From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
Gerd Hoffmann <kraxel@redhat.com>,
Leif Lindholm <quic_llindhol@quicinc.com>,
Sami Mujawar <sami.mujawar@arm.com>
Subject: [edk2-devel] [PATCH 1/9] ArmVirtPkg: introduce FdtSerialPortAddressLib
Date: Sun, 8 Oct 2023 17:39:04 +0200 [thread overview]
Message-ID: <20231008153912.175941-2-lersek@redhat.com> (raw)
In-Reply-To: <20231008153912.175941-1-lersek@redhat.com>
Introduce a new library class + instance for:
- collecting serial port base addresses from the device tree,
- collecting the /chosen stdout-path serial port base address from the
device tree.
The logic is loosely based on the following functions:
- SerialPortGetBaseAddress()
[ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c]
- PlatformPeim() [ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c]
- GetSerialConsolePortAddress()
[ArmVirtPkg/Library/Fdt16550SerialPortHookLib/EarlyFdt16550SerialPortHookLib.c]
which are going to be converted to clients of the new library later.
Copyright notices from those other files are preserved.
The new library fixes the following warts, found by reading the existent
code:
- Neither of the three functions check whether the "reg" property exists.
(This may be implicitly checked when they compare the property size to
16.)
- GetSerialConsolePortAddress() uses ScanMem8() for locating a colon (":")
node path separator in "stdout-path", when AsciiStrStr() could work just
as fine. While ScanMem8() is likely faster, "stdout-path" is presumably
very short, and ScanMem8() introduces an extra lib class dependency
(namely BaseMemoryLib).
- If ScanMem8() fails to locate a colon in "stdout-path", then
GetSerialConsolePortAddress() re-measures the length of the whole
"stdout-path" property. This is conceptually (if not performance-wise)
disturbing, because we know the whole size of the "stdout-path" property
from the property lookup just before, so we only need to subtract the
NUL-terminator for learning the length.
- GetSerialConsolePortAddress() does not check if the first (or only) node
path inside the "stdout-path" property is empty. (Not a big deal, the
subsequent alias resolution should simply fail.)
- GetSerialConsolePortAddress() does not verify if the node path retrieved
(and potentially alias-resolved) from "stdout-path" can be located in
the device tree; it assumes it.
- Code is duplicated (of course) between SerialPortGetBaseAddress() and
PlatformPeim(), but more surprisingly, all three functions embed the
same code for verifying the "status" property of the serial port node,
and for checking and reading its "reg" property.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/ArmVirtPkg.dec | 1 +
ArmVirtPkg/ArmVirt.dsc.inc | 1 +
ArmVirtPkg/Include/Library/FdtSerialPortAddressLib.h | 83 +++++++
ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.inf | 27 +++
ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.c | 256 ++++++++++++++++++++
5 files changed, 368 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 4645c91a8375..0f2d7873279f 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -27,6 +27,7 @@ [Includes.common]
[LibraryClasses]
ArmVirtMemInfoLib|Include/Library/ArmVirtMemInfoLib.h
+ FdtSerialPortAddressLib|Include/Library/FdtSerialPortAddressLib.h
[Guids.common]
gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 3f7bac6bf33a..0d7115525497 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -121,6 +121,7 @@ [LibraryClasses.common]
# ARM PL011 UART Driver
PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf
+ FdtSerialPortAddressLib|ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.inf
PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
#PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf
diff --git a/ArmVirtPkg/Include/Library/FdtSerialPortAddressLib.h b/ArmVirtPkg/Include/Library/FdtSerialPortAddressLib.h
new file mode 100644
index 000000000000..3d4c911f58aa
--- /dev/null
+++ b/ArmVirtPkg/Include/Library/FdtSerialPortAddressLib.h
@@ -0,0 +1,83 @@
+/** @file
+ Determine the base addresses of serial ports from the Device Tree.
+
+ Copyright (C) Red Hat
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef FDT_SERIAL_PORT_ADDRESS_LIB_H_
+#define FDT_SERIAL_PORT_ADDRESS_LIB_H_
+
+#include <Base.h>
+
+typedef struct {
+ UINTN NumberOfPorts;
+ UINT64 BaseAddress[2];
+} FDT_SERIAL_PORTS;
+
+/**
+ Collect the first ARRAY_SIZE (Ports->BaseAddress) serial ports into Ports from
+ DeviceTree.
+
+ @param[in] DeviceTree The flat device tree (FDT) to scan.
+
+ @param[in] Compatible Look for Compatible in the "compatible" property of the
+ scanned nodes.
+
+ @param[out] Ports On successful return, Ports->NumberOfPorts contains the
+ number of serial ports found; it is (a) positive and
+ (b) at most ARRAY_SIZE (Ports->BaseAddress). If the FDT
+ had more serial ports, those are not reported. On
+ error, the contents of Ports are indeterminate.
+
+ @retval RETURN_INVALID_PARAMETER DeviceTree does not point to a valid FDT
+ header.
+
+ @retval RETURN_NOT_FOUND No compatible and enabled serial port has
+ been found.
+
+ @retval RETURN_SUCCESS At least one compatible and enabled serial
+ port has been found; Ports has been filled
+ in.
+**/
+RETURN_STATUS
+EFIAPI
+FdtSerialGetPorts (
+ IN CONST VOID *DeviceTree,
+ IN CONST CHAR8 *Compatible,
+ OUT FDT_SERIAL_PORTS *Ports
+ );
+
+/**
+ Fetch the base address of the serial port identified in the "stdout-path"
+ property of the "/chosen" node in DeviceTree.
+
+ @param[in] DeviceTree The flat device tree (FDT) to scan.
+
+ @param[out] BaseAddress On success, the base address of the preferred serial
+ port (to be used as console). On error, BaseAddress
+ is not modified.
+
+ @retval RETURN_INVALID_PARAMETER DeviceTree does not point to a valid FDT
+ header.
+
+ @retval RETURN_NOT_FOUND No enabled console port has been found.
+
+ @retval RETURN_PROTOCOL_ERROR The first (or only) node path in the
+ "stdout-path" property is an empty string.
+
+ @retval RETURN_PROTOCOL_ERROR The console port has been found in the FDT,
+ but its base address is not correctly
+ represented.
+
+ @retval RETURN_SUCCESS BaseAddress has been populated.
+**/
+RETURN_STATUS
+EFIAPI
+FdtSerialGetConsolePort (
+ IN CONST VOID *DeviceTree,
+ OUT UINT64 *BaseAddress
+ );
+
+#endif
diff --git a/ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.inf b/ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.inf
new file mode 100644
index 000000000000..ae6d0d374b80
--- /dev/null
+++ b/ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.inf
@@ -0,0 +1,27 @@
+## @file
+# Determine the base addresses of serial ports from the Device Tree.
+#
+# Copyright (C) Red Hat
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 1.27
+ BASE_NAME = FdtSerialPortAddressLib
+ FILE_GUID = AEBE813B-25EA-40E5-95C4-2B864FE1E951
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = FdtSerialPortAddressLib
+
+[Sources]
+ FdtSerialPortAddressLib.c
+
+[Packages]
+ ArmVirtPkg/ArmVirtPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ FdtLib
diff --git a/ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.c b/ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.c
new file mode 100644
index 000000000000..f6508e09898a
--- /dev/null
+++ b/ArmVirtPkg/Library/FdtSerialPortAddressLib/FdtSerialPortAddressLib.c
@@ -0,0 +1,256 @@
+/** @file
+ Determine the base addresses of serial ports from the Device Tree.
+
+ Copyright (C) Red Hat
+ Copyright (c) 2011 - 2023, Arm Ltd. All rights reserved.<BR>
+ Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Linaro Ltd. All rights reserved.<BR>
+ Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/FdtSerialPortAddressLib.h>
+#include <libfdt.h>
+
+/**
+ Read the "reg" property of Node in DeviceTree as a UINT64 base address.
+
+ @param[in] DeviceTree The flat device tree (FDT) to scan.
+
+ @param[in] Node The node to read the "reg" property of.
+
+ @param[out] BaseAddress On success, the base address read out of Node's "reg"
+ property. On error, not modified.
+
+ @retval RETURN_DEVICE_ERROR Node has a "status" property with value
+ different from "okay".
+
+ @retval RETURN_NOT_FOUND Node does not have a "reg" property.
+
+ @retval RETURN_BAD_BUFFER_SIZE The size of Node's "reg" property is not 16
+ bytes.
+
+ @retval RETURN_SUCCESS BaseAddress has been populated.
+**/
+STATIC
+RETURN_STATUS
+GetBaseAddress (
+ IN CONST VOID *DeviceTree,
+ IN INT32 Node,
+ OUT UINT64 *BaseAddress
+ )
+{
+ CONST CHAR8 *NodeStatus;
+ CONST VOID *RegProp;
+ INT32 PropSize;
+
+ NodeStatus = fdt_getprop (DeviceTree, Node, "status", NULL);
+ if ((NodeStatus != NULL) && (AsciiStrCmp (NodeStatus, "okay") != 0)) {
+ return RETURN_DEVICE_ERROR;
+ }
+
+ RegProp = fdt_getprop (DeviceTree, Node, "reg", &PropSize);
+ if (RegProp == NULL) {
+ return RETURN_NOT_FOUND;
+ }
+
+ if (PropSize != 16) {
+ return RETURN_BAD_BUFFER_SIZE;
+ }
+
+ *BaseAddress = fdt64_to_cpu (ReadUnaligned64 (RegProp));
+ return RETURN_SUCCESS;
+}
+
+/**
+ Collect the first ARRAY_SIZE (Ports->BaseAddress) serial ports into Ports from
+ DeviceTree.
+
+ @param[in] DeviceTree The flat device tree (FDT) to scan.
+
+ @param[in] Compatible Look for Compatible in the "compatible" property of the
+ scanned nodes.
+
+ @param[out] Ports On successful return, Ports->NumberOfPorts contains the
+ number of serial ports found; it is (a) positive and
+ (b) at most ARRAY_SIZE (Ports->BaseAddress). If the FDT
+ had more serial ports, those are not reported. On
+ error, the contents of Ports are indeterminate.
+
+ @retval RETURN_INVALID_PARAMETER DeviceTree does not point to a valid FDT
+ header.
+
+ @retval RETURN_NOT_FOUND No compatible and enabled serial port has
+ been found.
+
+ @retval RETURN_SUCCESS At least one compatible and enabled serial
+ port has been found; Ports has been filled
+ in.
+**/
+RETURN_STATUS
+EFIAPI
+FdtSerialGetPorts (
+ IN CONST VOID *DeviceTree,
+ IN CONST CHAR8 *Compatible,
+ OUT FDT_SERIAL_PORTS *Ports
+ )
+{
+ INT32 Node;
+
+ if (fdt_check_header (DeviceTree) != 0) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ Ports->NumberOfPorts = 0;
+ Node = fdt_next_node (DeviceTree, 0, NULL);
+ while ((Node > 0) &&
+ (Ports->NumberOfPorts < ARRAY_SIZE (Ports->BaseAddress)))
+ {
+ CONST CHAR8 *CompatProp;
+ INT32 PropSize;
+
+ CompatProp = fdt_getprop (DeviceTree, Node, "compatible", &PropSize);
+ if (CompatProp != NULL) {
+ CONST CHAR8 *CompatItem;
+
+ CompatItem = CompatProp;
+ while ((CompatItem < CompatProp + PropSize) &&
+ (AsciiStrCmp (CompatItem, Compatible) != 0))
+ {
+ CompatItem += AsciiStrLen (CompatItem) + 1;
+ }
+
+ if (CompatItem < CompatProp + PropSize) {
+ RETURN_STATUS Status;
+ UINT64 BaseAddress;
+
+ Status = GetBaseAddress (DeviceTree, Node, &BaseAddress);
+ if (!RETURN_ERROR (Status)) {
+ Ports->BaseAddress[Ports->NumberOfPorts++] = BaseAddress;
+ }
+ }
+ }
+
+ Node = fdt_next_node (DeviceTree, Node, NULL);
+ }
+
+ return Ports->NumberOfPorts > 0 ? RETURN_SUCCESS : RETURN_NOT_FOUND;
+}
+
+/**
+ Fetch the base address of the serial port identified in the "stdout-path"
+ property of the "/chosen" node in DeviceTree.
+
+ @param[in] DeviceTree The flat device tree (FDT) to scan.
+
+ @param[out] BaseAddress On success, the base address of the preferred serial
+ port (to be used as console). On error, BaseAddress
+ is not modified.
+
+ @retval RETURN_INVALID_PARAMETER DeviceTree does not point to a valid FDT
+ header.
+
+ @retval RETURN_NOT_FOUND No enabled console port has been found.
+
+ @retval RETURN_PROTOCOL_ERROR The first (or only) node path in the
+ "stdout-path" property is an empty string.
+
+ @retval RETURN_PROTOCOL_ERROR The console port has been found in the FDT,
+ but its base address is not correctly
+ represented.
+
+ @retval RETURN_SUCCESS BaseAddress has been populated.
+**/
+RETURN_STATUS
+EFIAPI
+FdtSerialGetConsolePort (
+ IN CONST VOID *DeviceTree,
+ OUT UINT64 *BaseAddress
+ )
+{
+ INT32 ChosenNode;
+ CONST CHAR8 *StdoutPathProp;
+ INT32 PropSize;
+ CONST CHAR8 *StdoutPathEnd;
+ UINTN StdoutPathLength;
+ INT32 ConsoleNode;
+ RETURN_STATUS Status;
+
+ if (fdt_check_header (DeviceTree) != 0) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ ChosenNode = fdt_path_offset (DeviceTree, "/chosen");
+ if (ChosenNode < 0) {
+ return RETURN_NOT_FOUND;
+ }
+
+ StdoutPathProp = fdt_getprop (
+ DeviceTree,
+ ChosenNode,
+ "stdout-path",
+ &PropSize
+ );
+ if (StdoutPathProp == NULL) {
+ return RETURN_NOT_FOUND;
+ }
+
+ //
+ // If StdoutPathProp contains a colon (":"), then the colon terminates the
+ // path we're interested in.
+ //
+ StdoutPathEnd = AsciiStrStr (StdoutPathProp, ":");
+ if (StdoutPathEnd == NULL) {
+ StdoutPathLength = PropSize - 1;
+ } else {
+ StdoutPathLength = StdoutPathEnd - StdoutPathProp;
+ }
+
+ if (StdoutPathLength == 0) {
+ return RETURN_PROTOCOL_ERROR;
+ }
+
+ if (StdoutPathProp[0] == '/') {
+ //
+ // StdoutPathProp starts with an absolute node path.
+ //
+ ConsoleNode = fdt_path_offset_namelen (
+ DeviceTree,
+ StdoutPathProp,
+ (INT32)StdoutPathLength
+ );
+ } else {
+ //
+ // StdoutPathProp starts with an alias.
+ //
+ CONST CHAR8 *ResolvedStdoutPath;
+
+ ResolvedStdoutPath = fdt_get_alias_namelen (
+ DeviceTree,
+ StdoutPathProp,
+ (INT32)StdoutPathLength
+ );
+ if (ResolvedStdoutPath == NULL) {
+ return RETURN_NOT_FOUND;
+ }
+
+ ConsoleNode = fdt_path_offset (DeviceTree, ResolvedStdoutPath);
+ }
+
+ if (ConsoleNode < 0) {
+ return RETURN_NOT_FOUND;
+ }
+
+ Status = GetBaseAddress (DeviceTree, ConsoleNode, BaseAddress);
+ switch (Status) {
+ case RETURN_NOT_FOUND:
+ case RETURN_BAD_BUFFER_SIZE:
+ return RETURN_PROTOCOL_ERROR;
+ case RETURN_SUCCESS:
+ return RETURN_SUCCESS;
+ default:
+ return RETURN_NOT_FOUND;
+ }
+}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109391): https://edk2.groups.io/g/devel/message/109391
Mute This Topic: https://groups.io/mt/101834875/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2023-10-08 15:39 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-08 15:39 [edk2-devel] [PATCH 0/9] ArmVirtPkg: support two PL011 UARTs Laszlo Ersek
2023-10-08 15:39 ` Laszlo Ersek [this message]
2023-10-08 15:39 ` [edk2-devel] [PATCH 2/9] ArmVirtPkg/Fdt16550SerialPortHookLib: rebase to FdtSerialPortAddressLib Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 3/9] ArmVirtPkg: adjust whitespace in block scope declarations Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 4/9] ArmVirtPkg: adhere to the serial port selected by /chosen "stdout-path" Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 5/9] ArmVirtPkg: store separate console and debug PL011 addresses in GUID HOB Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 6/9] ArmVirtPkg: introduce DebugLibFdtPL011Uart Flash instance Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 7/9] ArmVirtPkg: introduce DebugLibFdtPL011Uart RAM instance Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 8/9] ArmVirtPkg: introduce DebugLibFdtPL011Uart DXE Runtime instance Laszlo Ersek
2023-10-08 15:39 ` [edk2-devel] [PATCH 9/9] ArmVirtPkg: steer DebugLib output away from SerialPortLib+console traffic Laszlo Ersek
2023-10-10 7:43 ` [edk2-devel] [PATCH 0/9] ArmVirtPkg: support two PL011 UARTs Ard Biesheuvel
2023-10-10 15:33 ` Laszlo Ersek
2023-10-26 14:21 ` Peter Maydell
2023-10-26 14:46 ` Julien Grall
2023-10-26 14:55 ` Ard Biesheuvel
2023-10-26 15:36 ` Laszlo Ersek
2023-10-26 15:30 ` Laszlo Ersek
2023-10-26 15:19 ` Laszlo Ersek
2023-10-26 15:21 ` Ard Biesheuvel
2023-10-26 19:00 ` Laszlo Ersek
2023-10-27 10:57 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231008153912.175941-2-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox