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

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

[Rebecca Cran]

Copy code from PrePi to PrePeiCore that prints the firmware version
and build date early in the boot process.

Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
---
 ArmPlatformPkg/PrePeiCore/PrePeiCore.c          | 17 +++++++++++++++++
 ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf  |  3 +++
 ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf |  3 +++
 3 files changed, 23 insertions(+)

diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
index 6dd9bcdea24f..9e2a0db676b9 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
@@ -90,6 +95,18 @@ CEntryPoint (
 
   // If not primary Jump to Secondary Main
   if (ArmPlatformIsPrimaryCore (MpId)) {
+    // Initialize the Serial Port
+    SerialPortInitialize ();
+    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);
     SaveAndSetDebugTimerInterrupt (TRUE);
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
index fb01dd1a113e..83029cf08d1a 100644
--- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
+++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
@@ -57,6 +57,9 @@
   gEfiTemporaryRamSupportPpiGuid
   gArmMpCoreInfoPpiGuid
 
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
+
 [FeaturePcd]
   gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
 
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
index e9eb092d3ac9..9f9ad81586ed 100644
--- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
@@ -55,6 +55,9 @@
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid
 
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
+
 [FeaturePcd]
   gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
 
-- 
2.34.1

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

[Ard Biesheuvel]

Hello Rebecca,

Apologies for the delay. I'm a bit behind on review, and few other
people appear to take an interest in these changes, unfortunately.


On Thu, 14 Apr 2022 at 00:46, Rebecca Cran <quic_rcran@quicinc.com> wrote:
>
> Copy code from PrePi to PrePeiCore that prints the firmware version
> and build date early in the boot process.
>
> Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
> ---
>  ArmPlatformPkg/PrePeiCore/PrePeiCore.c          | 17 +++++++++++++++++
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf  |  3 +++
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf |  3 +++
>  3 files changed, 23 insertions(+)
>
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> index 6dd9bcdea24f..9e2a0db676b9 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
> @@ -90,6 +95,18 @@ CEntryPoint (
>
>    // If not primary Jump to Secondary Main
>    if (ArmPlatformIsPrimaryCore (MpId)) {
> +    // Initialize the Serial Port
> +    SerialPortInitialize ();
> +    CharCount = AsciiSPrint (
> +                  Buffer,
> +                  sizeof (Buffer),
> +                  "UEFI firmware (version %s built at %a on %a)\n\r",
> +                  (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),

Given that this runs before any library constructors are called, I'd
be inclined to use FixedPcdGetPtr() here (and [FixedPcd] in the .INFs
below), to ensure that we don't dereference garbage.

I also wonder whether we need a call to
ProcessLibraryConstructorList() somewhere.

> +                  __TIME__,
> +                  __DATE__
> +                  );
> +    SerialPortWrite ((UINT8 *)Buffer, CharCount);
> +
>      // Initialize the Debug Agent for Source Level Debugging
>      InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, NULL, NULL);
>      SaveAndSetDebugTimerInterrupt (TRUE);
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> index fb01dd1a113e..83029cf08d1a 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> @@ -57,6 +57,9 @@
>    gEfiTemporaryRamSupportPpiGuid
>    gArmMpCoreInfoPpiGuid
>
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> +
>  [FeaturePcd]
>    gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
>
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> index e9eb092d3ac9..9f9ad81586ed 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> @@ -55,6 +55,9 @@
>  [Ppis]
>    gEfiTemporaryRamSupportPpiGuid
>
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> +
>  [FeaturePcd]
>    gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
>
> --
> 2.34.1
>
>
>
> 
>
>

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

[Sami Mujawar]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

Hi Rebecca, Ard,

On Tue, May 3, 2022 at 12:43 AM, Ard Biesheuvel wrote:

> 
> 
>> // Data Cache enabled on Primary core when MMU is enabled.
>> ArmDisableDataCache ();
>> // Invalidate instruction cache
>> @@ -90,6 +95,18 @@ CEntryPoint (
>> 
>> // If not primary Jump to Secondary Main
>> if (ArmPlatformIsPrimaryCore (MpId)) {
>> + // Initialize the Serial Port
>> + SerialPortInitialize ();
>> + CharCount = AsciiSPrint (
>> + Buffer,
>> + sizeof (Buffer),
>> + "UEFI firmware (version %s built at %a on %a)\n\r",
>> + (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
> 
> Given that this runs before any library constructors are called, I'd
> be inclined to use FixedPcdGetPtr() here (and [FixedPcd] in the .INFs
> below), to ensure that we don't dereference garbage.
> 
> I also wonder whether we need a call to
> ProcessLibraryConstructorList() somewhere.

[SAMI] There is a patch at https://edk2.groups.io/g/devel/message/91113 which aims to call ProcessLibraryConstructorList(). The idea being that the DebugLib constructor would be called which initializes the serial port.
So, maybe these patches need adjustment so that we avoid calling SerialPortInitialize () twice.
[/SAMI]

regards,

Sami Mujawar

> 
> 
> 
>> + __TIME__,
>> + __DATE__
>> + );
>> + SerialPortWrite ((UINT8 *)Buffer, CharCount);
> 
>

[-- Attachment #2: Type: text/html, Size: 1586 bytes --]

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

[Sami Mujawar]

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hi Rebecca,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 21/07/2022 05:23 pm, Sami Mujawar via groups.io wrote:
Hi Rebecca, Ard,

On Tue, May 3, 2022 at 12:43 AM, Ard Biesheuvel wrote:
// Data Cache enabled on Primary core when MMU is enabled.
ArmDisableDataCache ();
// Invalidate instruction cache
@@ -90,6 +95,18 @@ CEntryPoint (

// If not primary Jump to Secondary Main
if (ArmPlatformIsPrimaryCore (MpId)) {
+ // Initialize the Serial Port
+ SerialPortInitialize ();
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "UEFI firmware (version %s built at %a on %a)\n\r",
+ (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
Given that this runs before any library constructors are called, I'd
be inclined to use FixedPcdGetPtr() here (and [FixedPcd] in the .INFs
below), to ensure that we don't dereference garbage.

I also wonder whether we need a call to
ProcessLibraryConstructorList() somewhere.
[SAMI] There is a patch at https://edk2.groups.io/g/devel/message/91113 which aims to call ProcessLibraryConstructorList(). The idea being that the DebugLib constructor would be called which initializes the serial port.
So, maybe these patches need adjustment so that we avoid calling SerialPortInitialize () twice.
[/SAMI]

[/SAMI] I had a look at the SerialPortInitialize() implementation for PL011 and 16550 UARTs and it appears that both check the current settings before programming the UART. Therefore, please ignore my comment above.

[/SAMI]

regards,

Sami Mujawar

+ __TIME__,
+ __DATE__
+ );
+ SerialPortWrite ((UINT8 *)Buffer, CharCount);

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 2726 bytes --]

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

[Oliver Steffen]

[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]

Hi all,

Is anybody still interested in this?
Rohit's "Invoke constructors for SEC phase" patch was merged.

Link to the original thread:
https://edk2.groups.io/g/devel/message/88884

Regards,
 Oliver

On Thu, Jul 21, 2022 at 6:24 PM Sami Mujawar <sami.mujawar@arm.com> wrote:

> Hi Rebecca, Ard,
>
> On Tue, May 3, 2022 at 12:43 AM, Ard Biesheuvel wrote:
>
> // Data Cache enabled on Primary core when MMU is enabled.
> ArmDisableDataCache ();
> // Invalidate instruction cache
> @@ -90,6 +95,18 @@ CEntryPoint (
>
> // If not primary Jump to Secondary Main
> if (ArmPlatformIsPrimaryCore (MpId)) {
> + // Initialize the Serial Port
> + SerialPortInitialize ();
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "UEFI firmware (version %s built at %a on %a)\n\r",
> + (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
>
> Given that this runs before any library constructors are called, I'd
> be inclined to use FixedPcdGetPtr() here (and [FixedPcd] in the .INFs
> below), to ensure that we don't dereference garbage.
>
> I also wonder whether we need a call to
> ProcessLibraryConstructorList() somewhere.
>
> [SAMI] There is a patch at https://edk2.groups.io/g/devel/message/91113
> which aims to call ProcessLibraryConstructorList(). The idea being that the
> DebugLib constructor would be called which initializes the serial port.
> So, maybe these patches need adjustment so that we avoid calling
> SerialPortInitialize () twice.
> [/SAMI]
>
> regards,
>
> Sami Mujawar
>
>
> + __TIME__,
> + __DATE__
> + );
> + SerialPortWrite ((UINT8 *)Buffer, CharCount);
>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 2732 bytes --]

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

[Rebecca Cran]

I am still interested in it! I'm just currently working through a bit of 
a backlog (next on the list is working on my MP Services patch, since 
I've found it doesn't work on the N2 FVP).

-- 
Rebecca Cran

On 9/21/22 01:03, Oliver Steffen wrote:
> Hi all,
>
> Is anybody still interested in this?
> Rohit's "Invoke constructors for SEC phase" patch was merged.
>
> Link to the original thread:
> https://edk2.groups.io/g/devel/message/88884
>
> Regards,
>  Oliver
>
> On Thu, Jul 21, 2022 at 6:24 PM Sami Mujawar <sami.mujawar@arm.com> wrote:
>
>     Hi Rebecca, Ard,
>
>     On Tue, May 3, 2022 at 12:43 AM, Ard Biesheuvel wrote:
>
>             // Data Cache enabled on Primary core when MMU is enabled.
>             ArmDisableDataCache ();
>             // Invalidate instruction cache
>             @@ -90,6 +95,18 @@ CEntryPoint (
>
>             // If not primary Jump to Secondary Main
>             if (ArmPlatformIsPrimaryCore (MpId)) {
>             + // Initialize the Serial Port
>             + SerialPortInitialize ();
>             + CharCount = AsciiSPrint (
>             + Buffer,
>             + sizeof (Buffer),
>             + "UEFI firmware (version %s built at %a on %a)\n\r",
>             + (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString),
>
>         Given that this runs before any library constructors are
>         called, I'd
>         be inclined to use FixedPcdGetPtr() here (and [FixedPcd] in
>         the .INFs
>         below), to ensure that we don't dereference garbage.
>
>         I also wonder whether we need a call to
>         ProcessLibraryConstructorList() somewhere.
>
>     [SAMI] There is a patch at
>     https://edk2.groups.io/g/devel/message/91113 which aims to call
>     ProcessLibraryConstructorList(). The idea being that the DebugLib
>     constructor would be called which initializes the serial port.
>     So, maybe these patches need adjustment so that we avoid calling
>     SerialPortInitialize () twice.
>     [/SAMI]
>
>     regards,
>
>     Sami Mujawar
>
>
>             + __TIME__,
>             + __DATE__
>             + );
>             + SerialPortWrite ((UINT8 *)Buffer, CharCount);
>
>     
>

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

[Rebecca Cran]

Could I get some reviews on this please?

-- 
Rebecca Cran

On 4/13/22 16:45, Rebecca Cran wrote:
> Copy code from PrePi to PrePeiCore that prints the firmware version
> and build date early in the boot process.
>
> Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
> ---
>   ArmPlatformPkg/PrePeiCore/PrePeiCore.c          | 17 +++++++++++++++++
>   ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf  |  3 +++
>   ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf |  3 +++
>   3 files changed, 23 insertions(+)
>
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.c b/ArmPlatformPkg/PrePeiCore/PrePeiCore.c
> index 6dd9bcdea24f..9e2a0db676b9 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
> @@ -90,6 +95,18 @@ CEntryPoint (
>   
>     // If not primary Jump to Secondary Main
>     if (ArmPlatformIsPrimaryCore (MpId)) {
> +    // Initialize the Serial Port
> +    SerialPortInitialize ();
> +    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);
>       SaveAndSetDebugTimerInterrupt (TRUE);
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> index fb01dd1a113e..83029cf08d1a 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> @@ -57,6 +57,9 @@
>     gEfiTemporaryRamSupportPpiGuid
>     gArmMpCoreInfoPpiGuid
>   
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> +
>   [FeaturePcd]
>     gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
>   
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> index e9eb092d3ac9..9f9ad81586ed 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> @@ -55,6 +55,9 @@
>   [Ppis]
>     gEfiTemporaryRamSupportPpiGuid
>   
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> +
>   [FeaturePcd]
>     gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores
>