Reviewed-by: Guo Dong <guo.d...@intel.com>
-----Original Message-----
From: Benjamin Doron <benjamin.doro...@gmail.com>
Sent: Sunday, June 5, 2022 5:55 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.d...@intel.com>; Ni, Ray <ray...@intel.com>; Maurice Ma
<maurice...@intel.com>; You, Benjamin <benjamin....@intel.com>; Rhodes, Sean
<sean@starlabs.systems>
Subject: [PATCH v3] UefiPayloadPkg: Add support for logging to CBMEM console
Writes TianoCore debug logs into the CBMEM console ringbuffer, from where the
user can retrieve them with the `cbmem` userspace utility.
The intention is to aid in debugging non-fatal issues even in release builds,
or simply make TianoCore's logs available to those interested.
Consequently, MDEPKG_NDEBUG must be masked. As an in-memory debug logging
library, ASSERTs must be non-fatal to be seen, so they neither dead-loop nor
create a breakpoint. It is assumed that ASSERT() neither enforces fatal
conditions nor security integrity, as release builds do not call DebugAssert()
from the ASSERT macro.
More detailed debug logs are produced with the DEBUG_CODE macro, but this
guards other debug-related code throughout the codebase. To avoid changing
behaviour on release builds, this is only set for debug builds.
Tested on QEMU, dumping the appropriate memory region in the UEFI shell shows
the TianoCore log. An improved revision of the debug library used in several
coreboot-related EDK2 forks, including MrChromebox's.
Previous revisions also tested on an Acer Aspire VN7-572G laptop.
Cc: Guo Dong <guo.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Maurice Ma <maurice...@intel.com>
Cc: Benjamin You <benjamin....@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Benjamin Doron <benjamin.doro...@gmail.com>
---
UefiPayloadPkg/Include/Coreboot.h | 13 +
UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c | 264
++++++++++++++++++++
UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf | 30 +++
UefiPayloadPkg/UefiPayloadPkg.dsc | 25 +-
4 files changed, 327 insertions(+), 5 deletions(-)
diff --git a/UefiPayloadPkg/Include/Coreboot.h
b/UefiPayloadPkg/Include/Coreboot.h
index a3e1109fe84e..5892fae7682b 100644
--- a/UefiPayloadPkg/Include/Coreboot.h
+++ b/UefiPayloadPkg/Include/Coreboot.h
@@ -199,6 +199,13 @@ struct cb_forward {
UINT64 forward; }; +struct cb_cbmem_ref {+ UINT32 tag;+ // Field
contains size of this struct = 0x0010+ UINT32 size;+ UINT64
cbmem_addr;+};+ #define CB_TAG_FRAMEBUFFER 0x0012 struct cb_framebuffer {
UINT32 tag;@@ -229,6 +236,12 @@ struct cb_vdat {
#define CB_TAG_TIMESTAMPS 0x0016 #define CB_TAG_CBMEM_CONSOLE
0x0017+struct cbmem_console {+ UINT32 size;+ UINT32 cursor;+ UINT8
body[0];+} __attribute__((packed));+ #define CB_TAG_MRC_CACHE 0x0018
struct cb_cbmem_tab { UINT32 tag;diff --git
a/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c
b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c
new file mode 100644
index 000000000000..863fbcfef2f0
--- /dev/null
+++ b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c
@@ -0,0 +1,264 @@
+/** @file+ CBMEM console SerialPortLib instance++ Copyright (c) 2022, Baruch
Binyamin Doron+ SPDX-License-Identifier: BSD-2-Clause-Patent++**/++#include
<Base.h>+#include <Coreboot.h>++#include <Library/BaseMemoryLib.h>+#include
<Library/BlParseLib.h>+#include <Library/SerialPortLib.h>++// Upper nibble
contains flags+#define CBMC_CURSOR_MASK ((1 << 28) - 1)+#define CBMC_OVERFLOW
(1 << 31)++STATIC struct cbmem_console *mCbConsole = NULL;++/**+ Find
coreboot record with given Tag.+ NOTE: This coreboot-specific function
definition is absent+ from the common BlParseLib header.++ @param Tag
The tag id to be found++ @retval NULL The Tag is
not found.+ @retval Others The pointer to the record
found.++**/+VOID *+FindCbTag (+ IN UINT32 Tag+ );++/**+ Initialize
the serial device hardware.++ If no initialization is required, then return
RETURN_SUCCESS.+ If the serial device was successfully initialized, then
return RETURN_SUCCESS.+ If the serial device could not be initialized, then
return RETURN_DEVICE_ERROR.++ @retval RETURN_SUCCESS The serial device
was initialized.+ @retval RETURN_DEVICE_ERROR The serial device could not be
initialized.++**/+RETURN_STATUS+EFIAPI+SerialPortInitialize (+ VOID+ )+{+ //
The coreboot table contains large structures as references+ struct
cb_cbmem_ref *cbref = FindCbTag(CB_TAG_CBMEM_CONSOLE);+ if (!cbref) {+
return RETURN_DEVICE_ERROR;+ }++ mCbConsole = (VOID
*)(UINTN)cbref->cbmem_addr; // Support PEI and DXE+ if (mCbConsole == NULL)
{+ return RETURN_DEVICE_ERROR;+ }++ return RETURN_SUCCESS;+}++/**+ Write
data from buffer to serial device.++ Writes NumberOfBytes data bytes from
Buffer to the serial device.+ The number of bytes actually written to the
serial device is returned.+ If the return value is less than NumberOfBytes,
then the write operation failed.+ If Buffer is NULL, then ASSERT().+ If
NumberOfBytes is zero, then return 0.++ @param Buffer Pointer to
the data buffer to be written.+ @param NumberOfBytes Number of bytes to
written to the serial device.++ @retval 0 NumberOfBytes is 0.+
@retval >0 The number of bytes written to the serial device.+
If this value is less than NumberOfBytes, then the write
operation failed.++**/+UINTN+EFIAPI+SerialPortWrite (+ IN UINT8 *Buffer,+
IN UINTN NumberOfBytes+ )+{+ UINT32 cursor;+ UINT32
flags;++ if (Buffer == NULL || NumberOfBytes == 0) {+ return 0;+ }++ if
(!mCbConsole) {+ return 0;+ }++ cursor = mCbConsole->cursor &
CBMC_CURSOR_MASK;+ flags = mCbConsole->cursor & ~CBMC_CURSOR_MASK;+ if
(cursor >= mCbConsole->size) {+ // Already overflowed; bail out. TODO: Is
this unnecessarily cautious?+ // - Supports old coreboot version with legacy
overflow mechanism.+ return 0;+ }++ if (cursor + NumberOfBytes >
mCbConsole->size) {+ // Will overflow, zero cursor and set flag.+ cursor
= 0;+ flags |= CBMC_OVERFLOW;+ }++ if (NumberOfBytes > mCbConsole->size)
{+ // This one debug message is longer than the entire buffer. Truncate it.+
// - TODO: Is this unnecessarily cautious?+ NumberOfBytes =
mCbConsole->size;+ }++ CopyMem (&mCbConsole->body[cursor], Buffer,
NumberOfBytes);+ cursor += NumberOfBytes;++ if (cursor == mCbConsole->size)
{+ // Next message will overflow, zero cursor.+ // - Set flag
preemptively. This could not be determined later.+ cursor = 0;+ flags |=
CBMC_OVERFLOW;+ }++ mCbConsole->cursor = flags | cursor;++ return
NumberOfBytes;+}++/**+ Read data from serial device and save the datas in
buffer.++ Reads NumberOfBytes data bytes from a serial device into the buffer+
specified by Buffer. The number of bytes actually read is returned.+ If
Buffer is NULL, then ASSERT().+ If NumberOfBytes is zero, then return 0.++
@param Buffer Pointer to the data buffer to store the data read from
the serial device.+ @param NumberOfBytes Number of bytes which will be
read.++ @retval 0 Read data failed, no data is to be read.+
@retval >0 Actual number of bytes read from serial
device.++**/+UINTN+EFIAPI+SerialPortRead (+ OUT UINT8 *Buffer,+ IN UINTN
NumberOfBytes+)+{+ return 0;+}++/**+ Polls a serial device to see if
there is any data waiting to be read.++ @retval TRUE Data is
waiting to be read from the serial device.+ @retval FALSE There is
no data waiting to be read from the serial
device.++**/+BOOLEAN+EFIAPI+SerialPortPoll (+ VOID+ )+{+ return
FALSE;+}++/**+ Sets the control bits on a serial device.++ @param Control
Sets the bits of Control that are settable.++ @retval
RETURN_SUCCESS The new control bits were set on the serial device.+
@retval RETURN_UNSUPPORTED The serial device does not support this
operation.+ @retval RETURN_DEVICE_ERROR The serial device is not functioning
correctly.++**/+RETURN_STATUS+EFIAPI+SerialPortSetControl (+ IN UINT32
Control+ )+{+ return RETURN_UNSUPPORTED;+}++/**+ Retrieve the status of the
control bits on a serial device.++ @param Control A pointer to
return the current control signals from the serial device.++ @retval
RETURN_SUCCESS The control bits were read from the serial device.+
@retval RETURN_UNSUPPORTED The serial device does not support this
operation.+ @retval RETURN_DEVICE_ERROR The serial device is not functioning
correctly.++**/+RETURN_STATUS+EFIAPI+SerialPortGetControl (+ OUT UINT32
*Control+ )+{+ return RETURN_UNSUPPORTED;+}++/**+ Sets the baud rate,
receive FIFO depth, transmit/receive time out, parity,+ data bits, and stop
bits on a serial device.++ @param BaudRate The requested baud rate.
A BaudRate value of 0 will use the+ device's default
interface speed.+ On output, the value actually
set.+ @param ReceiveFifoDepth The requested depth of the FIFO on the receive
side of the+ serial interface. A ReceiveFifoDepth
value of 0 will use+ the device's default FIFO
depth.+ On output, the value actually set.+ @param
Timeout The requested time out for a single character in
microseconds.+ This timeout applies to both the
transmit and receive side of the+ interface. A
Timeout value of 0 will use the device's default time+
out value.+ On output, the value actually set.+
@param Parity The type of parity to use on this serial device. A
Parity value of+ DefaultParity will use the device's
default parity value.+ On output, the value actually
set.+ @param DataBits The number of data bits to use on the serial
device. A DataBits+ value of 0 will use the device's
default data bit setting.+ On output, the value
actually set.+ @param StopBits The number of stop bits to use on
this serial device. A StopBits+ value of
DefaultStopBits will use the device's default number of+
stop bits.+ On output, the value actually set.++
@retval RETURN_SUCCESS The new attributes were set on the serial
device.+ @retval RETURN_UNSUPPORTED The serial device does not support
this operation.+ @retval RETURN_INVALID_PARAMETER One or more of the
attributes has an unsupported value.+ @retval RETURN_DEVICE_ERROR The
serial device is not functioning
correctly.++**/+RETURN_STATUS+EFIAPI+SerialPortSetAttributes (+ IN OUT UINT64
*BaudRate,+ IN OUT UINT32 *ReceiveFifoDepth,+ IN OUT
UINT32 *Timeout,+ IN OUT EFI_PARITY_TYPE *Parity,+ IN OUT
UINT8 *DataBits,+ IN OUT EFI_STOP_BITS_TYPE *StopBits+ )+{+
return RETURN_UNSUPPORTED;+}+diff --git
a/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf
b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf
new file mode 100644
index 000000000000..3df6a0c736a5
--- /dev/null
+++ b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf
@@ -0,0 +1,30 @@
+## @file+# Component description file for CbSerialPortLib library.+#+#
Copyright (c) 2022, Baruch Binyamin Doron+# SPDX-License-Identifier:
BSD-2-Clause-Patent+#+##++[Defines]+ INF_VERSION =
0x00010005+ BASE_NAME = CbSerialPortLib+ FILE_GUID
= 0DB3EF12-1426-4086-B012-113184C4CE11+ MODULE_TYPE
= BASE+ VERSION_STRING = 1.0+ # Recall that debug
logging can be unsafe to core. Route over RSC.+ LIBRARY_CLASS
= SerialPortLib+ CONSTRUCTOR =
SerialPortInitialize++[Packages]+ MdePkg/MdePkg.dec+
MdeModulePkg/MdeModulePkg.dec+
UefiPayloadPkg/UefiPayloadPkg.dec++[LibraryClasses]+ BaseMemoryLib+
BlParseLib++[Sources]+ CbSerialPortLib.c+diff --git
a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 4d9bbc80c866..0e4248767756 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -37,6 +37,7 @@
DEFINE ABOVE_4G_MEMORY = TRUE DEFINE BOOT_MANAGER_ESCAPE
= FALSE DEFINE SD_MMC_TIMEOUT = 1000000+ DEFINE
USE_CBMEM_FOR_CONSOLE = FALSE # # SBL: UEFI payload for Slim
Bootloader # COREBOOT: UEFI payload for coreboot@@ -121,10 +122,11 @@
[BuildOptions] *_*_*_CC_FLAGS = -D
DISABLE_NEW_DEPRECATED_INTERFACES- GCC:*_UNIXGCC_*_CC_FLAGS =
-DMDEPKG_NDEBUG+!if $(USE_CBMEM_FOR_CONSOLE) == FALSE
GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG INTEL:RELEASE_*_*_CC_FLAGS
= /D MDEPKG_NDEBUG MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG+!endif
[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER] GCC:*_*_*_DLINK_FLAGS =
-z common-page-size=0x1000@@ -231,8 +233,13 @@
TimerLib|UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf !endif
ResetSystemLib|UefiPayloadPkg/Library/ResetSystemLib/ResetSystemLib.inf+!if
$(USE_CBMEM_FOR_CONSOLE) == TRUE+
SerialPortLib|UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf+
PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf+!else
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
PlatformHookLib|UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf+!endif
PlatformBootManagerLib|UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf @@ -422,10
+429,18 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c,
0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x7
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F-!if
$(SOURCE_DEBUG_ENABLE)- gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x17+!if
$(USE_CBMEM_FOR_CONSOLE) == FALSE+ !if $(SOURCE_DEBUG_ENABLE)+
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x17+ !else+
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F+ !endif !else-
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F+ !if $(TARGET) == DEBUG+
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x07+ !else+
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x03+ !endif !endif
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule|$(MAX_SIZE_NON_POPULATE_CAPSULE)
#@@ -471,7 +486,7 @@
gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode
gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress
gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize-!if $(TARGET) ==
DEBUG+!if ($(TARGET) == DEBUG || $(USE_CBMEM_FOR_CONSOLE) == TRUE)
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE !else
gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE--
2.36.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90655): https://edk2.groups.io/g/devel/message/90655
Mute This Topic: https://groups.io/mt/91568694/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
