From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web11.3463.1588760311715675831 for ; Wed, 06 May 2020 03:18:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=goDuABPh; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.68, mailfrom: pete@akeo.ie) Received: by mail-wm1-f68.google.com with SMTP id x4so1958434wmj.1 for ; Wed, 06 May 2020 03:18:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=cvm/2cBm4guLEMZg3kESr49LrFGPWh8lduiD++ES/wg=; b=goDuABPh2KpW5pJdGe4L3pj87ni+v55b1OKxPumDu/hKtxkvNBhknWHY1rhV/zskwG oOF2G2vJ3zz2GOaUtQPUNLrrM4K/05O5n3PLDj6xGFRvetuEMDgL3VOL2GvPgzPZ0f9c ih03fFIXRHvvTlTu0KsPpn//usAZm4YV7we/i50Q69esfm62Or5xqiyHapmij0a+s6Gj mZvwWCQaZYTfwLwHOKWJtNyhov8XruchlIkSq65ALFZFNwpEwOSRPAVtoHf/5SQ93j5U bmH0VPzamMs+Oi7jalKbofZCNStLUTEMsVuPUohE3vdr8QmdhwX8+4zXYHLSiJ3zdlmr WeWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=cvm/2cBm4guLEMZg3kESr49LrFGPWh8lduiD++ES/wg=; b=Ehh+S1PiySNvYf9J0kK669jxYzfwkWb93JL+BTcJIIbu86XbRLsNoFVTJ/RcTDIkZ7 irTuUO5NuuRy8kN6YuWh/ILVIFTLHNVHcQ8+BhHuX3Jok0mhg9+snpc6XXg30Bxo5fxW hLgJ3PSQ9jqyc4VORzdPxZOu2AkWBN6vZWIacZYk/YLteOeLN8sgSqmFv4dlel63i0qe hc7fZ4VYTDhX1B2cdTbgl3icCnPdBYJEFjXYER5fyOWkpzNWhuaIgBdxGC5pyFsveI4J QcLrhuCLhJ4GE7Sst0+OKLU2tsycFuL82SI6D9J53u+3FS10UynOElQn7+2KV6bAeGyG J1Zw== X-Gm-Message-State: AGi0Puaz3Hc4PGdZ47vrnyz3NV2Of6qGeyvP1TRee8/Kxxh5xpy0VmxC rADrYp0QGQpMd/kd7BDGTK1pFQ== X-Google-Smtp-Source: APiQypKEYJ/pkvNef2ub07kgLSQWXyV6t+7I1uJpmUYe8qcwqtA41xahnnjIJhrjG/7qrZHvrQPgVw== X-Received: by 2002:a1c:9a13:: with SMTP id c19mr3451388wme.159.1588760310243; Wed, 06 May 2020 03:18:30 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.75.87]) by smtp.googlemail.com with ESMTPSA id p8sm1980578wre.11.2020.05.06.03.18.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 May 2020 03:18:29 -0700 (PDT) Subject: Re: [PATCH edk2-platforms 2/5] Platform/RaspberryPi: introduce DebugDualSerialPortLib To: Ard Biesheuvel , devel@edk2.groups.io Cc: leif@nuviainc.com, andrey.warkentin@gmail.com References: <20200505145029.29826-1-ard.biesheuvel@arm.com> <20200505145029.29826-3-ard.biesheuvel@arm.com> From: "Pete Batard" Message-ID: <5ca612ab-a452-c9b0-223b-7192d81d214f@akeo.ie> Date: Wed, 6 May 2020 11:18:28 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200505145029.29826-3-ard.biesheuvel@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit On 2020.05.05 15:50, Ard Biesheuvel wrote: > On DEBUG builds that use the serial port directly for debug output, > every module reinitializes the UART hardware, through the DebugLib > constructor calling SerialPortInitialize. > > This is unnecessary, but usually harmless. However, in cases where this > requires information that is non-trivial to obtain (e.g., the rate of > the clock source feeding the baud clock), it results in a special kind > of dependency hell that can only be fully appreciated by seasoned EDK2 > connoisseurs [0]. > > As a first step towards solving this mess, implement a special version > of the Raspberry Pi dual serial port library that only implements the > SerialPortInitialize() and SerialPortWrite() library functions, and make > the former an empty stub. This makes it only suitable for use by modules > that inherit a dependency on SerialPortLib via DebugLib, and requires us > to ensure that the baud clock is programmed correctly by the SEC phase. > > Use this version of the library to satisfy all SerialPortLib dependencies > except the ones in PrePi and in SerialDxe. These will retain the full > version, which is the only one that still consumes PcdSerialClockRate. > > [0] There are two distinct problems making this mess almost unsolvable: > - SerialPortInitialize() is called directly in various places instead > of relying on constructor ordering, so adding a constructor to a > SerialPortLib implementation does not help, > - Constructor ordering resolution in the EDK2 tooling fails to take > transitive dependencies into account if an intermediate library has > no constructor it self. For instance, if LibA depends on LibB, which > depends on LibC, the constructors of LibA and LibC could be called in > any order if LibB does not have a constructor itself (and fixing this > breaks all the platforms in the tree) > > Signed-off-by: Ard Biesheuvel > --- > Platform/RaspberryPi/RPi3/RPi3.dsc | 12 +++-- > Platform/RaspberryPi/RPi4/RPi4.dsc | 12 +++-- > Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.inf | 46 ++++++++++++++++++++ > Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.c | 28 ++++++++++++ > 4 files changed, 92 insertions(+), 6 deletions(-) > > diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc > index 563fb891b841..d7218219fc5a 100644 > --- a/Platform/RaspberryPi/RPi3/RPi3.dsc > +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc > @@ -127,7 +127,7 @@ [LibraryClasses.common] > # Dual serial port library > PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf > PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf > - SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf > + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.inf > > # Cryptographic libraries > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf > @@ -521,7 +521,10 @@ [Components.common] > # > # PEI Phase modules > # > - ArmPlatformPkg/PrePi/PeiUniCore.inf > + ArmPlatformPkg/PrePi/PeiUniCore.inf { > + > + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf > + } > > # > # DXE > @@ -569,7 +572,10 @@ [Components.common] > MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf > MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > - MdeModulePkg/Universal/SerialDxe/SerialDxe.inf > + MdeModulePkg/Universal/SerialDxe/SerialDxe.inf { > + > + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf > + } > Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf > > MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc > index 4deccd9d3ecc..4fb015b077e6 100644 > --- a/Platform/RaspberryPi/RPi4/RPi4.dsc > +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc > @@ -127,7 +127,7 @@ [LibraryClasses.common] > # Dual serial port library > PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf > PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf > - SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf > + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.inf > > # Cryptographic libraries > IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf > @@ -536,7 +536,10 @@ [Components.common] > # > # PEI Phase modules > # > - ArmPlatformPkg/PrePi/PeiUniCore.inf > + ArmPlatformPkg/PrePi/PeiUniCore.inf { > + > + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf > + } > > # > # DXE > @@ -584,7 +587,10 @@ [Components.common] > MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf > MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf > MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf > - MdeModulePkg/Universal/SerialDxe/SerialDxe.inf > + MdeModulePkg/Universal/SerialDxe/SerialDxe.inf { > + > + SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf > + } > Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf > EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf > > diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.inf b/Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.inf > new file mode 100644 > index 000000000000..cd14d44c59d8 > --- /dev/null > +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.inf > @@ -0,0 +1,46 @@ > +## @file > +# > +# SerialPortLib instance for both PL011 and 16550 UART that satisfies > +# only the dependencies of DebugLib. > +# > +# Copyright (c) 2020, Pete Batard > +# Copyright (c) 2012 - 2020, ARM Ltd. All rights reserved.
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 1.27 > + BASE_NAME = DebugDualSerialPortLib > + FILE_GUID = 323bae1b-c2fc-4929-a2fe-9e9174f8ce0f > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = SerialPortLib > + > +[Packages] > + ArmPlatformPkg/ArmPlatformPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + Silicon/Broadcom/Bcm283x/Bcm283x.dec > + > +[LibraryClasses] > + IoLib > + PcdLib > + PL011UartLib > + > +[Sources] > + DebugDualSerialPortLib.c > + DualSerialPortLib.h > + DualSerialPortLibCommon.c > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialDetectCable ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride ## CONSUMES > + > +[FixedPcd] > + gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress ## CONSUMES > diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.c > new file mode 100644 > index 000000000000..fbb9fde08bac > --- /dev/null > +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DebugDualSerialPortLib.c > @@ -0,0 +1,28 @@ > +/** @file > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > + > +/** > + 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 > + ) > +{ > + return RETURN_SUCCESS; > +} > Reviewed-by: Pete Batard