Browse Source

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.schaefer@hpe.com>
Reviewed-by: Abner Chang <abner.chang@hpe.com>
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Abner Chang 3 years ago
parent
commit
b728a4ef50

+ 8 - 3
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;

+ 4 - 8
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 *FirmwareContext

+ 16 - 30
Silicon/RISC-V/ProcessorPkg/Library/RiscVEdk2SbiLib/RiscVEdk2SbiLib.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;
 }

+ 8 - 5
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);
 
   //