From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web09.2005.1583232316939198005 for ; Tue, 03 Mar 2020 02:45:17 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=QFgRktH0; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id j16so3721997wrt.3 for ; Tue, 03 Mar 2020 02:45:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wg05S0HtsLbhoHici2qibDMgD7ShHpWFthQoeE3LXJI=; b=QFgRktH0TVtlMzo1QgMW2adN3g2kIz13LcNbbg/ZyYweaNSrnjDXKWh+zk2o/hYy/0 ckquFmGRl8ZZA7/h5yQRROTATmhZOdwRPOceEA3HjL+7sCCkDYQB0tkgcsgu7TZLN1Zo OL/VwebadB6kA1CsVUJpTBwL/jny3JdFBX0/+xi2DsFqtA2QIPAIq5g4WG6631sQ5nxt ZkOVP/D9Hvf+vvZUmBXDLq4CXPJf+cIQXqZ1YR0PTt3rijVK4aTu30x0NG8/6snt4M2m xIDrU+W0fj98Eg7xa264jH7ah6kABmrK9v9/j+lZ01EsDLhCBFI87Ud5TZ7vegIO8P/e lJmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wg05S0HtsLbhoHici2qibDMgD7ShHpWFthQoeE3LXJI=; b=H50RzeMJg4Qzt4XFtRKG62n6oD1t6PlJVPQIQyiTU5Ew4KGoosdrTptr3imfHk21nh WJOTCMKkHzA2D+AJfc6JAOmouvt2kTGpzYlkCshIKJ/j9DOKEkiTxcdAdjjZbAJMebxm tDPm73UpEa0Iw/x96QT4VrruDpJ+m6YZfxYZvd/ZJ9Pa2//nM4rvucaMxk0oKDYzYZ1Z I6HaXFXd0GW+snax1zJ53tb5ddAbofzNCnbDjh8VsyQeoC+GGlLM7XiwFx4NRJHhgwCZ 92f4tEFgDalmiyE1hghsvyIFFKIv4eQG4MfHn3birmXXnhC9mQh/wmrF32jYMZedt4KV GkDg== X-Gm-Message-State: ANhLgQ2PmxOa230dBc18OE8N4rARdiZ70Rd3UlWff3fyJRgZidI83Pbt JUI69PR9hN0p0TMowDdubXMQgUqLhJ/jcGQi3S59qw== X-Google-Smtp-Source: ADFU+vvM4CpDXP6ftqqnM8cay1OeUSkTL80Yn6p7EuDwFUghiiM/3e2HM2J/b4yc5j+Eqoo0cPzbGypt3uEQLhoCkB8= X-Received: by 2002:a5d:6051:: with SMTP id j17mr4844072wrt.151.1583232315167; Tue, 03 Mar 2020 02:45:15 -0800 (PST) MIME-Version: 1.0 References: <20200303103339.7468-1-pete@akeo.ie> <20200303103339.7468-5-pete@akeo.ie> In-Reply-To: <20200303103339.7468-5-pete@akeo.ie> From: "Ard Biesheuvel" Date: Tue, 3 Mar 2020 11:45:04 +0100 Message-ID: Subject: Re: [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice To: Pete Batard Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Andrei Warkentin Content-Type: text/plain; charset="UTF-8" On Tue, 3 Mar 2020 at 11:33, Pete Batard wrote: > > With this and the previous commit, ACPI_BASIC_MODE_ENABLE becomes > superfluous so remove it. > > New option defaults to enabled on Pi 3, disabled on Pi 4. > > Signed-off-by: Pete Batard This touches a lot of different files at the same time, but I guess it makes sense as a single set, so Reviewed-by: Ard Biesheuvel > --- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 8 ++++++++ > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 1 + > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 +++++ > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 21 ++++++++++++++++++++ > Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 5 +++++ > Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 3 +++ > Platform/RaspberryPi/RPi3/RPi3.dsc | 5 +++++ > Platform/RaspberryPi/RPi4/RPi4.dsc | 10 +++++----- > Platform/RaspberryPi/RPi4/RPi4.fdf | 2 -- > Platform/RaspberryPi/RPi4/Readme.md | 15 ++++++-------- > Platform/RaspberryPi/RaspberryPi.dec | 4 +--- > 11 files changed, 60 insertions(+), 19 deletions(-) > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > index 451a419a5358..31a62094e33e 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > @@ -151,6 +151,14 @@ SetupVariables ( > PcdSet32 (PcdRamLimitTo3GB, 0); > } > > + Size = sizeof (UINT32); > + Status = gRT->GetVariable (L"OptDeviceTree", > + &gConfigDxeFormSetGuid, > + NULL, &Size, &Var32); > + if (EFI_ERROR (Status)) { > + PcdSet32 (PcdOptDeviceTree, PcdGet32 (PcdOptDeviceTree)); > + } > + > Size = sizeof (UINT32); > Status = gRT->GetVariable (L"SdIsArasan", > &gConfigDxeFormSetGuid, > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > index 407aac89c7b3..736d49df562b 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > @@ -72,6 +72,7 @@ [Pcd] > gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot > + gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree > gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB > gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > index 830533a9dc49..2e79e322e558 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > @@ -46,6 +46,11 @@ > #string STR_ADVANCED_3GB_OFF #language en-US "Disabled" > #string STR_ADVANCED_3GB_ON #language en-US "Enabled" > > +#string STR_ADVANCED_DT_PROMPT #language en-US "Device Tree" > +#string STR_ADVANCED_DT_HELP #language en-US "Disable this option to force OSes such as GNU/Linux to use ACPI" > +#string STR_ADVANCED_DT_OFF #language en-US "Disabled (Force ACPI)" > +#string STR_ADVANCED_DT_ON #language en-US "Enabled" > + > /* > * MMC/SD configuration. > */ > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > index 483edd7459c5..d16058da4926 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > @@ -85,6 +85,14 @@ typedef struct { > UINT32 Enabled; > } ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA; > > +typedef struct { > + /* > + * 0 - Do not provide a Device Tree to the OS > + * 1 - Provide a Device Tree to the OS > + */ > + UINT32 Enabled; > +} ADVANCED_DEVICE_TREE_VARSTORE_DATA; > + > typedef struct { > /* > * 0 - Don't disable multi-block. > @@ -162,6 +170,11 @@ formset > name = RamLimitTo3GB, > guid = CONFIGDXE_FORM_SET_GUID; > > + efivarstore ADVANCED_DEVICE_TREE_VARSTORE_DATA, > + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > + name = OptDeviceTree, > + guid = CONFIGDXE_FORM_SET_GUID; > + > efivarstore MMC_DISMULTI_VARSTORE_DATA, > attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > name = MmcDisableMulti, > @@ -279,6 +292,14 @@ formset > option text = STRING_TOKEN(STR_ADVANCED_3GB_ON), value = 1, flags = 0; > endoneof; > endif; > + > + oneof varid = OptDeviceTree.Enabled, > + prompt = STRING_TOKEN(STR_ADVANCED_DT_PROMPT), > + help = STRING_TOKEN(STR_ADVANCED_DT_HELP), > + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED, > + option text = STRING_TOKEN(STR_ADVANCED_DT_OFF), value = 0, flags = 0; > + option text = STRING_TOKEN(STR_ADVANCED_DT_ON), value = 1, flags = DEFAULT; > + endoneof; > endform; > > form formid = 0x1003, > diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c > index eb8048930c30..e7143f57b3b6 100644 > --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c > +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c > @@ -443,6 +443,11 @@ FdtDxeInitialize ( > UINT32 BoardRevision; > BOOLEAN Internal; > > + if (PcdGet32 (PcdOptDeviceTree) == 0) { > + DEBUG ((DEBUG_INFO, "Device Tree disabled per user configuration\n")); > + return EFI_SUCCESS; > + } > + > Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL, > (VOID**)&mFwProtocol); > ASSERT_EFI_ERROR (Status); > diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf > index bf9912b4f7d8..fc37353f883f 100644 > --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf > +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf > @@ -48,3 +48,6 @@ [Depex] > > [FixedPcd] > gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress > + > +[Pcd] > + gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree > diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc > index 48e1a32e1d24..91d5738afbc6 100644 > --- a/Platform/RaspberryPi/RPi3/RPi3.dsc > +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc > @@ -445,6 +445,11 @@ [PcdsDynamicHii.common.DEFAULT] > gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0 > gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0 > > + # > + # Device Tree > + # > + gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1 > + > # > # Common UEFI ones. > # > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc > index 3ce2c3e4d519..79295729f6ee 100644 > --- a/Platform/RaspberryPi/RPi4/RPi4.dsc > +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc > @@ -38,7 +38,6 @@ [Defines] > DEFINE SECURE_BOOT_ENABLE = FALSE > DEFINE INCLUDE_TFTP_COMMAND = FALSE > DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F > - DEFINE ACPI_BASIC_MODE_ENABLE = FALSE > > !ifndef TFA_BUILD_ARTIFACTS > # > @@ -271,8 +270,6 @@ [PcdsFeatureFlag.common] > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE > > - gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|$(ACPI_BASIC_MODE_ENABLE) > - > [PcdsFixedAtBuild.common] > gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000 > gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000 > @@ -483,6 +480,11 @@ [PcdsDynamicHii.common.DEFAULT] > gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0 > gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1 > > + # > + # Device Tree > + # > + gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0 > + > # > # Common UEFI ones. > # > @@ -575,9 +577,7 @@ [Components.common] > > ArmPkg/Drivers/ArmGic/ArmGicDxe.inf > Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf > -!if $(ACPI_BASIC_MODE_ENABLE) == FALSE > Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf > -!endif > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > ArmPkg/Drivers/TimerDxe/TimerDxe.inf > MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf > diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf > index cb5e8e9ae92e..c38320350213 100644 > --- a/Platform/RaspberryPi/RPi4/RPi4.fdf > +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf > @@ -208,9 +208,7 @@ [FV.FvMain] > > INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf > INF Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf > -!if $(ACPI_BASIC_MODE_ENABLE) == FALSE > INF Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf > -!endif > INF Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf > INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf > diff --git a/Platform/RaspberryPi/RPi4/Readme.md b/Platform/RaspberryPi/RPi4/Readme.md > index 758d0124a6cf..21c9fd4f0a4a 100644 > --- a/Platform/RaspberryPi/RPi4/Readme.md > +++ b/Platform/RaspberryPi/RPi4/Readme.md > @@ -16,9 +16,12 @@ Raspberry Pi is a trademark of the [Raspberry Pi Foundation](https://www.raspber > This firmware is still in development stage, meaning that it comes with the > following __major__ limitations: > > -- USB may only work in pre-OS phase at this stage due to nonstandard ECAM, > - missing/incomplete ACPI tables as well as other factors. For Linux, using > - the `ACPI_BASIC_MODE_ENABLE` build option may help. > +- xHCI USB may only work in pre-OS phase due to nonstandard DMA constraints. > + For 4 GB models running Linux, limiting the RAM to 3 GB should help. > + The 3 GB limitation is currently enabled by default in the user settings. > +- Device Tree boot of OSes such as Linux may not work at all. > + For this reason, the provision of a Device Tree is disabled by default in > + the user settings, in order to enforce ACPI boot. > - Serial I/O from the OS may not work due to CPU throttling affecting the > miniUART baudrate. This can be worked around by using the PL011 UART > through the device tree overlays. > @@ -27,12 +30,6 @@ following __major__ limitations: > > Build instructions from the top level edk2-platforms Readme.md apply. > > -The following additional build options are also available: > -- `-D ACPI_BASIC_MODE_ENABLE=1`: Limits OS visible memory to 3 GB and forces > - ACPI (by disabling the Device Tree driver altogether). This may be required > - to boot Operating Systems such as Linux on account of the current PCIe/xHCI > - limitations. > - > # Booting the firmware > > 1. Format a uSD card as FAT16 or FAT32 > diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec > index 7f2c37ac9a7f..25058ccc7783 100644 > --- a/Platform/RaspberryPi/RaspberryPi.dec > +++ b/Platform/RaspberryPi/RaspberryPi.dec > @@ -57,8 +57,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016 > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0|UINT8|0x00000017 > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018 > + gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|1|UINT32|0x0000001B > gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019 > gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A > - > -[PcdsFeatureFlag.common] > - gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x0000001B > -- > 2.21.0.windows.1 >