* [PATCH 0/3] ArmPlatformPkg, ArmVirtPkg: Add early hello message @ 2022-07-26 7:28 Oliver Steffen 2022-07-26 7:28 ` [PATCH 1/3] ArmPlatformPkg: introduce fixed PCD for " Oliver Steffen ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Oliver Steffen @ 2022-07-26 7:28 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Sami Mujawar, Star Zeng, Andrew Fish, Oliver Steffen Add the ability to print an early hello message independent of debug mask to the serial port when the firmware starts. Introduce a PCD entry to set the message text (ArmPlatformPkg). If the message text is empty (default) then nothing is printed. The message is useful for debugging boot problems, especially with silent firmware builds. It can take some seconds until the first line is printed when booting the firmware, for example when running ArmVirt in Qemu. Use the above in ArmVirtPkg by defining a message text. These changes have already been proposed by Laszlo Ersek in 2015. I am reposting because I find this useful. Example of a VM starting up, AARCH64, Qemu on X64). First line is the new message. (Timestamp in seconds). -VM start- 0000.094 | UEFI firmware starting. 00004.06 | BdsDxe: failed to load Boot0001 "UEFI Misc Device" from VenHw(93E34C7E-B50E-11DF-9223-2443DFD72085,00): Not Found 00004.08 | BdsDxe: loading Boot0002 "UEFI Misc Device 2" from PciRoot(0x0)/Pci(0x2,0x0) 00004.08 | BdsDxe: starting Boot0002 "UEFI Misc Device 2" from PciRoot(0x0)/Pci(0x2,0x0) 00004.11 | System BootOrder not found. Initializing defaults. 00004.11 | Creating boot entry "Boot0005" with label "Red Hat Enterprise Linux" for file "\EFI\redhat\shimaa64.efi" 00004.15 | 00008.39 | EFI stub: Booting Linux Kernel... [...] PR: https://github.com/tianocore/edk2/pull/3140 Signed-off-by: Oliver Steffen <osteffen@redhat.com> # Everything here is editable! You can modify the patch name, author, # date, commit message, and the diff (if --diff was given). # Lines starting with '#' will be ignored, and an empty message # aborts the edit. Laszlo Ersek (3): ArmPlatformPkg: introduce fixed PCD for early hello message ArmPlatformPkg: PrePeiCore: write early hello message to the serial port ArmVirtPkg: set early hello message ArmPlatformPkg/ArmPlatformPkg.dec | 7 +++++++ ArmVirtPkg/ArmVirtQemu.dsc | 1 + ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + ArmPlatformPkg/PrePeiCore/MainMPCore.c | 5 +++++ ArmPlatformPkg/PrePeiCore/MainUniCore.c | 5 +++++ 7 files changed, 23 insertions(+) -- 2.37.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] ArmPlatformPkg: introduce fixed PCD for early hello message 2022-07-26 7:28 [PATCH 0/3] ArmPlatformPkg, ArmVirtPkg: Add early hello message Oliver Steffen @ 2022-07-26 7:28 ` Oliver Steffen 2022-07-26 7:28 ` [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port Oliver Steffen 2022-07-26 7:28 ` [PATCH 3/3] ArmVirtPkg: set early hello message Oliver Steffen 2 siblings, 0 replies; 9+ messages in thread From: Oliver Steffen @ 2022-07-26 7:28 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Sami Mujawar, Star Zeng, Andrew Fish, Laszlo Ersek, Oliver Steffen From: Laszlo Ersek <lersek@redhat.com> Add a PCD for defining a hello message that gets printed to the serial port very early in the boot process, regardless of debug masks. This is useful for debugging boot problems (especially in virtual machines) and informs interactive users that the firmware is running. If a platform doesn't want this feature, it should stick with the default empty string. Singed-off-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- ArmPlatformPkg/ArmPlatformPkg.dec | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec index dd6e78f62aa1..ac726417cbe7 100644 --- a/ArmPlatformPkg/ArmPlatformPkg.dec +++ b/ArmPlatformPkg/ArmPlatformPkg.dec @@ -122,6 +122,13 @@ [PcdsFixedAtBuild.common] ## If set, this will swap settings for HDLCD RED_SELECT and BLUE_SELECT registers gArmPlatformTokenSpaceGuid.PcdArmHdLcdSwapBlueRedSelect|FALSE|BOOLEAN|0x00000045 + # + # Early hello message (ASCII string), printed to the serial port. + # If set to the empty string, nothing is printed. + # Otherwise, a trailing CRLF should be specified explicitly. + # + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage|""|VOID*|0x00000100 + [PcdsFixedAtBuild.common,PcdsDynamic.common] ## PL031 RealTimeClock gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024 -- 2.37.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port 2022-07-26 7:28 [PATCH 0/3] ArmPlatformPkg, ArmVirtPkg: Add early hello message Oliver Steffen 2022-07-26 7:28 ` [PATCH 1/3] ArmPlatformPkg: introduce fixed PCD for " Oliver Steffen @ 2022-07-26 7:28 ` Oliver Steffen 2022-07-26 10:42 ` Sami Mujawar 2022-07-26 7:28 ` [PATCH 3/3] ArmVirtPkg: set early hello message Oliver Steffen 2 siblings, 1 reply; 9+ messages in thread From: Oliver Steffen @ 2022-07-26 7:28 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Sami Mujawar, Star Zeng, Andrew Fish, Laszlo Ersek, Oliver Steffen From: Laszlo Ersek <lersek@redhat.com> Print the early hello message to the serial port. The FixedPcdGetSize() macro expands to an integer constant, therefore an optimizing compiler can eliminate the new code, if the platform DSC doesn't override the empty string (size=1) default of PcdEarlyHelloMessage. Signed-off-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + ArmPlatformPkg/PrePeiCore/MainMPCore.c | 7 +++++++ ArmPlatformPkg/PrePeiCore/MainUniCore.c | 7 +++++++ 5 files changed, 19 insertions(+) diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf index a5b4722459d1..ea7b220bc831 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf @@ -66,6 +66,8 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage + gArmTokenSpaceGuid.PcdGicDistributorBase gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase gArmTokenSpaceGuid.PcdGicSgiIntId diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf index 466a2b01c384..29fb8737cb2f 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf @@ -64,4 +64,6 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h index 0345dd7bdd2a..ae8302becda2 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h @@ -16,6 +16,7 @@ #include <Library/DebugLib.h> #include <Library/IoLib.h> #include <Library/PcdLib.h> +#include <Library/SerialPortLib.h> #include <PiPei.h> #include <Ppi/TemporaryRamSupport.h> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c index b5d0d3a6442f..21c9d5f6da8f 100644 --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c @@ -116,6 +116,13 @@ PrimaryMain ( UINTN TemporaryRamBase; UINTN TemporaryRamSize; + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { + SerialPortWrite ( + FixedPcdGetPtr (PcdEarlyHelloMessage), + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 + ); + } + CreatePpiList (&PpiListSize, &PpiList); // Enable the GIC Distributor diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c index 1c2580eb923b..37560540e14f 100644 --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c @@ -29,6 +29,13 @@ PrimaryMain ( UINTN TemporaryRamBase; UINTN TemporaryRamSize; + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { + SerialPortWrite ( + FixedPcdGetPtr (PcdEarlyHelloMessage), + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 + ); + } + CreatePpiList (&PpiListSize, &PpiList); // Adjust the Temporary Ram as the new Ppi List (Common + Platform Ppi Lists) is created at -- 2.37.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port 2022-07-26 7:28 ` [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port Oliver Steffen @ 2022-07-26 10:42 ` Sami Mujawar 2022-07-26 15:50 ` Oliver Steffen 0 siblings, 1 reply; 9+ messages in thread From: Sami Mujawar @ 2022-07-26 10:42 UTC (permalink / raw) To: Oliver Steffen, devel@edk2.groups.io, Rebecca Cran, quic_rcran@quicinc.com Cc: Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Star Zeng, Andrew Fish, Laszlo Ersek Hi Oliver, There is a patch for review at https://edk2.groups.io/g/devel/message/88884 which prints the firmware version string. I believe with Ard's comments addressed that patch can be merged. Would that patch solve your purpose to print an early message for debug purpose? Regards, Sami Mujawar On 26/07/2022, 08:29, "Oliver Steffen" <osteffen@redhat.com> wrote: From: Laszlo Ersek <lersek@redhat.com> Print the early hello message to the serial port. The FixedPcdGetSize() macro expands to an integer constant, therefore an optimizing compiler can eliminate the new code, if the platform DSC doesn't override the empty string (size=1) default of PcdEarlyHelloMessage. Signed-off-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + ArmPlatformPkg/PrePeiCore/MainMPCore.c | 7 +++++++ ArmPlatformPkg/PrePeiCore/MainUniCore.c | 7 +++++++ 5 files changed, 19 insertions(+) diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf index a5b4722459d1..ea7b220bc831 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf @@ -66,6 +66,8 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage + gArmTokenSpaceGuid.PcdGicDistributorBase gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase gArmTokenSpaceGuid.PcdGicSgiIntId diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf index 466a2b01c384..29fb8737cb2f 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf @@ -64,4 +64,6 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h index 0345dd7bdd2a..ae8302becda2 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h @@ -16,6 +16,7 @@ #include <Library/DebugLib.h> #include <Library/IoLib.h> #include <Library/PcdLib.h> +#include <Library/SerialPortLib.h> #include <PiPei.h> #include <Ppi/TemporaryRamSupport.h> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c index b5d0d3a6442f..21c9d5f6da8f 100644 --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c @@ -116,6 +116,13 @@ PrimaryMain ( UINTN TemporaryRamBase; UINTN TemporaryRamSize; + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { + SerialPortWrite ( + FixedPcdGetPtr (PcdEarlyHelloMessage), + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 + ); + } + CreatePpiList (&PpiListSize, &PpiList); // Enable the GIC Distributor diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c index 1c2580eb923b..37560540e14f 100644 --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c @@ -29,6 +29,13 @@ PrimaryMain ( UINTN TemporaryRamBase; UINTN TemporaryRamSize; + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { + SerialPortWrite ( + FixedPcdGetPtr (PcdEarlyHelloMessage), + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 + ); + } + CreatePpiList (&PpiListSize, &PpiList); // Adjust the Temporary Ram as the new Ppi List (Common + Platform Ppi Lists) is created at -- 2.37.1 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port 2022-07-26 10:42 ` Sami Mujawar @ 2022-07-26 15:50 ` Oliver Steffen 2022-07-26 16:22 ` Sami Mujawar 0 siblings, 1 reply; 9+ messages in thread From: Oliver Steffen @ 2022-07-26 15:50 UTC (permalink / raw) To: Sami Mujawar Cc: devel@edk2.groups.io, Rebecca Cran, quic_rcran@quicinc.com, Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Star Zeng, Andrew Fish, Laszlo Ersek [-- Attachment #1: Type: text/plain, Size: 5575 bytes --] Hi Sami, Thank you for pointing to that patch. It pretty much does the same thing and I would be OK with using it instead of these changes here. Except that the message only shows up in debug mode and seems to depend on the debug mask. It would be nice if it would show up all the time. We are building ArmVirt in debug, but with DEBUG_PRINT_ERROR_LEVEL=0x80000000, aka a "silent" debug build, and need the message there too. What was proposed in this series prints the message independently of the debug mask. Regards, Oliver On Tue, Jul 26, 2022 at 12:42 PM Sami Mujawar <Sami.Mujawar@arm.com> wrote: > Hi Oliver, > > There is a patch for review at > https://edk2.groups.io/g/devel/message/88884 which prints the firmware > version string. I believe with Ard's comments addressed that patch can be > merged. > Would that patch solve your purpose to print an early message for debug > purpose? > > Regards, > > Sami Mujawar > > On 26/07/2022, 08:29, "Oliver Steffen" <osteffen@redhat.com> wrote: > > From: Laszlo Ersek <lersek@redhat.com> > > Print the early hello message to the serial port. > > The FixedPcdGetSize() macro expands to an integer constant, therefore > an > optimizing compiler can eliminate the new code, if the platform DSC > doesn't override the empty string (size=1) default of > PcdEarlyHelloMessage. > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Oliver Steffen <osteffen@redhat.com> > --- > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ > ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + > ArmPlatformPkg/PrePeiCore/MainMPCore.c | 7 +++++++ > ArmPlatformPkg/PrePeiCore/MainUniCore.c | 7 +++++++ > 5 files changed, 19 insertions(+) > > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > index a5b4722459d1..ea7b220bc831 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > @@ -66,6 +66,8 @@ [FixedPcd] > gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize > gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize > > + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage > + > gArmTokenSpaceGuid.PcdGicDistributorBase > gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase > gArmTokenSpaceGuid.PcdGicSgiIntId > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > index 466a2b01c384..29fb8737cb2f 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > @@ -64,4 +64,6 @@ [FixedPcd] > gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize > gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize > > + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage > + > gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > index 0345dd7bdd2a..ae8302becda2 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > @@ -16,6 +16,7 @@ > #include <Library/DebugLib.h> > #include <Library/IoLib.h> > #include <Library/PcdLib.h> > +#include <Library/SerialPortLib.h> > > #include <PiPei.h> > #include <Ppi/TemporaryRamSupport.h> > diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c > b/ArmPlatformPkg/PrePeiCore/MainMPCore.c > index b5d0d3a6442f..21c9d5f6da8f 100644 > --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c > +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c > @@ -116,6 +116,13 @@ PrimaryMain ( > UINTN TemporaryRamBase; > UINTN TemporaryRamSize; > > + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { > + SerialPortWrite ( > + FixedPcdGetPtr (PcdEarlyHelloMessage), > + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 > + ); > + } > + > CreatePpiList (&PpiListSize, &PpiList); > > // Enable the GIC Distributor > diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c > b/ArmPlatformPkg/PrePeiCore/MainUniCore.c > index 1c2580eb923b..37560540e14f 100644 > --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c > +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c > @@ -29,6 +29,13 @@ PrimaryMain ( > UINTN TemporaryRamBase; > UINTN TemporaryRamSize; > > + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { > + SerialPortWrite ( > + FixedPcdGetPtr (PcdEarlyHelloMessage), > + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 > + ); > + } > + > CreatePpiList (&PpiListSize, &PpiList); > > // Adjust the Temporary Ram as the new Ppi List (Common + Platform > Ppi Lists) is created at > -- > 2.37.1 > > > 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: 6865 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port 2022-07-26 15:50 ` Oliver Steffen @ 2022-07-26 16:22 ` Sami Mujawar 2022-07-28 9:53 ` Oliver Steffen 0 siblings, 1 reply; 9+ messages in thread From: Sami Mujawar @ 2022-07-26 16:22 UTC (permalink / raw) To: Oliver Steffen Cc: devel@edk2.groups.io, Rebecca Cran, quic_rcran@quicinc.com, Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Star Zeng, Andrew Fish, Laszlo Ersek, nd [-- Attachment #1: Type: text/plain, Size: 6796 bytes --] Hi Oliver, Both your and Rebecca’s patches are using SerialPortWrite() which should work regardless of the debug print level and for release builds as well. Rebecca’s patch additionally initialises the serial port which is required if the UART has not been setup by any previous firmware. I believe Qemu might handoff with the UART initialised, therefore you might not see the difference. However, this would be required for some hardware platforms. I do not have a setup with Qemu ready to try. Is it possible for you to apply Rebecca’s patch and check, please? Alternatively, I will prepare a setup tomorrow to confirm. Regards, Sami Mujawar From: Oliver Steffen <osteffen@redhat.com> Date: Tuesday, 26 July 2022 at 16:50 To: Sami Mujawar <Sami.Mujawar@arm.com> Cc: "devel@edk2.groups.io" <devel@edk2.groups.io>, Rebecca Cran <rebecca@bsdio.com>, "quic_rcran@quicinc.com" <quic_rcran@quicinc.com>, Ard Biesheuvel <ardb+tianocore@kernel.org>, Chasel Chiu <chasel.chiu@intel.com>, Gerd Hoffmann <kraxel@redhat.com>, Leif Lindholm <quic_llindhol@quicinc.com>, Nate DeSimone <nathaniel.l.desimone@intel.com>, Star Zeng <star.zeng@intel.com>, Andrew Fish <afish@apple.com>, Laszlo Ersek <lersek@redhat.com> Subject: Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port Hi Sami, Thank you for pointing to that patch. It pretty much does the same thing and I would be OK with using it instead of these changes here. Except that the message only shows up in debug mode and seems to depend on the debug mask. It would be nice if it would show up all the time. We are building ArmVirt in debug, but with DEBUG_PRINT_ERROR_LEVEL=0x80000000, aka a "silent" debug build, and need the message there too. What was proposed in this series prints the message independently of the debug mask. Regards, Oliver On Tue, Jul 26, 2022 at 12:42 PM Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>> wrote: Hi Oliver, There is a patch for review at https://edk2.groups.io/g/devel/message/88884 which prints the firmware version string. I believe with Ard's comments addressed that patch can be merged. Would that patch solve your purpose to print an early message for debug purpose? Regards, Sami Mujawar On 26/07/2022, 08:29, "Oliver Steffen" <osteffen@redhat.com<mailto:osteffen@redhat.com>> wrote: From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> Print the early hello message to the serial port. The FixedPcdGetSize() macro expands to an integer constant, therefore an optimizing compiler can eliminate the new code, if the platform DSC doesn't override the empty string (size=1) default of PcdEarlyHelloMessage. Signed-off-by: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> Signed-off-by: Oliver Steffen <osteffen@redhat.com<mailto:osteffen@redhat.com>> --- ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + ArmPlatformPkg/PrePeiCore/MainMPCore.c | 7 +++++++ ArmPlatformPkg/PrePeiCore/MainUniCore.c | 7 +++++++ 5 files changed, 19 insertions(+) diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf index a5b4722459d1..ea7b220bc831 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf @@ -66,6 +66,8 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage + gArmTokenSpaceGuid.PcdGicDistributorBase gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase gArmTokenSpaceGuid.PcdGicSgiIntId diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf index 466a2b01c384..29fb8737cb2f 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf @@ -64,4 +64,6 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h index 0345dd7bdd2a..ae8302becda2 100644 --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h @@ -16,6 +16,7 @@ #include <Library/DebugLib.h> #include <Library/IoLib.h> #include <Library/PcdLib.h> +#include <Library/SerialPortLib.h> #include <PiPei.h> #include <Ppi/TemporaryRamSupport.h> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c b/ArmPlatformPkg/PrePeiCore/MainMPCore.c index b5d0d3a6442f..21c9d5f6da8f 100644 --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c @@ -116,6 +116,13 @@ PrimaryMain ( UINTN TemporaryRamBase; UINTN TemporaryRamSize; + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { + SerialPortWrite ( + FixedPcdGetPtr (PcdEarlyHelloMessage), + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 + ); + } + CreatePpiList (&PpiListSize, &PpiList); // Enable the GIC Distributor diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c b/ArmPlatformPkg/PrePeiCore/MainUniCore.c index 1c2580eb923b..37560540e14f 100644 --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c @@ -29,6 +29,13 @@ PrimaryMain ( UINTN TemporaryRamBase; UINTN TemporaryRamSize; + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { + SerialPortWrite ( + FixedPcdGetPtr (PcdEarlyHelloMessage), + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 + ); + } + CreatePpiList (&PpiListSize, &PpiList); // Adjust the Temporary Ram as the new Ppi List (Common + Platform Ppi Lists) is created at -- 2.37.1 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: 12391 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port 2022-07-26 16:22 ` Sami Mujawar @ 2022-07-28 9:53 ` Oliver Steffen 2022-08-04 13:20 ` [edk2-devel] " Ard Biesheuvel 0 siblings, 1 reply; 9+ messages in thread From: Oliver Steffen @ 2022-07-28 9:53 UTC (permalink / raw) To: Sami Mujawar Cc: devel@edk2.groups.io, Rebecca Cran, quic_rcran@quicinc.com, Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Star Zeng, Andrew Fish, Laszlo Ersek, nd [-- Attachment #1: Type: text/plain, Size: 7384 bytes --] Hi everyone, I checked Rebecca's patch with my Qemu use case. All fine - we can use that patch instead of the series I posted here. Even better, because it prints the version information. Thanks! Regards, Oliver On Tue, Jul 26, 2022 at 6:22 PM Sami Mujawar <Sami.Mujawar@arm.com> wrote: > Hi Oliver, > > > > Both your and Rebecca’s patches are using SerialPortWrite() which should > work regardless of the debug print level and for release builds as well. > > Rebecca’s patch additionally initialises the serial port which is required > if the UART has not been setup by any previous firmware. > > I believe Qemu might handoff with the UART initialised, therefore you > might not see the difference. However, this would be required for some > hardware platforms. > > > > I do not have a setup with Qemu ready to try. Is it possible for you to > apply Rebecca’s patch and check, please? > > Alternatively, I will prepare a setup tomorrow to confirm. > > > > Regards, > > > > Sami Mujawar > > > > *From: *Oliver Steffen <osteffen@redhat.com> > *Date: *Tuesday, 26 July 2022 at 16:50 > *To: *Sami Mujawar <Sami.Mujawar@arm.com> > *Cc: *"devel@edk2.groups.io" <devel@edk2.groups.io>, Rebecca Cran < > rebecca@bsdio.com>, "quic_rcran@quicinc.com" <quic_rcran@quicinc.com>, > Ard Biesheuvel <ardb+tianocore@kernel.org>, Chasel Chiu < > chasel.chiu@intel.com>, Gerd Hoffmann <kraxel@redhat.com>, Leif Lindholm < > quic_llindhol@quicinc.com>, Nate DeSimone <nathaniel.l.desimone@intel.com>, > Star Zeng <star.zeng@intel.com>, Andrew Fish <afish@apple.com>, Laszlo > Ersek <lersek@redhat.com> > *Subject: *Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello > message to the serial port > > > > Hi Sami, > > Thank you for pointing to that patch. It pretty much does the same thing > and I would be OK with using it instead of these changes here. Except > that the message only shows up in debug mode and seems to depend on the > debug mask. It would be nice if it would show up all the time. > > We are building ArmVirt in debug, but with > DEBUG_PRINT_ERROR_LEVEL=0x80000000, aka a "silent" debug build, and need > the message there too. > > What was proposed in this series prints the message independently of the > debug mask. > > > Regards, > Oliver > > > > On Tue, Jul 26, 2022 at 12:42 PM Sami Mujawar <Sami.Mujawar@arm.com> > wrote: > > Hi Oliver, > > There is a patch for review at > https://edk2.groups.io/g/devel/message/88884 which prints the firmware > version string. I believe with Ard's comments addressed that patch can be > merged. > Would that patch solve your purpose to print an early message for debug > purpose? > > Regards, > > Sami Mujawar > > On 26/07/2022, 08:29, "Oliver Steffen" <osteffen@redhat.com> wrote: > > From: Laszlo Ersek <lersek@redhat.com> > > Print the early hello message to the serial port. > > The FixedPcdGetSize() macro expands to an integer constant, therefore > an > optimizing compiler can eliminate the new code, if the platform DSC > doesn't override the empty string (size=1) default of > PcdEarlyHelloMessage. > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Oliver Steffen <osteffen@redhat.com> > --- > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ > ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + > ArmPlatformPkg/PrePeiCore/MainMPCore.c | 7 +++++++ > ArmPlatformPkg/PrePeiCore/MainUniCore.c | 7 +++++++ > 5 files changed, 19 insertions(+) > > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > index a5b4722459d1..ea7b220bc831 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > @@ -66,6 +66,8 @@ [FixedPcd] > gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize > gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize > > + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage > + > gArmTokenSpaceGuid.PcdGicDistributorBase > gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase > gArmTokenSpaceGuid.PcdGicSgiIntId > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > index 466a2b01c384..29fb8737cb2f 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > @@ -64,4 +64,6 @@ [FixedPcd] > gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize > gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize > > + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage > + > gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > index 0345dd7bdd2a..ae8302becda2 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > @@ -16,6 +16,7 @@ > #include <Library/DebugLib.h> > #include <Library/IoLib.h> > #include <Library/PcdLib.h> > +#include <Library/SerialPortLib.h> > > #include <PiPei.h> > #include <Ppi/TemporaryRamSupport.h> > diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c > b/ArmPlatformPkg/PrePeiCore/MainMPCore.c > index b5d0d3a6442f..21c9d5f6da8f 100644 > --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c > +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c > @@ -116,6 +116,13 @@ PrimaryMain ( > UINTN TemporaryRamBase; > UINTN TemporaryRamSize; > > + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { > + SerialPortWrite ( > + FixedPcdGetPtr (PcdEarlyHelloMessage), > + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 > + ); > + } > + > CreatePpiList (&PpiListSize, &PpiList); > > // Enable the GIC Distributor > diff --git a/ArmPlatformPkg/PrePeiCore/MainUniCore.c > b/ArmPlatformPkg/PrePeiCore/MainUniCore.c > index 1c2580eb923b..37560540e14f 100644 > --- a/ArmPlatformPkg/PrePeiCore/MainUniCore.c > +++ b/ArmPlatformPkg/PrePeiCore/MainUniCore.c > @@ -29,6 +29,13 @@ PrimaryMain ( > UINTN TemporaryRamBase; > UINTN TemporaryRamSize; > > + if (FixedPcdGetSize (PcdEarlyHelloMessage) > 1) { > + SerialPortWrite ( > + FixedPcdGetPtr (PcdEarlyHelloMessage), > + FixedPcdGetSize (PcdEarlyHelloMessage) - 1 > + ); > + } > + > CreatePpiList (&PpiListSize, &PpiList); > > // Adjust the Temporary Ram as the new Ppi List (Common + Platform > Ppi Lists) is created at > -- > 2.37.1 > > > 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: 12124 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port 2022-07-28 9:53 ` Oliver Steffen @ 2022-08-04 13:20 ` Ard Biesheuvel 0 siblings, 0 replies; 9+ messages in thread From: Ard Biesheuvel @ 2022-08-04 13:20 UTC (permalink / raw) To: edk2-devel-groups-io, Oliver Steffen Cc: Sami Mujawar, Rebecca Cran, quic_rcran@quicinc.com, Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Star Zeng, Andrew Fish, Laszlo Ersek, nd On Thu, 28 Jul 2022 at 11:53, Oliver Steffen <osteffen@redhat.com> wrote: > > Hi everyone, > > I checked Rebecca's patch with my Qemu use case. > All fine - we can use that patch instead of the series I posted here. > Even better, because it prints the version information. > > Thanks! > Thanks for confirming. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ArmVirtPkg: set early hello message 2022-07-26 7:28 [PATCH 0/3] ArmPlatformPkg, ArmVirtPkg: Add early hello message Oliver Steffen 2022-07-26 7:28 ` [PATCH 1/3] ArmPlatformPkg: introduce fixed PCD for " Oliver Steffen 2022-07-26 7:28 ` [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port Oliver Steffen @ 2022-07-26 7:28 ` Oliver Steffen 2 siblings, 0 replies; 9+ messages in thread From: Oliver Steffen @ 2022-07-26 7:28 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Chasel Chiu, Gerd Hoffmann, Leif Lindholm, Nate DeSimone, Sami Mujawar, Star Zeng, Andrew Fish, Laszlo Ersek, Oliver Steffen From: Laszlo Ersek <lersek@redhat.com> Set the text for the early hello message in ArmVirtPkg. This prints a friendly banner on QEMU, regardless of debug mask settings. Signed-off-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Oliver Steffen <osteffen@redhat.com> --- ArmVirtPkg/ArmVirtQemu.dsc | 1 + 1 file changed, 1 insertion(+) diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index 9369a88858fd..eaa784c1573a 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -135,6 +135,7 @@ [PcdsFeatureFlag.common] gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE) [PcdsFixedAtBuild.common] + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage|"UEFI firmware starting.\r\n" !if $(ARCH) == AARCH64 gArmTokenSpaceGuid.PcdVFPEnabled|1 !endif -- 2.37.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-04 13:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-26 7:28 [PATCH 0/3] ArmPlatformPkg, ArmVirtPkg: Add early hello message Oliver Steffen 2022-07-26 7:28 ` [PATCH 1/3] ArmPlatformPkg: introduce fixed PCD for " Oliver Steffen 2022-07-26 7:28 ` [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port Oliver Steffen 2022-07-26 10:42 ` Sami Mujawar 2022-07-26 15:50 ` Oliver Steffen 2022-07-26 16:22 ` Sami Mujawar 2022-07-28 9:53 ` Oliver Steffen 2022-08-04 13:20 ` [edk2-devel] " Ard Biesheuvel 2022-07-26 7:28 ` [PATCH 3/3] ArmVirtPkg: set early hello message Oliver Steffen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox