Hey Benjamin<br /><br />The commit has been around for a while now and is perfectly functional. Why don't we upstream it and then `FindCbTag()` and `CopyMem()` can be a follow-up patch when/if time allows?<br /><br />Cheers<br />Sean<br /><br />Squashing that commit in:<br />
<div>[PATCH] UefiPayloadPkg: Add support for logging to CBMEM console</div>
<div> </div>
<div>Tested on QEMU, dumping the appropriate memory region in UEFI shell</div>
<div>shows the TianoCore log. `find_cb_subtable` is sourced from STM/SeaBIOS.</div>
<div> </div>
<div>Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com></div>
<div>---</div>
<div> UefiPayloadPkg/Include/Coreboot.h | 14 +</div>
<div> .../Library/CbSerialPortLib/CbSerialPortLib.c | 272 ++++++++++++++++++</div>
<div> .../CbSerialPortLib/CbSerialPortLib.inf | 27 ++</div>
<div> UefiPayloadPkg/UefiPayloadPkg.dsc | 5 +</div>
<div> UefiPayloadPkg/UefiPayloadPkg.fdf | 6 +-</div>
<div> 5 files changed, 323 insertions(+), 1 deletion(-)</div>
<div> create mode 100644 UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c</div>
<div> create mode 100644 UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf</div>
<div> </div>
<div>diff --git a/UefiPayloadPkg/Include/Coreboot.h b/UefiPayloadPkg/Include/Coreboot.h</div>
<div>index a3e1109fe8..f2f577a02e 100644</div>
<div>--- a/UefiPayloadPkg/Include/Coreboot.h</div>
<div>+++ b/UefiPayloadPkg/Include/Coreboot.h</div>
<div>@@ -199,7 +199,14 @@ struct cb_forward {</div>
<div> UINT64 forward;</div>
<div> };</div>
<div> </div>
<div>+struct cb_cbmem_ref {</div>
<div>+ UINT32 tag;</div>
<div>+ UINT32 size;</div>
<div>+ UINT64 cbmem_addr;</div>
<div>+};</div>
<div>+</div>
<div> #define CB_TAG_FRAMEBUFFER 0x0012</div>
<div>+</div>
<div> struct cb_framebuffer {</div>
<div> UINT32 tag;</div>
<div> UINT32 size;</div>
<div>@@ -230,6 +237,13 @@ struct cb_vdat {</div>
<div> #define CB_TAG_TIMESTAMPS 0x0016</div>
<div> #define CB_TAG_CBMEM_CONSOLE 0x0017</div>
<div> #define CB_TAG_MRC_CACHE 0x0018</div>
<div>+</div>
<div>+struct cbmem_console {</div>
<div>+ UINT32 size;</div>
<div>+ UINT32 cursor;</div>
<div>+ UINT8 body[0];</div>
<div>+} __attribute__((packed));</div>
<div>+</div>
<div> struct cb_cbmem_tab {</div>
<div> UINT32 tag;</div>
<div> UINT32 size;</div>
<div>diff --git a/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c</div>
<div>new file mode 100644</div>
<div>index 0000000000..7b26c08dd7</div>
<div>--- /dev/null</div>
<div>+++ b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c</div>
<div>@@ -0,0 +1,272 @@</div>
<div>+/** @file</div>
<div>+ CBMEM console SerialPortLib instance</div>
<div>+</div>
<div>+ SPDX-License-Identifier: BSD-2-Clause-Patent</div>
<div>+</div>
<div>+**/</div>
<div>+</div>
<div>+#include <Base.h></div>
<div>+#include <Coreboot.h></div>
<div>+</div>
<div>+#include <Library/BaseLib.h></div>
<div>+#include <Library/BlParseLib.h></div>
<div>+#include <Library/SerialPortLib.h></div>
<div>+</div>
<div>+//</div>
<div>+// We can't use DebugLib due to a constructor dependency cycle between DebugLib</div>
<div>+// and ourselves.</div>
<div>+//</div>
<div>+#define ASSERT(Expression) \</div>
<div>+ do { \</div>
<div>+ if (!(Expression)) { \</div>
<div>+ CpuDeadLoop (); \</div>
<div>+ } \</div>
<div>+ } while (FALSE)</div>
<div>+</div>
<div>+#define CBMC_CURSOR_MASK ((1 << 28) - 1)</div>
<div>+#define CBMC_OVERFLOW (1 << 31)</div>
<div>+</div>
<div>+STATIC struct cbmem_console *gCbConsole = NULL;</div>
<div>+STATIC UINT32 STM_cursor = 0;</div>
<div>+</div>
<div>+// Try to find the coreboot memory table in the given coreboot table.</div>
<div>+static void *</div>
<div>+find_cb_subtable(struct cb_header *cbh, UINT32 tag)</div>
<div>+{</div>
<div>+ char *tbl = (char *)cbh + sizeof(*cbh);</div>
<div>+ UINT32 count = cbh->table_entries;</div>
<div>+ int i;</div>
<div>+ for (i=0; i<count; i++) {</div>
<div>+ struct cb_memory *cbm = (void*)tbl;</div>
<div>+ tbl += cbm->size;</div>
<div>+ if (cbm->tag == tag)</div>
<div>+ return cbm;</div>
<div>+ }</div>
<div>+ return NULL;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Initialize the serial device hardware.</div>
<div>+</div>
<div>+ If no initialization is required, then return RETURN_SUCCESS.</div>
<div>+ If the serial device was successfully initialized, then return RETURN_SUCCESS.</div>
<div>+ If the serial device could not be initialized, then return RETURN_DEVICE_ERROR.</div>
<div>+</div>
<div>+ @retval RETURN_SUCCESS The serial device was initialized.</div>
<div>+ @retval RETURN_DEVICE_ERROR The serial device could not be initialized.</div>
<div>+</div>
<div>+**/</div>
<div>+RETURN_STATUS</div>
<div>+EFIAPI</div>
<div>+SerialPortInitialize (</div>
<div>+ VOID</div>
<div>+ )</div>
<div>+{</div>
<div>+ /* `FindCbTag` doesn't work because we need direct access to the memory addresses? */</div>
<div>+ struct cb_header *cbh = GetParameterBase();</div>
<div>+ if (!cbh) {</div>
<div>+ return RETURN_DEVICE_ERROR;</div>
<div>+ }</div>
<div>+</div>
<div>+ struct cb_cbmem_ref *cbref = find_cb_subtable(cbh, CB_TAG_CBMEM_CONSOLE);</div>
<div>+ if (!cbref) {</div>
<div>+ return RETURN_DEVICE_ERROR;</div>
<div>+ }</div>
<div>+</div>
<div>+ gCbConsole = (void *)(UINTN)cbref->cbmem_addr; // Support PEI and DXE</div>
<div>+ if (gCbConsole == NULL) {</div>
<div>+ return RETURN_DEVICE_ERROR;</div>
<div>+ }</div>
<div>+</div>
<div>+ // set the cursor such that the STM console will not overwrite the</div>
<div>+ // coreboot console output</div>
<div>+ STM_cursor = gCbConsole->cursor & CBMC_CURSOR_MASK;</div>
<div>+</div>
<div>+ return RETURN_SUCCESS;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Write data from buffer to serial device.</div>
<div>+</div>
<div>+ Writes NumberOfBytes data bytes from Buffer to the serial device.</div>
<div>+ The number of bytes actually written to the serial device is returned.</div>
<div>+ If the return value is less than NumberOfBytes, then the write operation failed.</div>
<div>+ If Buffer is NULL, then ASSERT().</div>
<div>+ If NumberOfBytes is zero, then return 0.</div>
<div>+</div>
<div>+ @param Buffer Pointer to the data buffer to be written.</div>
<div>+ @param NumberOfBytes Number of bytes to written to the serial device.</div>
<div>+</div>
<div>+ @retval 0 NumberOfBytes is 0.</div>
<div>+ @retval >0 The number of bytes written to the serial device.</div>
<div>+ If this value is less than NumberOfBytes, then the write operation failed.</div>
<div>+</div>
<div>+**/</div>
<div>+UINTN</div>
<div>+EFIAPI</div>
<div>+SerialPortWrite (</div>
<div>+ IN UINT8 *Buffer,</div>
<div>+ IN UINTN NumberOfBytes</div>
<div>+ )</div>
<div>+{</div>
<div>+ UINTN Sent;</div>
<div>+ UINT32 cursor;</div>
<div>+ UINT32 flags;</div>
<div>+</div>
<div>+ ASSERT (Buffer != NULL);</div>
<div>+</div>
<div>+ if (NumberOfBytes == 0) {</div>
<div>+ return 0;</div>
<div>+ }</div>
<div>+</div>
<div>+ if (!gCbConsole) {</div>
<div>+ return 0;</div>
<div>+ }</div>
<div>+</div>
<div>+ Sent = 0;</div>
<div>+ do {</div>
<div>+ cursor = gCbConsole->cursor & CBMC_CURSOR_MASK;</div>
<div>+ flags = gCbConsole->cursor & ~CBMC_CURSOR_MASK;</div>
<div>+</div>
<div>+ if (cursor >= gCbConsole->size) {</div>
<div>+ return 0; // Old coreboot version with legacy overflow mechanism.</div>
<div>+ }</div>
<div>+</div>
<div>+ gCbConsole->body[cursor++] = Buffer[Sent++];</div>
<div>+</div>
<div>+ if (cursor >= gCbConsole->size) {</div>
<div>+ cursor = STM_cursor;</div>
<div>+ flags |= CBMC_OVERFLOW;</div>
<div>+ }</div>
<div>+</div>
<div>+ gCbConsole->cursor = flags | cursor;</div>
<div>+ } while (Sent < NumberOfBytes);</div>
<div>+</div>
<div>+ return Sent;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Read data from serial device and save the datas in buffer.</div>
<div>+</div>
<div>+ Reads NumberOfBytes data bytes from a serial device into the buffer</div>
<div>+ specified by Buffer. The number of bytes actually read is returned.</div>
<div>+ If Buffer is NULL, then ASSERT().</div>
<div>+ If NumberOfBytes is zero, then return 0.</div>
<div>+</div>
<div>+ @param Buffer Pointer to the data buffer to store the data read from the serial device.</div>
<div>+ @param NumberOfBytes Number of bytes which will be read.</div>
<div>+</div>
<div>+ @retval 0 Read data failed, no data is to be read.</div>
<div>+ @retval >0 Actual number of bytes read from serial device.</div>
<div>+</div>
<div>+**/</div>
<div>+UINTN</div>
<div>+EFIAPI</div>
<div>+SerialPortRead (</div>
<div>+ OUT UINT8 *Buffer,</div>
<div>+ IN UINTN NumberOfBytes</div>
<div>+)</div>
<div>+{</div>
<div>+ return 0;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Polls a serial device to see if there is any data waiting to be read.</div>
<div>+</div>
<div>+ @retval TRUE Data is waiting to be read from the serial device.</div>
<div>+ @retval FALSE There is no data waiting to be read from the serial device.</div>
<div>+</div>
<div>+**/</div>
<div>+BOOLEAN</div>
<div>+EFIAPI</div>
<div>+SerialPortPoll (</div>
<div>+ VOID</div>
<div>+ )</div>
<div>+{</div>
<div>+ return FALSE;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Sets the control bits on a serial device.</div>
<div>+</div>
<div>+ @param Control Sets the bits of Control that are settable.</div>
<div>+</div>
<div>+ @retval RETURN_SUCCESS The new control bits were set on the serial device.</div>
<div>+ @retval RETURN_UNSUPPORTED The serial device does not support this operation.</div>
<div>+ @retval RETURN_DEVICE_ERROR The serial device is not functioning correctly.</div>
<div>+</div>
<div>+**/</div>
<div>+RETURN_STATUS</div>
<div>+EFIAPI</div>
<div>+SerialPortSetControl (</div>
<div>+ IN UINT32 Control</div>
<div>+ )</div>
<div>+{</div>
<div>+ return RETURN_UNSUPPORTED;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Retrieve the status of the control bits on a serial device.</div>
<div>+</div>
<div>+ @param Control A pointer to return the current control signals from the serial device.</div>
<div>+</div>
<div>+ @retval RETURN_SUCCESS The control bits were read from the serial device.</div>
<div>+ @retval RETURN_UNSUPPORTED The serial device does not support this operation.</div>
<div>+ @retval RETURN_DEVICE_ERROR The serial device is not functioning correctly.</div>
<div>+</div>
<div>+**/</div>
<div>+RETURN_STATUS</div>
<div>+EFIAPI</div>
<div>+SerialPortGetControl (</div>
<div>+ OUT UINT32 *Control</div>
<div>+ )</div>
<div>+{</div>
<div>+ return RETURN_UNSUPPORTED;</div>
<div>+}</div>
<div>+</div>
<div>+/**</div>
<div>+ Sets the baud rate, receive FIFO depth, transmit/receive time out, parity,</div>
<div>+ data bits, and stop bits on a serial device.</div>
<div>+</div>
<div>+ @param BaudRate The requested baud rate. A BaudRate value of 0 will use the</div>
<div>+ device's default interface speed.</div>
<div>+ On output, the value actually set.</div>
<div>+ @param ReceiveFifoDepth The requested depth of the FIFO on the receive side of the</div>
<div>+ serial interface. A ReceiveFifoDepth value of 0 will use</div>
<div>+ the device's default FIFO depth.</div>
<div>+ On output, the value actually set.</div>
<div>+ @param Timeout The requested time out for a single character in microseconds.</div>
<div>+ This timeout applies to both the transmit and receive side of the</div>
<div>+ interface. A Timeout value of 0 will use the device's default time</div>
<div>+ out value.</div>
<div>+ On output, the value actually set.</div>
<div>+ @param Parity The type of parity to use on this serial device. A Parity value of</div>
<div>+ DefaultParity will use the device's default parity value.</div>
<div>+ On output, the value actually set.</div>
<div>+ @param DataBits The number of data bits to use on the serial device. A DataBits</div>
<div>+ value of 0 will use the device's default data bit setting.</div>
<div>+ On output, the value actually set.</div>
<div>+ @param StopBits The number of stop bits to use on this serial device. A StopBits</div>
<div>+ value of DefaultStopBits will use the device's default number of</div>
<div>+ stop bits.</div>
<div>+ On output, the value actually set.</div>
<div>+</div>
<div>+ @retval RETURN_SUCCESS The new attributes were set on the serial device.</div>
<div>+ @retval RETURN_UNSUPPORTED The serial device does not support this operation.</div>
<div>+ @retval RETURN_INVALID_PARAMETER One or more of the attributes has an unsupported value.</div>
<div>+ @retval RETURN_DEVICE_ERROR The serial device is not functioning correctly.</div>
<div>+</div>
<div>+**/</div>
<div>+RETURN_STATUS</div>
<div>+EFIAPI</div>
<div>+SerialPortSetAttributes (</div>
<div>+ IN OUT UINT64 *BaudRate,</div>
<div>+ IN OUT UINT32 *ReceiveFifoDepth,</div>
<div>+ IN OUT UINT32 *Timeout,</div>
<div>+ IN OUT EFI_PARITY_TYPE *Parity,</div>
<div>+ IN OUT UINT8 *DataBits,</div>
<div>+ IN OUT EFI_STOP_BITS_TYPE *StopBits</div>
<div>+ )</div>
<div>+{</div>
<div>+ return RETURN_UNSUPPORTED;</div>
<div>+}</div>
<div>diff --git a/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf</div>
<div>new file mode 100644</div>
<div>index 0000000000..37b33c24ad</div>
<div>--- /dev/null</div>
<div>+++ b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf</div>
<div>@@ -0,0 +1,27 @@</div>
<div>+## @file</div>
<div>+# Component description file for CbSerialPortLib module.</div>
<div>+#</div>
<div>+# SPDX-License-Identifier: BSD-2-Clause-Patent</div>
<div>+#</div>
<div>+##</div>
<div>+</div>
<div>+[Defines]</div>
<div>+ INF_VERSION = 0x00010005</div>
<div>+ BASE_NAME = CbSerialPortLib</div>
<div>+ FILE_GUID = 0DB3EF12-1426-4086-B012-113184C4CE11</div>
<div>+ MODULE_TYPE = BASE</div>
<div>+ VERSION_STRING = 0.5</div>
<div>+ LIBRARY_CLASS = SerialPortLib</div>
<div>+ CONSTRUCTOR = SerialPortInitialize</div>
<div>+</div>
<div>+[Sources]</div>
<div>+ CbSerialPortLib.c</div>
<div>+</div>
<div>+[Packages]</div>
<div>+ MdeModulePkg/MdeModulePkg.dec</div>
<div>+ MdePkg/MdePkg.dec</div>
<div>+ UefiPayloadPkg/UefiPayloadPkg.dec</div>
<div>+</div>
<div>+[LibraryClasses]</div>
<div>+ BaseLib</div>
<div>+ BlParseLib</div>
<div>diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc</div>
<div>index 3d08edfe31..b3770b9be6 100644</div>
<div>--- a/UefiPayloadPkg/UefiPayloadPkg.dsc</div>
<div>+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc</div>
<div>@@ -33,6 +33,7 @@</div>
<div> DEFINE UNIVERSAL_PAYLOAD = FALSE</div>
<div> DEFINE SECURITY_STUB_ENABLE = TRUE</div>
<div> DEFINE SMM_SUPPORT = FALSE</div>
<div>+ DEFINE USE_CBMEM_FOR_CONSOLE = FALSE</div>
<div> #</div>
<div> # SBL: UEFI payload for Slim Bootloader</div>
<div> # COREBOOT: UEFI payload for coreboot</div>
<div>@@ -223,7 +224,11 @@</div>
<div> TimerLib|UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf</div>
<div> !endif</div>
<div> ResetSystemLib|UefiPayloadPkg/Library/ResetSystemLib/ResetSystemLib.inf</div>
<div>+!if $(USE_CBMEM_FOR_CONSOLE) == TRUE</div>
<div>+ SerialPortLib|UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf</div>
<div>+!else</div>
<div> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf</div>
<div>+!endif</div>
<div> PlatformHookLib|UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf</div>
<div> PlatformBootManagerLib|UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf</div>
<div> IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf</div>
<div>diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf</div>
<div>index f619a23139..0df8342252 100644</div>
<div>--- a/UefiPayloadPkg/UefiPayloadPkg.fdf</div>
<div>+++ b/UefiPayloadPkg/UefiPayloadPkg.fdf</div>
<div>@@ -15,8 +15,12 @@ DEFINE FD_BLOCK_SIZE = 0x00001000</div>
<div> !if $(TARGET) == "NOOPT"</div>
<div> DEFINE FD_SIZE = 0x00850000</div>
<div> DEFINE NUM_BLOCKS = 0x850</div>
<div>-!else</div>
<div> </div>
<div>+!elseif $(USE_CBMEM_FOR_CONSOLE) == TRUE</div>
<div>+DEFINE FD_SIZE = 0x00600000</div>
<div>+DEFINE NUM_BLOCKS = 0x600</div>
<div>+</div>
<div>+!else</div>
<div> DEFINE FD_SIZE = 0x00590000</div>
<div> DEFINE NUM_BLOCKS = 0x590</div>
<div> !endif</div>
<div>-- </div>
<div>2.32.0</div>
<div> </div>
<div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links:<p> You receive all messages sent to this group. <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/86201">View/Reply Online (#86201)</a> | | <a target="_blank" href="https://groups.io/mt/88756033/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>