* [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib
@ 2020-08-29 13:41 Leif Lindholm
2020-08-30 8:14 ` Abner Chang
0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2020-08-29 13:41 UTC (permalink / raw)
To: devel; +Cc: Abner Chang, Gilbert Chen, Daniel Schaefer
In function CreateU54E51CoreProcessorSpecificDataHob(), the
FirmwareContext variable gets initialized an ASSERT_EFI_ERROR
macro, meaning the initialization gets folded out for RELEASE
builds. Use a temporary variable for the ASSERT instead.
Fixes a build error of both u540 and u500 with gcc 8.3.
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Gilbert Chen <gilbert.chen@hpe.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
index 6ddae632fd17..edeabf028ff8 100644
--- a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
+++ b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
@@ -56,6 +56,7 @@ 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__));
@@ -63,7 +64,8 @@ CreateU54E51CoreProcessorSpecificDataHob (
return EFI_INVALID_PARAMETER;
}
- ASSERT_EFI_ERROR (SbiGetFirmwareContext (&FirmwareContext));
+ Status = SbiGetFirmwareContext (&FirmwareContext);
+ ASSERT_EFI_ERROR (Status);
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));
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib
2020-08-29 13:41 [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib Leif Lindholm
@ 2020-08-30 8:14 ` Abner Chang
2020-08-30 11:34 ` Leif Lindholm
0 siblings, 1 reply; 5+ messages in thread
From: Abner Chang @ 2020-08-30 8:14 UTC (permalink / raw)
To: Leif Lindholm, devel@edk2.groups.io; +Cc: Chen, Gilbert, Schaefer, Daniel
> -----Original Message-----
> From: Leif Lindholm [mailto:leif@nuviainc.com]
> Sent: Saturday, August 29, 2020 9:41 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> Chen, Gilbert <gilbert.chen@hpe.com>; Schaefer, Daniel
> <daniel.schaefer@hpe.com>
> Subject: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54
> PeiCoreInfoHobLib
>
> In function CreateU54E51CoreProcessorSpecificDataHob(), the
> FirmwareContext variable gets initialized an ASSERT_EFI_ERROR macro,
> meaning the initialization gets folded out for RELEASE builds. Use a
> temporary variable for the ASSERT instead.
>
> Fixes a build error of both u540 and u500 with gcc 8.3.
>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Gilbert Chen <gilbert.chen@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> ---
> Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> index 6ddae632fd17..edeabf028ff8 100644
> --- a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> +++ b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> @@ -56,6 +56,7 @@ 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__));
>
> @@ -63,7 +64,8 @@ CreateU54E51CoreProcessorSpecificDataHob (
> return EFI_INVALID_PARAMETER;
> }
>
> - ASSERT_EFI_ERROR (SbiGetFirmwareContext (&FirmwareContext));
> + Status = SbiGetFirmwareContext (&FirmwareContext);
> ASSERT_EFI_ERROR
ASSERT_EFI_ERROR() should start at the new line. I see this macro just attached to the end of Status = SbiGetFirmwareContext (&FirmwareContext). Is my editor problem?
> + (Status);
Please add error condition check for the Status. Return EFI_INVALID_PARAMETERS to caller, and also update the function header for the case of @retval is EFI_INVALID_PARAMETER. Just realized that we use @return in the function header instead of @retval.
Thanks for this.
Abner
> 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));
> --
> 2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib
2020-08-30 8:14 ` Abner Chang
@ 2020-08-30 11:34 ` Leif Lindholm
2020-08-30 12:59 ` Abner Chang
0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2020-08-30 11:34 UTC (permalink / raw)
To: Chang, Abner (HPS SW/FW Technologist)
Cc: devel@edk2.groups.io, Chen, Gilbert, Schaefer, Daniel
On Sun, Aug 30, 2020 at 08:14:36 +0000, Chang, Abner (HPS SW/FW Technologist) wrote:
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif@nuviainc.com]
> > Sent: Saturday, August 29, 2020 9:41 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> > Chen, Gilbert <gilbert.chen@hpe.com>; Schaefer, Daniel
> > <daniel.schaefer@hpe.com>
> > Subject: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54
> > PeiCoreInfoHobLib
> >
> > In function CreateU54E51CoreProcessorSpecificDataHob(), the
> > FirmwareContext variable gets initialized an ASSERT_EFI_ERROR macro,
> > meaning the initialization gets folded out for RELEASE builds. Use a
> > temporary variable for the ASSERT instead.
> >
> > Fixes a build error of both u540 and u500 with gcc 8.3.
> >
> > Cc: Abner Chang <abner.chang@hpe.com>
> > Cc: Gilbert Chen <gilbert.chen@hpe.com>
> > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> > ---
> > Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > index 6ddae632fd17..edeabf028ff8 100644
> > --- a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > +++ b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > @@ -56,6 +56,7 @@ 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__));
> >
> > @@ -63,7 +64,8 @@ CreateU54E51CoreProcessorSpecificDataHob (
> > return EFI_INVALID_PARAMETER;
> > }
> >
> > - ASSERT_EFI_ERROR (SbiGetFirmwareContext (&FirmwareContext));
> > + Status = SbiGetFirmwareContext (&FirmwareContext);
> > ASSERT_EFI_ERROR
> ASSERT_EFI_ERROR() should start at the new line. I see this macro
> just attached to the end of Status = SbiGetFirmwareContext
> (&FirmwareContext). Is my editor problem?
https://edk2.groups.io/g/devel/message/64794
It's your editor.
> > + (Status);
> Please add error condition check for the Status. Return
> EFI_INVALID_PARAMETERS to caller, and also update the function
> header for the case of @retval is EFI_INVALID_PARAMETER. Just
> realized that we use @return in the function header instead of
> @retval.
I agree this looks like a better way of handling the failure, but I'll
leave that to you. This patch simply stops the current handling from
breaking RELEASE builds.
Regards,
Leif
>
> Thanks for this.
> Abner
>
> > 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));
> > --
> > 2.20.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib
2020-08-30 11:34 ` Leif Lindholm
@ 2020-08-30 12:59 ` Abner Chang
2020-08-30 13:30 ` Leif Lindholm
0 siblings, 1 reply; 5+ messages in thread
From: Abner Chang @ 2020-08-30 12:59 UTC (permalink / raw)
To: Leif Lindholm; +Cc: devel@edk2.groups.io, Chen, Gilbert, Schaefer, Daniel
> -----Original Message-----
> From: Leif Lindholm [mailto:leif@nuviainc.com]
> Sent: Sunday, August 30, 2020 7:34 PM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> Cc: devel@edk2.groups.io; Chen, Gilbert <gilbert.chen@hpe.com>; Schaefer,
> Daniel <daniel.schaefer@hpe.com>
> Subject: Re: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of
> U54 PeiCoreInfoHobLib
>
> On Sun, Aug 30, 2020 at 08:14:36 +0000, Chang, Abner (HPS SW/FW
> Technologist) wrote:
> > > -----Original Message-----
> > > From: Leif Lindholm [mailto:leif@nuviainc.com]
> > > Sent: Saturday, August 29, 2020 9:41 PM
> > > To: devel@edk2.groups.io
> > > Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> > > Chen, Gilbert <gilbert.chen@hpe.com>; Schaefer, Daniel
> > > <daniel.schaefer@hpe.com>
> > > Subject: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE
> > > builds of U54 PeiCoreInfoHobLib
> > >
> > > In function CreateU54E51CoreProcessorSpecificDataHob(), the
> > > FirmwareContext variable gets initialized an ASSERT_EFI_ERROR macro,
> > > meaning the initialization gets folded out for RELEASE builds. Use a
> > > temporary variable for the ASSERT instead.
> > >
> > > Fixes a build error of both u540 and u500 with gcc 8.3.
> > >
> > > Cc: Abner Chang <abner.chang@hpe.com>
> > > Cc: Gilbert Chen <gilbert.chen@hpe.com>
> > > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > > Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> > > ---
> > > Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > > a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > > b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > > index 6ddae632fd17..edeabf028ff8 100644
> > > --- a/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > > +++ b/Silicon/SiFive/U54/Library/PeiCoreInfoHobLib/CoreInfoHob.c
> > > @@ -56,6 +56,7 @@ 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__));
> > >
> > > @@ -63,7 +64,8 @@ CreateU54E51CoreProcessorSpecificDataHob (
> > > return EFI_INVALID_PARAMETER;
> > > }
> > >
> > > - ASSERT_EFI_ERROR (SbiGetFirmwareContext (&FirmwareContext));
> > > + Status = SbiGetFirmwareContext (&FirmwareContext);
> > > ASSERT_EFI_ERROR
> > ASSERT_EFI_ERROR() should start at the new line. I see this macro just
> > attached to the end of Status = SbiGetFirmwareContext
> > (&FirmwareContext). Is my editor problem?
>
> INVALID URI REMOVED
> 3A__edk2.groups.io_g_devel_message_64794&d=DwIBAg&c=C5b8zRQO1mi
> GmBeVZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=
> UkdQYKg5exPcuGmzwjtBQvaRGX5SPNiaP9ko8nTDWgQ&s=r0p7ssf1Q8Y2070
> L71hNFsILUzkHznlm5k8mpyVeV9k&e=
> It's your editor.
Ok.
>
> > > + (Status);
> > Please add error condition check for the Status. Return
> > EFI_INVALID_PARAMETERS to caller, and also update the function header
> > for the case of @retval is EFI_INVALID_PARAMETER. Just realized that
> > we use @return in the function header instead of @retval.
>
> I agree this looks like a better way of handling the failure, but I'll leave that to
> you. This patch simply stops the current handling from breaking RELEASE
> builds.
Sure!
Reviewed-by: Abner Chang <abner.chang@hpe.com>
>
> Regards,
>
> Leif
>
> >
> > Thanks for this.
> > Abner
> >
> > > 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));
> > > --
> > > 2.20.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib
2020-08-30 12:59 ` Abner Chang
@ 2020-08-30 13:30 ` Leif Lindholm
0 siblings, 0 replies; 5+ messages in thread
From: Leif Lindholm @ 2020-08-30 13:30 UTC (permalink / raw)
To: Chang, Abner (HPS SW/FW Technologist)
Cc: devel@edk2.groups.io, Chen, Gilbert, Schaefer, Daniel
On Sun, Aug 30, 2020 at 12:59:06 +0000, Chang, Abner (HPS SW/FW Technologist) wrote:
> > > > @@ -63,7 +64,8 @@ CreateU54E51CoreProcessorSpecificDataHob (
> > > > return EFI_INVALID_PARAMETER;
> > > > }
> > > >
> > > > - ASSERT_EFI_ERROR (SbiGetFirmwareContext (&FirmwareContext));
> > > > + Status = SbiGetFirmwareContext (&FirmwareContext);
> > > > ASSERT_EFI_ERROR
> > > ASSERT_EFI_ERROR() should start at the new line. I see this macro just
> > > attached to the end of Status = SbiGetFirmwareContext
> > > (&FirmwareContext). Is my editor problem?
> >
> > INVALID URI REMOVED
> > 3A__edk2.groups.io_g_devel_message_64794&d=DwIBAg&c=C5b8zRQO1mi
> > GmBeVZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=
> > UkdQYKg5exPcuGmzwjtBQvaRGX5SPNiaP9ko8nTDWgQ&s=r0p7ssf1Q8Y2070
> > L71hNFsILUzkHznlm5k8mpyVeV9k&e=
> > It's your editor.
> Ok.
> >
> > > > + (Status);
> > > Please add error condition check for the Status. Return
> > > EFI_INVALID_PARAMETERS to caller, and also update the function header
> > > for the case of @retval is EFI_INVALID_PARAMETER. Just realized that
> > > we use @return in the function header instead of @retval.
> >
> > I agree this looks like a better way of handling the failure, but I'll leave that to
> > you. This patch simply stops the current handling from breaking RELEASE
> > builds.
> Sure!
>
> Reviewed-by: Abner Chang <abner.chang@hpe.com>
Thanks.
Pushed as 50639477fc0f.
> >
> > Regards,
> >
> > Leif
> >
> > >
> > > Thanks for this.
> > > Abner
> > >
> > > > 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));
> > > > --
> > > > 2.20.1
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-30 13:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-29 13:41 [PATCH edk2-platforms 1/1] Silicon/SiFive: fix RELEASE builds of U54 PeiCoreInfoHobLib Leif Lindholm
2020-08-30 8:14 ` Abner Chang
2020-08-30 11:34 ` Leif Lindholm
2020-08-30 12:59 ` Abner Chang
2020-08-30 13:30 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox