[PATCH 1/1] Silicon/SiFive: Handle case of NULL FirmwareContext

["Daniel Schaefer" <daniel.schaefer@hpe.com>]

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>
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Leif Lindholm <leif@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 *FirmwareContext
diff --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/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;
 }
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