From 5087a07736452518a508a7776002afbce360128b Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Sat, 30 Sep 2023 23:23:43 +0200 Subject: ArmVirtPkg/FdtPL011SerialPortLib: initialize implicitly FdtPL011SerialPortLib claims that it's usable from the DXE_CORE. That's not correct: the DXE_CORE calls DEBUG() and ASSERT() before it calls ProcessLibraryConstructorList(). Via the BaseDebugLibSerialPort instance, those DEBUG() and ASSERT() calls result in SerialPortWrite() calls, before ProcessLibraryConstructorList() called either our constructor FdtPL011SerialPortLibInitialize(), or BaseDebugLibSerialPortConstructor(). (And even if the DXE_CORE called the latter function early enough, it would just invoke our SerialPortInitialize() function -- which does nothing.) This means that the earliest DXE_CORE debug messages are lost. Rename FdtPL011SerialPortLibInitialize() to SerialPortInitialize(), so that the same initialization occur through the constructor and the public SerialPortInitialize() library API. Turn SerialPortInitialize() calls after the first one into no-ops. Our SerialPortLib APIs already use (mSerialBaseAddress != 0) to track initialization. Rework those checks to actually initialize the library if that hasn't happened yet. The following new lines appear in the log: > CoreInitializeMemoryServices: > BaseAddress - 0x48000000 Length - 0xF8000000 MinimalMemorySizeNeeded - 0x38C8000 > InstallProtocolInterface: [EfiLoadedImageProtocol] 46EFC3E0 > ProtectUefiImageCommon - 0x46EFC3E0 > - 0x0000000046EB2000 - 0x0000000000068000 (0x46EB2000 is the load address of the DXE Core.) Reported-by: Oliver Smith-Denny Signed-off-by: Laszlo Ersek Reviewed-by: Ard Biesheuvel --- .../FdtPL011SerialPortLib/FdtPL011SerialPortLib.c | 84 +++++++++++++--------- .../FdtPL011SerialPortLib.inf | 2 +- 2 files changed, 52 insertions(+), 34 deletions(-) (limited to 'ArmVirtPkg') diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c index 614ea5a860..9d71f36957 100644 --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c @@ -23,49 +23,57 @@ #include #include -STATIC UINTN mSerialBaseAddress; - -RETURN_STATUS -EFIAPI -SerialPortInitialize ( - VOID - ) -{ - return RETURN_SUCCESS; -} +STATIC UINTN mSerialBaseAddress; +STATIC RETURN_STATUS mPermanentStatus = RETURN_SUCCESS; /** - Program hardware of Serial port - @return RETURN_NOT_FOUND if no PL011 base address could be found - Otherwise, result of PL011UartInitializePort () is returned + @retval RETURN_SUCCESS If the serial port was initialized successfully by + this call, or an earlier call, to + SerialPortInitialize(). + @retval RETURN_NOT_FOUND If no PL011 base address could be found. + + @return Error codes forwarded from + PL011UartInitializePort(). **/ RETURN_STATUS EFIAPI -FdtPL011SerialPortLibInitialize ( +SerialPortInitialize ( VOID ) { VOID *Hob; + RETURN_STATUS Status; CONST UINT64 *UartBase; + UINTN SerialBaseAddress; UINT64 BaudRate; UINT32 ReceiveFifoDepth; EFI_PARITY_TYPE Parity; UINT8 DataBits; EFI_STOP_BITS_TYPE StopBits; + if (mSerialBaseAddress != 0) { + return RETURN_SUCCESS; + } + + if (RETURN_ERROR (mPermanentStatus)) { + return mPermanentStatus; + } + Hob = GetFirstGuidHob (&gEarlyPL011BaseAddressGuid); if ((Hob == NULL) || (GET_GUID_HOB_DATA_SIZE (Hob) != sizeof *UartBase)) { - return RETURN_NOT_FOUND; + Status = RETURN_NOT_FOUND; + goto Failed; } UartBase = GET_GUID_HOB_DATA (Hob); - mSerialBaseAddress = (UINTN)*UartBase; - if (mSerialBaseAddress == 0) { - return RETURN_NOT_FOUND; + SerialBaseAddress = (UINTN)*UartBase; + if (SerialBaseAddress == 0) { + Status = RETURN_NOT_FOUND; + goto Failed; } BaudRate = (UINTN)PcdGet64 (PcdUartDefaultBaudRate); @@ -74,15 +82,25 @@ FdtPL011SerialPortLibInitialize ( DataBits = PcdGet8 (PcdUartDefaultDataBits); StopBits = (EFI_STOP_BITS_TYPE)PcdGet8 (PcdUartDefaultStopBits); - return PL011UartInitializePort ( - mSerialBaseAddress, - FixedPcdGet32 (PL011UartClkInHz), - &BaudRate, - &ReceiveFifoDepth, - &Parity, - &DataBits, - &StopBits - ); + Status = PL011UartInitializePort ( + SerialBaseAddress, + FixedPcdGet32 (PL011UartClkInHz), + &BaudRate, + &ReceiveFifoDepth, + &Parity, + &DataBits, + &StopBits + ); + if (RETURN_ERROR (Status)) { + goto Failed; + } + + mSerialBaseAddress = SerialBaseAddress; + return RETURN_SUCCESS; + +Failed: + mPermanentStatus = Status; + return Status; } /** @@ -102,7 +120,7 @@ SerialPortWrite ( IN UINTN NumberOfBytes ) { - if (mSerialBaseAddress != 0) { + if (!RETURN_ERROR (SerialPortInitialize ())) { return PL011UartWrite (mSerialBaseAddress, Buffer, NumberOfBytes); } @@ -126,7 +144,7 @@ SerialPortRead ( IN UINTN NumberOfBytes ) { - if (mSerialBaseAddress != 0) { + if (!RETURN_ERROR (SerialPortInitialize ())) { return PL011UartRead (mSerialBaseAddress, Buffer, NumberOfBytes); } @@ -146,7 +164,7 @@ SerialPortPoll ( VOID ) { - if (mSerialBaseAddress != 0) { + if (!RETURN_ERROR (SerialPortInitialize ())) { return PL011UartPoll (mSerialBaseAddress); } @@ -199,7 +217,7 @@ SerialPortSetAttributes ( { RETURN_STATUS Status; - if (mSerialBaseAddress == 0) { + if (RETURN_ERROR (SerialPortInitialize ())) { Status = RETURN_UNSUPPORTED; } else { Status = PL011UartInitializePort ( @@ -234,7 +252,7 @@ SerialPortSetControl ( { RETURN_STATUS Status; - if (mSerialBaseAddress == 0) { + if (RETURN_ERROR (SerialPortInitialize ())) { Status = RETURN_UNSUPPORTED; } else { Status = PL011UartSetControl (mSerialBaseAddress, Control); @@ -261,7 +279,7 @@ SerialPortGetControl ( { RETURN_STATUS Status; - if (mSerialBaseAddress == 0) { + if (RETURN_ERROR (SerialPortInitialize ())) { Status = RETURN_UNSUPPORTED; } else { Status = PL011UartGetControl (mSerialBaseAddress, Control); diff --git a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf index 2b9a34aa30..c417514e3e 100644 --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf @@ -15,7 +15,7 @@ MODULE_TYPE = BASE VERSION_STRING = 1.0 LIBRARY_CLASS = SerialPortLib|DXE_CORE DXE_DRIVER UEFI_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION - CONSTRUCTOR = FdtPL011SerialPortLibInitialize + CONSTRUCTOR = SerialPortInitialize [Sources.common] FdtPL011SerialPortLib.c -- cgit v1.2.3