Merged to edk2-platforms. > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Abner Chang > Sent: Wednesday, September 30, 2020 9:37 PM > To: Schaefer, Daniel <daniel.schae...@hpe.com>; devel@edk2.groups.io > Cc: Leif Lindholm <l...@nuviainc.com> > Subject: Re: [edk2-devel] [PATCH 1/1] Silicon/SiFive: Handle case of NULL > FirmwareContext > > Reviewed-by: Abner Chang <abner.ch...@hpe.com> > > > -----Original Message----- > > From: Schaefer, Daniel > > Sent: Wednesday, September 30, 2020 4:50 PM > > To: devel@edk2.groups.io > > Cc: Schaefer, Daniel <daniel.schae...@hpe.com>; Chang, Abner (HPS > > SW/FW Technologist) <abner.ch...@hpe.com>; Leif Lindholm > > <l...@nuviainc.com> > > Subject: [PATCH 1/1] Silicon/SiFive: Handle case of NULL > > FirmwareContext > > > > Abort creating the SMBIOS HOBs if there's no firmware context to get > > the information from. > > Turn SbiLib functions for getting mscratch into VOID since they can > > never practically fail. > > > > Signed-off-by: Daniel Schaefer <daniel.schae...@hpe.com> > > Cc: Abner Chang <abner.ch...@hpe.com> > > Cc: Leif Lindholm <l...@nuviainc.com> > > --- > > .../Include/Library/RiscVEdk2SbiLib.h | 12 ++--- > > .../PlatformPkg/Universal/Sec/SecMain.c | 11 +++-- > > .../Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c | 46 +++++++------------ > > .../Library/PeiCoreInfoHobLib/CoreInfoHob.c | 13 ++++-- > > 4 files changed, 36 insertions(+), 46 deletions(-) > > > > diff --git > > a/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h > > b/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h > > index 558841a970ce..f81ea06b05b0 100644 > > --- a/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h > > +++ b/Silicon/RISC-V/ProcessorPkg/Include/Library/RiscVEdk2SbiLib.h > > @@ -514,9 +514,8 @@ SbiVendorCall ( > > access the firmware context. @param[out] ScratchSpace The > > scratch > > space pointer.- @retval EFI_SUCCESS The operation succeeds. > > **/- > > EFI_STATUS+VOID EFIAPI SbiGetMscratch ( OUT SBI_SCRATCH > > **ScratchSpace@@ -527,9 +526,8 @@ SbiGetMscratch ( > > @param[in] HartId The hart id. @param[out] ScratchSpace > The > > scratch space pointer.- @retval EFI_SUCCESS The operation > succeeds. > > **/-EFI_STATUS+VOID EFIAPI SbiGetMscratchHartid ( IN UINTN > > HartId,@@ -540,9 +538,8 @@ SbiGetMscratchHartid ( > > Get firmware context of the calling hart. @param[out] FirmwareContext > > The firmware context pointer.- @retval EFI_SUCCESS The > > operation > > succeeds. **/-EFI_STATUS+VOID EFIAPI SbiGetFirmwareContext ( OUT > > EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT **FirmwareContext@@ -552,9 > > +549,8 @@ SbiGetFirmwareContext ( > > Set firmware context of the calling hart. @param[in] FirmwareContext > > The firmware context pointer.- @retval EFI_SUCCESS The > > operation > > succeeds. **/-EFI_STATUS+VOID EFIAPI SbiSetFirmwareContext ( IN > > EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *FirmwareContextdiff --git > > a/Platform/RISC-V/PlatformPkg/Universal/Sec/SecMain.c > b/Platform/RISC- > > V/PlatformPkg/Universal/Sec/SecMain.c > > index 877777bfa1ab..fa9ecd789a57 100644 > > --- a/Platform/RISC-V/PlatformPkg/Universal/Sec/SecMain.c > > +++ b/Platform/RISC-V/PlatformPkg/Universal/Sec/SecMain.c > > @@ -415,7 +415,10 @@ EFI_STATUS EFIAPI TemporaryRamDone ( > > return EFI_SUCCESS; } -/** Handles SBI calls of EDK2's SBI FW > > extension+/**+ Handles SBI calls of EDK2's SBI FW extension.++ The > > extension+return > > value is the error code returned by the SBI call. @param[in] ExtId > > The > > extension ID of the FW extension. @param[in] FuncId The called > > function ID.@@ -424,7 +427,7 @@ EFI_STATUS EFIAPI TemporaryRamDone > ( > > @param[out] OutTrap Trap info for trapping further, see OpenSBI > > code. > > Is ignored if return value is not SBI_ETRAP. - @retval 0 If > > the > handler > > succeeds.+ @retval SBI_OK If the handler succeeds. @retval > > SBI_ENOTSUPP If there's no function with the given ID. @retval > > SBI_ETRAP If the called SBI functions wants to trap further. **/@@ - > 436,7 > > +439,7 @@ STATIC int SbiEcallFirmwareHandler ( > > OUT struct sbi_trap_info *OutTrap ) {- int Ret = 0;+ int Ret = > > SBI_OK; > > switch (FuncId) { case SBI_EXT_FW_MSCRATCH_FUNC:@@ -447,6 +450,8 > > @@ STATIC int SbiEcallFirmwareHandler ( > > break; default: Ret = SBI_ENOTSUPP;+ DEBUG > > ((DEBUG_ERROR, > > "%a: Called SBI firmware ecall with invalid function ID\n", > __FUNCTION__));+ > > ASSERT (FALSE); }; return Ret;diff --git a/Silicon/RISC- > > V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c > > b/Silicon/RISC- > > V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.c > > index 0df505d2675b..9bbeaaec3f7a 100644 > > --- > > a/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib. > > c > > +++ b/Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2Sbi > > +++ Li > > +++ b.c > > @@ -801,9 +801,8 @@ SbiVendorCall ( > > access the firmware context. @param[out] ScratchSpace The > > scratch > > space pointer.- @retval EFI_SUCCESS The operation succeeds. > > **/- > > EFI_STATUS+VOID EFIAPI SbiGetMscratch ( OUT SBI_SCRATCH > > **ScratchSpace@@ -811,11 +810,10 @@ SbiGetMscratch ( > > { SbiRet Ret = SbiCall (SBI_EDK2_FW_EXT, > SBI_EXT_FW_MSCRATCH_FUNC, > > 0); - if (!Ret.Error) {- *ScratchSpace = (SBI_SCRATCH *)Ret.Value;- }+ > > // > > Our ecall handler never returns an error, only when the func id is > > invalid+ ASSERT (Ret.Error == SBI_OK); - return EFI_SUCCESS;+ > > *ScratchSpace = (SBI_SCRATCH *)Ret.Value; } /**@@ -823,9 +821,8 @@ > SbiGetMscratch ( > > @param[in] HartId The hart id. @param[out] ScratchSpace > The > > scratch space pointer.- @retval EFI_SUCCESS The operation > succeeds. > > **/-EFI_STATUS+VOID EFIAPI SbiGetMscratchHartid ( IN UINTN > > HartId,@@ -839,11 +836,10 @@ SbiGetMscratchHartid ( > > HartId ); - if (!Ret.Error) {- > > *ScratchSpace = > (SBI_SCRATCH > > *)Ret.Value;- }+ // Our ecall handler never returns an error, only > > when the func id is invalid+ ASSERT (Ret.Error == SBI_OK); - return > > EFI_SUCCESS;+ *ScratchSpace = (SBI_SCRATCH *)Ret.Value; } /**@@ > > -852,7 +848,7 @@ SbiGetMscratchHartid ( > > @param[out] FirmwareContext The firmware context pointer. > @retval > > EFI_SUCCESS The operation succeeds. **/-EFI_STATUS+VOID EFIAPI > > SbiGetFirmwareContext ( OUT > EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT > > **FirmwareContext@@ -860,24 +856,18 @@ SbiGetFirmwareContext ( > > { SBI_SCRATCH *ScratchSpace; SBI_PLATFORM *SbiPlatform;- SbiRet > Ret > > = SbiCall (SBI_EDK2_FW_EXT, SBI_EXT_FW_MSCRATCH_FUNC, 0); - if > > (!Ret.Error) {- ScratchSpace = (SBI_SCRATCH *)Ret.Value;- SbiPlatform > > = > > (SBI_PLATFORM *)sbi_platform_ptr(ScratchSpace);- *FirmwareContext = > > (EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *)SbiPlatform- > > >firmware_context;- }-- return EFI_SUCCESS;+ > > SbiGetMscratch(&ScratchSpace);+ SbiPlatform = (SBI_PLATFORM > > *)sbi_platform_ptr(ScratchSpace);+ *FirmwareContext = > > (EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *)SbiPlatform- > > >firmware_context; } /** Set firmware context of the calling hart. > > @param[in] FirmwareContext The firmware context pointer.- @retval > > EFI_SUCCESS The operation succeeds. **/-EFI_STATUS+VOID EFIAPI > > SbiSetFirmwareContext ( IN EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT > > *FirmwareContext@@ -885,13 +875,9 @@ SbiSetFirmwareContext ( > > { SBI_SCRATCH *ScratchSpace; SBI_PLATFORM *SbiPlatform;- SbiRet > Ret > > = SbiCall (SBI_EDK2_FW_EXT, SBI_EXT_FW_MSCRATCH_FUNC, 0); - if > > (!Ret.Error) {- ScratchSpace = (SBI_SCRATCH *)Ret.Value;- SbiPlatform > > = > > (SBI_PLATFORM *)sbi_platform_ptr (ScratchSpace);- SbiPlatform- > > >firmware_context = (UINTN)FirmwareContext;- }+ > > SbiGetMscratch(&ScratchSpace); - return EFI_SUCCESS;+ SbiPlatform = > > (SBI_PLATFORM *)sbi_platform_ptr (ScratchSpace);+ SbiPlatform- > > >firmware_context = (UINTN)FirmwareContext; }diff --git > > a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c > > b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c > > index edeabf028ff8..88f36cbbe299 100644 > > --- a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c > > +++ b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c > > @@ -38,7 +38,7 @@ > > @return EFI_SUCCESS The PEIM initialized successfully. > > EFI_UNSUPPORTED HART is ignored by platform.-+ EFI_NOT_FOUND > > Processor specific data hob is not available. **/ EFI_STATUS EFIAPI@@ > > -56,7 > > +56,6 @@ CreateU54E51CoreProcessorSpecificDataHob ( > > RISC_V_PROCESSOR_SPECIFIC_HOB_DATA ProcessorSpecDataHob; > > EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT *FirmwareContext; > > EFI_RISCV_FIRMWARE_CONTEXT_HART_SPECIFIC > > *FirmwareContextHartSpecific;- EFI_STATUS Status; DEBUG > > ((DEBUG_INFO, "%a: Entry.\n", __FUNCTION__)); @@ -64,9 +63,14 @@ > > CreateU54E51CoreProcessorSpecificDataHob ( > > return EFI_INVALID_PARAMETER; } - Status = SbiGetFirmwareContext > > (&FirmwareContext);- ASSERT_EFI_ERROR (Status);+ > > SbiGetFirmwareContext (&FirmwareContext);+ ASSERT > > (FirmwareContext != NULL);+ if (FirmwareContext == NULL) {+ DEBUG > > ((DEBUG_ERROR, "Failed to get the pointer of > > EFI_RISCV_OPENSBI_FIRMWARE_CONTEXT of hart %d\n", HartId));+ > return > > EFI_NOT_FOUND;+ } DEBUG ((DEBUG_INFO, " Firmware Context is at > > 0x%x.\n", FirmwareContext));+ FirmwareContextHartSpecific = > > FirmwareContext->HartSpecific[HartId]; DEBUG ((DEBUG_INFO, " > > Firmware Context Hart specific is at 0x%x.\n", > > FirmwareContextHartSpecific)); if (FirmwareContextHartSpecific == > > NULL) {@@ -102,7 +106,6 @@ > CreateU54E51CoreProcessorSpecificDataHob ( > > ProcessorSpecDataHob.ProcessorSpecificData.SupervisorModeXlen = > > RegisterLen64; } - DebugPrintHartSpecificInfo (&ProcessorSpecDataHob); > > //-- > > 2.28.0 > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65828): https://edk2.groups.io/g/devel/message/65828 Mute This Topic: https://groups.io/mt/77213797/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
