[edk2-devel][PATCH v2 0/6] Platform/RPi: User config improvements

[edk2-devel][PATCH v2 0/6] Platform/RPi: User config improvements

[Pete Batard]

Changes from v1:
* Split first commit into 2 patches.
* Clarify commit message for 3/6 and remove extra line-removal hunk.
* Improve and clarify computation of 3G+ segment size in
  ConfigDxe.c/ApplyVariables ().
* Minor whitespace fixes (e.g. "3GB" -> "3 GB" in strings).

Andrei Warkentin (4):
  Platform/RPi: Add firmware call to read installed memory size
  Platform/RPi: Use GetModelInstalledMB () to read RAM size
  Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB
  Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice

Pete Batard (2):
  Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice
  Platform/RPi/ConfigDxe: Improve RPi configuration form

 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                 | 80 ++++++++++++++++---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf               |  9 ++-
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni            | 38 ++++++---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr            | 84 +++++++++++++++++---
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c                       |  5 ++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf                     |  3 +
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 10 +--
 Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c       | 29 ++++++-
 Platform/RaspberryPi/Include/Protocol/RpiFirmware.h                | 47 ++++++-----
 Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf           |  3 -
 Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c          | 32 +++++---
 Platform/RaspberryPi/RPi3/RPi3.dsc                                 | 11 +++
 Platform/RaspberryPi/RPi4/RPi4.dsc                                 | 16 ++--
 Platform/RaspberryPi/RPi4/RPi4.fdf                                 |  2 -
 Platform/RaspberryPi/RPi4/Readme.md                                | 15 ++--
 Platform/RaspberryPi/RaspberryPi.dec                               |  6 +-
 16 files changed, 297 insertions(+), 93 deletions(-)

-- 
2.21.0.windows.1

[edk2-devel][PATCH v2 1/6] Platform/RPi: Add firmware call to read installed memory size

[Pete Batard]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

Add a new RPiFirmwareGetModelInstalledMB () call in RpiFirmwareDxe
to return the amount of detected installed RAM on the system (in MB).

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 29 +++++++++++-
 Platform/RaspberryPi/Include/Protocol/RpiFirmware.h          | 47 +++++++++++---------
 2 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 75826fdc0e53..40c78b5d57cf 100644
--- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -606,6 +606,32 @@ RpiFirmwareGetModelName (
   }
 }
 
+STATIC
+EFI_STATUS
+EFIAPI
+RPiFirmwareGetModelInstalledMB (
+  OUT   UINT32 *InstalledMB
+  )
+{
+  EFI_STATUS Status;
+  UINT32     Revision;
+
+  Status = RpiFirmwareGetModelRevision(&Revision);
+  if (EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Could not get the board revision: Status == %r\n",
+      __FUNCTION__, Status));
+    return EFI_DEVICE_ERROR;
+  }
+
+  //
+  // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+  // Bits [20-22] indicate the amount of memory starting with 256MB (000b)
+  // and doubling in size for each value (001b = 512 MB, 010b = 1GB, etc.)
+  //
+  *InstalledMB = 256 << ((Revision >> 20) & 0x07);
+  return EFI_SUCCESS;
+}
+
 STATIC
 EFI_STATUS
 EFIAPI
@@ -1236,7 +1262,8 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
   RpiFirmwareGetFirmwareRevision,
   RpiFirmwareGetManufacturerName,
   RpiFirmwareGetCpuName,
-  RpiFirmwareGetArmMemory
+  RpiFirmwareGetArmMemory,
+  RPiFirmwareGetModelInstalledMB,
 };
 
 /**
diff --git a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
index e3287e3c040f..108becbd3b6d 100644
--- a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
+++ b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
@@ -115,6 +115,12 @@ EFI_STATUS
   UINT32 *Revision
   );
 
+typedef
+EFI_STATUS
+(EFIAPI *GET_MODEL_INSTALLED_MB) (
+  UINT32 *InstalledMB
+  );
+
 typedef
 CHAR8*
 (EFIAPI *GET_MANUFACTURER_NAME) (
@@ -135,26 +141,27 @@ EFI_STATUS
   );
 
 typedef struct {
-  SET_POWER_STATE       SetPowerState;
-  GET_MAC_ADDRESS       GetMacAddress;
-  GET_COMMAND_LINE      GetCommandLine;
-  GET_CLOCK_RATE        GetClockRate;
-  GET_CLOCK_RATE        GetMaxClockRate;
-  GET_CLOCK_RATE        GetMinClockRate;
-  SET_CLOCK_RATE        SetClockRate;
-  GET_FB                GetFB;
-  FREE_FB               FreeFB;
-  GET_FB_SIZE           GetFBSize;
-  SET_LED               SetLed;
-  GET_SERIAL            GetSerial;
-  GET_MODEL             GetModel;
-  GET_MODEL_REVISION    GetModelRevision;
-  GET_MODEL_NAME        GetModelName;
-  GET_MODEL_FAMILY      GetModelFamily;
-  GET_FIRMWARE_REVISION GetFirmwareRevision;
-  GET_MANUFACTURER_NAME GetManufacturerName;
-  GET_CPU_NAME          GetCpuName;
-  GET_ARM_MEM           GetArmMem;
+  SET_POWER_STATE        SetPowerState;
+  GET_MAC_ADDRESS        GetMacAddress;
+  GET_COMMAND_LINE       GetCommandLine;
+  GET_CLOCK_RATE         GetClockRate;
+  GET_CLOCK_RATE         GetMaxClockRate;
+  GET_CLOCK_RATE         GetMinClockRate;
+  SET_CLOCK_RATE         SetClockRate;
+  GET_FB                 GetFB;
+  FREE_FB                FreeFB;
+  GET_FB_SIZE            GetFBSize;
+  SET_LED                SetLed;
+  GET_SERIAL             GetSerial;
+  GET_MODEL              GetModel;
+  GET_MODEL_REVISION     GetModelRevision;
+  GET_MODEL_NAME         GetModelName;
+  GET_MODEL_FAMILY       GetModelFamily;
+  GET_FIRMWARE_REVISION  GetFirmwareRevision;
+  GET_MANUFACTURER_NAME  GetManufacturerName;
+  GET_CPU_NAME           GetCpuName;
+  GET_ARM_MEM            GetArmMem;
+  GET_MODEL_INSTALLED_MB GetModelInstalledMB;
 } RASPBERRY_PI_FIRMWARE_PROTOCOL;
 
 extern EFI_GUID gRaspberryPiFirmwareProtocolGuid;
-- 
2.21.0.windows.1

[edk2-devel][PATCH v2 2/6] Platform/RPi: Use GetModelInstalledMB () to read RAM size

[Pete Batard]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

Use the call introduced in the previous commit to read the
platform's installed memory.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index f25c439f89c8..5585cb846f41 100644
--- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -870,21 +870,19 @@ MemArrMapInfoUpdateSmbiosType19 (
   )
 {
   EFI_STATUS Status;
-  UINT32 BoardRevision = 0;
+  UINT32 InstalledMB = 0;
 
   // Note: Type 19 addresses are expressed in KB, not bytes
   // The memory layout used in all known Pi SoC's starts at 0
   mMemArrMapInfoType19.StartingAddress = 0;
+
   // The minimum RAM size used on any Raspberry Pi model is 256 MB
   mMemArrMapInfoType19.EndingAddress = 256 * 1024;
-  Status = mFwProtocol->GetModelRevision (&BoardRevision);
+  Status = mFwProtocol->GetModelInstalledMB (&InstalledMB);
   if (Status != EFI_SUCCESS) {
     DEBUG ((DEBUG_WARN, "Couldn't get the board memory size - defaulting to 256 MB: %r\n", Status));
   } else {
-    // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
-    // Bits [20-22] indicate the amount of memory starting with 256MB (000b)
-    // and doubling in size for each value (001b = 512 MB, 010b = 1GB, etc.)
-    mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) & 0x07;
+    mMemArrMapInfoType19.EndingAddress = InstalledMB * 1024;
   }
   mMemArrMapInfoType19.EndingAddress -= 1;
 
-- 
2.21.0.windows.1

[edk2-devel][PATCH v2 3/6] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB

[Pete Batard]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

Splitting the RAM descriptors is required so that we can make the 3 to 4 GB
segment of the Raspberry Pi 4 a user-configurable option.

This also removes the need for the PcdAcpiBasicMode PCD.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf  |  3 --
 Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 32 ++++++++++++++------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
index 9abcc2cc0075..77cdbe399a06 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
@@ -59,8 +59,5 @@ [FixedPcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
-[FeaturePcd]
-  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode
-
 [Ppis]
   gArmMpCoreInfoPpiGuid
diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
index 4b388465cdde..901e5e3ee924 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
+++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
@@ -25,7 +25,7 @@ UINT32 mBoardRevision;
 
 
 // The total number of descriptors, including the final "end-of-table" descriptor.
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 10
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 11
 
 STATIC BOOLEAN                  VirtualMemoryInfoInitialized = FALSE;
 STATIC RPI_MEMORY_REGION_INFO   VirtualMemoryInfo[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
@@ -60,6 +60,7 @@ ArmPlatformGetVirtualMemoryMap (
 {
   UINTN                         Index = 0;
   UINTN                         GpuIndex;
+  INT64                         OrigMemorySize;
   INT64                         SystemMemorySize;
   ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
 
@@ -155,13 +156,13 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryInfo[Index].Type             = RPI_MEM_UNMAPPED_REGION;
   VirtualMemoryInfo[Index++].Name           = L"SoC Reserved (283x)";
 
-  if (FeaturePcdGet (PcdAcpiBasicMode)) {
-    //
-    // Limit the memory to 3 GB to work around the DMA bugs in the SoC without
-    // having to rely on IORT or _DMA descriptions.
-    //
-    SystemMemorySize = MIN(SystemMemorySize, 3U * SIZE_1GB);
-  }
+  //
+  // By default we limit the memory to 3 GB to work around the DMA bugs in the SoC,
+  // for OSes that don't support _DMA range descriptors. On 4GB boards, it's runtime
+  // setting to boot with 4 GB, and the additional 1 GB is added by ConfigDxe.
+  //
+  OrigMemorySize = SystemMemorySize;
+  SystemMemorySize = MIN(SystemMemorySize, 3UL * SIZE_1GB);
 
   // If we have RAM above the 1 GB mark, declare it
   if (SystemMemorySize - SIZE_1GB > 0) {
@@ -170,7 +171,20 @@ ArmPlatformGetVirtualMemoryMap (
     VirtualMemoryTable[Index].Length        = SystemMemorySize - SIZE_1GB;
     VirtualMemoryTable[Index].Attributes    = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
     VirtualMemoryInfo[Index].Type           = RPI_MEM_BASIC_REGION;
-    VirtualMemoryInfo[Index++].Name         = L"Extended System RAM";
+    VirtualMemoryInfo[Index++].Name         = L"Extended System RAM below 3 GB";
+  }
+
+  //
+  // If we have RAM above 3 GB mark, declare it so it's mapped, but
+  // don't add it to the memory map. This is done later by ConfigDxe if necessary.
+  //
+  if (OrigMemorySize > (3UL * SIZE_1GB)) {
+    VirtualMemoryTable[Index].PhysicalBase  = 3UL * SIZE_1GB;
+    VirtualMemoryTable[Index].VirtualBase   = VirtualMemoryTable[Index].PhysicalBase;
+    VirtualMemoryTable[Index].Length        = OrigMemorySize - VirtualMemoryTable[Index].PhysicalBase;
+    VirtualMemoryTable[Index].Attributes    = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+    VirtualMemoryInfo[Index].Type           = RPI_MEM_UNMAPPED_REGION;
+    VirtualMemoryInfo[Index++].Name         = L"Extended System RAM above 3 GB";
   }
 
   // End of Table
-- 
2.21.0.windows.1

[edk2-devel][PATCH v2 4/6] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice

[Pete Batard]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

Currently some OSes (e.g FreeBSD) can make full use of the maximum
4 GB of RAM a Raspberry Pi 4 can offer, whereas others (e.g. Linux)
must be restricted to only the first 3 GB.

Previously this was a compile-time choice chosen by PcdAcpiBasicMode,
and now we make it user-selectable. The default is a 3 GB limit.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c      | 72 +++++++++++++++++---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |  8 ++-
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 11 +++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 41 +++++++++++
 Platform/RaspberryPi/RPi3/RPi3.dsc                      |  6 ++
 Platform/RaspberryPi/RPi4/RPi4.dsc                      |  6 ++
 Platform/RaspberryPi/RaspberryPi.dec                    |  4 +-
 7 files changed, 134 insertions(+), 14 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 2f48ca0dd758..964ca6d369da 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -11,11 +11,13 @@
 #include <Library/AcpiLib.h>
 #include <Library/HiiLib.h>
 #include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
 #include <Library/IoLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/DevicePathLib.h>
 #include <IndustryStandard/RpiMbox.h>
+#include <IndustryStandard/Bcm2711.h>
 #include <IndustryStandard/Bcm2836.h>
 #include <IndustryStandard/Bcm2836Gpio.h>
 #include <Library/GpioLib.h>
@@ -26,6 +28,8 @@ extern UINT8 ConfigDxeHiiBin[];
 extern UINT8 ConfigDxeStrings[];
 
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
+STATIC UINT32 mModelFamily = 0;
+STATIC UINT32 mModelInstalledMB = 0;
 
 /*
  * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and
@@ -129,6 +133,24 @@ SetupVariables (
     PcdSet32 (PcdCustomCpuClock, PcdGet32 (PcdCustomCpuClock));
   }
 
+  if (mModelFamily >= 4 && mModelInstalledMB > 3 * 1024) {
+    /*
+     * This allows changing PcdRamLimitTo3GB in forms.
+     */
+    PcdSet32 (PcdRamMoreThan3GB, 1);
+
+    Size = sizeof (UINT32);
+    Status = gRT->GetVariable (L"RamLimitTo3GB",
+                               &gConfigDxeFormSetGuid,
+                               NULL, &Size, &Var32);
+    if (EFI_ERROR (Status)) {
+      PcdSet32 (PcdRamLimitTo3GB, PcdGet32 (PcdRamLimitTo3GB));
+    }
+  } else {
+    PcdSet32 (PcdRamMoreThan3GB, 0);
+    PcdSet32 (PcdRamLimitTo3GB, 0);
+  }
+
   Size = sizeof (UINT32);
   Status = gRT->GetVariable (L"SdIsArasan",
                   &gConfigDxeFormSetGuid,
@@ -224,7 +246,7 @@ ApplyVariables (
   UINT32 CpuClock = PcdGet32 (PcdCpuClock);
   UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
   UINT32 Rate = 0;
-  UINT32 ModelFamily = 0;
+  UINT64 SystemMemorySize;
 
   if (CpuClock != 0) {
     if (CpuClock == 2) {
@@ -258,15 +280,31 @@ ApplyVariables (
     DEBUG ((DEBUG_INFO, "Current CPU speed is %uHz\n", Rate));
   }
 
-  Status = mFwProtocol->GetModelFamily (&ModelFamily);
-  if (Status != EFI_SUCCESS) {
-    DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi model family: %r\n", Status));
-  } else {
-    DEBUG ((DEBUG_INFO, "Current Raspberry Pi model family is 0x%x\n", ModelFamily));
+  if (mModelFamily >= 4 && PcdGet32 (PcdRamLimitTo3GB) == 0) {
+    /*
+     * Similar to how we compute the > 3 GB RAM segment's size in PlatformLib/
+     * RaspberryPiMem.c, with some overlap protection for the Bcm2xxx register
+     * spaces. This computation should also work for models with more than 4 GB
+     * RAM, if there ever exist ones.
+     */
+    SystemMemorySize = (UINT64)mModelInstalledMB * SIZE_1MB;
+    ASSERT (SystemMemorySize > 3UL * SIZE_1GB);
+    SystemMemorySize = MIN(SystemMemorySize, BCM2836_SOC_REGISTERS);
+    if (BCM2711_SOC_REGISTERS > 0) {
+      SystemMemorySize = MIN(SystemMemorySize, BCM2711_SOC_REGISTERS);
+    }
+
+    Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, 3UL * BASE_1GB,
+                    SystemMemorySize - (3UL * SIZE_1GB),
+                    EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB);
+    ASSERT_EFI_ERROR (Status);
+    Status = gDS->SetMemorySpaceAttributes (3UL * BASE_1GB,
+                    SystemMemorySize - (3UL * SIZE_1GB),
+                    EFI_MEMORY_WB);
+    ASSERT_EFI_ERROR (Status);
   }
 
-
-  if (ModelFamily == 3) {
+  if (mModelFamily == 3) {
     /*
      * Pi 3: either Arasan or SdHost goes to SD card.
      *
@@ -316,7 +354,7 @@ ApplyVariables (
     GpioPinFuncSet (52, Gpio48Group);
     GpioPinFuncSet (53, Gpio48Group);
 
-  } else if (ModelFamily == 4) {
+  } else if (mModelFamily == 4) {
     /*
      * Pi 4: either Arasan or eMMC2 goes to SD card.
      */
@@ -352,7 +390,7 @@ ApplyVariables (
       GpioPinFuncSet (39, GPIO_FSEL_ALT3);
     }
   } else {
-    DEBUG ((DEBUG_ERROR, "Model Family %d not supported...\n", ModelFamily));
+    DEBUG ((DEBUG_ERROR, "Model Family %d not supported...\n", mModelFamily));
   }
 
   /*
@@ -400,6 +438,20 @@ ConfigInitialize (
     return Status;
   }
 
+  Status = mFwProtocol->GetModelFamily (&mModelFamily);
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi model family: %r\n", Status));
+  } else {
+    DEBUG ((DEBUG_INFO, "Current Raspberry Pi model family is %d\n", mModelFamily));
+  }
+
+  Status = mFwProtocol->GetModelInstalledMB (&mModelInstalledMB);
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi installed RAM size: %r\n", Status));
+  } else {
+    DEBUG ((DEBUG_INFO, "Current Raspberry Pi installed RAM size is %d MB\n", mModelInstalledMB));
+  }
+
   Status = SetupVariables ();
   if (Status != EFI_SUCCESS) {
     DEBUG ((DEBUG_ERROR, "Couldn't not setup NV vars: %r\n", Status));
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index dc726cc6d934..407aac89c7b3 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -33,6 +33,7 @@ [Packages]
   ArmPlatformPkg/ArmPlatformPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  Silicon/Broadcom/Bcm27xx/Bcm27xx.dec
   Silicon/Broadcom/Bcm283x/Bcm283x.dec
   Platform/RaspberryPi/RaspberryPi.dec
   EmbeddedPkg/EmbeddedPkg.dec
@@ -53,10 +54,11 @@ [Guids]
   gConfigDxeFormSetGuid
 
 [Protocols]
-  gRaspberryPiFirmwareProtocolGuid ## CONSUMES
+  gRaspberryPiFirmwareProtocolGuid      ## CONSUMES
   gRaspberryPiConfigAppliedProtocolGuid ## PRODUCES
 
 [Pcd]
+  gBcm27xxTokenSpaceGuid.PcdBcm27xxRegistersAddress
   gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
   gRaspberryPiTokenSpaceGuid.PcdCpuClock
   gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock
@@ -70,8 +72,8 @@ [Pcd]
   gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot
-
-[FeaturePcd]
+  gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
+  gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 9b4076635f05..830533a9dc49 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -35,6 +35,17 @@
 #string STR_CHIPSET_SD_SDHOST        #language en-US "Broadcom SDHOST"
 #string STR_CHIPSET_SD_ARASAN        #language en-US "Arasan SDHCI"
 
+/*
+ * Advanced configuration.
+ */
+
+#string STR_ADVANCED_FORM_TITLE      #language en-US "Advanced Configuration"
+
+#string STR_ADVANCED_3GB_PROMPT      #language en-US "Limit RAM to 3 GB"
+#string STR_ADVANCED_3GB_HELP        #language en-US "OSes not supporting ACPI DMA constraints require a 3 GB limit or face broken xHCI USB"
+#string STR_ADVANCED_3GB_OFF         #language en-US "Disabled"
+#string STR_ADVANCED_3GB_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 60bfdbd4d17e..483edd7459c5 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -73,6 +73,18 @@ typedef struct {
   UINT32 Routing;
 } CHIPSET_SD_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 - Don't disable multi-block.
@@ -140,6 +152,16 @@ formset
       name  = SdIsArasan,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
+    efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = RamMoreThan3GB,
+      guid  = CONFIGDXE_FORM_SET_GUID;
+
+    efivarstore ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = RamLimitTo3GB,
+      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,
@@ -193,6 +215,10 @@ formset
            prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
 
+        goto 0x1006,
+           prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE),
+           help = STRING_TOKEN(STR_NULL_STRING);
+
         goto 0x1003,
            prompt = STRING_TOKEN(STR_MMC_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
@@ -240,6 +266,21 @@ formset
         endoneof;
     endform;
 
+    form formid = 0x1006,
+        title  = STRING_TOKEN(STR_ADVANCED_FORM_TITLE);
+        subtitle text = STRING_TOKEN(STR_NULL_STRING);
+
+        grayoutif ideqval RamMoreThan3GB.Supported == 0;
+          oneof varid = RamLimitTo3GB.Enabled,
+              prompt      = STRING_TOKEN(STR_ADVANCED_3GB_PROMPT),
+              help        = STRING_TOKEN(STR_ADVANCED_3GB_HELP),
+              flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+              option text = STRING_TOKEN(STR_ADVANCED_3GB_OFF), value = 0, flags = DEFAULT;
+              option text = STRING_TOKEN(STR_ADVANCED_3GB_ON), value = 1, flags = 0;
+          endoneof;
+        endif;
+    endform;
+
     form formid = 0x1003,
         title  = STRING_TOKEN(STR_MMC_FORM_TITLE);
         subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE);
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index df5b246af1f8..48e1a32e1d24 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -439,6 +439,12 @@ [PcdsDynamicHii.common.DEFAULT]
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0xff
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|L"DisplayEnableSShot"|gConfigDxeFormSetGuid|0x0|1
 
+  #
+  # Supporting > 3GB of memory.
+  #
+  gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0
+  gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 60a5e38da778..3ce2c3e4d519 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -477,6 +477,12 @@ [PcdsDynamicHii.common.DEFAULT]
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0xff
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|L"DisplayEnableSShot"|gConfigDxeFormSetGuid|0x0|1
 
+  #
+  # Supporting > 3GB of memory.
+  #
+  gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0
+  gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index bc378ffbfb8d..7f2c37ac9a7f 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -57,6 +57,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0|UINT8|0x00000017
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018
+  gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
+  gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
 
 [PcdsFeatureFlag.common]
-  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x00000019
+  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x0000001B
-- 
2.21.0.windows.1

[edk2-devel][PATCH v2 5/6] Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice

[Pete Batard]

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 <pete@akeo.ie>
---
 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 964ca6d369da..ccac8daad402 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

[edk2-devel][PATCH v2 6/6] Platform/RPi/ConfigDxe: Improve RPi configuration form

[Pete Batard]

Group all SD/MMC settings under the SD/MMC form.
Make CPU settings more prominent.
Harmonise form titles and text content.
Reorder forms in the order they are most likely to be queried.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 24 +++++-----
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 50 ++++++++++----------
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 2e79e322e558..77eda96d8136 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -14,11 +14,11 @@
 #string STR_FORM_SET_TITLE_HELP  #language en-US "Press <Enter> to configure system settings."
 
 /*
- * Chipset config.
+ * Chipset configuration.
  */
 
-#string STR_CHIPSET_FORM_TITLE      #language en-US "Chipset Configuration"
-#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings when booted."
+#string STR_CHIPSET_FORM_TITLE      #language en-US "CPU Configuration"
+#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings."
 
 #string STR_CHIPSET_CLOCK_CPU_PROMPT #language en-US "CPU Clock"
 #string STR_CHIPSET_CLOCK_CPU_HELP   #language en-US "CPU Speed"
@@ -30,11 +30,6 @@
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_PROMPT #language en-US "CPU Clock Rate (MHz)"
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_HELP   #language en-US "Adjust the CPU speed.\nMin value: 100 MHz\nMax value: 1600 MHz\n\nWarning: Overclocking can make the system unbootable!"
 
-#string STR_CHIPSET_SD_PROMPT        #language en-US "uSD Routing"
-#string STR_CHIPSET_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
-#string STR_CHIPSET_SD_SDHOST        #language en-US "Broadcom SDHOST"
-#string STR_CHIPSET_SD_ARASAN        #language en-US "Arasan SDHCI"
-
 /*
  * Advanced configuration.
  */
@@ -55,8 +50,13 @@
  * MMC/SD configuration.
  */
 
-#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Tweaks"
-#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS will override settings when booted."
+#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Configuration"
+#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS may override settings."
+
+#string STR_MMC_SD_PROMPT        #language en-US "uSD Routing"
+#string STR_MMC_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
+#string STR_MMC_SD_SDHOST        #language en-US "Broadcom SDHOST"
+#string STR_MMC_SD_ARASAN        #language en-US "Arasan SDHCI"
 
 #string STR_MMC_DISMULTI_PROMPT  #language en-US "Multi-Block Support"
 #string STR_MMC_DISMULTI_HELP    #language en-US "Use CMD18/CMD25 for transfers when possible"
@@ -84,7 +84,7 @@
  * Display settings.
  */
 
-#string STR_DISPLAY_FORM_TITLE      #language en-US "Display"
+#string STR_DISPLAY_FORM_TITLE      #language en-US "Display Configuration"
 #string STR_DISPLAY_FORM_SUBTITLE   #language en-US "UEFI video driver settings"
 
 #string STR_DISPLAY_VMODES_640_PROMPT  #language en-US "Virtual 640x480"
@@ -109,7 +109,7 @@
 /*
  * Debugging settings go here.
  */
-#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging"
+#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging Configuration"
 #string STR_DEBUG_FORM_SUBTITLE     #language en-US "For UEFI/OS Developers"
 
 #string STR_DEBUG_JTAG_PROMPT       #language en-US "JTAG Routing"
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index d16058da4926..9c2fd64a8e27 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -65,14 +65,6 @@ typedef struct {
   UINT32 Clock;
 } CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
 
-typedef struct {
-  /*
-   * 0 - uSD slot routed to Broadcom SDHOST.
-   * 1 - uSD slot routed to Arasan SDHCI.
-   */
-  UINT32 Routing;
-} CHIPSET_SD_VARSTORE_DATA;
-
 typedef struct {
   /*
    * Always set by ConfigDxe prior to HII init to reflect
@@ -93,6 +85,14 @@ typedef struct {
   UINT32 Enabled;
 } ADVANCED_DEVICE_TREE_VARSTORE_DATA;
 
+typedef struct {
+  /*
+   * 0 - uSD slot routed to Broadcom SDHOST.
+   * 1 - uSD slot routed to Arasan SDHCI.
+   */
+  UINT32 Routing;
+} MMC_SD_VARSTORE_DATA;
+
 typedef struct {
   /*
    * 0 - Don't disable multi-block.
@@ -155,11 +155,6 @@ formset
       name  = CustomCpuClock,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
-    efivarstore CHIPSET_SD_VARSTORE_DATA,
-      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
-      name  = SdIsArasan,
-      guid  = CONFIGDXE_FORM_SET_GUID;
-
     efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = RamMoreThan3GB,
@@ -175,6 +170,11 @@ formset
       name  = OptDeviceTree,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
+    efivarstore MMC_SD_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = SdIsArasan,
+      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,
@@ -228,6 +228,10 @@ formset
            prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
 
+        goto 0x1004,
+            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
+            help = STRING_TOKEN(STR_NULL_STRING);
+
         goto 0x1006,
            prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
@@ -236,10 +240,6 @@ formset
            prompt = STRING_TOKEN(STR_MMC_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
 
-        goto 0x1004,
-            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
-            help = STRING_TOKEN(STR_NULL_STRING);
-
         goto 0x1005,
             prompt = STRING_TOKEN(STR_DEBUG_FORM_TITLE),
             help = STRING_TOKEN(STR_NULL_STRING);
@@ -269,14 +269,6 @@ formset
               default = 600,
           endnumeric;
         endif;
-
-        oneof varid = SdIsArasan.Routing,
-            prompt      = STRING_TOKEN(STR_CHIPSET_SD_PROMPT),
-            help        = STRING_TOKEN(STR_CHIPSET_SD_HELP),
-            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
-            option text = STRING_TOKEN(STR_CHIPSET_SD_ARASAN), value = 1, flags = 0;
-            option text = STRING_TOKEN(STR_CHIPSET_SD_SDHOST), value = 0, flags = DEFAULT;
-        endoneof;
     endform;
 
     form formid = 0x1006,
@@ -306,6 +298,14 @@ formset
         title  = STRING_TOKEN(STR_MMC_FORM_TITLE);
         subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE);
 
+        oneof varid = SdIsArasan.Routing,
+            prompt      = STRING_TOKEN(STR_MMC_SD_PROMPT),
+            help        = STRING_TOKEN(STR_MMC_SD_HELP),
+            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+            option text = STRING_TOKEN(STR_MMC_SD_ARASAN), value = 1, flags = 0;
+            option text = STRING_TOKEN(STR_MMC_SD_SDHOST), value = 0, flags = DEFAULT;
+        endoneof;
+
         oneof varid = MmcDisableMulti.DisableMulti,
             prompt      = STRING_TOKEN(STR_MMC_DISMULTI_PROMPT),
             help        = STRING_TOKEN(STR_MMC_DISMULTI_HELP),
-- 
2.21.0.windows.1

Re: [edk2-devel][PATCH v2 0/6] Platform/RPi: User config improvements

[Ard Biesheuvel]

On Tue, 3 Mar 2020 at 14:08, Pete Batard <pete@akeo.ie> wrote:
>
> Changes from v1:
> * Split first commit into 2 patches.
> * Clarify commit message for 3/6 and remove extra line-removal hunk.
> * Improve and clarify computation of 3G+ segment size in
>   ConfigDxe.c/ApplyVariables ().
> * Minor whitespace fixes (e.g. "3GB" -> "3 GB" in strings).
>
> Andrei Warkentin (4):
>   Platform/RPi: Add firmware call to read installed memory size
>   Platform/RPi: Use GetModelInstalledMB () to read RAM size
>   Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB
>   Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice
>
> Pete Batard (2):
>   Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice
>   Platform/RPi/ConfigDxe: Improve RPi configuration form

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as d5f7c68948a3..dac18ca51781

Thanks!

>
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                 | 80 ++++++++++++++++---
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf               |  9 ++-
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni            | 38 ++++++---
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr            | 84 +++++++++++++++++---
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c                       |  5 ++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf                     |  3 +
>  Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 10 +--
>  Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c       | 29 ++++++-
>  Platform/RaspberryPi/Include/Protocol/RpiFirmware.h                | 47 ++++++-----
>  Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf           |  3 -
>  Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c          | 32 +++++---
>  Platform/RaspberryPi/RPi3/RPi3.dsc                                 | 11 +++
>  Platform/RaspberryPi/RPi4/RPi4.dsc                                 | 16 ++--
>  Platform/RaspberryPi/RPi4/RPi4.fdf                                 |  2 -
>  Platform/RaspberryPi/RPi4/Readme.md                                | 15 ++--
>  Platform/RaspberryPi/RaspberryPi.dec                               |  6 +-
>  16 files changed, 297 insertions(+), 93 deletions(-)
>
> --
> 2.21.0.windows.1
>

Re: [edk2-devel][PATCH v2 6/6] Platform/RPi/ConfigDxe: Improve RPi configuration form

[Andrei Warkentin]

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>

________________________________________
From: Pete Batard <pete@akeo.ie>
Sent: Tuesday, March 3, 2020 8:08 AM
To: devel@edk2.groups.io
Cc: ard.biesheuvel@linaro.org; leif@nuviainc.com; philmd@redhat.com; Andrei Warkentin
Subject: [edk2-devel][PATCH v2 6/6] Platform/RPi/ConfigDxe: Improve RPi configuration form

Group all SD/MMC settings under the SD/MMC form.
Make CPU settings more prominent.
Harmonise form titles and text content.
Reorder forms in the order they are most likely to be queried.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 24 +++++-----
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 50 ++++++++++----------
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 2e79e322e558..77eda96d8136 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -14,11 +14,11 @@
 #string STR_FORM_SET_TITLE_HELP  #language en-US "Press <Enter> to configure system settings."

 /*
- * Chipset config.
+ * Chipset configuration.
  */

-#string STR_CHIPSET_FORM_TITLE      #language en-US "Chipset Configuration"
-#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings when booted."
+#string STR_CHIPSET_FORM_TITLE      #language en-US "CPU Configuration"
+#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings."

 #string STR_CHIPSET_CLOCK_CPU_PROMPT #language en-US "CPU Clock"
 #string STR_CHIPSET_CLOCK_CPU_HELP   #language en-US "CPU Speed"
@@ -30,11 +30,6 @@
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_PROMPT #language en-US "CPU Clock Rate (MHz)"
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_HELP   #language en-US "Adjust the CPU speed.\nMin value: 100 MHz\nMax value: 1600 MHz\n\nWarning: Overclocking can make the system unbootable!"

-#string STR_CHIPSET_SD_PROMPT        #language en-US "uSD Routing"
-#string STR_CHIPSET_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
-#string STR_CHIPSET_SD_SDHOST        #language en-US "Broadcom SDHOST"
-#string STR_CHIPSET_SD_ARASAN        #language en-US "Arasan SDHCI"
-
 /*
  * Advanced configuration.
  */
@@ -55,8 +50,13 @@
  * MMC/SD configuration.
  */

-#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Tweaks"
-#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS will override settings when booted."
+#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Configuration"
+#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS may override settings."
+
+#string STR_MMC_SD_PROMPT        #language en-US "uSD Routing"
+#string STR_MMC_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
+#string STR_MMC_SD_SDHOST        #language en-US "Broadcom SDHOST"
+#string STR_MMC_SD_ARASAN        #language en-US "Arasan SDHCI"

 #string STR_MMC_DISMULTI_PROMPT  #language en-US "Multi-Block Support"
 #string STR_MMC_DISMULTI_HELP    #language en-US "Use CMD18/CMD25 for transfers when possible"
@@ -84,7 +84,7 @@
  * Display settings.
  */

-#string STR_DISPLAY_FORM_TITLE      #language en-US "Display"
+#string STR_DISPLAY_FORM_TITLE      #language en-US "Display Configuration"
 #string STR_DISPLAY_FORM_SUBTITLE   #language en-US "UEFI video driver settings"

 #string STR_DISPLAY_VMODES_640_PROMPT  #language en-US "Virtual 640x480"
@@ -109,7 +109,7 @@
 /*
  * Debugging settings go here.
  */
-#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging"
+#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging Configuration"
 #string STR_DEBUG_FORM_SUBTITLE     #language en-US "For UEFI/OS Developers"

 #string STR_DEBUG_JTAG_PROMPT       #language en-US "JTAG Routing"
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index d16058da4926..9c2fd64a8e27 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -65,14 +65,6 @@ typedef struct {
   UINT32 Clock;
 } CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;

-typedef struct {
-  /*
-   * 0 - uSD slot routed to Broadcom SDHOST.
-   * 1 - uSD slot routed to Arasan SDHCI.
-   */
-  UINT32 Routing;
-} CHIPSET_SD_VARSTORE_DATA;
-
 typedef struct {
   /*
    * Always set by ConfigDxe prior to HII init to reflect
@@ -93,6 +85,14 @@ typedef struct {
   UINT32 Enabled;
 } ADVANCED_DEVICE_TREE_VARSTORE_DATA;

+typedef struct {
+  /*
+   * 0 - uSD slot routed to Broadcom SDHOST.
+   * 1 - uSD slot routed to Arasan SDHCI.
+   */
+  UINT32 Routing;
+} MMC_SD_VARSTORE_DATA;
+
 typedef struct {
   /*
    * 0 - Don't disable multi-block.
@@ -155,11 +155,6 @@ formset
       name  = CustomCpuClock,
       guid  = CONFIGDXE_FORM_SET_GUID;

-    efivarstore CHIPSET_SD_VARSTORE_DATA,
-      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
-      name  = SdIsArasan,
-      guid  = CONFIGDXE_FORM_SET_GUID;
-
     efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = RamMoreThan3GB,
@@ -175,6 +170,11 @@ formset
       name  = OptDeviceTree,
       guid  = CONFIGDXE_FORM_SET_GUID;

+    efivarstore MMC_SD_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = SdIsArasan,
+      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,
@@ -228,6 +228,10 @@ formset
            prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);

+        goto 0x1004,
+            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
+            help = STRING_TOKEN(STR_NULL_STRING);
+
         goto 0x1006,
            prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
@@ -236,10 +240,6 @@ formset
            prompt = STRING_TOKEN(STR_MMC_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);

-        goto 0x1004,
-            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
-            help = STRING_TOKEN(STR_NULL_STRING);
-
         goto 0x1005,
             prompt = STRING_TOKEN(STR_DEBUG_FORM_TITLE),
             help = STRING_TOKEN(STR_NULL_STRING);
@@ -269,14 +269,6 @@ formset
               default = 600,
           endnumeric;
         endif;
-
-        oneof varid = SdIsArasan.Routing,
-            prompt      = STRING_TOKEN(STR_CHIPSET_SD_PROMPT),
-            help        = STRING_TOKEN(STR_CHIPSET_SD_HELP),
-            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
-            option text = STRING_TOKEN(STR_CHIPSET_SD_ARASAN), value = 1, flags = 0;
-            option text = STRING_TOKEN(STR_CHIPSET_SD_SDHOST), value = 0, flags = DEFAULT;
-        endoneof;
     endform;

     form formid = 0x1006,
@@ -306,6 +298,14 @@ formset
         title  = STRING_TOKEN(STR_MMC_FORM_TITLE);
         subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE);

+        oneof varid = SdIsArasan.Routing,
+            prompt      = STRING_TOKEN(STR_MMC_SD_PROMPT),
+            help        = STRING_TOKEN(STR_MMC_SD_HELP),
+            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+            option text = STRING_TOKEN(STR_MMC_SD_ARASAN), value = 1, flags = 0;
+            option text = STRING_TOKEN(STR_MMC_SD_SDHOST), value = 0, flags = DEFAULT;
+        endoneof;
+
         oneof varid = MmcDisableMulti.DisableMulti,
             prompt      = STRING_TOKEN(STR_MMC_DISMULTI_PROMPT),
             help        = STRING_TOKEN(STR_MMC_DISMULTI_HELP),
--
2.21.0.windows.1

Re: [edk2-devel][PATCH v2 6/6] Platform/RPi/ConfigDxe: Improve RPi configuration form

[Andrei Warkentin]

Nevermind, I missed all the current reviews and the code is already in.

A

________________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Andrei Warkentin via Groups.Io <awarkentin=vmware.com@groups.io>
Sent: Tuesday, March 3, 2020 11:17 AM
To: Pete Batard; devel@edk2.groups.io
Cc: ard.biesheuvel@linaro.org; leif@nuviainc.com; philmd@redhat.com
Subject: Re: [edk2-devel][PATCH v2 6/6] Platform/RPi/ConfigDxe: Improve RPi configuration form

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>

________________________________________
From: Pete Batard <pete@akeo.ie>
Sent: Tuesday, March 3, 2020 8:08 AM
To: devel@edk2.groups.io
Cc: ard.biesheuvel@linaro.org; leif@nuviainc.com; philmd@redhat.com; Andrei Warkentin
Subject: [edk2-devel][PATCH v2 6/6] Platform/RPi/ConfigDxe: Improve RPi configuration form

Group all SD/MMC settings under the SD/MMC form.
Make CPU settings more prominent.
Harmonise form titles and text content.
Reorder forms in the order they are most likely to be queried.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 24 +++++-----
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 50 ++++++++++----------
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 2e79e322e558..77eda96d8136 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -14,11 +14,11 @@
 #string STR_FORM_SET_TITLE_HELP  #language en-US "Press <Enter> to configure system settings."

 /*
- * Chipset config.
+ * Chipset configuration.
  */

-#string STR_CHIPSET_FORM_TITLE      #language en-US "Chipset Configuration"
-#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings when booted."
+#string STR_CHIPSET_FORM_TITLE      #language en-US "CPU Configuration"
+#string STR_CHIPSET_FORM_SUBTITLE   #language en-US "Note: OS may override settings."

 #string STR_CHIPSET_CLOCK_CPU_PROMPT #language en-US "CPU Clock"
 #string STR_CHIPSET_CLOCK_CPU_HELP   #language en-US "CPU Speed"
@@ -30,11 +30,6 @@
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_PROMPT #language en-US "CPU Clock Rate (MHz)"
 #string STR_CHIPSET_CUSTOM_CPU_CLOCK_HELP   #language en-US "Adjust the CPU speed.\nMin value: 100 MHz\nMax value: 1600 MHz\n\nWarning: Overclocking can make the system unbootable!"

-#string STR_CHIPSET_SD_PROMPT        #language en-US "uSD Routing"
-#string STR_CHIPSET_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
-#string STR_CHIPSET_SD_SDHOST        #language en-US "Broadcom SDHOST"
-#string STR_CHIPSET_SD_ARASAN        #language en-US "Arasan SDHCI"
-
 /*
  * Advanced configuration.
  */
@@ -55,8 +50,13 @@
  * MMC/SD configuration.
  */

-#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Tweaks"
-#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS will override settings when booted."
+#string STR_MMC_FORM_TITLE       #language en-US "SD/MMC Configuration"
+#string STR_MMC_FORM_SUBTITLE    #language en-US "Note: UEFI only, OS may override settings."
+
+#string STR_MMC_SD_PROMPT        #language en-US "uSD Routing"
+#string STR_MMC_SD_HELP          #language en-US "Choose host controller to drive uSD slot"
+#string STR_MMC_SD_SDHOST        #language en-US "Broadcom SDHOST"
+#string STR_MMC_SD_ARASAN        #language en-US "Arasan SDHCI"

 #string STR_MMC_DISMULTI_PROMPT  #language en-US "Multi-Block Support"
 #string STR_MMC_DISMULTI_HELP    #language en-US "Use CMD18/CMD25 for transfers when possible"
@@ -84,7 +84,7 @@
  * Display settings.
  */

-#string STR_DISPLAY_FORM_TITLE      #language en-US "Display"
+#string STR_DISPLAY_FORM_TITLE      #language en-US "Display Configuration"
 #string STR_DISPLAY_FORM_SUBTITLE   #language en-US "UEFI video driver settings"

 #string STR_DISPLAY_VMODES_640_PROMPT  #language en-US "Virtual 640x480"
@@ -109,7 +109,7 @@
 /*
  * Debugging settings go here.
  */
-#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging"
+#string STR_DEBUG_FORM_TITLE        #language en-US "Debugging Configuration"
 #string STR_DEBUG_FORM_SUBTITLE     #language en-US "For UEFI/OS Developers"

 #string STR_DEBUG_JTAG_PROMPT       #language en-US "JTAG Routing"
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index d16058da4926..9c2fd64a8e27 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -65,14 +65,6 @@ typedef struct {
   UINT32 Clock;
 } CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;

-typedef struct {
-  /*
-   * 0 - uSD slot routed to Broadcom SDHOST.
-   * 1 - uSD slot routed to Arasan SDHCI.
-   */
-  UINT32 Routing;
-} CHIPSET_SD_VARSTORE_DATA;
-
 typedef struct {
   /*
    * Always set by ConfigDxe prior to HII init to reflect
@@ -93,6 +85,14 @@ typedef struct {
   UINT32 Enabled;
 } ADVANCED_DEVICE_TREE_VARSTORE_DATA;

+typedef struct {
+  /*
+   * 0 - uSD slot routed to Broadcom SDHOST.
+   * 1 - uSD slot routed to Arasan SDHCI.
+   */
+  UINT32 Routing;
+} MMC_SD_VARSTORE_DATA;
+
 typedef struct {
   /*
    * 0 - Don't disable multi-block.
@@ -155,11 +155,6 @@ formset
       name  = CustomCpuClock,
       guid  = CONFIGDXE_FORM_SET_GUID;

-    efivarstore CHIPSET_SD_VARSTORE_DATA,
-      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
-      name  = SdIsArasan,
-      guid  = CONFIGDXE_FORM_SET_GUID;
-
     efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = RamMoreThan3GB,
@@ -175,6 +170,11 @@ formset
       name  = OptDeviceTree,
       guid  = CONFIGDXE_FORM_SET_GUID;

+    efivarstore MMC_SD_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = SdIsArasan,
+      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,
@@ -228,6 +228,10 @@ formset
            prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);

+        goto 0x1004,
+            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
+            help = STRING_TOKEN(STR_NULL_STRING);
+
         goto 0x1006,
            prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);
@@ -236,10 +240,6 @@ formset
            prompt = STRING_TOKEN(STR_MMC_FORM_TITLE),
            help = STRING_TOKEN(STR_NULL_STRING);

-        goto 0x1004,
-            prompt = STRING_TOKEN(STR_DISPLAY_FORM_TITLE),
-            help = STRING_TOKEN(STR_NULL_STRING);
-
         goto 0x1005,
             prompt = STRING_TOKEN(STR_DEBUG_FORM_TITLE),
             help = STRING_TOKEN(STR_NULL_STRING);
@@ -269,14 +269,6 @@ formset
               default = 600,
           endnumeric;
         endif;
-
-        oneof varid = SdIsArasan.Routing,
-            prompt      = STRING_TOKEN(STR_CHIPSET_SD_PROMPT),
-            help        = STRING_TOKEN(STR_CHIPSET_SD_HELP),
-            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
-            option text = STRING_TOKEN(STR_CHIPSET_SD_ARASAN), value = 1, flags = 0;
-            option text = STRING_TOKEN(STR_CHIPSET_SD_SDHOST), value = 0, flags = DEFAULT;
-        endoneof;
     endform;

     form formid = 0x1006,
@@ -306,6 +298,14 @@ formset
         title  = STRING_TOKEN(STR_MMC_FORM_TITLE);
         subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE);

+        oneof varid = SdIsArasan.Routing,
+            prompt      = STRING_TOKEN(STR_MMC_SD_PROMPT),
+            help        = STRING_TOKEN(STR_MMC_SD_HELP),
+            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+            option text = STRING_TOKEN(STR_MMC_SD_ARASAN), value = 1, flags = 0;
+            option text = STRING_TOKEN(STR_MMC_SD_SDHOST), value = 0, flags = DEFAULT;
+        endoneof;
+
         oneof varid = MmcDisableMulti.DisableMulti,
             prompt      = STRING_TOKEN(STR_MMC_DISMULTI_PROMPT),
             help        = STRING_TOKEN(STR_MMC_DISMULTI_HELP),
--
2.21.0.windows.1