Reviewed-by: Abner Chang <[email protected]> > -----Original Message----- > From: Schaefer, Daniel > Sent: Wednesday, September 30, 2020 4:50 PM > To: [email protected] > Cc: Schaefer, Daniel <[email protected]>; Chang, Abner (HPS > SW/FW Technologist) <[email protected]>; Leif Lindholm > <[email protected]> > 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 <[email protected]> > Cc: Abner Chang <[email protected]> > Cc: Leif Lindholm <[email protected]> > --- > .../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 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/RiscVEdk2SbiLi > +++ 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 (#65768): https://edk2.groups.io/g/devel/message/65768 Mute This Topic: https://groups.io/mt/77213797/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
