From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=PQCcrzF8; spf=pass (domain: linaro.org, ip: 209.85.221.45, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by groups.io with SMTP; Wed, 02 Oct 2019 12:43:15 -0700 Received: by mail-wr1-f45.google.com with SMTP id i1so368932wro.4 for ; Wed, 02 Oct 2019 12:43:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=FfLYmJdR3VVacel37wGcJ4bMxrUf03UpOsBjNtz4nhs=; b=PQCcrzF8AU4ZxAoboczs7NyjxSl8tVcbOTkXE0JmxZZiAZdTzMjMNIawASafB29Q95 HkShduCAoex+2ID8kU+ohEXtcQOTx8HqvszigxznwRq9hNclfM5V3DegLq1EobuQYUGi R9ZHA6nxA+Z0IRxdL2KXKMUde5fDnJgdzYAzUwsyRQC8G/+cOv646ZNfhT/5iwARl6JB WvTeBI988v0xUAanY+MejYlveLQaaJ+AdyJU4acgfBrVH0OaVMQTxCU9xxg2tDiCHoeO sdLXyihF6FbQ0g6DD8gPExh2FKXAbwRuh+UK7c91d81IUCVGPDuBlVj1dHDf75Q7PUJC JCNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=FfLYmJdR3VVacel37wGcJ4bMxrUf03UpOsBjNtz4nhs=; b=kQAsdx0KIsU5aNSELqzMoBlzHzKRVhuP91ixvxZY0eCRG50R/gYXU3wY/z0VAs8ugI RbW5R/4AhAcN7Et7EIjUTonXoEsYNbpAZ0MYaWBSmiLXOYiaok/nWUODsbi+nDKn8y8L nYaNx1XgRQuRZ3V8ga6szmFTp1RHmQXuhUKMEYvxW/u66Zyrrzs0BL7ou8rkwUoUVN06 s+YlaYKVbENn/cqBltwIOC6iunl/wrQCdP9pxNonJA8X/oOxoSclSxFcwZH6NtYNV1bH BR74D/pB3xtONWKmpmepzaM0S8UL3lPFup3Rl+WuKOe9pZ3S+35bwQXpkmSNiZ+2xtiA Qppw== X-Gm-Message-State: APjAAAWW9wbjOjAVakxnpfuWxlowiJD3l5aQJFyGimgu3taU6C6IsH6+ +3dWXvR29C/6/029piSptZ8luA4/tIU= X-Google-Smtp-Source: APXvYqw8QNKLzcC0heDsCdKneWrVVS48jhHeBDgA/KawaAhwCWXidJ7qGrzDNY5/5OYX9gsB8F/Apw== X-Received: by 2002:a5d:51d2:: with SMTP id n18mr4073003wrv.10.1570045392781; Wed, 02 Oct 2019 12:43:12 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id z142sm399221wmc.24.2019.10.02.12.43.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Oct 2019 12:43:11 -0700 (PDT) Date: Wed, 2 Oct 2019 20:43:10 +0100 From: "Leif Lindholm" To: devel@edk2.groups.io, gilbert.chen@hpe.com Cc: Palmer Dabbelt Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 06/14] RiscV/Universal: Initial version of common RISC-V SEC module Message-ID: <20191002194310.GF25504@bivouac.eciton.net> References: <20190919035131.4700-1-gilbert.chen@hpe.com> <20190919035131.4700-7-gilbert.chen@hpe.com> MIME-Version: 1.0 In-Reply-To: <20190919035131.4700-7-gilbert.chen@hpe.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Sep 19, 2019 at 11:51:23AM +0800, Gilbert Chen wrote: > Common RISC-V SEC module for RISC-V platforms. If this is common to RISC-V platforms, this really should live in edk2/RiscVPkg. > Signed-off-by: Gilbert Chen > --- > Platform/RiscV/Universal/Sec/Riscv64/SecEntry.S | 438 ++++++++++++++++++++ > Platform/RiscV/Universal/Sec/SecMain.c | 524 ++++++++++++++++++++++++ > Platform/RiscV/Universal/Sec/SecMain.h | 57 +++ > Platform/RiscV/Universal/Sec/SecMain.inf | 75 ++++ > 4 files changed, 1094 insertions(+) > create mode 100644 Platform/RiscV/Universal/Sec/Riscv64/SecEntry.S > create mode 100644 Platform/RiscV/Universal/Sec/SecMain.c > create mode 100644 Platform/RiscV/Universal/Sec/SecMain.h > create mode 100644 Platform/RiscV/Universal/Sec/SecMain.inf > > diff --git a/Platform/RiscV/Universal/Sec/Riscv64/SecEntry.S b/Platform/RiscV/Universal/Sec/Riscv64/SecEntry.S > new file mode 100644 > index 00000000..18b54b84 > --- /dev/null > +++ b/Platform/RiscV/Universal/Sec/Riscv64/SecEntry.S > @@ -0,0 +1,438 @@ > +/* > + * Copyright (c) 2019 , Hewlett Packard Enterprise Development LP. All rights reserved. > + * > + * SPDX-License-Identifier: BSD-2-Clause > + * > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > + * > + * Authors: > + * Anup Patel If Anup is the author, he should be indicated in the commit as the Author. If you further modify the patch, you can add a comment above your signed-off-by: of the form [Updated coding style issues.] Signed-off-by: ... > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +.text > +.align 3 > +.global ASM_PFX(_ModuleEntryPoint) > +ASM_PFX(_ModuleEntryPoint): > + /* > + * Jump to warm-boot if this is not the selected core booting, > + */ > + csrr a6, CSR_MHARTID > + li a5, FixedPcdGet32 (PcdBootHartId) > + bne a6, a5, _wait_for_boot_hart We don't actually have a coding style for assembler, but I find it really helps readability to have the arguments aligned across all lines. Compare (for example) with OvmfPkg/Sec/X64/SecEntry.nasm. > + > + // light LED on > + li a5, 0x54002000 > + li a4, 0xff > + sw a4, 0x08(a5) > + li a4, 0x11 > + sw a4, 0x0c(a5) > + > + li ra, 0 > + call _reset_regs > + > + /* Preload HART details > + * s7 -> HART Count > + * s8 -> HART Stack Size > + */ > + li s7, FixedPcdGet32 (PcdHartCount) > + li s8, FixedPcdGet32 (PcdOpenSbiStackSize) > + > + /* Setup scratch space for all the HARTs*/ > + li tp, FixedPcdGet32 (PcdScratchRamBase) > + mul a5, s7, s8 > + add tp, tp, a5 > + /* Keep a copy of tp */ > + add t3, tp, zero > + /* Counter */ > + li t2, 1 > + /* hartid 0 is mandated by ISA */ > + li t1, 0 > +_scratch_init: > + add tp, t3, zero > + mul a5, s8, t1 > + sub tp, tp, a5 > + li a5, SBI_SCRATCH_SIZE > + sub tp, tp, a5 > + > + /* Initialize scratch space */ > + li a4, FixedPcdGet32 (PcdFwStartAddress) > + li a5, FixedPcdGet32 (PcdFwEndAddress) > + sub a5, a5, a4 > + sd a4, SBI_SCRATCH_FW_START_OFFSET(tp) > + sd a5, SBI_SCRATCH_FW_SIZE_OFFSET(tp) Can you add a blank line here? > + /* Note: fw_next_arg1() uses a0, a1, and ra */ > + call fw_next_arg1 > + sd a0, SBI_SCRATCH_NEXT_ARG1_OFFSET(tp) Can you add a blank line here? > + /* Note: fw_next_addr() uses a0, a1, and ra */ > + call fw_next_addr Can you add a blank line here? > + sd a0, SBI_SCRATCH_NEXT_ADDR_OFFSET(tp) > + li a4, PRV_S > + sd a4, SBI_SCRATCH_NEXT_MODE_OFFSET(tp) > + la a4, _start_warm > + sd a4, SBI_SCRATCH_WARMBOOT_ADDR_OFFSET(tp) > + la a4, platform > + sd a4, SBI_SCRATCH_PLATFORM_ADDR_OFFSET(tp) > + la a4, _hartid_to_scratch > + sd a4, SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET(tp) > + sd zero, SBI_SCRATCH_TMP0_OFFSET(tp) Can you add a blank line here? > +#ifdef FW_OPTIONS > + li a4, FW_OPTIONS > + sd a4, SBI_SCRATCH_OPTIONS_OFFSET(tp) > +#else > + sd zero, SBI_SCRATCH_OPTIONS_OFFSET(tp) > +#endif Can you add a blank line here? > + add t1, t1, t2 > + blt t1, s7, _scratch_init > + > + /* Fill-out temporary memory with 55aa*/ > + li a4, FixedPcdGet32 (PcdTemporaryRamBase) > + li a5, FixedPcdGet32 (PcdTemporaryRamSize) > + add a5, a4, a5 > +1: > + li a3, 0x5AA55AA55AA55AA5 > + sd a3, (a4) > + add a4, a4, __SIZEOF_POINTER__ > + blt a4, a5, 1b > + > + /* Update boot hart flag */ > + la a4, _boot_hart_done > + li a5, 1 > + sd a5, (a4) > + > + /* Wait for boot hart */ > +_wait_for_boot_hart: > + la a4, _boot_hart_done > + ld a5, (a4) > + /* Reduce the bus traffic so that boot hart may proceed faster */ > + nop > + nop > + nop > + beqz a5, _wait_for_boot_hart > + > +_start_warm: > + li ra, 0 > + call _reset_regs > + > + /* Disable and clear all interrupts */ > + csrw CSR_MIE, zero > + csrw CSR_MIP, zero > + > + li s7, FixedPcdGet32 (PcdHartCount) > + li s8, FixedPcdGet32 (PcdOpenSbiStackSize) > + > + /* HART ID should be within expected limit */ > + csrr s6, CSR_MHARTID > + bge s6, s7, _start_hang > + > + /* find the scratch space for this hart */ > + li tp, FixedPcdGet32 (PcdScratchRamBase) > + mul a5, s7, s8 > + add tp, tp, a5 > + mul a5, s8, s6 > + sub tp, tp, a5 > + li a5, SBI_SCRATCH_SIZE > + sub tp, tp, a5 > + > + /* update the mscratch */ > + csrw CSR_MSCRATCH, tp > + > + /*make room for Hart specific Firmware Context*/ > + li a5, FIRMWARE_CONTEXT_HART_SPECIFIC_SIZE > + sub tp, tp, a5 > + > + /* Setup stack */ > + add sp, tp, zero > + > + /* Setup stack for the Hart executing EFI to top of temporary ram*/ > + csrr a6, CSR_MHARTID > + li a5, FixedPcdGet32 (PcdBootHartId) > + bne a6, a5, 1f > + > + li a4, FixedPcdGet32(PcdTemporaryRamBase) > + li a5, FixedPcdGet32(PcdTemporaryRamSize) > + add sp, a4, a5 > + 1: > + > + /* Setup trap handler */ > + la a4, _trap_handler > + csrw CSR_MTVEC, a4 Can you add a blank line here? > + /* Make sure that mtvec is updated */ > + 1: > + csrr a5, CSR_MTVEC > + bne a4, a5, 1b > + > + /* Call library constructors before jup to SEC core */ > + call ProcessLibraryConstructorList > + > + /* jump to SEC Core C */ > + csrr a0, CSR_MHARTID > + csrr a1, CSR_MSCRATCH > + call SecCoreStartUpWithStack > + > + /* We do not expect to reach here hence just hang */ > + j _start_hang > + > + .align 3 > + .section .data, "aw" > +_boot_hart_done: > + RISCV_PTR 0 > + > + .align 3 > + .section .entry, "ax", %progbits > + .globl _hartid_to_scratch > +_hartid_to_scratch: > + add sp, sp, -(3 * __SIZEOF_POINTER__) > + sd s0, (sp) > + sd s1, (__SIZEOF_POINTER__)(sp) > + sd s2, (__SIZEOF_POINTER__ * 2)(sp) Can you add a blank line here? > + /* > + * a0 -> HART ID (passed by caller) > + * s0 -> HART Stack Size > + * s1 -> HART Stack End > + * s2 -> Temporary > + */ > + la s2, platform > +#if __riscv_xlen == 64 > + lwu s0, SBI_PLATFORM_HART_STACK_SIZE_OFFSET(s2) > + lwu s2, SBI_PLATFORM_HART_COUNT_OFFSET(s2) > +#else > + lw s0, SBI_PLATFORM_HART_STACK_SIZE_OFFSET(s2) > + lw s2, SBI_PLATFORM_HART_COUNT_OFFSET(s2) > +#endif > + mul s2, s2, s0 > + li s1, FixedPcdGet32 (PcdScratchRamBase) > + add s1, s1, s2 > + mul s2, s0, a0 > + sub s1, s1, s2 > + li s2, SBI_SCRATCH_SIZE > + sub a0, s1, s2 > + ld s0, (sp) > + ld s1, (__SIZEOF_POINTER__)(sp) > + ld s2, (__SIZEOF_POINTER__ * 2)(sp) > + add sp, sp, (3 * __SIZEOF_POINTER__) > + ret > + > + .align 3 > + .section .entry, "ax", %progbits > + .globl _start_hang > +_start_hang: > + wfi > + j _start_hang > + > + .align 3 > + .section .entry, "ax", %progbits > + .globl _trap_handler > +_trap_handler: > + /* Swap TP and MSCRATCH */ > + csrrw tp, CSR_MSCRATCH, tp > + > + /* Save T0 in scratch space */ > + sd t0, SBI_SCRATCH_TMP0_OFFSET(tp) > + > + /* Check which mode we came from */ > + csrr t0, CSR_MSTATUS > + srl t0, t0, MSTATUS_MPP_SHIFT > + and t0, t0, PRV_M > + xori t0, t0, PRV_M > + beq t0, zero, _trap_handler_m_mode > + > + /* We came from S-mode or U-mode */ > +_trap_handler_s_mode: > + /* Set T0 to original SP */ > + add t0, sp, zero > + > + /* Setup exception stack */ > + add sp, tp, -(SBI_TRAP_REGS_SIZE) > + > + /* Jump to code common for all modes */ > + j _trap_handler_all_mode > + > + /* We came from M-mode */ > +_trap_handler_m_mode: > + /* Set T0 to original SP */ > + add t0, sp, zero > + > + /* Re-use current SP as exception stack */ > + add sp, sp, -(SBI_TRAP_REGS_SIZE) > + > +_trap_handler_all_mode: > + /* Save original SP (from T0) on stack */ > + sd t0, SBI_TRAP_REGS_OFFSET(sp)(sp) > + > + /* Restore T0 from scratch space */ > + ld t0, SBI_SCRATCH_TMP0_OFFSET(tp) > + > + /* Save T0 on stack */ > + sd t0, SBI_TRAP_REGS_OFFSET(t0)(sp) > + > + /* Swap TP and MSCRATCH */ > + csrrw tp, CSR_MSCRATCH, tp > + > + /* Save MEPC and MSTATUS CSRs */ > + csrr t0, CSR_MEPC > + sd t0, SBI_TRAP_REGS_OFFSET(mepc)(sp) > + csrr t0, CSR_MSTATUS > + sd t0, SBI_TRAP_REGS_OFFSET(mstatus)(sp) > + > + /* Save all general regisers except SP and T0 */ > + sd zero, SBI_TRAP_REGS_OFFSET(zero)(sp) > + sd ra, SBI_TRAP_REGS_OFFSET(ra)(sp) > + sd gp, SBI_TRAP_REGS_OFFSET(gp)(sp) > + sd tp, SBI_TRAP_REGS_OFFSET(tp)(sp) > + sd t1, SBI_TRAP_REGS_OFFSET(t1)(sp) > + sd t2, SBI_TRAP_REGS_OFFSET(t2)(sp) > + sd s0, SBI_TRAP_REGS_OFFSET(s0)(sp) > + sd s1, SBI_TRAP_REGS_OFFSET(s1)(sp) > + sd a0, SBI_TRAP_REGS_OFFSET(a0)(sp) > + sd a1, SBI_TRAP_REGS_OFFSET(a1)(sp) > + sd a2, SBI_TRAP_REGS_OFFSET(a2)(sp) > + sd a3, SBI_TRAP_REGS_OFFSET(a3)(sp) > + sd a4, SBI_TRAP_REGS_OFFSET(a4)(sp) > + sd a5, SBI_TRAP_REGS_OFFSET(a5)(sp) > + sd a6, SBI_TRAP_REGS_OFFSET(a6)(sp) > + sd a7, SBI_TRAP_REGS_OFFSET(a7)(sp) > + sd s2, SBI_TRAP_REGS_OFFSET(s2)(sp) > + sd s3, SBI_TRAP_REGS_OFFSET(s3)(sp) > + sd s4, SBI_TRAP_REGS_OFFSET(s4)(sp) > + sd s5, SBI_TRAP_REGS_OFFSET(s5)(sp) > + sd s6, SBI_TRAP_REGS_OFFSET(s6)(sp) > + sd s7, SBI_TRAP_REGS_OFFSET(s7)(sp) > + sd s8, SBI_TRAP_REGS_OFFSET(s8)(sp) > + sd s9, SBI_TRAP_REGS_OFFSET(s9)(sp) > + sd s10, SBI_TRAP_REGS_OFFSET(s10)(sp) > + sd s11, SBI_TRAP_REGS_OFFSET(s11)(sp) > + sd t3, SBI_TRAP_REGS_OFFSET(t3)(sp) > + sd t4, SBI_TRAP_REGS_OFFSET(t4)(sp) > + sd t5, SBI_TRAP_REGS_OFFSET(t5)(sp) > + sd t6, SBI_TRAP_REGS_OFFSET(t6)(sp) > + > + /* Call C routine */ > + add a0, sp, zero > + csrr a1, CSR_MSCRATCH > + call sbi_trap_handler > + > + /* Restore all general regisers except SP and T0 */ > + ld ra, SBI_TRAP_REGS_OFFSET(ra)(sp) > + ld gp, SBI_TRAP_REGS_OFFSET(gp)(sp) > + ld tp, SBI_TRAP_REGS_OFFSET(tp)(sp) > + ld t1, SBI_TRAP_REGS_OFFSET(t1)(sp) > + ld t2, SBI_TRAP_REGS_OFFSET(t2)(sp) > + ld s0, SBI_TRAP_REGS_OFFSET(s0)(sp) > + ld s1, SBI_TRAP_REGS_OFFSET(s1)(sp) > + ld a0, SBI_TRAP_REGS_OFFSET(a0)(sp) > + ld a1, SBI_TRAP_REGS_OFFSET(a1)(sp) > + ld a2, SBI_TRAP_REGS_OFFSET(a2)(sp) > + ld a3, SBI_TRAP_REGS_OFFSET(a3)(sp) > + ld a4, SBI_TRAP_REGS_OFFSET(a4)(sp) > + ld a5, SBI_TRAP_REGS_OFFSET(a5)(sp) > + ld a6, SBI_TRAP_REGS_OFFSET(a6)(sp) > + ld a7, SBI_TRAP_REGS_OFFSET(a7)(sp) > + ld s2, SBI_TRAP_REGS_OFFSET(s2)(sp) > + ld s3, SBI_TRAP_REGS_OFFSET(s3)(sp) > + ld s4, SBI_TRAP_REGS_OFFSET(s4)(sp) > + ld s5, SBI_TRAP_REGS_OFFSET(s5)(sp) > + ld s6, SBI_TRAP_REGS_OFFSET(s6)(sp) > + ld s7, SBI_TRAP_REGS_OFFSET(s7)(sp) > + ld s8, SBI_TRAP_REGS_OFFSET(s8)(sp) > + ld s9, SBI_TRAP_REGS_OFFSET(s9)(sp) > + ld s10, SBI_TRAP_REGS_OFFSET(s10)(sp) > + ld s11, SBI_TRAP_REGS_OFFSET(s11)(sp) > + ld t3, SBI_TRAP_REGS_OFFSET(t3)(sp) > + ld t4, SBI_TRAP_REGS_OFFSET(t4)(sp) > + ld t5, SBI_TRAP_REGS_OFFSET(t5)(sp) > + ld t6, SBI_TRAP_REGS_OFFSET(t6)(sp) > + > + /* Restore MEPC and MSTATUS CSRs */ > + ld t0, SBI_TRAP_REGS_OFFSET(mepc)(sp) > + csrw CSR_MEPC, t0 > + ld t0, SBI_TRAP_REGS_OFFSET(mstatus)(sp) > + csrw CSR_MSTATUS, t0 > + > + /* Restore T0 */ > + ld t0, SBI_TRAP_REGS_OFFSET(t0)(sp) > + > + /* Restore SP */ > + ld sp, SBI_TRAP_REGS_OFFSET(sp)(sp) > + > + mret > + > + .align 3 > + .section .entry, "ax", %progbits > + .globl _reset_regs > +_reset_regs: > + > + /* flush the instruction cache */ > + fence.i > + /* Reset all registers except ra, a0,a1 */ > + li sp, 0 > + li gp, 0 > + li tp, 0 > + li t0, 0 > + li t1, 0 > + li t2, 0 > + li s0, 0 > + li s1, 0 > + li a2, 0 > + li a3, 0 > + li a4, 0 > + li a5, 0 > + li a6, 0 > + li a7, 0 > + li s2, 0 > + li s3, 0 > + li s4, 0 > + li s5, 0 > + li s6, 0 > + li s7, 0 > + li s8, 0 > + li s9, 0 > + li s10, 0 > + li s11, 0 > + li t3, 0 > + li t4, 0 > + li t5, 0 > + li t6, 0 > + csrw CSR_MSCRATCH, 0 > + ret > + > + .align 3 > + .section .entry, "ax", %progbits > + .global fw_prev_arg1 > +fw_prev_arg1: > + /* We return previous arg1 in 'a0' */ > + add a0, zero, zero > + ret > + > + .align 3 > + .section .entry, "ax", %progbits > + .global fw_next_arg1 > +fw_next_arg1: > + /* We return next arg1 in 'a0' */ > + li a0, FixedPcdGet32(PcdRiscVPeiFvBase) > + ret > + > + .align 3 > + .section .entry, "ax", %progbits > + .global fw_next_addr > +fw_next_addr: > + /* We return next address in 'a0' */ > + la a0, _jump_addr > + ld a0, (a0) > + ret > + > + .align 3 > + .section .entry, "ax", %progbits > +_jump_addr: > + RISCV_PTR SecCoreStartUpWithStack > diff --git a/Platform/RiscV/Universal/Sec/SecMain.c b/Platform/RiscV/Universal/Sec/SecMain.c > new file mode 100644 > index 00000000..40b351ca > --- /dev/null > +++ b/Platform/RiscV/Universal/Sec/SecMain.c > @@ -0,0 +1,524 @@ > +/** @file > + RISC-V SEC phase module. > + > + Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include "SecMain.h" > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Please sort includes alphabetically. > +#include > + > +int HartsIn = 0; UINTN? > + > +EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI mTemporaryRamSupportPpi = { > + TemporaryRamMigration > +}; > + > +EFI_PEI_TEMPORARY_RAM_DONE_PPI mTemporaryRamDonePpi = { > + TemporaryRamDone > +}; > + > +EFI_PEI_PPI_DESCRIPTOR mPrivateDispatchTable[] = { > + { > + EFI_PEI_PPI_DESCRIPTOR_PPI, > + &gEfiTemporaryRamSupportPpiGuid, > + &mTemporaryRamSupportPpi > + }, > + { > + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > + &gEfiTemporaryRamDonePpiGuid, > + &mTemporaryRamDonePpi > + }, > +}; Can the above three variables all be made STATIC? > + > +/** > + Locates a section within a series of sections > + with the specified section type. > + > + The Instance parameter indicates which instance of the section > + type to return. (0 is first instance, 1 is second...) > + > + @param[in] Sections The sections to search > + @param[in] SizeOfSections Total size of all sections > + @param[in] SectionType The section type to locate > + @param[in] Instance The section instance number > + @param[out] FoundSection The FFS section if found > + > + @retval EFI_SUCCESS The file and section was found > + @retval EFI_NOT_FOUND The file and section was not found > + @retval EFI_VOLUME_CORRUPTED The firmware volume was corrupted > + > +**/ > +EFI_STATUS > +FindFfsSectionInstance ( This is a note, not something that needs to be addressed for setting up the -devel branch: but this function is identical to the one in /OvmfPkg/Sec/SecMain.c and the one in Platform/Intel/SimicsOpenBoardPkg/SecCore/SecMain.c We should address this. > + IN VOID *Sections, > + IN UINTN SizeOfSections, > + IN EFI_SECTION_TYPE SectionType, > + IN UINTN Instance, > + OUT EFI_COMMON_SECTION_HEADER **FoundSection > + ) > +{ > + EFI_PHYSICAL_ADDRESS CurrentAddress; > + UINT32 Size; > + EFI_PHYSICAL_ADDRESS EndOfSections; > + EFI_COMMON_SECTION_HEADER *Section; > + EFI_PHYSICAL_ADDRESS EndOfSection; > + > + // > + // Loop through the FFS file sections within the PEI Core FFS file > + // > + EndOfSection = (EFI_PHYSICAL_ADDRESS)(UINTN) Sections; > + EndOfSections = EndOfSection + SizeOfSections; > + for (;;) { > + if (EndOfSection == EndOfSections) { > + break; > + } > + CurrentAddress = (EndOfSection + 3) & ~(3ULL); > + if (CurrentAddress >= EndOfSections) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + Section = (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAddress; > + > + Size = SECTION_SIZE (Section); > + if (Size < sizeof (*Section)) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + EndOfSection = CurrentAddress + Size; > + if (EndOfSection > EndOfSections) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + // > + // Look for the requested section type > + // > + if (Section->Type == SectionType) { > + if (Instance == 0) { > + *FoundSection = Section; > + return EFI_SUCCESS; > + } else { > + Instance--; > + } > + } > + } > + > + return EFI_NOT_FOUND; > +} > + > +/** > + Locates a section within a series of sections > + with the specified section type. > + > + @param[in] Sections The sections to search > + @param[in] SizeOfSections Total size of all sections > + @param[in] SectionType The section type to locate > + @param[out] FoundSection The FFS section if found > + > + @retval EFI_SUCCESS The file and section was found > + @retval EFI_NOT_FOUND The file and section was not found > + @retval EFI_VOLUME_CORRUPTED The firmware volume was corrupted > + > +**/ > +EFI_STATUS > +FindFfsSectionInSections ( The same applies to this one. > + IN VOID *Sections, > + IN UINTN SizeOfSections, > + IN EFI_SECTION_TYPE SectionType, > + OUT EFI_COMMON_SECTION_HEADER **FoundSection > + ) > +{ > + return FindFfsSectionInstance ( > + Sections, > + SizeOfSections, > + SectionType, > + 0, > + FoundSection > + ); > +} > + > +/** > + Locates a FFS file with the specified file type and a section > + within that file with the specified section type. > + > + @param[in] Fv The firmware volume to search > + @param[in] FileType The file type to locate > + @param[in] SectionType The section type to locate > + @param[out] FoundSection The FFS section if found > + > + @retval EFI_SUCCESS The file and section was found > + @retval EFI_NOT_FOUND The file and section was not found > + @retval EFI_VOLUME_CORRUPTED The firmware volume was corrupted > + > +**/ > +EFI_STATUS > +FindFfsFileAndSection ( And this one. > + IN EFI_FIRMWARE_VOLUME_HEADER *Fv, > + IN EFI_FV_FILETYPE FileType, > + IN EFI_SECTION_TYPE SectionType, > + OUT EFI_COMMON_SECTION_HEADER **FoundSection > + ) > +{ > + EFI_STATUS Status; > + EFI_PHYSICAL_ADDRESS CurrentAddress; > + EFI_PHYSICAL_ADDRESS EndOfFirmwareVolume; > + EFI_FFS_FILE_HEADER *File; > + UINT32 Size; > + EFI_PHYSICAL_ADDRESS EndOfFile; > + > + if (Fv->Signature != EFI_FVH_SIGNATURE) { > + DEBUG ((DEBUG_ERROR, "FV at %p does not have FV header signature\n", Fv)); > + return EFI_VOLUME_CORRUPTED; > + } > + > + CurrentAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) Fv; > + EndOfFirmwareVolume = CurrentAddress + Fv->FvLength; > + > + // > + // Loop through the FFS files in the Boot Firmware Volume > + // > + for (EndOfFile = CurrentAddress + Fv->HeaderLength; ; ) { > + > + CurrentAddress = (EndOfFile + 7) & ~(7ULL); > + if (CurrentAddress > EndOfFirmwareVolume) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; > + Size = *(UINT32*) File->Size & 0xffffff; > + if (Size < (sizeof (*File) + sizeof (EFI_COMMON_SECTION_HEADER))) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + EndOfFile = CurrentAddress + Size; > + if (EndOfFile > EndOfFirmwareVolume) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + // > + // Look for the request file type > + // > + if (File->Type != FileType) { > + continue; > + } > + > + Status = FindFfsSectionInSections ( > + (VOID*) (File + 1), > + (UINTN) EndOfFile - (UINTN) (File + 1), > + SectionType, > + FoundSection > + ); > + if (!EFI_ERROR (Status) || (Status == EFI_VOLUME_CORRUPTED)) { > + return Status; > + } > + } > +} > + > +/** > + Locates the PEI Core entry point address > + > + @param[in] Fv The firmware volume to search > + @param[out] PeiCoreEntryPoint The entry point of the PEI Core image > + > + @retval EFI_SUCCESS The file and section was found > + @retval EFI_NOT_FOUND The file and section was not found > + @retval EFI_VOLUME_CORRUPTED The firmware volume was corrupted > + > +**/ > +EFI_STATUS > +FindPeiCoreImageBaseInFv ( And this. > + IN EFI_FIRMWARE_VOLUME_HEADER *Fv, > + OUT EFI_PHYSICAL_ADDRESS *PeiCoreImageBase > + ) > +{ > + EFI_STATUS Status; > + EFI_COMMON_SECTION_HEADER *Section; > + > + Status = FindFfsFileAndSection ( > + Fv, > + EFI_FV_FILETYPE_PEI_CORE, > + EFI_SECTION_PE32, > + &Section > + ); > + if (EFI_ERROR (Status)) { > + Status = FindFfsFileAndSection ( > + Fv, > + EFI_FV_FILETYPE_PEI_CORE, > + EFI_SECTION_TE, > + &Section > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Unable to find PEI Core image\n")); > + return Status; > + } > + } > + DEBUG ((DEBUG_ERROR, "PeiCoreImageBase found\n")); > + *PeiCoreImageBase = (EFI_PHYSICAL_ADDRESS)(UINTN)(Section + 1); > + return EFI_SUCCESS; > +} > + > +/** > + Locates the PEI Core entry point address > + > + @param[in,out] Fv The firmware volume to search > + @param[out] PeiCoreEntryPoint The entry point of the PEI Core image > + > + @retval EFI_SUCCESS The file and section was found > + @retval EFI_NOT_FOUND The file and section was not found > + @retval EFI_VOLUME_CORRUPTED The firmware volume was corrupted > + > +**/ > +VOID > +FindPeiCoreImageBase ( > + IN OUT EFI_FIRMWARE_VOLUME_HEADER **BootFv, > + OUT EFI_PHYSICAL_ADDRESS *PeiCoreImageBase > + ) > +{ > + *PeiCoreImageBase = 0; > + > + DEBUG ((DEBUG_INFO, "FindPeiCoreImageBaseInFv\n")); This one is the same with S3 support ripped out, and the above line added. Can you delete the message, or change it to something more human friendly? I personally would interpret that as something that was printed _from_ that function (and in that situation should be using %a __FUNCTION__). > + FindPeiCoreImageBaseInFv (*BootFv, PeiCoreImageBase); > +} > + > +/* > + Find and return Pei Core entry point. > + > + It also find SEC and PEI Core file debug inforamtion. It will report them if > + remote debug is enabled. > + > +**/ > +VOID > +FindAndReportEntryPoints ( > + IN EFI_FIRMWARE_VOLUME_HEADER **BootFirmwareVolumePtr, > + OUT EFI_PEI_CORE_ENTRY_POINT *PeiCoreEntryPoint > + ) > +{ > + EFI_STATUS Status; > + EFI_PHYSICAL_ADDRESS PeiCoreImageBase; > + > + DEBUG ((DEBUG_INFO, "FindAndReportEntryPoints\n")); Use %a __FUNCTION__. > + > + FindPeiCoreImageBase (BootFirmwareVolumePtr, &PeiCoreImageBase); > + // > + // Find PEI Core entry point > + // > + Status = PeCoffLoaderGetEntryPoint ((VOID *) (UINTN) PeiCoreImageBase, (VOID**) PeiCoreEntryPoint); > + if (EFI_ERROR(Status)) { > + *PeiCoreEntryPoint = 0; > + } > + DEBUG ((DEBUG_INFO, "PeCoffLoaderGetEntryPoint success: %x\n", *PeiCoreEntryPoint)); > + > + return; > +} > +/* > + Print out the content of firmware context. > + > +**/ > +VOID > +DebutPrintFirmwareContext ( Debut -> Debug? > + EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *FirmwareContext > + ) > +{ > + DEBUG ((DEBUG_INFO, "[OpenSBI]: OpenSBI Firmware Context at 0x%x\n", FirmwareContext)); Please drop the [OpenSBI] prefix. > + DEBUG ((DEBUG_INFO, " PEI Service at 0x%x\n\n", FirmwareContext->PeiServiceTable)); > +} > + > +EFI_STATUS > +EFIAPI > +TemporaryRamMigration ( > + IN CONST EFI_PEI_SERVICES **PeiServices, > + IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase, > + IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase, > + IN UINTN CopySize > + ) > +{ > + VOID *OldHeap; > + VOID *NewHeap; > + VOID *OldStack; > + VOID *NewStack; > + struct sbi_platform *ThisSbiPlatform; > + > + DEBUG ((DEBUG_INFO, > + "TemporaryRamMigration(0x%Lx, 0x%Lx, 0x%Lx)\n", %a __FUNCTION__ > + TemporaryMemoryBase, > + PermanentMemoryBase, > + (UINT64)CopySize > + )); > + > + OldHeap = (VOID*)(UINTN)TemporaryMemoryBase; > + NewHeap = (VOID*)((UINTN)PermanentMemoryBase + (CopySize >> 1)); > + > + OldStack = (VOID*)((UINTN)TemporaryMemoryBase + (CopySize >> 1)); > + NewStack = (VOID*)(UINTN)PermanentMemoryBase; > + > + CopyMem (NewHeap, OldHeap, CopySize >> 1); // Migrate Heap > + CopyMem (NewStack, OldStack, CopySize >> 1); // Migrate Stack > + > + // > + // Reset firmware context pointer > + // > + ThisSbiPlatform = (struct sbi_platform *)sbi_platform_ptr(sbi_scratch_thishart_ptr()); > + ThisSbiPlatform->firmware_context += (unsigned long)((UINTN)NewStack - (UINTN)OldStack); "unsigned long" is not a type that exists in UEFI. Please use UINTN. Applies throughout. > + // > + // Relocate PEI Service ** > + // > + ((EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *)ThisSbiPlatform->firmware_context)->PeiServiceTable += (unsigned long)((UINTN)NewStack - (UINTN)OldStack); The above line is begging for a temporary pointer for the firmware_context. > + DEBUG ((DEBUG_INFO, "[OpenSBI]: OpenSBI Firmware Context is relocated to 0x%x\n", ThisSbiPlatform->firmware_context)); Please > + DebutPrintFirmwareContext ((EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *)ThisSbiPlatform->firmware_context); > + > + register uintptr_t a0 asm ("a0") = (uintptr_t)((UINTN)NewStack - (UINTN)OldStack); > + asm volatile ("add sp, sp, a0"::"r"(a0):); Urgh. Is there any way this could be broken out into a separate assembler function instead? If not, we still need to get rid of the uintptr_t:s. > + return EFI_SUCCESS; > +} > + > +EFI_STATUS EFIAPI TemporaryRamDone (VOID) > +{ > + DEBUG ((DEBUG_INFO, "2nd time PEI core, temporary ram done.\n")); > + return EFI_SUCCESS; > +} > + > +#if 1 Just delete the #if 1? > +#define GPIO_CTRL_ADDR 0x54002000 > +#define GPIO_OUTPUT_VAL 0x0C > +static volatile UINT32 * const gpio = (void *)GPIO_CTRL_ADDR; > +#define REG32(p, i) ((p)[(i)>>2]) > +#endif And the #endif Moreover, please move the ADDRESS/VALUE macros to SecMain.h, and use IoLib instead of defining your own variables and macros. > + > +static VOID EFIAPI PeiCore(VOID) STATIC Also, please follow coding style for function definition, this is supposed to be over multiple lines. > +{ > + EFI_SEC_PEI_HAND_OFF SecCoreData; > + EFI_PEI_CORE_ENTRY_POINT PeiCoreEntryPoint; > + EFI_FIRMWARE_VOLUME_HEADER *BootFv = (EFI_FIRMWARE_VOLUME_HEADER *)FixedPcdGet32(PcdRiscVPeiFvBase); > + EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT FirmwareContext; > + struct sbi_platform *ThisSbiPlatform; > + UINT32 HartId; > + > + REG32(gpio, GPIO_OUTPUT_VAL) = 0x88; > + FindAndReportEntryPoints (&BootFv, &PeiCoreEntryPoint); > + > + SecCoreData.DataSize = sizeof(EFI_SEC_PEI_HAND_OFF); > + SecCoreData.BootFirmwareVolumeBase = BootFv; > + SecCoreData.BootFirmwareVolumeSize = (UINTN) BootFv->FvLength; > + SecCoreData.TemporaryRamBase = (VOID*)(UINT64) FixedPcdGet32(PcdTemporaryRamBase); > + SecCoreData.TemporaryRamSize = (UINTN) FixedPcdGet32(PcdTemporaryRamSize); > + SecCoreData.PeiTemporaryRamBase = SecCoreData.TemporaryRamBase; > + SecCoreData.PeiTemporaryRamSize = SecCoreData.TemporaryRamSize >> 1; > + SecCoreData.StackBase = (UINT8 *)SecCoreData.TemporaryRamBase + (SecCoreData.TemporaryRamSize >> 1); Please don't use UINT8 * to get away from pointer arithmetic. It disguises what function is actually being performed. (VOID *)((UINTN) ...) is a much preferred form where that level of description is required. > + SecCoreData.StackSize = SecCoreData.TemporaryRamSize >> 1; > + > + // > + // Print out scratch address of each hart > + // > + DEBUG ((DEBUG_INFO, "[OpenSBI]: OpenSBI scratch address for each hart:\n")); Please drop [OpenSBI] tag. > + for (HartId = 0; HartId < FixedPcdGet32 (PcdHartCount); HartId ++) { > + DEBUG ((DEBUG_INFO, " Hart %d: 0x%x\n", HartId, sbi_hart_id_to_scratch(sbi_scratch_thishart_ptr(), HartId))); Please wrap long line. > + } > + > + // > + // Set up OpepSBI firmware context poitner on boot hart OpenSbi scratch. Firmware context residents in stack and will be Please wrap long line. > + // switched to memory when temporary ram migration. > + // > + ZeroMem ((VOID *)&FirmwareContext, sizeof (EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT)); > + ThisSbiPlatform = (struct sbi_platform *)sbi_platform_ptr(sbi_scratch_thishart_ptr()); > + if (ThisSbiPlatform->opensbi_version > OPENSBI_VERSION) { > + DEBUG ((DEBUG_ERROR, "[OpenSBI]: OpenSBI platform table version 0x%x is newer than OpenSBI version 0x%x.\n" > + "There maybe be some backward compatable issues.\n", Please drop [OpenSBI] tag. Please convert the two lines to two separate DEBUG prints. > + ThisSbiPlatform->opensbi_version, > + OPENSBI_VERSION > + )); > + ASSERT(FALSE); > + } > + DEBUG ((DEBUG_INFO, "[OpenSBI]: OpenSBI platform table at address: 0x%x\nFirmware Context is located at 0x%x\n", Please drop [OpenSBI] tag. > + ThisSbiPlatform, > + &FirmwareContext > + )); > + ThisSbiPlatform->firmware_context = (unsigned long)&FirmwareContext; UINTN (probably best to do a global search and replace on this). > + // > + // Set firmware context Hart-specific pointer > + // > + for (HartId = 0; HartId < FixedPcdGet32 (PcdHartCount); HartId ++) { > + FirmwareContext.HartSpecific [HartId] = \ > + (EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC *)((UINT8 *)sbi_hart_id_to_scratch(sbi_scratch_thishart_ptr(), HartId) - FIRMWARE_CONTEXT_HART_SPECIFIC_SIZE); Please don't use (UINT8 *) to get away from pointer arithmetic. UINTN should work fine in this context. Screaming out for a temporary variable. > + DEBUG ((DEBUG_INFO, "[OpenSBI]: OpenSBI Hart %d Firmware Context Hart-specific at address: 0x%x\n", Please drop [OpenSBI] tag. > + HartId, > + FirmwareContext.HartSpecific [HartId] > + )); > + } > + > + // > + // Transfer the control to the PEI core > + // > + (*PeiCoreEntryPoint) (&SecCoreData, (EFI_PEI_PPI_DESCRIPTOR *)&mPrivateDispatchTable); > +} > +/** > + This function initilizes hart specific information and SBI. > + For the boot hart, it boots system through PEI core and initial SBI in the DXE IPL. > + For others, it goes to initial SBI and halt. > + > + the lay out of memory region for each hart is as below delineates, > + > + _ ____ > + |----Scratch ends | | > + | | sizeof (sbi_scratch) | > + | _| | > + |----Scratch buffer start s <----- *scratch | > + |----Firmware Context Hart-specific ends _ | > + | | | > + | | FIRMWARE_CONTEXT_HART_SPECIFIC_SIZE | > + | | | PcdOpenSbiStackSize > + | _| | > + |----Firmware Context Hart-specific starts <----- **HartFirmwareContext | > + |----Hart stack top _ | > + | | | > + | | | > + | | Stack | > + | | | > + | _| ____| > + |----Hart stack bottom > + > +**/ > +VOID EFIAPI SecCoreStartUpWithStack(unsigned long hartid, struct sbi_scratch *scratch) Split up over multiple lines. > +{ > + EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC *HartFirmwareContext; > + > + // > + // Setup EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC for each hart. > + // > + HartFirmwareContext = (EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC *)((UINT8 *)scratch - FIRMWARE_CONTEXT_HART_SPECIFIC_SIZE); Please don't use UINT8 * to get away from pointer arithmetic. In this context, UINTN should work just fine. > + HartFirmwareContext->IsaExtensionSupported = RiscVReadMisa (); > + HartFirmwareContext->MachineVendorId.Value64_L = RiscVReadMVendorId (); > + HartFirmwareContext->MachineVendorId.Value64_H = 0; > + HartFirmwareContext->MachineArchId.Value64_L = RiscVReadMArchId (); > + HartFirmwareContext->MachineArchId.Value64_H = 0; > + HartFirmwareContext->MachineImplId.Value64_L = RiscVReadMImplId (); > + HartFirmwareContext->MachineImplId.Value64_H = 0; > + > +#if 0 Please don't leave unused code in. If you want to keep the printouts for extreme debugging sessions, change the loglevel to DEBUG_VERBOSE. > + while (HartsIn != hartid); > + DEBUG ((DEBUG_INFO, "[OpenSBI]: Initial Firmware Context Hart-specific for HART ID:%d\n", hartid)); Please drop [OpenSBI] tag. > + DEBUG ((DEBUG_INFO, " Scratch at address: 0x%x\n", scratch)); > + DEBUG ((DEBUG_INFO, " Firmware Context Hart-specific at address: 0x%x\n", HartFirmwareContext)); > + DEBUG ((DEBUG_INFO, " stack pointer at address: 0x%x\n", stack_point)); > + DEBUG ((DEBUG_INFO, " MISA: 0x%x\n", HartFirmwareContext->IsaExtensionSupported)); > + DEBUG ((DEBUG_INFO, " MVENDORID: 0x%x\n", HartFirmwareContext->MachineVendorId.Value64_L)); > + DEBUG ((DEBUG_INFO, " MARCHID: 0x%x\n", HartFirmwareContext->MachineArchId.Value64_L)); > + DEBUG ((DEBUG_INFO, " MIMPID: 0x%x\n\n", HartFirmwareContext->MachineImplId.Value64_L)); > + HartsIn ++; > + for (;;); > +#endif > + > + if (hartid == FixedPcdGet32(PcdBootHartId)) { > + PeiCore(); > + } > + sbi_init(scratch); > +} > diff --git a/Platform/RiscV/Universal/Sec/SecMain.h b/Platform/RiscV/Universal/Sec/SecMain.h > new file mode 100644 > index 00000000..e7565f5e > --- /dev/null > +++ b/Platform/RiscV/Universal/Sec/SecMain.h > @@ -0,0 +1,57 @@ > +/** @file > + RISC-V SEC phase module definitions.. > + > + Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef _SECMAIN_H_ > +#define _SECMAIN_H_ Plese drop leading _. > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Please drop all include statements not required by this file, and add them instead in the files that do use them. > + > +VOID > +SecMachineModeTrapHandler ( > + IN VOID > + ); > + > +VOID > +EFIAPI > +SecStartupPhase2 ( > + IN VOID *Context > + ); > + > +EFI_STATUS > +EFIAPI > +TemporaryRamMigration ( > + IN CONST EFI_PEI_SERVICES **PeiServices, > + IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase, > + IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase, > + IN UINTN CopySize > + ); > + > +EFI_STATUS > +EFIAPI > +TemporaryRamDone ( > + VOID > + ); > + > +#endif // _SECMAIN_H_ > diff --git a/Platform/RiscV/Universal/Sec/SecMain.inf b/Platform/RiscV/Universal/Sec/SecMain.inf > new file mode 100644 > index 00000000..c408fc8d > --- /dev/null > +++ b/Platform/RiscV/Universal/Sec/SecMain.inf > @@ -0,0 +1,75 @@ > +## @file > +# RISC-V SEC module. > +# > +# Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 Can this be updated to a recent specification version. > + BASE_NAME = SecMain > + FILE_GUID = df1ccef6-f301-4a63-9661-fc6030dcc880 > + MODULE_TYPE = SEC > + VERSION_STRING = 1.0 > + ENTRY_POINT = SecMain > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = RISCV64 EBC Pretty sure EBC is not valid here. > +# > + > +[Sources] > + SecMain.c > + > +[Sources.RISCV64] > + Riscv64/SecEntry.s > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + RiscVPkg/RiscVPkg.dec > + Platform/RiscV/RiscVPlatformPkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + BaseMemoryLib > + PcdLib > + DebugAgentLib > + IoLib > + PeCoffLib > + PeCoffGetEntryPointLib > + PeCoffExtraActionLib > + ExtractGuidedSectionLib > + RiscVCpuLib > + PrintLib > + SerialPortLib > + RiscVOpensbiLib > + OpenSbiPlatformLib # This is required library which > + # provides platform level opensbi > + # functions. I don't doubt it, but I can tell it's required from the fact that it is listed. And the name itself says all of the rest. The comment is superfluous and can be deleted. > + > +[Ppis] > + gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED > + gEfiTemporaryRamDonePpiGuid # PPI ALWAYS_PRODUCED > + > +[Guids] > + gUefiRiscVMachineContextGuid > + > +[FixedPcd] > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdRiscVPeiFvBase > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdRiscVPeiFvSize > + > +[Pcd] > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdFwStartAddress > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdFwEndAddress > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdHartCount > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdBootHartId > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdScratchRamBase > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdScratchRamSize > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdOpenSbiStackSize > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdTemporaryRamBase > + gUefiRiscVPlatformPkgTokenSpaceGuid.PcdTemporaryRamSize Please sort the above alphabetically ( apart from where it makes sense not to - for example PcdFwStartAddress before PcdFwEndAddress makes more sense than the other way around). / Leif > -- > 2.12.0.windows.1 > > > >