Re: [PATCH v3 1/1] ArmPlatformPkg/PrePeiCore: Print the firmware version early in boot

["Rebecca Cran" <quic_rcran@quicinc.com>]

Thanks, that's a good change.

Reviewed-by: Rebecca Cran <rebecca@quicinc.com>

On 10/25/22 05:41, Leif Lindholm wrote:
> On Tue, Oct 11, 2022 at 14:59:52 -0600, Rebecca Cran wrote:
>> Copy code from PrePi to PrePeiCore that prints the firmware version
>> and build date early in the boot process.
> I'm good with this, but I'd prefer to break the printout into a helper
> function in order to reduce clutter in CEntryPoint.
>
> i.e. fold in
>
> ---
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> index 674f4f7df498..225e22f75c23 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> @@ -54,15 +54,32 @@ CreatePpiList (
>     *PpiListSize = sizeof (gCommonPpiTable) + PlatformPpiListSize;
>      }
>
> +STATIC
> +VOID
> +PrintFirmwareVersion (
> +  VOID
> +  )
> +{
> +  CHAR8  Buffer[100];
> +  UINTN  CharCount;
> +
> +  CharCount = AsciiSPrint (
> +                Buffer,
> +                sizeof (Buffer),
> +                "UEFI firmware (version %s built at %a on %a)\n\r",
> +                (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
> +                __TIME__,
> +                __DATE__
> +                );
> +  SerialPortWrite ((UINT8 *)Buffer, CharCount);
> +}
> +
>   VOID
>   CEntryPoint (
>     IN  UINTN                     MpId,
>     IN  EFI_PEI_CORE_ENTRY_POINT  PeiCoreEntryPoint
>     )
>   {
> -  CHAR8  Buffer[100];
> -  UINTN  CharCount;
> -
>     if (!ArmMmuEnabled ()) {
>       // Data Cache enabled on Primary core when MMU is enabled.
>       ArmDisableDataCache ();
> @@ -100,15 +117,8 @@ CEntryPoint (
>       // Invoke "ProcessLibraryConstructorList" to have all library constructors
>       // called.
>       ProcessLibraryConstructorList ();
> -    CharCount = AsciiSPrint (
> -                  Buffer,
> -                  sizeof (Buffer),
> -                  "UEFI firmware (version %s built at %a on %a)\n\r",
> -                  (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
> -                  __TIME__,
> -                  __DATE__
> -                  );
> -    SerialPortWrite ((UINT8 *)Buffer, CharCount);
> +
> +    PrintFirmwareVersion();
>
>       // Initialize the Debug Agent for Source Level Debugging
>            InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL,
>            NULL);
> ---
>
> If you're happy with that, I can apply and push.
>
> /
>      Leif
>
>> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>> Tested-by: Oliver Steffen <osteffen@redhat.com>
>> ---
>>   ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf  |  3 +++
>>   ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf |  3 +++
>>   ArmPlatformPkg/PrePeiCore/PrePeiCore.c          | 14 ++++++++++++++
>>   3 files changed, 20 insertions(+)
>>
>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
>> index a5b4722459d1..4a3112b58dcb 100644
>> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
>> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
>> @@ -54,6 +54,9 @@ [Ppis]
>>     gEfiTemporaryRamSupportPpiGuid
>>     gArmMpCoreInfoPpiGuid
>>   
>> +[Pcd]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
>> +
>>   [FeaturePcd]
>>     gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
>>   
>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>> index 466a2b01c384..ab5bf1dac2d8 100644
>> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>> @@ -52,6 +52,9 @@ [LibraryClasses]
>>   [Ppis]
>>     gEfiTemporaryRamSupportPpiGuid
>>   
>> +[Pcd]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
>> +
>>   [FeaturePcd]
>>     gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
>>   
>> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>> index 9c4b25df953d..1d4f6969b660 100644
>> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
>> @@ -11,6 +11,8 @@
>>   #include <Library/CacheMaintenanceLib.h>
>>   #include <Library/DebugAgentLib.h>
>>   #include <Library/ArmLib.h>
>> +#include <Library/PrintLib.h>
>> +#include <Library/SerialPortLib.h>
>>   
>>   #include "PrePeiCore.h"
>>   
>> @@ -58,6 +60,9 @@ CEntryPoint (
>>     IN  EFI_PEI_CORE_ENTRY_POINT  PeiCoreEntryPoint
>>     )
>>   {
>> +  CHAR8  Buffer[100];
>> +  UINTN  CharCount;
>> +
>>     // Data Cache enabled on Primary core when MMU is enabled.
>>     ArmDisableDataCache ();
>>     // Invalidate instruction cache
>> @@ -93,6 +98,15 @@ CEntryPoint (
>>       // Invoke "ProcessLibraryConstructorList" to have all library constructors
>>       // called.
>>       ProcessLibraryConstructorList ();
>> +    CharCount = AsciiSPrint (
>> +                  Buffer,
>> +                  sizeof (Buffer),
>> +                  "UEFI firmware (version %s built at %a on %a)\n\r",
>> +                  (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
>> +                  __TIME__,
>> +                  __DATE__
>> +                  );
>> +    SerialPortWrite ((UINT8 *)Buffer, CharCount);
>>   
>>       // Initialize the Debug Agent for Source Level Debugging
>>       InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>> -- 
>> 2.30.2
>>