Initially, FdtDxe used an internal (embedded in UEFI) FDT,
because it was neither understood how to consume the
one loaded by the VideoCore firmware, nor understood just
how important it is to use the DTB provided by config.txt.
Embedding the DT was a bad idea, because:
- Permanently stale
- No overlays
Also, on devices like the Pi 4 you _have_ to have a DT
around for the start4 VPU firmware to pick up, otherwise
the board is left in an inconsistent state. So we're being
proscriptive now about DT use with config.txt, which means
this internal DT logic is deadc code.
Further FdtDxe cleanups are possible and will be handled
separately, specifically:
- probably no need to use a separate allocation for patched DT (optimize memory
used)
- suspicious use of EfiBootServicesData (I filed
https://github.com/ARM-software/ebbr/issues/45 to sort out the real
requirements)
Testing: Booted Ubuntu 18.04 on Pi 2B (1.2).
Signed-off-by: Andrei Warkentin <andrey.warken...@gmail.com>
---
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 206 ++++----------------
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 4 -
Platform/RaspberryPi/RPi3/RPi3.fdf | 11 --
Platform/RaspberryPi/RPi4/RPi4.fdf | 8 -
Platform/RaspberryPi/RaspberryPi.dec | 7 -
5 files changed, 38 insertions(+), 198 deletions(-)
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index e7143f57..187b9566 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -335,90 +335,6 @@ CleanSimpleFramebuffer (
return EFI_SUCCESS;
}
-#define MAX_CMDLINE_SIZE 512
-
-STATIC
-EFI_STATUS
-UpdateBootArgs (
- VOID
- )
-{
- INTN Node;
- INTN Retval;
- EFI_STATUS Status;
- CHAR8 *CommandLine;
-
- //
- // Locate the /chosen node
- //
- Node = fdt_path_offset (mFdtImage, "/chosen");
- if (Node < 0) {
- DEBUG ((DEBUG_ERROR, "%a: failed to locate /chosen node\n", __FUNCTION__));
- return EFI_NOT_FOUND;
- }
-
- //
- // If /chosen/bootargs already exists, we want to add a space character
- // before adding the firmware supplied arguments. However, the RpiFirmware
- // protocol expects a 32-bit aligned buffer. So let's allocate 4 bytes of
- // slack, and skip the first 3 when passing this buffer into libfdt.
- //
- CommandLine = AllocatePool (MAX_CMDLINE_SIZE) + 4;
- if (!CommandLine) {
- DEBUG ((DEBUG_ERROR, "%a: failed to allocate memory\n", __FUNCTION__));
- return EFI_OUT_OF_RESOURCES;
- }
-
- //
- // Get the command line from the firmware
- //
- Status = mFwProtocol->GetCommandLine (MAX_CMDLINE_SIZE, CommandLine + 4);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a: failed to retrieve command line\n",
__FUNCTION__));
- return Status;
- }
-
- if (AsciiStrLen (CommandLine + 4) == 0) {
- DEBUG ((DEBUG_INFO, "%a: empty command line received\n", __FUNCTION__));
- return EFI_SUCCESS;
- }
-
- CommandLine[3] = ' ';
-
- Retval = fdt_appendprop_string (mFdtImage, Node, "bootargs",
&CommandLine[3]);
- if (Retval != 0) {
- DEBUG ((DEBUG_ERROR, "%a: failed to set /chosen/bootargs property (%d)\n",
- __FUNCTION__, Retval));
- }
-
- DEBUG_CODE_BEGIN ();
- CONST CHAR8 *Prop;
- INT32 Length;
- INT32 Index;
-
- Node = fdt_path_offset (mFdtImage, "/chosen");
- ASSERT (Node >= 0);
-
- Prop = fdt_getprop (mFdtImage, Node, "bootargs", &Length);
- ASSERT (Prop != NULL);
-
- DEBUG ((DEBUG_INFO, "Command line set from firmware (length %d):\n'",
Length));
-
- for (Index = 0; Index < Length; Index++, Prop++) {
- if (*Prop == '\0') {
- continue;
- }
- DEBUG ((DEBUG_INFO, "%c", *Prop));
- }
-
- DEBUG ((DEBUG_INFO, "'\n"));
- DEBUG_CODE_END ();
-
- FreePool (CommandLine - 4);
- return EFI_SUCCESS;
-}
-
-
/**
@param ImageHandle of the loaded driver
@param SystemTable Pointer to the System Table
@@ -435,13 +351,10 @@ FdtDxeInitialize (
IN EFI_SYSTEM_TABLE *SystemTable
)
{
+ INT32 Retval;
EFI_STATUS Status;
- EFI_GUID *FdtGuid;
- VOID *FdtImage;
UINTN FdtSize;
- INT32 Retval;
- UINT32 BoardRevision;
- BOOLEAN Internal;
+ VOID *FdtImage = NULL;
if (PcdGet32 (PcdOptDeviceTree) == 0) {
DEBUG ((DEBUG_INFO, "Device Tree disabled per user configuration\n"));
@@ -450,77 +363,28 @@ FdtDxeInitialize (
Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
(VOID**)&mFwProtocol);
- ASSERT_EFI_ERROR (Status);
+ if (mFwProtocol == NULL) {
+ /*
+ * Impossible because of the depex...
+ */
+ ASSERT_EFI_ERROR (Status);
+ return EFI_NOT_FOUND;
+ }
- Internal = FALSE;
FdtImage = (VOID*)(UINTN)PcdGet32 (PcdFdtBaseAddress);
Retval = fdt_check_header (FdtImage);
- if (Retval == 0) {
- /*
- * Have FDT passed via config.txt.
- */
- FdtSize = fdt_totalsize (FdtImage);
- DEBUG ((DEBUG_INFO, "Device Tree passed via config.txt (0x%lx bytes)\n",
FdtSize));
- Status = EFI_SUCCESS;
- } else {
- /*
- * Use one of the embedded FDT's.
- */
- Internal = TRUE;
- DEBUG ((DEBUG_INFO, "No/Bad Device Tree found at address 0x%p (%a), "
- "looking up internal one...\n", FdtImage, fdt_strerror (Retval)));
+ if (Retval != 0) {
/*
- * Query the board revision to differentiate between models.
+ * Any one of:
+ * - Invalid config.txt device_tree_address (not PcdFdtBaseAddress)
+ * - Missing FDT for your Pi variant (if not overriding via device_tree=)
*/
- Status = mFwProtocol->GetModelRevision (&BoardRevision);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Failed to get board type: %r\n", Status));
- DEBUG ((DEBUG_INFO, "Using default internal Device Tree\n"));
- FdtGuid = &gRaspberryPiDefaultFdtGuid;
- } else {
- //
www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
- switch ((BoardRevision >> 4) & 0xFF) {
- case 0x08:
- DEBUG ((DEBUG_INFO, "Using Raspberry Pi 3 Model B internal Device
Tree\n"));
- FdtGuid = &gRaspberryPi3ModelBFdtGuid;
- break;
- case 0x0D:
- DEBUG ((DEBUG_INFO, "Using Raspberry Pi 3 Model B+ internal Device
Tree\n"));
- FdtGuid = &gRaspberryPi3ModelBPlusFdtGuid;
- break;
- case 0x11:
- DEBUG ((DEBUG_INFO, "Using Raspberry Pi 4 Model B internal Device
Tree\n"));
- FdtGuid = &gRaspberryPi4ModelBFdtGuid;
- break;
- default:
- DEBUG ((DEBUG_INFO, "Using default internal Device Tree\n"));
- FdtGuid = &gRaspberryPiDefaultFdtGuid;
- break;
- }
- }
- do {
- Status = GetSectionFromAnyFv (FdtGuid, EFI_SECTION_RAW, 0, &FdtImage,
&FdtSize);
- if (Status == EFI_SUCCESS) {
- if (fdt_check_header (FdtImage) != 0) {
- Status = EFI_INCOMPATIBLE_VERSION;
- }
- }
- // No retry needed if we are successful or are dealing with the default
Fdt.
- if ( (Status == EFI_SUCCESS) ||
- (CompareGuid (FdtGuid, &gRaspberryPiDefaultFdtGuid)) )
- break;
- // Otherwise, try one more time with the default Fdt. An example of this
- // is if we detected a non-default Fdt, that isn't included in the FDF.
- DEBUG ((DEBUG_INFO, "Internal Device Tree was not found for this platform,
"
- "falling back to default...\n"));
- FdtGuid = &gRaspberryPiDefaultFdtGuid;
- } while (1);
+ DEBUG ((DEBUG_ERROR, "No devicetree passed via config.txt\n"));
+ return EFI_NOT_FOUND;
}
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Failed to locate Device Tree: %r\n", Status));
- return Status;
- }
+ FdtSize = fdt_totalsize (FdtImage);
+ DEBUG ((DEBUG_INFO, "Devicetree passed via config.txt (0x%lx bytes)\n",
FdtSize));
/*
* Probably overkill.
@@ -529,12 +393,19 @@ FdtDxeInitialize (
Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData,
EFI_SIZE_TO_PAGES (FdtSize),
(EFI_PHYSICAL_ADDRESS*)&mFdtImage);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Failed to allocate Device Tree: %r\n", Status));
- return Status;
+ DEBUG ((DEBUG_ERROR, "Failed to allocate devicetree: %r\n", Status));
+ goto out;
}
Retval = fdt_open_into (FdtImage, mFdtImage, FdtSize);
- ASSERT (Retval == 0);
+ if (Retval != 0) {
+ DEBUG ((DEBUG_ERROR, "fdt_open_into failed: %d\n", Retval));
+ goto out;
+ }
+
+ /*
+ * These are all best-effort.
+ */
Status = SanitizePSCI ();
if (EFI_ERROR (Status)) {
@@ -566,19 +437,18 @@ FdtDxeInitialize (
Print (L"Failed to update USB compatible properties: %r\n", Status);
}
- if (Internal) {
- /*
- * A GPU-provided DTB already has the full command line.
- */
- Status = UpdateBootArgs ();
- if (EFI_ERROR (Status)) {
- Print (L"Failed to update boot arguments: %r\n", Status);
- }
- }
-
- DEBUG ((DEBUG_INFO, "Installed Device Tree at address 0x%p\n", mFdtImage));
+ DEBUG ((DEBUG_INFO, "Installed devicetree at address %p\n", mFdtImage));
Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mFdtImage);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Couldn't register devicetree: %r\n", Status));
+ goto out;
+ }
+out:
+ if (EFI_ERROR(Status)) {
+ if (mFdtImage != NULL) {
+ gBS->FreePages ((EFI_PHYSICAL_ADDRESS) mFdtImage, EFI_SIZE_TO_PAGES
(FdtSize));
+ }
+ }
return Status;
}
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
index fc37353f..49ba5ba1 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
@@ -35,10 +35,6 @@
[Guids]
gFdtTableGuid
- gRaspberryPi3ModelBFdtGuid
- gRaspberryPi3ModelBPlusFdtGuid
- gRaspberryPi4ModelBFdtGuid
- gRaspberryPiDefaultFdtGuid
[Protocols]
gRaspberryPiFirmwareProtocolGuid ## CONSUMES
diff --git a/Platform/RaspberryPi/RPi3/RPi3.fdf
b/Platform/RaspberryPi/RPi3/RPi3.fdf
index daedc443..e854cd21 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.fdf
+++ b/Platform/RaspberryPi/RPi3/RPi3.fdf
@@ -303,17 +303,6 @@ READ_LOCK_STATUS = TRUE
#
INF Platform/RaspberryPi/Drivers/LogoDxe/LogoDxe.inf
- #
- # Device Tree support (used by FdtDxe)
- # GUIDs should match gRaspberryPi#####FdtGuid's from the .dec
- #
- FILE FREEFORM = DF5DA223-1D27-47C3-8D1B-9A41B55A18BC {
- SECTION RAW =
Platform/RaspberryPi/$(PLATFORM_NAME)/DeviceTree/bcm2710-rpi-3-b.dtb
- }
- FILE FREEFORM = 3D523012-73FE-40E5-892E-1A4DF60F3C0C {
- SECTION RAW =
Platform/RaspberryPi/$(PLATFORM_NAME)/DeviceTree/bcm2710-rpi-3-b-plus.dtb
- }
-
[FV.FVMAIN_COMPACT]
FvAlignment = 16
ERASE_POLARITY = 1
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf
b/Platform/RaspberryPi/RPi4/RPi4.fdf
index c3e9cfc4..b1f7aa23 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -307,14 +307,6 @@ READ_LOCK_STATUS = TRUE
#
INF Platform/RaspberryPi/Drivers/LogoDxe/LogoDxe.inf
- #
- # Device Tree support (used by FdtDxe)
- # GUIDs should match gRaspberryPi#####FdtGuid's from the .dec
- #
- FILE FREEFORM = 80AB6833-CAE4-4CEE-B59D-EB2039B05551 {
- SECTION RAW =
Platform/RaspberryPi/$(PLATFORM_NAME)/DeviceTree/bcm2711-rpi-4-b.dtb
- }
-
[FV.FVMAIN_COMPACT]
FvAlignment = 16
ERASE_POLARITY = 1
diff --git a/Platform/RaspberryPi/RaspberryPi.dec
b/Platform/RaspberryPi/RaspberryPi.dec
index b66322be..66ef6186 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -26,13 +26,6 @@
gRaspberryPiTokenSpaceGuid = {0xCD7CC258, 0x31DB, 0x11E6, {0x9F, 0xD3,
0x63, 0xB0, 0xB8, 0xEE, 0xD6, 0xB5}}
gRaspberryPiEventResetGuid = {0xCD7CC258, 0x31DB, 0x11E6, {0x9F, 0xD3,
0x63, 0xB4, 0xB4, 0xE4, 0xD4, 0xB4}}
gConfigDxeFormSetGuid = {0xCD7CC258, 0x31DB, 0x22E6, {0x9F, 0x22, 0x63,
0xB0, 0xB8, 0xEE, 0xD6, 0xB5}}
- # GUIDs used by FdtDxe to serve a Device Tree at runtime. Not all of these
need to apply
- # to the current platform or match an actual FDF binary, but they need to be
defined.
- gRaspberryPi3ModelBFdtGuid = { 0xDF5DA223, 0x1D27, 0x47C3, { 0x8D, 0x1B,
0x9A, 0x41, 0xB5, 0x5A, 0x18, 0xBC } }
- gRaspberryPi3ModelBPlusFdtGuid = { 0x3D523012, 0x73FE, 0x40E5, { 0x89, 0x2E,
0x1A, 0x4D, 0xF6, 0x0F, 0x3C, 0x0C } }
- gRaspberryPi4ModelBFdtGuid = { 0x80AB6833, 0xCAE4, 0x4CEE, { 0xB5, 0x9D,
0xEB, 0x20, 0x39, 0xB0, 0x55, 0x51 } }
- # Default Fdt to serve if the hardware model can't be detected. Should match
one of the above.
- gRaspberryPiDefaultFdtGuid = {0xDF5DA223, 0x1D27, 0x47C3, { 0x8D, 0x1B,
0x9A, 0x41, 0xB5, 0x5A, 0x18, 0xBC}}
[PcdsFixedAtBuild.common]
#
