Skip to content

Commit

Permalink
ArmVirtPkg/FdtPL011SerialPortLib: initialize implicitly
Browse files Browse the repository at this point in the history
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 <osde@linux.microsoft.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
  • Loading branch information
lersek authored and mergify[bot] committed Oct 7, 2023
1 parent 82191f8 commit 5087a07
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 34 deletions.
84 changes: 51 additions & 33 deletions ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,49 +23,57 @@
#include <Library/HobLib.h>
#include <Guid/EarlyPL011BaseAddress.h>

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);
Expand All @@ -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;
}

/**
Expand All @@ -102,7 +120,7 @@ SerialPortWrite (
IN UINTN NumberOfBytes
)
{
if (mSerialBaseAddress != 0) {
if (!RETURN_ERROR (SerialPortInitialize ())) {
return PL011UartWrite (mSerialBaseAddress, Buffer, NumberOfBytes);
}

Expand All @@ -126,7 +144,7 @@ SerialPortRead (
IN UINTN NumberOfBytes
)
{
if (mSerialBaseAddress != 0) {
if (!RETURN_ERROR (SerialPortInitialize ())) {
return PL011UartRead (mSerialBaseAddress, Buffer, NumberOfBytes);
}

Expand All @@ -146,7 +164,7 @@ SerialPortPoll (
VOID
)
{
if (mSerialBaseAddress != 0) {
if (!RETURN_ERROR (SerialPortInitialize ())) {
return PL011UartPoll (mSerialBaseAddress);
}

Expand Down Expand Up @@ -199,7 +217,7 @@ SerialPortSetAttributes (
{
RETURN_STATUS Status;

if (mSerialBaseAddress == 0) {
if (RETURN_ERROR (SerialPortInitialize ())) {
Status = RETURN_UNSUPPORTED;
} else {
Status = PL011UartInitializePort (
Expand Down Expand Up @@ -234,7 +252,7 @@ SerialPortSetControl (
{
RETURN_STATUS Status;

if (mSerialBaseAddress == 0) {
if (RETURN_ERROR (SerialPortInitialize ())) {
Status = RETURN_UNSUPPORTED;
} else {
Status = PL011UartSetControl (mSerialBaseAddress, Control);
Expand All @@ -261,7 +279,7 @@ SerialPortGetControl (
{
RETURN_STATUS Status;

if (mSerialBaseAddress == 0) {
if (RETURN_ERROR (SerialPortInitialize ())) {
Status = RETURN_UNSUPPORTED;
} else {
Status = PL011UartGetControl (mSerialBaseAddress, Control);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5087a07

Please sign in to comment.