[edk2-devel] [edk2-platforms][PATCH 2/2] RPi: allow selecting DT-only mode
Ard Biesheuvel
ard.biesheuvel at arm.com
Mon May 11 11:25:16 UTC 2020
On 5/11/20 12:58 PM, Pete Batard wrote:
> On 2020.05.10 22:34, Andrei Warkentin wrote:
>> Today the Pies can be booted in a way where only ACPI is exposed,
>> or both ACPI and DT are exposed.
>>
>> This adds one more mode - DT only, no ACPI. The target audience
>> is developers. When both are exposed, it's up to the OS to decide
>> which gets used, and that choice can differ between OSes,
>>
>> Note: this does _not_ change defaults. Pi 3 still defaults to
>> ACPI + DT, while Pi 4 still defaults to ACPI only.
>>
>> We don't really want to remove DT + ACPI mode - it is the default
>> on Pi 3, and removing it is bound to just annoy users - WoA and
>> NetBSD (voa UEFI) on Pi 3 only work with ACPI, while everything
>> else (Linux, FreeBSD) only work with DT. I'd make an analogy of
>> MPS and ACPI being exposed for the longest time ever together on
>> PCs.
>
> I agree with this.
>
> Even if other platforms may avoid this configuration, we did make DT +
> ACPI mode the default for the Pi 3, and folks might already be using
> this setup on the Pi 3 to boot Linux in DT mode and Windows in ACPI mode
> without having to change their settings. So removing that dual mode may
> be seen as a regression.
>
Fair enough.
>>
>> Testing: OpenBSD on Pi 4 with DT-only and ACPI-only boots.
>>
>> Signed-off-by: Andrei Warkentin <andrey.warkentin at gmail.com>
>> ---
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11
>> +++++++----
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 +-
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 9 +++++----
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 15
>> ++++++++-------
>> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 3 ++-
>> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 2 +-
>> Platform/RaspberryPi/Include/ConfigVars.h | 11
>> +++++------
>> Platform/RaspberryPi/RPi3/RPi3.dsc | 8 ++++++--
>> Platform/RaspberryPi/RPi4/RPi4.dsc | 8 ++++++--
>> Platform/RaspberryPi/RaspberryPi.dec | 2 +-
>> 10 files changed, 42 insertions(+), 29 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> index 8c9609f3..29701db2 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> @@ -155,11 +155,11 @@ SetupVariables (
>> }
>> Size = sizeof (UINT32);
>> - Status = gRT->GetVariable (L"OptDeviceTree",
>> + Status = gRT->GetVariable (L"SystemTableMode",
>> &gConfigDxeFormSetGuid,
>> NULL, &Size, &Var32);
>> if (EFI_ERROR (Status)) {
>> - PcdSet32 (PcdOptDeviceTree, PcdGet32 (PcdOptDeviceTree));
>> + PcdSet32 (PcdSystemTableMode, PcdGet32 (PcdSystemTableMode));
>> }
>> Size = sizeof (UINT32);
>> @@ -488,8 +488,11 @@ ConfigInitialize (
>> DEBUG ((DEBUG_ERROR, "Couldn't install ConfigDxe configuration
>> pages: %r\n", Status));
>> }
>> - Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
>> - ASSERT_EFI_ERROR (Status);
>> + if (PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_ACPI ||
>> + PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {
>
> Overall, I think code readability might have been improved by using
> bitmasks (e.g. bit 0 for ACPI, bit 1 for DT) rather than a triplet of
> values, but I'm fine with the patch as it stands.
>
>> + Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
>> + ASSERT_EFI_ERROR (Status);
>> + }
>> return EFI_SUCCESS;
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> index 57963baf..e47f3af6 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> @@ -77,7 +77,7 @@
>> gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit
>> gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes
>> gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot
>> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
>> + gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
>> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> index 07660072..db36e200 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> @@ -41,10 +41,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"
>> +#string STR_ADVANCED_SYSTAB_PROMPT #language en-US "System Table
>> Selection"
>> +#string STR_ADVANCED_SYSTAB_HELP #language en-US "ACPI/DT choice
>> for specific OSes"
>> +#string STR_ADVANCED_SYSTAB_ACPI #language en-US "ACPI"
>> +#string STR_ADVANCED_SYSTAB_BOTH #language en-US "ACPI + Devicetree"
>> +#string STR_ADVANCED_SYSTAB_DT #language en-US "Devicetree"
>> /*
>> * MMC/SD configuration.
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> index 576eabe9..91a0ea2d 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> @@ -44,9 +44,9 @@ formset
>> name = RamLimitTo3GB,
>> guid = CONFIGDXE_FORM_SET_GUID;
>> - efivarstore ADVANCED_DEVICE_TREE_VARSTORE_DATA,
>> + efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
>> attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>> - name = OptDeviceTree,
>> + name = SystemTableMode,
>> guid = CONFIGDXE_FORM_SET_GUID;
>> efivarstore MMC_SD_VARSTORE_DATA,
>> @@ -164,12 +164,13 @@ formset
>> endoneof;
>> endif;
>> - oneof varid = OptDeviceTree.Enabled,
>> - prompt = STRING_TOKEN(STR_ADVANCED_DT_PROMPT),
>> - help = STRING_TOKEN(STR_ADVANCED_DT_HELP),
>> + oneof varid = SystemTableMode.Mode,
>> + prompt = STRING_TOKEN(STR_ADVANCED_SYSTAB_PROMPT),
>> + help = STRING_TOKEN(STR_ADVANCED_SYSTAB_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;
>> + option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_ACPI),
>> value = SYSTEM_TABLE_MODE_ACPI, flags = 0;
>> + option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_BOTH),
>> value = SYSTEM_TABLE_MODE_BOTH, flags = DEFAULT;
>> + option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value
>> = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
>> endoneof;
>> endform;
>> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
>> b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
>> index 3aaa0a7f..44c6c87c 100644
>> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
>> @@ -355,7 +355,8 @@ FdtDxeInitialize (
>> UINTN FdtSize;
>> VOID *FdtImage = NULL;
>> - if (PcdGet32 (PcdOptDeviceTree) == 0) {
>> + if (PcdGet32 (PcdSystemTableMode) != SYSTEM_TABLE_MODE_BOTH &&
>> + PcdGet32 (PcdSystemTableMode) != SYSTEM_TABLE_MODE_DT) {
>> DEBUG ((DEBUG_INFO, "Device Tree disabled per user
>> configuration\n"));
>> return EFI_SUCCESS;
>> }
>> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
>> b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
>> index 49ba5ba1..26f84e59 100644
>> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
>> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
>> @@ -46,4 +46,4 @@
>> gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress
>> [Pcd]
>> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
>> + gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
>> diff --git a/Platform/RaspberryPi/Include/ConfigVars.h
>> b/Platform/RaspberryPi/Include/ConfigVars.h
>> index a0959b4b..28837f98 100644
>> --- a/Platform/RaspberryPi/Include/ConfigVars.h
>> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
>> @@ -76,12 +76,11 @@ typedef struct {
>> } 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;
>> +#define SYSTEM_TABLE_MODE_ACPI 0
>> +#define SYSTEM_TABLE_MODE_BOTH 1
>> +#define SYSTEM_TABLE_MODE_DT 2
>> + UINT32 Mode;
>> +} SYSTEM_TABLE_MODE_VARSTORE_DATA;
>> typedef struct {
>> /*
>> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc
>> b/Platform/RaspberryPi/RPi3/RPi3.dsc
>> index 563fb891..a138c874 100644
>> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
>> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
>> @@ -481,9 +481,13 @@
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
>>
>> #
>> - # Device Tree
>> + # Device Tree and ACPI selection.
>> #
>> -
>> gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
>>
>> + # 0 - SYSTEM_TABLE_MODE_ACPI
>> + # 1 - SYSTEM_TABLE_MODE_BOTH (default)
>> + # 2 - SYSTEM_TABLE_MODE_DT
>> + #
>> +
>> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|1
>>
>> #
>> # Common UEFI ones.
>> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> index 4deccd9d..75867f03 100644
>> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> @@ -496,9 +496,13 @@
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
>>
>> #
>> - # Device Tree
>> + # Device Tree and ACPI selection.
>> #
>> -
>> gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
>>
>> + # 0 - SYSTEM_TABLE_MODE_ACPI (default)
>> + # 1 - SYSTEM_TABLE_MODE_BOTH
>> + # 2 - SYSTEM_TABLE_MODE_DT
>> + #
>> +
>> gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|0
>>
>> #
>> # Common UEFI ones.
>> diff --git a/Platform/RaspberryPi/RaspberryPi.dec
>> b/Platform/RaspberryPi/RaspberryPi.dec
>> index 66ef6186..1a3c44e0 100644
>> --- a/Platform/RaspberryPi/RaspberryPi.dec
>> +++ b/Platform/RaspberryPi/RaspberryPi.dec
>> @@ -65,6 +65,6 @@
>> gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016
>>
>> gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0x3F|UINT8|0x00000017
>>
>> gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018
>> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|1|UINT32|0x0000001B
>> + gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|1|UINT32|0x0000001B
>> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
>>
>
> Reviewed-by: Pete Batard <pete at akeo.ie>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59064): https://edk2.groups.io/g/devel/message/59064
Mute This Topic: https://groups.io/mt/74124181/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list