* [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