From f4bbef1dd7c15bec5ac164802842d55d740c36fe Mon Sep 17 00:00:00 2001 From: Luigi Leonardi Date: Wed, 17 Jun 2026 13:48:39 +0200 Subject: [PATCH] ArmVirtPkg: introduce compile and runtime control of serial debug log level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new PCD `PcdSerialDebugPrintErrorLevel` that overrides the verbosity set in `DebugPrintErrorLevel` for the serial port debug output and does not affect memory debug logging. Accepted values are: - "silent": DEBUG_ERROR only - "verbose": use the `DebugPrintErrorLevel` value. - a hex bitmask (e.g. "0x80000040"). This PCD value can be overridden at runtime using the fw_cfg entry "opt/org.tianocore/DebugLevel" without rebuilding the firmware. The runtime override takes priority over the compile-time one. When neither is set, the serial output uses the compiled-in PcdDebugPrintErrorLevel. SEC and PEI_CORE phases do not support this override because they run from flash, where global variables are not available. Supporting it would require parsing the device tree on every debug write to locate the fw_cfg device and read the DebugLevel value — unnecessary overhead given the low volume of logs in those phases. ParseSerialDebugLevel() is duplicated as a STATIC function in Flash.c and PlatformPeiLib.c rather than shared via a header, because EDK2 coding style forbids function definitions in headers. A dedicated library class would be excessive for a small helper with only two consumers. Example QEMU command line: -fw_cfg name=opt/org.tianocore/DebugLevel,string=silent -fw_cfg name=opt/org.tianocore/DebugLevel,string=verbose -fw_cfg name=opt/org.tianocore/DebugLevel,string=0x80000000 Suggested-by: Gerd Hoffmann Signed-off-by: Luigi Leonardi --- ArmVirtPkg/ArmVirtCloudHv.dsc | 2 + ArmVirtPkg/ArmVirtPkg.dec | 5 ++ ArmVirtPkg/ArmVirtQemuKernel.dsc | 3 + .../Include/Guid/EarlyPL011BaseAddress.h | 11 ++- .../Library/DebugLibFdtPL011Uart/DebugLib.c | 28 +++++-- .../DebugLibFdtPL011UartFlash.inf | 1 + .../Library/DebugLibFdtPL011Uart/Flash.c | 84 +++++++++++++++++++ ArmVirtPkg/Library/DebugLibFdtPL011Uart/Ram.c | 40 +++++++++ .../Library/DebugLibFdtPL011Uart/Write.h | 14 ++++ .../Library/PlatformPeiLib/PlatformPeiLib.c | 72 ++++++++++++++++ .../Library/PlatformPeiLib/PlatformPeiLib.inf | 3 + 11 files changed, 256 insertions(+), 7 deletions(-) diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc index d76b17c602..122e0e8f6d 100644 --- a/ArmVirtPkg/ArmVirtCloudHv.dsc +++ b/ArmVirtPkg/ArmVirtCloudHv.dsc @@ -46,6 +46,8 @@ TpmPlatformHierarchyLib|SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHierarchyLib.inf ArmTransferListLib|ArmPkg/Library/ArmTransferListLib/ArmTransferListLib.inf + QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf + QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf !include MdePkg/MdeLibs.dsc.inc diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec index a05958ec64..8386ec0ad8 100644 --- a/ArmVirtPkg/ArmVirtPkg.dec +++ b/ArmVirtPkg/ArmVirtPkg.dec @@ -51,3 +51,8 @@ # Cloud Hypervisor has no other way to pass Rsdp address to the guest except use a PCD. # gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x00000005 + + ## Serial debug log level override. Accepts "silent", "verbose", or a + # hex bitmask (e.g. "0x80000040"). "verbose" falls back to the + # compiled-in PcdDebugPrintErrorLevel. + gArmVirtTokenSpaceGuid.PcdSerialDebugPrintErrorLevel|"verbose"|VOID*|0x00000006 diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 703eae9c07..7fdb3b55b9 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -75,6 +75,9 @@ ArmMonitorLib|ArmVirtPkg/Library/ArmVirtMonitorLib/ArmVirtMonitorLib.inf +[LibraryClasses.common.SEC] + QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf + [LibraryClasses.common.DXE_DRIVER] AcpiPlatformLib|OvmfPkg/Library/AcpiPlatformLib/DxeAcpiPlatformLib.inf ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf diff --git a/ArmVirtPkg/Include/Guid/EarlyPL011BaseAddress.h b/ArmVirtPkg/Include/Guid/EarlyPL011BaseAddress.h index 2857a3b3c2..e56e7e3370 100644 --- a/ArmVirtPkg/Include/Guid/EarlyPL011BaseAddress.h +++ b/ArmVirtPkg/Include/Guid/EarlyPL011BaseAddress.h @@ -21,9 +21,16 @@ typedef struct { // // for SerialPortLib and console IO // - UINT64 ConsoleAddress; + UINT64 ConsoleAddress; // // for DebugLib; may equal ConsoleAddress if there's only one PL011 UART // - UINT64 DebugAddress; + UINT64 DebugAddress; + // + // Serial debug log level override from fw_cfg "opt/org.tianocore/DebugLevel" + // or PcdSerialDebugPrintErrorLevel. DebugLevelSet is TRUE when an override + // is active; DebugLevel contains the parsed bitmask. + // + BOOLEAN DebugLevelSet; + UINT32 DebugLevel; } EARLY_PL011_BASE_ADDRESS; diff --git a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLib.c b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLib.c index 18222aaebc..128db7c754 100644 --- a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLib.c +++ b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLib.c @@ -94,17 +94,28 @@ DebugPrintMarker ( IN BASE_LIST BaseListMarker ) { - CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; + UINT32 SerialDebugLevel; + UINT32 DebugLevel; + EFI_STATUS Status; // // If Format is NULL, then ASSERT(). // ASSERT (Format != NULL); + DebugLevel = GetDebugPrintErrorLevel (); + + Status = GetSerialDebugPrintErrorLevel (&SerialDebugLevel); + if (EFI_ERROR (Status)) { + SerialDebugLevel = DebugLevel; + } + // - // Check driver debug mask value and global mask + // SerialDebugLevel can be more verbose than DebugLevel. + // Check if we have something to print // - if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) { + if (((ErrorLevel & DebugLevel) == 0) && ((ErrorLevel & SerialDebugLevel) == 0)) { return; } @@ -118,12 +129,19 @@ DebugPrintMarker ( } // - // Send string to Memory Debug Log if enabled + // Send string to Memory Debug Log if enabled and error level matches // - if (MemDebugLogEnabled ()) { + if (((ErrorLevel & DebugLevel) != 0) && MemDebugLogEnabled ()) { MemDebugLogWrite ((CHAR8 *)Buffer, AsciiStrLen (Buffer)); } + // + // Check runtime debug mask value and global mask + // + if ((ErrorLevel & SerialDebugLevel) == 0) { + return; + } + // // Send the print string to a Serial Port // diff --git a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLibFdtPL011UartFlash.inf b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLibFdtPL011UartFlash.inf index 6b3b3316a2..9951edcd20 100644 --- a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLibFdtPL011UartFlash.inf +++ b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/DebugLibFdtPL011UartFlash.inf @@ -50,6 +50,7 @@ [FixedPcd] gArmPlatformTokenSpaceGuid.PL011UartClkInHz + gArmVirtTokenSpaceGuid.PcdSerialDebugPrintErrorLevel gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity diff --git a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Flash.c b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Flash.c index a624e0860d..7419fa0dc4 100644 --- a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Flash.c +++ b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Flash.c @@ -6,12 +6,64 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include +#include #include #include #include #include "Write.h" +// +// Duplicated in ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c — +// keep both copies in sync. +// + +/** + Parse a serial debug level string. + + Accepted values are "silent" (DEBUG_ERROR only), "verbose" (no + override), or a hex bitmask (e.g. "0x80000040"). + + @param[in] String NUL-terminated ASCII string to parse. + @param[out] DebugLevel On success, the parsed debug level bitmask. + + @retval TRUE String was recognised; *DebugLevel is valid. + @retval FALSE String is NULL, "verbose", or unrecognised; no override. +**/ +STATIC +BOOLEAN +ParseSerialDebugLevel ( + IN CONST CHAR8 *String, + OUT UINT32 *DebugLevel + ) +{ + UINT64 Value; + CHAR8 *End; + + if ((String == NULL) || (DebugLevel == NULL)) { + return FALSE; + } + + if (AsciiStrCmp (String, "silent") == 0) { + *DebugLevel = DEBUG_ERROR; + return TRUE; + } + + if (AsciiStrCmp (String, "verbose") == 0) { + return FALSE; + } + + if (!EFI_ERROR (AsciiStrHexToUint64S (String, &End, &Value)) && + (*End == '\0')) + { + *DebugLevel = (UINT32)Value; + return TRUE; + } + + return FALSE; +} + /** (Copied from SerialPortWrite() in "MdePkg/Include/Library/SerialPortLib.h" at commit c4547aefb3d0, with the Buffer non-nullity assertion removed:) @@ -105,3 +157,35 @@ DebugLibFdtPL011UartWrite ( return PL011UartWrite ((UINTN)DebugAddress, Buffer, NumberOfBytes); } + +/** + Retrieve the serial debug print error level override. + + The flash variant parses the compile-time PCD only. The fw_cfg + override is not available in SEC/PEI_CORE phases. + + @param[out] Value On success, the debug log level bitmask. + + @retval EFI_SUCCESS The debug level was retrieved successfully. + @retval EFI_INVALID_PARAMETER Value is NULL. + @retval EFI_NOT_FOUND No override is configured (e.g. "verbose"). +**/ +EFI_STATUS +GetSerialDebugPrintErrorLevel ( + OUT UINT32 *Value + ) +{ + CONST CHAR8 *String; + + if (Value == NULL) { + return EFI_INVALID_PARAMETER; + } + + String = (CONST CHAR8 *)PcdGetPtr (PcdSerialDebugPrintErrorLevel); + + if (ParseSerialDebugLevel (String, Value)) { + return EFI_SUCCESS; + } + + return EFI_NOT_FOUND; +} diff --git a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Ram.c b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Ram.c index bc5be015bd..1d923a0f87 100644 --- a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Ram.c +++ b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Ram.c @@ -20,6 +20,8 @@ UINTN mDebugLibFdtPL011UartAddress; RETURN_STATUS mDebugLibFdtPL011UartPermanentStatus = RETURN_SUCCESS; +BOOLEAN mSerialDebugLevelSet; +UINT32 mSerialDebugLevel; /** Statefully initialize both the library instance and the debug PL011 UART. @@ -62,6 +64,11 @@ Initialize ( goto Failed; } + if (UartBase->DebugLevelSet) { + mSerialDebugLevelSet = TRUE; + mSerialDebugLevel = UartBase->DebugLevel; + } + BaudRate = (UINTN)PcdGet64 (PcdUartDefaultBaudRate); ReceiveFifoDepth = 0; // Use the default value for Fifo depth Parity = (EFI_PARITY_TYPE)PcdGet8 (PcdUartDefaultParity); @@ -122,3 +129,36 @@ DebugLibFdtPL011UartWrite ( return PL011UartWrite (mDebugLibFdtPL011UartAddress, Buffer, NumberOfBytes); } + +/** + Retrieve the serial debug print error level override. + + @param[out] Value On success, the debug log level bitmask. + + @retval EFI_SUCCESS The debug level was retrieved successfully. + @retval EFI_INVALID_PARAMETER Value is NULL. + @retval EFI_NOT_FOUND The debug level is not available. +**/ +EFI_STATUS +GetSerialDebugPrintErrorLevel ( + OUT UINT32 *Value + ) +{ + RETURN_STATUS Status; + + if (Value == NULL) { + return EFI_INVALID_PARAMETER; + } + + Status = Initialize (); + if (RETURN_ERROR (Status)) { + return EFI_NOT_FOUND; + } + + if (mSerialDebugLevelSet) { + *Value = mSerialDebugLevel; + return EFI_SUCCESS; + } + + return EFI_NOT_FOUND; +} diff --git a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Write.h b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Write.h index b57861f67c..074d1ac448 100644 --- a/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Write.h +++ b/ArmVirtPkg/Library/DebugLibFdtPL011Uart/Write.h @@ -34,3 +34,17 @@ DebugLibFdtPL011UartWrite ( IN UINT8 *Buffer, IN UINTN NumberOfBytes ); + +/** + Retrieve the serial debug print error level override. + + @param[out] Value On success, the debug log level bitmask. + + @retval EFI_SUCCESS The debug level was retrieved successfully. + @retval EFI_INVALID_PARAMETER Value is NULL. + @retval EFI_NOT_FOUND The debug level is not available. +**/ +EFI_STATUS +GetSerialDebugPrintErrorLevel ( + OUT UINT32 *Value + ); diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c index 7d3f8c4c86..e039810dc1 100644 --- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c +++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -17,10 +18,61 @@ #include #include #include +#include #include #include +// +// Duplicated in ArmVirtPkg/Library/DebugLibFdtPL011Uart/Flash.c — +// keep both copies in sync. +// + +/** + Parse a serial debug level string. + + Accepted values are "silent" (DEBUG_ERROR only), "verbose" (no + override), or a hex bitmask (e.g. "0x80000040"). + + @param[in] String NUL-terminated ASCII string to parse. + @param[out] DebugLevel On success, the parsed debug level bitmask. + + @retval TRUE String was recognised; *DebugLevel is valid. + @retval FALSE String is NULL, "verbose", or unrecognised; no override. +**/ +STATIC +BOOLEAN +ParseSerialDebugLevel ( + IN CONST CHAR8 *String, + OUT UINT32 *DebugLevel + ) +{ + UINT64 Value; + CHAR8 *End; + + if ((String == NULL) || (DebugLevel == NULL)) { + return FALSE; + } + + if (AsciiStrCmp (String, "silent") == 0) { + *DebugLevel = DEBUG_ERROR; + return TRUE; + } + + if (AsciiStrCmp (String, "verbose") == 0) { + return FALSE; + } + + if (!EFI_ERROR (AsciiStrHexToUint64S (String, &End, &Value)) && + (*End == '\0')) + { + *DebugLevel = (UINT32)Value; + return TRUE; + } + + return FALSE; +} + STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpm2DiscoveredPpi = { EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST, &gOvmfTpmDiscoveredPpiGuid, @@ -56,6 +108,9 @@ PlatformPeim ( CONST UINT32 *RangesProp; UINT64 TpmBase; EFI_STATUS Status; + CHAR8 DebugLevelBuf[32]; + UINTN DebugLevelBufSize; + CONST CHAR8 *DebugLevelStr; Base = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress); ASSERT (Base != NULL); @@ -124,6 +179,23 @@ PlatformPeim ( )); } + DebugLevelBufSize = sizeof (DebugLevelBuf); + Status = QemuFwCfgGetAsString ( + "opt/org.tianocore/DebugLevel", + &DebugLevelBufSize, + DebugLevelBuf + ); + if (!RETURN_ERROR (Status)) { + DebugLevelStr = DebugLevelBuf; + } else { + DebugLevelStr = (CONST CHAR8 *)PcdGetPtr (PcdSerialDebugPrintErrorLevel); + } + + UartHobData->DebugLevelSet = ParseSerialDebugLevel ( + DebugLevelStr, + &UartHobData->DebugLevel + ); + TpmBase = 0; // diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf index d6c1b135c2..731d940333 100644 --- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf +++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf @@ -30,6 +30,7 @@ gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled [LibraryClasses] + BaseLib BaseMemoryLib DebugLib HobLib @@ -37,9 +38,11 @@ FdtSerialPortAddressLib PcdLib PeiServicesLib + QemuFwCfgSimpleParserLib [FixedPcd] gArmTokenSpaceGuid.PcdFvSize + gArmVirtTokenSpaceGuid.PcdSerialDebugPrintErrorLevel gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeAllocationPadding [Pcd]