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

["Leif Lindholm" <quic_llindhol@quicinc.com>]

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
>