From: "Abner Chang" <abner.chang@hpe.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"Chang, Abner (HPS SW/FW Technologist)" <abner.chang@hpe.com>,
"Schaefer, Daniel" <daniel.schaefer@hpe.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-devel] [PATCH 1/1] Silicon/SiFive: Handle case of NULL FirmwareContext
Date: Fri, 2 Oct 2020 02:40:56 +0000 [thread overview]
Message-ID: <CS1PR8401MB11447228A21D11C3C6A1DFD0FF310@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <1639933E5766B4C0.8589@groups.io>
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.schaefer@hpe.com>; devel@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>
> Subject: Re: [edk2-devel] [PATCH 1/1] Silicon/SiFive: Handle case of NULL
> FirmwareContext
>
> Reviewed-by: Abner Chang <abner.chang@hpe.com>
>
> > -----Original Message-----
> > From: Schaefer, Daniel
> > Sent: Wednesday, September 30, 2020 4:50 PM
> > To: devel@edk2.groups.io
> > Cc: Schaefer, Daniel <daniel.schaefer@hpe.com>; Chang, Abner (HPS
> > SW/FW Technologist) <abner.chang@hpe.com>; Leif Lindholm
> > <leif@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.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 *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
>
>
>
>
>
prev parent reply other threads:[~2020-10-02 2:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-30 8:50 [PATCH 0/1] Silicon/SiFive: Handle case of NULL FirmwareContext Daniel Schaefer
2020-09-30 8:50 ` [PATCH 1/1] " Daniel Schaefer
2020-09-30 13:37 ` Abner Chang
[not found] ` <1639933E5766B4C0.8589@groups.io>
2020-10-02 2:40 ` Abner Chang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CS1PR8401MB11447228A21D11C3C6A1DFD0FF310@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox