* [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
@ 2020-05-06 0:59 Andrei Warkentin
2020-05-06 6:38 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Andrei Warkentin @ 2020-05-06 0:59 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, pete, philmd
Today we just have an option to hide DT entirely, while ACPI
is always exposed. This change extends the option to
provide all three choices:
- ACPI only
- ACPI + DT
- DT only
Why? Because not all OSes will prefer DT over ACPI when both are available.
To do this cleanly, move the variable structure and value definitions
into a separate header, ConfigVars.h.
Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 20 +--
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 +-
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 9 +-
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 146 ++------------------
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 6 +-
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 2 +-
Platform/RaspberryPi/Include/ConfigVars.h | 131 ++++++++++++++++++
Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
Platform/RaspberryPi/RPi4/RPi4.dsc | 4 +-
Platform/RaspberryPi/RaspberryPi.dec | 2 +-
10 files changed, 171 insertions(+), 155 deletions(-)
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index c90c2530..00867879 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -22,6 +22,7 @@
#include <IndustryStandard/Bcm2836Gpio.h>
#include <Library/GpioLib.h>
#include <Protocol/RpiFirmware.h>
+#include <ConfigVars.h>
#include "ConfigDxeFormSetGuid.h"
#define FREQ_1_MHZ 1000000
@@ -154,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);
@@ -259,10 +260,10 @@ ApplyVariables (
UINT64 SystemMemorySize;
switch (CpuClock) {
- case 0: // Low
+ case CHIPSET_CPU_CLOCK_LOW:
Rate = FixedPcdGet32 (PcdCpuLowSpeedMHz) * FREQ_1_MHZ;
break;
- case 1: // Default
+ case CHIPSET_CPU_CLOCK_DEFAULT:
/*
* What the Raspberry Pi Foundation calls "max clock rate" is really the default value
* from: https://www.raspberrypi.org/documentation/configuration/config-txt/overclocking.md
@@ -272,10 +273,10 @@ ApplyVariables (
DEBUG ((DEBUG_ERROR, "Couldn't read default CPU speed %r\n", Status));
}
break;
- case 2: // Max
+ case CHIPSET_CPU_CLOCK_MAX:
Rate = FixedPcdGet32 (PcdCpuMaxSpeedMHz) * FREQ_1_MHZ;
break;
- case 3: // Custom
+ case CHIPSET_CPU_CLOCK_CUSTOM:
Rate = CustomCpuClock * FREQ_1_MHZ;
break;
}
@@ -487,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) {
+ 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..7195e497 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 0a650a94..72cc90ae 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -8,128 +8,7 @@
#include <Guid/HiiPlatformSetupFormset.h>
#include "ConfigDxeFormSetGuid.h"
-
-#pragma pack(1)
-typedef struct {
- /*
- * One bit for each scaled resolution supported,
- * these are ordered exactly like mGopModeData
- * in DisplayDxe.
- *
- * 800x600, 640x480, 1024x768, 720p, 1080p, native.
- */
- UINT8 v640 : 1;
- UINT8 v800 : 1;
- UINT8 v1024 : 1;
- UINT8 v720p : 1;
- UINT8 v1080p : 1;
- UINT8 Physical : 1;
-} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
-#pragma pack()
-
-typedef struct {
- /*
- * 0 - No screenshot support.
- * 1 - Screenshot support via hotkey.
- */
- UINT32 Enable;
-} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - No JTAG.
- * 1 - JTAG mode.
- */
- UINT32 Enable;
-} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't show UEFI exit message.
- * 1 - Show UEFI exit message.
- */
- UINT32 Show;
-} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - low.
- * 1 - default.
- * 2 - maximum.
- * 3 - custom.
- */
- UINT32 Clock;
-} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
-
-typedef struct {
- UINT32 Clock;
-} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
-
-typedef struct {
- /*
- * Always set by ConfigDxe prior to HII init to reflect
- * platform capability.
- */
- UINT32 Supported;
-} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
-
-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 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
- * 1 - uSD slot routed to Arasan SDHCI.
- */
- UINT32 Routing;
-} MMC_SD_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't disable multi-block.
- * 1 - Disable multi-block commands.
- */
- UINT32 DisableMulti;
-} MMC_DISMULTI_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't force 1 bit mode.
- * 1 - Force 1 bit mode.
- */
- UINT32 Force1Bit;
-} MMC_FORCE1BIT_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't force default speed.
- * 1 - Force default speed.
- */
- UINT32 ForceDS;
-} MMC_FORCEDS_VARSTORE_DATA;
-
-typedef struct {
- /*
- * Default Speed MHz override (25MHz default).
- */
- UINT32 MHz;
-} MMC_SD_DS_MHZ_VARSTORE_DATA;
-
-typedef struct {
- /*
- * High Speed MHz override (50MHz default).
- */
- UINT32 MHz;
-} MMC_SD_HS_MHZ_VARSTORE_DATA;
+#include <ConfigVars.h>
//
// EFI Variable attributes
@@ -165,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,
@@ -253,10 +132,10 @@ formset
prompt = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_PROMPT),
help = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_HELP),
flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = 0, flags = 0;
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = 1, flags = DEFAULT;
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = 2, flags = 0;
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = 3, flags = 0;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = CHIPSET_CPU_CLOCK_LOW, flags = 0;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = CHIPSET_CPU_CLOCK_DEFAULT, flags = DEFAULT;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = CHIPSET_CPU_CLOCK_MAX, flags = 0;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = CHIPSET_CPU_CLOCK_CUSTOM, flags = 0;
endoneof;
grayoutif NOT ideqval CpuClock.Clock == 3;
@@ -285,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 cb11256e..0472d8ec 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -15,10 +15,9 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <libfdt.h>
-
#include <Protocol/RpiFirmware.h>
-
#include <Guid/Fdt.h>
+#include <ConfigVars.h>
STATIC VOID *mFdtImage;
@@ -356,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
new file mode 100644
index 00000000..28837f98
--- /dev/null
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -0,0 +1,131 @@
+/** @file
+ *
+ * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#ifndef CONFIG_VARS_H
+#define CONFIG_VARS_H
+
+#pragma pack(1)
+typedef struct {
+ /*
+ * One bit for each scaled resolution supported,
+ * these are ordered exactly like mGopModeData
+ * in DisplayDxe.
+ *
+ * 800x600, 640x480, 1024x768, 720p, 1080p, native.
+ */
+ UINT8 v640 : 1;
+ UINT8 v800 : 1;
+ UINT8 v1024 : 1;
+ UINT8 v720p : 1;
+ UINT8 v1080p : 1;
+ UINT8 Physical : 1;
+} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
+#pragma pack()
+
+typedef struct {
+ /*
+ * 0 - No screenshot support.
+ * 1 - Screenshot support via hotkey.
+ */
+ UINT32 Enable;
+} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - No JTAG.
+ * 1 - JTAG mode.
+ */
+ UINT32 Enable;
+} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't show UEFI exit message.
+ * 1 - Show UEFI exit message.
+ */
+ UINT32 Show;
+} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
+
+typedef struct {
+#define CHIPSET_CPU_CLOCK_LOW 0
+#define CHIPSET_CPU_CLOCK_DEFAULT 1
+#define CHIPSET_CPU_CLOCK_MAX 2
+#define CHIPSET_CPU_CLOCK_CUSTOM 3
+ UINT32 Clock;
+} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
+
+typedef struct {
+ UINT32 Clock;
+} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * Always set by ConfigDxe prior to HII init to reflect
+ * platform capability.
+ */
+ UINT32 Supported;
+} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
+
+typedef struct {
+ UINT32 Enabled;
+} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
+
+typedef struct {
+#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 {
+ /*
+ * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
+ * 1 - uSD slot routed to Arasan SDHCI.
+ */
+ UINT32 Routing;
+} MMC_SD_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't disable multi-block.
+ * 1 - Disable multi-block commands.
+ */
+ UINT32 DisableMulti;
+} MMC_DISMULTI_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't force 1 bit mode.
+ * 1 - Force 1 bit mode.
+ */
+ UINT32 Force1Bit;
+} MMC_FORCE1BIT_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't force default speed.
+ * 1 - Force default speed.
+ */
+ UINT32 ForceDS;
+} MMC_FORCEDS_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * Default Speed MHz override (25MHz default).
+ */
+ UINT32 MHz;
+} MMC_SD_DS_MHZ_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * High Speed MHz override (50MHz default).
+ */
+ UINT32 MHz;
+} MMC_SD_HS_MHZ_VARSTORE_DATA;
+
+#endif /* CONFIG_VARS_H */
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 563fb891..721b8c20 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -481,9 +481,9 @@
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
#
- # Device Tree
+ # Device Tree and ACPI selection.
#
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
+ 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..c9ab999a 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -496,9 +496,9 @@
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
#
- # Device Tree
+ # Device Tree and ACPI selection.
#
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
+ 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
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
2020-05-06 0:59 [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed Andrei Warkentin
@ 2020-05-06 6:38 ` Ard Biesheuvel
2020-05-06 18:26 ` [edk2-devel] " Andrei Warkentin
[not found] ` <160C83BCD8110E81.4282@groups.io>
0 siblings, 2 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2020-05-06 6:38 UTC (permalink / raw)
To: Andrei Warkentin, devel; +Cc: leif, pete, philmd
On 5/6/20 2:59 AM, Andrei Warkentin wrote:
> Today we just have an option to hide DT entirely, while ACPI
> is always exposed. This change extends the option to
> provide all three choices:
> - ACPI only
> - ACPI + DT
> - DT only
>
> Why? Because not all OSes will prefer DT over ACPI when both are available.
>
> To do this cleanly, move the variable structure and value definitions
> into a separate header, ConfigVars.h.
>
> Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
I'd prefer it if DT and ACPI were mutually exclusive, so that it is
never left up to the OS to reason about which one is the correct one.
This aligns with other platforms we have in the tree.
> ---
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 20 +--
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 9 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 146 ++------------------
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 6 +-
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 2 +-
> Platform/RaspberryPi/Include/ConfigVars.h | 131 ++++++++++++++++++
> Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 4 +-
> Platform/RaspberryPi/RaspberryPi.dec | 2 +-
> 10 files changed, 171 insertions(+), 155 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index c90c2530..00867879 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -22,6 +22,7 @@
> #include <IndustryStandard/Bcm2836Gpio.h>
> #include <Library/GpioLib.h>
> #include <Protocol/RpiFirmware.h>
> +#include <ConfigVars.h>
> #include "ConfigDxeFormSetGuid.h"
>
> #define FREQ_1_MHZ 1000000
> @@ -154,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);
> @@ -259,10 +260,10 @@ ApplyVariables (
> UINT64 SystemMemorySize;
>
> switch (CpuClock) {
> - case 0: // Low
> + case CHIPSET_CPU_CLOCK_LOW:
> Rate = FixedPcdGet32 (PcdCpuLowSpeedMHz) * FREQ_1_MHZ;
> break;
> - case 1: // Default
> + case CHIPSET_CPU_CLOCK_DEFAULT:
> /*
> * What the Raspberry Pi Foundation calls "max clock rate" is really the default value
> * from: https://www.raspberrypi.org/documentation/configuration/config-txt/overclocking.md
> @@ -272,10 +273,10 @@ ApplyVariables (
> DEBUG ((DEBUG_ERROR, "Couldn't read default CPU speed %r\n", Status));
> }
> break;
> - case 2: // Max
> + case CHIPSET_CPU_CLOCK_MAX:
> Rate = FixedPcdGet32 (PcdCpuMaxSpeedMHz) * FREQ_1_MHZ;
> break;
> - case 3: // Custom
> + case CHIPSET_CPU_CLOCK_CUSTOM:
> Rate = CustomCpuClock * FREQ_1_MHZ;
> break;
> }
> @@ -487,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) {
> + 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..7195e497 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 0a650a94..72cc90ae 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -8,128 +8,7 @@
>
> #include <Guid/HiiPlatformSetupFormset.h>
> #include "ConfigDxeFormSetGuid.h"
> -
> -#pragma pack(1)
> -typedef struct {
> - /*
> - * One bit for each scaled resolution supported,
> - * these are ordered exactly like mGopModeData
> - * in DisplayDxe.
> - *
> - * 800x600, 640x480, 1024x768, 720p, 1080p, native.
> - */
> - UINT8 v640 : 1;
> - UINT8 v800 : 1;
> - UINT8 v1024 : 1;
> - UINT8 v720p : 1;
> - UINT8 v1080p : 1;
> - UINT8 Physical : 1;
> -} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
> -#pragma pack()
> -
> -typedef struct {
> - /*
> - * 0 - No screenshot support.
> - * 1 - Screenshot support via hotkey.
> - */
> - UINT32 Enable;
> -} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - No JTAG.
> - * 1 - JTAG mode.
> - */
> - UINT32 Enable;
> -} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't show UEFI exit message.
> - * 1 - Show UEFI exit message.
> - */
> - UINT32 Show;
> -} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - low.
> - * 1 - default.
> - * 2 - maximum.
> - * 3 - custom.
> - */
> - UINT32 Clock;
> -} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
> -
> -typedef struct {
> - UINT32 Clock;
> -} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * Always set by ConfigDxe prior to HII init to reflect
> - * platform capability.
> - */
> - UINT32 Supported;
> -} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
> -
> -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 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
> - * 1 - uSD slot routed to Arasan SDHCI.
> - */
> - UINT32 Routing;
> -} MMC_SD_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't disable multi-block.
> - * 1 - Disable multi-block commands.
> - */
> - UINT32 DisableMulti;
> -} MMC_DISMULTI_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't force 1 bit mode.
> - * 1 - Force 1 bit mode.
> - */
> - UINT32 Force1Bit;
> -} MMC_FORCE1BIT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't force default speed.
> - * 1 - Force default speed.
> - */
> - UINT32 ForceDS;
> -} MMC_FORCEDS_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * Default Speed MHz override (25MHz default).
> - */
> - UINT32 MHz;
> -} MMC_SD_DS_MHZ_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * High Speed MHz override (50MHz default).
> - */
> - UINT32 MHz;
> -} MMC_SD_HS_MHZ_VARSTORE_DATA;
> +#include <ConfigVars.h>
>
> //
> // EFI Variable attributes
> @@ -165,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,
> @@ -253,10 +132,10 @@ formset
> prompt = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_PROMPT),
> help = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_HELP),
> flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = 0, flags = 0;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = 1, flags = DEFAULT;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = 2, flags = 0;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = 3, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = CHIPSET_CPU_CLOCK_LOW, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = CHIPSET_CPU_CLOCK_DEFAULT, flags = DEFAULT;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = CHIPSET_CPU_CLOCK_MAX, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = CHIPSET_CPU_CLOCK_CUSTOM, flags = 0;
> endoneof;
>
> grayoutif NOT ideqval CpuClock.Clock == 3;
> @@ -285,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 cb11256e..0472d8ec 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -15,10 +15,9 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
> #include <libfdt.h>
> -
> #include <Protocol/RpiFirmware.h>
> -
> #include <Guid/Fdt.h>
> +#include <ConfigVars.h>
>
> STATIC VOID *mFdtImage;
>
> @@ -356,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
> new file mode 100644
> index 00000000..28837f98
> --- /dev/null
> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
> @@ -0,0 +1,131 @@
> +/** @file
> + *
> + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#ifndef CONFIG_VARS_H
> +#define CONFIG_VARS_H
> +
> +#pragma pack(1)
> +typedef struct {
> + /*
> + * One bit for each scaled resolution supported,
> + * these are ordered exactly like mGopModeData
> + * in DisplayDxe.
> + *
> + * 800x600, 640x480, 1024x768, 720p, 1080p, native.
> + */
> + UINT8 v640 : 1;
> + UINT8 v800 : 1;
> + UINT8 v1024 : 1;
> + UINT8 v720p : 1;
> + UINT8 v1080p : 1;
> + UINT8 Physical : 1;
> +} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
> +#pragma pack()
> +
> +typedef struct {
> + /*
> + * 0 - No screenshot support.
> + * 1 - Screenshot support via hotkey.
> + */
> + UINT32 Enable;
> +} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - No JTAG.
> + * 1 - JTAG mode.
> + */
> + UINT32 Enable;
> +} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't show UEFI exit message.
> + * 1 - Show UEFI exit message.
> + */
> + UINT32 Show;
> +} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
> +
> +typedef struct {
> +#define CHIPSET_CPU_CLOCK_LOW 0
> +#define CHIPSET_CPU_CLOCK_DEFAULT 1
> +#define CHIPSET_CPU_CLOCK_MAX 2
> +#define CHIPSET_CPU_CLOCK_CUSTOM 3
> + UINT32 Clock;
> +} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
> +
> +typedef struct {
> + UINT32 Clock;
> +} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * Always set by ConfigDxe prior to HII init to reflect
> + * platform capability.
> + */
> + UINT32 Supported;
> +} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
> +
> +typedef struct {
> + UINT32 Enabled;
> +} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
> +
> +typedef struct {
> +#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 {
> + /*
> + * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
> + * 1 - uSD slot routed to Arasan SDHCI.
> + */
> + UINT32 Routing;
> +} MMC_SD_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't disable multi-block.
> + * 1 - Disable multi-block commands.
> + */
> + UINT32 DisableMulti;
> +} MMC_DISMULTI_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't force 1 bit mode.
> + * 1 - Force 1 bit mode.
> + */
> + UINT32 Force1Bit;
> +} MMC_FORCE1BIT_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't force default speed.
> + * 1 - Force default speed.
> + */
> + UINT32 ForceDS;
> +} MMC_FORCEDS_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * Default Speed MHz override (25MHz default).
> + */
> + UINT32 MHz;
> +} MMC_SD_DS_MHZ_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * High Speed MHz override (50MHz default).
> + */
> + UINT32 MHz;
> +} MMC_SD_HS_MHZ_VARSTORE_DATA;
> +
> +#endif /* CONFIG_VARS_H */
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 563fb891..721b8c20 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -481,9 +481,9 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
>
> #
> - # Device Tree
> + # Device Tree and ACPI selection.
> #
> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
> + 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..c9ab999a 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -496,9 +496,9 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
>
> #
> - # Device Tree
> + # Device Tree and ACPI selection.
> #
> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
> + 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
2020-05-06 6:38 ` Ard Biesheuvel
@ 2020-05-06 18:26 ` Andrei Warkentin
[not found] ` <160C83BCD8110E81.4282@groups.io>
1 sibling, 0 replies; 4+ messages in thread
From: Andrei Warkentin @ 2020-05-06 18:26 UTC (permalink / raw)
To: Andrei Warkentin, devel@edk2.groups.io, ard.biesheuvel@arm.com
Cc: leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com
[-- Attachment #1: Type: text/plain, Size: 19753 bytes --]
Ok that sounds reasonable to me. I'll remove the "both" option entirely.
A
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ard Biesheuvel via groups.io <ard.biesheuvel=arm.com@groups.io>
Sent: Wednesday, May 6, 2020 1:38 AM
To: Andrei Warkentin <andrey.warkentin@gmail.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; philmd@redhat.com <philmd@redhat.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
On 5/6/20 2:59 AM, Andrei Warkentin wrote:
> Today we just have an option to hide DT entirely, while ACPI
> is always exposed. This change extends the option to
> provide all three choices:
> - ACPI only
> - ACPI + DT
> - DT only
>
> Why? Because not all OSes will prefer DT over ACPI when both are available.
>
> To do this cleanly, move the variable structure and value definitions
> into a separate header, ConfigVars.h.
>
> Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
I'd prefer it if DT and ACPI were mutually exclusive, so that it is
never left up to the OS to reason about which one is the correct one.
This aligns with other platforms we have in the tree.
> ---
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 20 +--
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 9 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 146 ++------------------
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 6 +-
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 2 +-
> Platform/RaspberryPi/Include/ConfigVars.h | 131 ++++++++++++++++++
> Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 4 +-
> Platform/RaspberryPi/RaspberryPi.dec | 2 +-
> 10 files changed, 171 insertions(+), 155 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index c90c2530..00867879 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -22,6 +22,7 @@
> #include <IndustryStandard/Bcm2836Gpio.h>
> #include <Library/GpioLib.h>
> #include <Protocol/RpiFirmware.h>
> +#include <ConfigVars.h>
> #include "ConfigDxeFormSetGuid.h"
>
> #define FREQ_1_MHZ 1000000
> @@ -154,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);
> @@ -259,10 +260,10 @@ ApplyVariables (
> UINT64 SystemMemorySize;
>
> switch (CpuClock) {
> - case 0: // Low
> + case CHIPSET_CPU_CLOCK_LOW:
> Rate = FixedPcdGet32 (PcdCpuLowSpeedMHz) * FREQ_1_MHZ;
> break;
> - case 1: // Default
> + case CHIPSET_CPU_CLOCK_DEFAULT:
> /*
> * What the Raspberry Pi Foundation calls "max clock rate" is really the default value
> * from: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.raspberrypi.org%2Fdocumentation%2Fconfiguration%2Fconfig-txt%2Foverclocking.md&data=02%7C01%7Cawarkentin%40vmware.com%7Cc02f0ea3653949b6847c08d7f1882716%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637243439445722301&sdata=BQl1mDfJ3Yt6YB%2FQGs6z6DoQ%2BQEiuGTuBAhIvsYrZ0w%3D&reserved=0
> @@ -272,10 +273,10 @@ ApplyVariables (
> DEBUG ((DEBUG_ERROR, "Couldn't read default CPU speed %r\n", Status));
> }
> break;
> - case 2: // Max
> + case CHIPSET_CPU_CLOCK_MAX:
> Rate = FixedPcdGet32 (PcdCpuMaxSpeedMHz) * FREQ_1_MHZ;
> break;
> - case 3: // Custom
> + case CHIPSET_CPU_CLOCK_CUSTOM:
> Rate = CustomCpuClock * FREQ_1_MHZ;
> break;
> }
> @@ -487,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) {
> + 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..7195e497 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 0a650a94..72cc90ae 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -8,128 +8,7 @@
>
> #include <Guid/HiiPlatformSetupFormset.h>
> #include "ConfigDxeFormSetGuid.h"
> -
> -#pragma pack(1)
> -typedef struct {
> - /*
> - * One bit for each scaled resolution supported,
> - * these are ordered exactly like mGopModeData
> - * in DisplayDxe.
> - *
> - * 800x600, 640x480, 1024x768, 720p, 1080p, native.
> - */
> - UINT8 v640 : 1;
> - UINT8 v800 : 1;
> - UINT8 v1024 : 1;
> - UINT8 v720p : 1;
> - UINT8 v1080p : 1;
> - UINT8 Physical : 1;
> -} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
> -#pragma pack()
> -
> -typedef struct {
> - /*
> - * 0 - No screenshot support.
> - * 1 - Screenshot support via hotkey.
> - */
> - UINT32 Enable;
> -} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - No JTAG.
> - * 1 - JTAG mode.
> - */
> - UINT32 Enable;
> -} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't show UEFI exit message.
> - * 1 - Show UEFI exit message.
> - */
> - UINT32 Show;
> -} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - low.
> - * 1 - default.
> - * 2 - maximum.
> - * 3 - custom.
> - */
> - UINT32 Clock;
> -} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
> -
> -typedef struct {
> - UINT32 Clock;
> -} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * Always set by ConfigDxe prior to HII init to reflect
> - * platform capability.
> - */
> - UINT32 Supported;
> -} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
> -
> -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 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
> - * 1 - uSD slot routed to Arasan SDHCI.
> - */
> - UINT32 Routing;
> -} MMC_SD_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't disable multi-block.
> - * 1 - Disable multi-block commands.
> - */
> - UINT32 DisableMulti;
> -} MMC_DISMULTI_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't force 1 bit mode.
> - * 1 - Force 1 bit mode.
> - */
> - UINT32 Force1Bit;
> -} MMC_FORCE1BIT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't force default speed.
> - * 1 - Force default speed.
> - */
> - UINT32 ForceDS;
> -} MMC_FORCEDS_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * Default Speed MHz override (25MHz default).
> - */
> - UINT32 MHz;
> -} MMC_SD_DS_MHZ_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * High Speed MHz override (50MHz default).
> - */
> - UINT32 MHz;
> -} MMC_SD_HS_MHZ_VARSTORE_DATA;
> +#include <ConfigVars.h>
>
> //
> // EFI Variable attributes
> @@ -165,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,
> @@ -253,10 +132,10 @@ formset
> prompt = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_PROMPT),
> help = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_HELP),
> flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = 0, flags = 0;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = 1, flags = DEFAULT;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = 2, flags = 0;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = 3, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = CHIPSET_CPU_CLOCK_LOW, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = CHIPSET_CPU_CLOCK_DEFAULT, flags = DEFAULT;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = CHIPSET_CPU_CLOCK_MAX, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = CHIPSET_CPU_CLOCK_CUSTOM, flags = 0;
> endoneof;
>
> grayoutif NOT ideqval CpuClock.Clock == 3;
> @@ -285,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 cb11256e..0472d8ec 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -15,10 +15,9 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
> #include <libfdt.h>
> -
> #include <Protocol/RpiFirmware.h>
> -
> #include <Guid/Fdt.h>
> +#include <ConfigVars.h>
>
> STATIC VOID *mFdtImage;
>
> @@ -356,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
> new file mode 100644
> index 00000000..28837f98
> --- /dev/null
> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
> @@ -0,0 +1,131 @@
> +/** @file
> + *
> + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#ifndef CONFIG_VARS_H
> +#define CONFIG_VARS_H
> +
> +#pragma pack(1)
> +typedef struct {
> + /*
> + * One bit for each scaled resolution supported,
> + * these are ordered exactly like mGopModeData
> + * in DisplayDxe.
> + *
> + * 800x600, 640x480, 1024x768, 720p, 1080p, native.
> + */
> + UINT8 v640 : 1;
> + UINT8 v800 : 1;
> + UINT8 v1024 : 1;
> + UINT8 v720p : 1;
> + UINT8 v1080p : 1;
> + UINT8 Physical : 1;
> +} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
> +#pragma pack()
> +
> +typedef struct {
> + /*
> + * 0 - No screenshot support.
> + * 1 - Screenshot support via hotkey.
> + */
> + UINT32 Enable;
> +} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - No JTAG.
> + * 1 - JTAG mode.
> + */
> + UINT32 Enable;
> +} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't show UEFI exit message.
> + * 1 - Show UEFI exit message.
> + */
> + UINT32 Show;
> +} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
> +
> +typedef struct {
> +#define CHIPSET_CPU_CLOCK_LOW 0
> +#define CHIPSET_CPU_CLOCK_DEFAULT 1
> +#define CHIPSET_CPU_CLOCK_MAX 2
> +#define CHIPSET_CPU_CLOCK_CUSTOM 3
> + UINT32 Clock;
> +} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
> +
> +typedef struct {
> + UINT32 Clock;
> +} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * Always set by ConfigDxe prior to HII init to reflect
> + * platform capability.
> + */
> + UINT32 Supported;
> +} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
> +
> +typedef struct {
> + UINT32 Enabled;
> +} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
> +
> +typedef struct {
> +#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 {
> + /*
> + * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
> + * 1 - uSD slot routed to Arasan SDHCI.
> + */
> + UINT32 Routing;
> +} MMC_SD_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't disable multi-block.
> + * 1 - Disable multi-block commands.
> + */
> + UINT32 DisableMulti;
> +} MMC_DISMULTI_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't force 1 bit mode.
> + * 1 - Force 1 bit mode.
> + */
> + UINT32 Force1Bit;
> +} MMC_FORCE1BIT_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't force default speed.
> + * 1 - Force default speed.
> + */
> + UINT32 ForceDS;
> +} MMC_FORCEDS_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * Default Speed MHz override (25MHz default).
> + */
> + UINT32 MHz;
> +} MMC_SD_DS_MHZ_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * High Speed MHz override (50MHz default).
> + */
> + UINT32 MHz;
> +} MMC_SD_HS_MHZ_VARSTORE_DATA;
> +
> +#endif /* CONFIG_VARS_H */
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 563fb891..721b8c20 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -481,9 +481,9 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
>
> #
> - # Device Tree
> + # Device Tree and ACPI selection.
> #
> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
> + 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..c9ab999a 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -496,9 +496,9 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
>
> #
> - # Device Tree
> + # Device Tree and ACPI selection.
> #
> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
> + 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
>
[-- Attachment #2: Type: text/html, Size: 32994 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
[not found] ` <160C83BCD8110E81.4282@groups.io>
@ 2020-05-09 7:16 ` Andrei Warkentin
0 siblings, 0 replies; 4+ messages in thread
From: Andrei Warkentin @ 2020-05-09 7:16 UTC (permalink / raw)
To: devel@edk2.groups.io, ard.biesheuvel@arm.com
Cc: leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com,
Andrei Warkentin
[-- Attachment #1: Type: text/plain, Size: 21424 bytes --]
Acting out on the request, I'm having second thoughts and wanted to get feedback.
The current behavior is "both" ACPI and DT get exposed today. I'd rather not remove that option (or make it non-default) as it's just going to annoy users - say, both the Pi 3 users running WoA (e.g. why are you defaulting to DT?) and the Pi 4 users running Linux/BSD via DT. And you can flip the defaults and it won't matter since someone somewhere will have relied on the ability of the current firmware to present whatever systab they expected.
Sigh.
Can I keep all 3 options? Default is still ACPI + DT, but have only ACPI and only DT? These last two are more for validation/development. This really is a replay of MPS/ACPI all over again...
A
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Andrei Warkentin via groups.io <awarkentin=vmware.com@groups.io>
Sent: Wednesday, May 6, 2020 1:26 PM
To: Andrei Warkentin <andrey.warkentin@gmail.com>; devel@edk2.groups.io <devel@edk2.groups.io>; ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>
Cc: leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; philmd@redhat.com <philmd@redhat.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
Ok that sounds reasonable to me. I'll remove the "both" option entirely.
A
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ard Biesheuvel via groups.io <ard.biesheuvel=arm.com@groups.io>
Sent: Wednesday, May 6, 2020 1:38 AM
To: Andrei Warkentin <andrey.warkentin@gmail.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; philmd@redhat.com <philmd@redhat.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed
On 5/6/20 2:59 AM, Andrei Warkentin wrote:
> Today we just have an option to hide DT entirely, while ACPI
> is always exposed. This change extends the option to
> provide all three choices:
> - ACPI only
> - ACPI + DT
> - DT only
>
> Why? Because not all OSes will prefer DT over ACPI when both are available.
>
> To do this cleanly, move the variable structure and value definitions
> into a separate header, ConfigVars.h.
>
> Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
I'd prefer it if DT and ACPI were mutually exclusive, so that it is
never left up to the OS to reason about which one is the correct one.
This aligns with other platforms we have in the tree.
> ---
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 20 +--
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 9 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 146 ++------------------
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 6 +-
> Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 2 +-
> Platform/RaspberryPi/Include/ConfigVars.h | 131 ++++++++++++++++++
> Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
> Platform/RaspberryPi/RPi4/RPi4.dsc | 4 +-
> Platform/RaspberryPi/RaspberryPi.dec | 2 +-
> 10 files changed, 171 insertions(+), 155 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index c90c2530..00867879 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -22,6 +22,7 @@
> #include <IndustryStandard/Bcm2836Gpio.h>
> #include <Library/GpioLib.h>
> #include <Protocol/RpiFirmware.h>
> +#include <ConfigVars.h>
> #include "ConfigDxeFormSetGuid.h"
>
> #define FREQ_1_MHZ 1000000
> @@ -154,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);
> @@ -259,10 +260,10 @@ ApplyVariables (
> UINT64 SystemMemorySize;
>
> switch (CpuClock) {
> - case 0: // Low
> + case CHIPSET_CPU_CLOCK_LOW:
> Rate = FixedPcdGet32 (PcdCpuLowSpeedMHz) * FREQ_1_MHZ;
> break;
> - case 1: // Default
> + case CHIPSET_CPU_CLOCK_DEFAULT:
> /*
> * What the Raspberry Pi Foundation calls "max clock rate" is really the default value
> * from: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.raspberrypi.org%2Fdocumentation%2Fconfiguration%2Fconfig-txt%2Foverclocking.md&data=02%7C01%7Cawarkentin%40vmware.com%7Cc02f0ea3653949b6847c08d7f1882716%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637243439445722301&sdata=BQl1mDfJ3Yt6YB%2FQGs6z6DoQ%2BQEiuGTuBAhIvsYrZ0w%3D&reserved=0<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.raspberrypi.org%2Fdocumentation%2Fconfiguration%2Fconfig-txt%2Foverclocking.md&data=02%7C01%7Cawarkentin%40vmware.com%7C9a09e4f0735d4d244f0408d7f1eb0eee%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637243864184420407&sdata=ZHg5wEeDH8cGtF59g9li1FZ58ZL72pubCUBV8D2zXfU%3D&reserved=0>
> @@ -272,10 +273,10 @@ ApplyVariables (
> DEBUG ((DEBUG_ERROR, "Couldn't read default CPU speed %r\n", Status));
> }
> break;
> - case 2: // Max
> + case CHIPSET_CPU_CLOCK_MAX:
> Rate = FixedPcdGet32 (PcdCpuMaxSpeedMHz) * FREQ_1_MHZ;
> break;
> - case 3: // Custom
> + case CHIPSET_CPU_CLOCK_CUSTOM:
> Rate = CustomCpuClock * FREQ_1_MHZ;
> break;
> }
> @@ -487,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) {
> + 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..7195e497 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 0a650a94..72cc90ae 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -8,128 +8,7 @@
>
> #include <Guid/HiiPlatformSetupFormset.h>
> #include "ConfigDxeFormSetGuid.h"
> -
> -#pragma pack(1)
> -typedef struct {
> - /*
> - * One bit for each scaled resolution supported,
> - * these are ordered exactly like mGopModeData
> - * in DisplayDxe.
> - *
> - * 800x600, 640x480, 1024x768, 720p, 1080p, native.
> - */
> - UINT8 v640 : 1;
> - UINT8 v800 : 1;
> - UINT8 v1024 : 1;
> - UINT8 v720p : 1;
> - UINT8 v1080p : 1;
> - UINT8 Physical : 1;
> -} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
> -#pragma pack()
> -
> -typedef struct {
> - /*
> - * 0 - No screenshot support.
> - * 1 - Screenshot support via hotkey.
> - */
> - UINT32 Enable;
> -} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - No JTAG.
> - * 1 - JTAG mode.
> - */
> - UINT32 Enable;
> -} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't show UEFI exit message.
> - * 1 - Show UEFI exit message.
> - */
> - UINT32 Show;
> -} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - low.
> - * 1 - default.
> - * 2 - maximum.
> - * 3 - custom.
> - */
> - UINT32 Clock;
> -} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
> -
> -typedef struct {
> - UINT32 Clock;
> -} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * Always set by ConfigDxe prior to HII init to reflect
> - * platform capability.
> - */
> - UINT32 Supported;
> -} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
> -
> -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 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
> - * 1 - uSD slot routed to Arasan SDHCI.
> - */
> - UINT32 Routing;
> -} MMC_SD_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't disable multi-block.
> - * 1 - Disable multi-block commands.
> - */
> - UINT32 DisableMulti;
> -} MMC_DISMULTI_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't force 1 bit mode.
> - * 1 - Force 1 bit mode.
> - */
> - UINT32 Force1Bit;
> -} MMC_FORCE1BIT_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * 0 - Don't force default speed.
> - * 1 - Force default speed.
> - */
> - UINT32 ForceDS;
> -} MMC_FORCEDS_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * Default Speed MHz override (25MHz default).
> - */
> - UINT32 MHz;
> -} MMC_SD_DS_MHZ_VARSTORE_DATA;
> -
> -typedef struct {
> - /*
> - * High Speed MHz override (50MHz default).
> - */
> - UINT32 MHz;
> -} MMC_SD_HS_MHZ_VARSTORE_DATA;
> +#include <ConfigVars.h>
>
> //
> // EFI Variable attributes
> @@ -165,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,
> @@ -253,10 +132,10 @@ formset
> prompt = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_PROMPT),
> help = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_HELP),
> flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = 0, flags = 0;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = 1, flags = DEFAULT;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = 2, flags = 0;
> - option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = 3, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = CHIPSET_CPU_CLOCK_LOW, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = CHIPSET_CPU_CLOCK_DEFAULT, flags = DEFAULT;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = CHIPSET_CPU_CLOCK_MAX, flags = 0;
> + option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = CHIPSET_CPU_CLOCK_CUSTOM, flags = 0;
> endoneof;
>
> grayoutif NOT ideqval CpuClock.Clock == 3;
> @@ -285,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 cb11256e..0472d8ec 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -15,10 +15,9 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
> #include <libfdt.h>
> -
> #include <Protocol/RpiFirmware.h>
> -
> #include <Guid/Fdt.h>
> +#include <ConfigVars.h>
>
> STATIC VOID *mFdtImage;
>
> @@ -356,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
> new file mode 100644
> index 00000000..28837f98
> --- /dev/null
> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
> @@ -0,0 +1,131 @@
> +/** @file
> + *
> + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#ifndef CONFIG_VARS_H
> +#define CONFIG_VARS_H
> +
> +#pragma pack(1)
> +typedef struct {
> + /*
> + * One bit for each scaled resolution supported,
> + * these are ordered exactly like mGopModeData
> + * in DisplayDxe.
> + *
> + * 800x600, 640x480, 1024x768, 720p, 1080p, native.
> + */
> + UINT8 v640 : 1;
> + UINT8 v800 : 1;
> + UINT8 v1024 : 1;
> + UINT8 v720p : 1;
> + UINT8 v1080p : 1;
> + UINT8 Physical : 1;
> +} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
> +#pragma pack()
> +
> +typedef struct {
> + /*
> + * 0 - No screenshot support.
> + * 1 - Screenshot support via hotkey.
> + */
> + UINT32 Enable;
> +} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - No JTAG.
> + * 1 - JTAG mode.
> + */
> + UINT32 Enable;
> +} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't show UEFI exit message.
> + * 1 - Show UEFI exit message.
> + */
> + UINT32 Show;
> +} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
> +
> +typedef struct {
> +#define CHIPSET_CPU_CLOCK_LOW 0
> +#define CHIPSET_CPU_CLOCK_DEFAULT 1
> +#define CHIPSET_CPU_CLOCK_MAX 2
> +#define CHIPSET_CPU_CLOCK_CUSTOM 3
> + UINT32 Clock;
> +} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
> +
> +typedef struct {
> + UINT32 Clock;
> +} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * Always set by ConfigDxe prior to HII init to reflect
> + * platform capability.
> + */
> + UINT32 Supported;
> +} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
> +
> +typedef struct {
> + UINT32 Enabled;
> +} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
> +
> +typedef struct {
> +#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 {
> + /*
> + * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
> + * 1 - uSD slot routed to Arasan SDHCI.
> + */
> + UINT32 Routing;
> +} MMC_SD_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't disable multi-block.
> + * 1 - Disable multi-block commands.
> + */
> + UINT32 DisableMulti;
> +} MMC_DISMULTI_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't force 1 bit mode.
> + * 1 - Force 1 bit mode.
> + */
> + UINT32 Force1Bit;
> +} MMC_FORCE1BIT_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * 0 - Don't force default speed.
> + * 1 - Force default speed.
> + */
> + UINT32 ForceDS;
> +} MMC_FORCEDS_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * Default Speed MHz override (25MHz default).
> + */
> + UINT32 MHz;
> +} MMC_SD_DS_MHZ_VARSTORE_DATA;
> +
> +typedef struct {
> + /*
> + * High Speed MHz override (50MHz default).
> + */
> + UINT32 MHz;
> +} MMC_SD_HS_MHZ_VARSTORE_DATA;
> +
> +#endif /* CONFIG_VARS_H */
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 563fb891..721b8c20 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -481,9 +481,9 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
>
> #
> - # Device Tree
> + # Device Tree and ACPI selection.
> #
> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
> + 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..c9ab999a 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -496,9 +496,9 @@
> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
>
> #
> - # Device Tree
> + # Device Tree and ACPI selection.
> #
> - gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
> + 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
>
[-- Attachment #2: Type: text/html, Size: 35957 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-09 7:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-06 0:59 [edk2-platforms][PATCH 1/1] RPi: allow selecting which system config tables are exposed Andrei Warkentin
2020-05-06 6:38 ` Ard Biesheuvel
2020-05-06 18:26 ` [edk2-devel] " Andrei Warkentin
[not found] ` <160C83BCD8110E81.4282@groups.io>
2020-05-09 7:16 ` Andrei Warkentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox