Hi, Thanks for looking at my patch. I probably need to refine the library before it's ready to be upstreamed. Two items on my list were finally debugging how to make `FindCbTag()` work, and using `CopyMem()` instead of byte-assignment. This commit is missing my follow-up: We must not define MDEPKG_NDEBUG or the messages are stripped. We must not make asserts fatal, because then they cannot be seen; we must try to continue anyways. PlatformHookLibNull must be used, or asserts occur when UefiPayload's instance cannot find the serial table in CBMEM. See https://github.com/MrChromebox/edk2/commit/5fec6d995cc2f453d4b580e941fca6662e2ed367 for the updated commit, I don't believe it ever made it to my GitHub. However, I now know that the Status Code stuff is irrelevant, because we aren't logging through its infrastructure. That should be it, I suppose? I'll get back to it when I can if upstream/coreboot's fork wants. Regards, Benjamin On Fri., Jan. 28, 2022, 4:38 p.m. Sean Rhodes, wrote: > From: Benjamin Doron > > Tested on QEMU, dumping the appropriate memory region in UEFI shell > shows the TianoCore log. `find_cb_subtable` is sourced from STM/SeaBIOS. > > Signed-off-by: Benjamin Doron > --- > UefiPayloadPkg/Include/Coreboot.h | 14 + > .../Library/CbSerialPortLib/CbSerialPortLib.c | 272 ++++++++++++++++++ > .../CbSerialPortLib/CbSerialPortLib.inf | 27 ++ > UefiPayloadPkg/UefiPayloadPkg.dsc | 5 + > UefiPayloadPkg/UefiPayloadPkg.fdf | 6 +- > 5 files changed, 323 insertions(+), 1 deletion(-) > create mode 100644 > UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c > create mode 100644 > UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf > > diff --git a/UefiPayloadPkg/Include/Coreboot.h > b/UefiPayloadPkg/Include/Coreboot.h > index a3e1109fe8..f2f577a02e 100644 > --- a/UefiPayloadPkg/Include/Coreboot.h > +++ b/UefiPayloadPkg/Include/Coreboot.h > @@ -199,7 +199,14 @@ struct cb_forward { > UINT64 forward; > }; > > +struct cb_cbmem_ref { > + UINT32 tag; > + UINT32 size; > + UINT64 cbmem_addr; > +}; > + > #define CB_TAG_FRAMEBUFFER 0x0012 > + > struct cb_framebuffer { > UINT32 tag; > UINT32 size; > @@ -230,6 +237,13 @@ struct cb_vdat { > #define CB_TAG_TIMESTAMPS 0x0016 > #define CB_TAG_CBMEM_CONSOLE 0x0017 > #define CB_TAG_MRC_CACHE 0x0018 > + > +struct cbmem_console { > + UINT32 size; > + UINT32 cursor; > + UINT8 body[0]; > +} __attribute__((packed)); > + > struct cb_cbmem_tab { > UINT32 tag; > UINT32 size; > diff --git a/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c > b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c > new file mode 100644 > index 0000000000..7b26c08dd7 > --- /dev/null > +++ b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.c > @@ -0,0 +1,272 @@ > +/** @file > + CBMEM console SerialPortLib instance > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > + > +#include > +#include > +#include > + > +// > +// We can't use DebugLib due to a constructor dependency cycle between > DebugLib > +// and ourselves. > +// > +#define ASSERT(Expression) \ > + do { \ > + if (!(Expression)) { \ > + CpuDeadLoop (); \ > + } \ > + } while (FALSE) > + > +#define CBMC_CURSOR_MASK ((1 << 28) - 1) > +#define CBMC_OVERFLOW (1 << 31) > + > +STATIC struct cbmem_console *gCbConsole = NULL; > +STATIC UINT32 STM_cursor = 0; > + > +// Try to find the coreboot memory table in the given coreboot table. > +static void * > +find_cb_subtable(struct cb_header *cbh, UINT32 tag) > +{ > + char *tbl = (char *)cbh + sizeof(*cbh); > + UINT32 count = cbh->table_entries; > + int i; > + for (i=0; i + struct cb_memory *cbm = (void*)tbl; > + tbl += cbm->size; > + if (cbm->tag == tag) > + return cbm; > + } > + return NULL; > +} > + > +/** > + 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 > + ) > +{ > + /* `FindCbTag` doesn't work because we need direct access to the memory > addresses? */ > + struct cb_header *cbh = GetParameterBase(); > + if (!cbh) { > + return RETURN_DEVICE_ERROR; > + } > + > + struct cb_cbmem_ref *cbref = find_cb_subtable(cbh, > CB_TAG_CBMEM_CONSOLE); > + if (!cbref) { > + return RETURN_DEVICE_ERROR; > + } > + > + gCbConsole = (void *)(UINTN)cbref->cbmem_addr; // Support PEI and > DXE > + if (gCbConsole == NULL) { > + return RETURN_DEVICE_ERROR; > + } > + > + // set the cursor such that the STM console will not overwrite the > + // coreboot console output > + STM_cursor = gCbConsole->cursor & CBMC_CURSOR_MASK; > + > + 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 > + ) > +{ > + UINTN Sent; > + UINT32 cursor; > + UINT32 flags; > + > + ASSERT (Buffer != NULL); > + > + if (NumberOfBytes == 0) { > + return 0; > + } > + > + if (!gCbConsole) { > + return 0; > + } > + > + Sent = 0; > + do { > + cursor = gCbConsole->cursor & CBMC_CURSOR_MASK; > + flags = gCbConsole->cursor & ~CBMC_CURSOR_MASK; > + > + if (cursor >= gCbConsole->size) { > + return 0; // Old coreboot version with legacy overflow > mechanism. > + } > + > + gCbConsole->body[cursor++] = Buffer[Sent++]; > + > + if (cursor >= gCbConsole->size) { > + cursor = STM_cursor; > + flags |= CBMC_OVERFLOW; > + } > + > + gCbConsole->cursor = flags | cursor; > + } while (Sent < NumberOfBytes); > + > + return Sent; > +} > + > +/** > + 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 0000000000..37b33c24ad > --- /dev/null > +++ b/UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf > @@ -0,0 +1,27 @@ > +## @file > +# Component description file for CbSerialPortLib module. > +# > +# 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 = 0.5 > + LIBRARY_CLASS = SerialPortLib > + CONSTRUCTOR = SerialPortInitialize > + > +[Sources] > + CbSerialPortLib.c > + > +[Packages] > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + UefiPayloadPkg/UefiPayloadPkg.dec > + > +[LibraryClasses] > + BaseLib > + BlParseLib > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc > b/UefiPayloadPkg/UefiPayloadPkg.dsc > index 3d08edfe31..b3770b9be6 100644 > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc > @@ -33,6 +33,7 @@ > DEFINE UNIVERSAL_PAYLOAD = FALSE > DEFINE SECURITY_STUB_ENABLE = TRUE > DEFINE SMM_SUPPORT = FALSE > + DEFINE USE_CBMEM_FOR_CONSOLE = FALSE > # > # SBL: UEFI payload for Slim Bootloader > # COREBOOT: UEFI payload for coreboot > @@ -223,7 +224,11 @@ > 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 > +!else > > SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf > +!endif > > PlatformHookLib|UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf > > PlatformBootManagerLib|UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf > diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf > b/UefiPayloadPkg/UefiPayloadPkg.fdf > index f619a23139..0df8342252 100644 > --- a/UefiPayloadPkg/UefiPayloadPkg.fdf > +++ b/UefiPayloadPkg/UefiPayloadPkg.fdf > @@ -15,8 +15,12 @@ DEFINE FD_BLOCK_SIZE = 0x00001000 > !if $(TARGET) == "NOOPT" > DEFINE FD_SIZE = 0x00850000 > DEFINE NUM_BLOCKS = 0x850 > -!else > > +!elseif $(USE_CBMEM_FOR_CONSOLE) == TRUE > +DEFINE FD_SIZE = 0x00600000 > +DEFINE NUM_BLOCKS = 0x600 > + > +!else > DEFINE FD_SIZE = 0x00590000 > DEFINE NUM_BLOCKS = 0x590 > !endif > -- > 2.32.0 > >