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

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements
@ 2020-03-03 10:33 Pete Batard
  2020-03-03 10:33 ` [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size Pete Batard
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Pete Batard @ 2020-03-03 10:33 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, philmd, awarkentin

This series improves on the configuration options made available to users
in the firmware/BIOS setup.

It adds 2 new options, one aimed at limiting the total amount of RAM
presented to the OS to 3 GB (currently needed for xHCI on Linux) and the
other toggling the provision of the Device Tree, which can be used to
force ACPI mode.

Combined, these two options remove the need for the ACPI_BASIC_MODE_ENABLE
build parameter we were previously using, and allow more fine grained
control, including adding the ability for Pi 3 users to enforce ACPI.

Finally, some cleanup and harmonization of the user settings menu forms
is applied.


Andrei Warkentin (3):
  Platform/RPi: Add firmware call to read installed memory 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 forms

 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                 | 66 ++++++++++++---
 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          | 33 +++++---
 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, 283 insertions(+), 94 deletions(-)

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size
  2020-03-03 10:33 [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements Pete Batard
@ 2020-03-03 10:33 ` Pete Batard
  2020-03-03 10:37   ` Ard Biesheuvel
  2020-03-03 10:33 ` [edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB Pete Batard
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Pete Batard @ 2020-03-03 10:33 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, philmd, awarkentin

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).

Use the new call in PlatformSmbiosDxe.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 10 ++---
 Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c       | 29 +++++++++++-
 Platform/RaspberryPi/Include/Protocol/RpiFirmware.h                | 47 +++++++++++---------
 3 files changed, 59 insertions(+), 27 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;
 
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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB
  2020-03-03 10:33 [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements Pete Batard
  2020-03-03 10:33 ` [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size Pete Batard
@ 2020-03-03 10:33 ` Pete Batard
  2020-03-03 10:39   ` Ard Biesheuvel
  2020-03-03 10:33 ` [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice Pete Batard
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Pete Batard @ 2020-03-03 10:33 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, philmd, awarkentin

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

This 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 | 33 ++++++++++++++------
 2 files changed, 23 insertions(+), 13 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..3f257d4fa528 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;
 
@@ -78,7 +79,6 @@ ArmPlatformGetVirtualMemoryMap (
     return;
   }
 
-
   // Firmware Volume
   VirtualMemoryTable[Index].PhysicalBase    = FixedPcdGet64 (PcdFdBaseAddress);
   VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
@@ -155,13 +155,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 4GB, and the additional 1GB is added by ConfigDxe.
+  //
+  OrigMemorySize = SystemMemorySize;
+  SystemMemorySize = MIN(SystemMemorySize, 3U * SIZE_1GB);
 
   // If we have RAM above the 1 GB mark, declare it
   if (SystemMemorySize - SIZE_1GB > 0) {
@@ -170,7 +170,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 3GB";
+  }
+
+  //
+  // 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 3GB";
   }
 
   // End of Table
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice
  2020-03-03 10:33 [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements Pete Batard
  2020-03-03 10:33 ` [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size Pete Batard
  2020-03-03 10:33 ` [edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB Pete Batard
@ 2020-03-03 10:33 ` Pete Batard
  2020-03-03 10:42   ` Ard Biesheuvel
  2020-03-03 10:33 ` [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision " Pete Batard
  2020-03-03 10:33 ` [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms Pete Batard
  4 siblings, 1 reply; 11+ messages in thread
From: Pete Batard @ 2020-03-03 10:33 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, philmd, awarkentin

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      | 58 ++++++++++++++++----
 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, 120 insertions(+), 14 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 2f48ca0dd758..451a419a5358 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,6 @@ ApplyVariables (
   UINT32 CpuClock = PcdGet32 (PcdCpuClock);
   UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
   UINT32 Rate = 0;
-  UINT32 ModelFamily = 0;
 
   if (CpuClock != 0) {
     if (CpuClock == 2) {
@@ -258,15 +279,18 @@ 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) {
+    Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, BASE_1GB + SIZE_2GB,
+                    SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS),
+                    EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB);
+    ASSERT_EFI_ERROR (Status);
+    Status = gDS->SetMemorySpaceAttributes (BASE_1GB + SIZE_2GB,
+                    SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS),
+                    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 +340,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 +376,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 +424,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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice
  2020-03-03 10:33 [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements Pete Batard
                   ` (2 preceding siblings ...)
  2020-03-03 10:33 ` [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice Pete Batard
@ 2020-03-03 10:33 ` Pete Batard
  2020-03-03 10:45   ` Ard Biesheuvel
  2020-03-03 10:33 ` [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms Pete Batard
  4 siblings, 1 reply; 11+ messages in thread
From: Pete Batard @ 2020-03-03 10:33 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, philmd, awarkentin

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 451a419a5358..31a62094e33e 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -151,6 +151,14 @@ SetupVariables (
     PcdSet32 (PcdRamLimitTo3GB, 0);
   }
 
+  Size = sizeof (UINT32);
+  Status = gRT->GetVariable (L"OptDeviceTree",
+                  &gConfigDxeFormSetGuid,
+                  NULL, &Size, &Var32);
+  if (EFI_ERROR (Status)) {
+    PcdSet32 (PcdOptDeviceTree, PcdGet32 (PcdOptDeviceTree));
+  }
+
   Size = sizeof (UINT32);
   Status = gRT->GetVariable (L"SdIsArasan",
                   &gConfigDxeFormSetGuid,
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 407aac89c7b3..736d49df562b 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -72,6 +72,7 @@ [Pcd]
   gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot
+  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
   gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
   gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
 
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 830533a9dc49..2e79e322e558 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -46,6 +46,11 @@
 #string STR_ADVANCED_3GB_OFF         #language en-US "Disabled"
 #string STR_ADVANCED_3GB_ON          #language en-US "Enabled"
 
+#string STR_ADVANCED_DT_PROMPT       #language en-US "Device Tree"
+#string STR_ADVANCED_DT_HELP         #language en-US "Disable this option to force OSes such as GNU/Linux to use ACPI"
+#string STR_ADVANCED_DT_OFF          #language en-US "Disabled (Force ACPI)"
+#string STR_ADVANCED_DT_ON           #language en-US "Enabled"
+
 /*
  * MMC/SD configuration.
  */
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index 483edd7459c5..d16058da4926 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -85,6 +85,14 @@ typedef struct {
   UINT32 Enabled;
 } ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
 
+typedef struct {
+  /*
+   * 0 - Do not provide a Device Tree to the OS
+   * 1 - Provide a Device Tree to the OS
+   */
+  UINT32 Enabled;
+} ADVANCED_DEVICE_TREE_VARSTORE_DATA;
+
 typedef struct {
   /*
    * 0 - Don't disable multi-block.
@@ -162,6 +170,11 @@ formset
       name  = RamLimitTo3GB,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
+    efivarstore ADVANCED_DEVICE_TREE_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = OptDeviceTree,
+      guid  = CONFIGDXE_FORM_SET_GUID;
+
     efivarstore MMC_DISMULTI_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = MmcDisableMulti,
@@ -279,6 +292,14 @@ formset
               option text = STRING_TOKEN(STR_ADVANCED_3GB_ON), value = 1, flags = 0;
           endoneof;
         endif;
+
+        oneof varid = OptDeviceTree.Enabled,
+            prompt      = STRING_TOKEN(STR_ADVANCED_DT_PROMPT),
+            help        = STRING_TOKEN(STR_ADVANCED_DT_HELP),
+            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+            option text = STRING_TOKEN(STR_ADVANCED_DT_OFF), value = 0, flags = 0;
+            option text = STRING_TOKEN(STR_ADVANCED_DT_ON), value = 1, flags = DEFAULT;
+        endoneof;
     endform;
 
     form formid = 0x1003,
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index eb8048930c30..e7143f57b3b6 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -443,6 +443,11 @@ FdtDxeInitialize (
   UINT32     BoardRevision;
   BOOLEAN    Internal;
 
+  if (PcdGet32 (PcdOptDeviceTree) == 0) {
+    DEBUG ((DEBUG_INFO, "Device Tree disabled per user configuration\n"));
+    return EFI_SUCCESS;
+  }
+
   Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
                   (VOID**)&mFwProtocol);
   ASSERT_EFI_ERROR (Status);
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
index bf9912b4f7d8..fc37353f883f 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
@@ -48,3 +48,6 @@ [Depex]
 
 [FixedPcd]
   gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress
+
+[Pcd]
+  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 48e1a32e1d24..91d5738afbc6 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -445,6 +445,11 @@ [PcdsDynamicHii.common.DEFAULT]
   gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
 
+  #
+  # Device Tree
+  #
+  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 3ce2c3e4d519..79295729f6ee 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -38,7 +38,6 @@ [Defines]
   DEFINE SECURE_BOOT_ENABLE      = FALSE
   DEFINE INCLUDE_TFTP_COMMAND    = FALSE
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
-  DEFINE ACPI_BASIC_MODE_ENABLE  = FALSE
 
 !ifndef TFA_BUILD_ARTIFACTS
   #
@@ -271,8 +270,6 @@ [PcdsFeatureFlag.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
 
-  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|$(ACPI_BASIC_MODE_ENABLE)
-
 [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
   gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
@@ -483,6 +480,11 @@ [PcdsDynamicHii.common.DEFAULT]
   gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
 
+  #
+  # Device Tree
+  #
+  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
+
   #
   # Common UEFI ones.
   #
@@ -575,9 +577,7 @@ [Components.common]
 
   ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
   Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
-!if $(ACPI_BASIC_MODE_ENABLE) == FALSE
   Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
-!endif
   Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
   ArmPkg/Drivers/TimerDxe/TimerDxe.inf
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
index cb5e8e9ae92e..c38320350213 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -208,9 +208,7 @@ [FV.FvMain]
 
   INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
   INF Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
-!if $(ACPI_BASIC_MODE_ENABLE) == FALSE
   INF Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
-!endif
   INF Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
   INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
   INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
diff --git a/Platform/RaspberryPi/RPi4/Readme.md b/Platform/RaspberryPi/RPi4/Readme.md
index 758d0124a6cf..21c9fd4f0a4a 100644
--- a/Platform/RaspberryPi/RPi4/Readme.md
+++ b/Platform/RaspberryPi/RPi4/Readme.md
@@ -16,9 +16,12 @@ Raspberry Pi is a trademark of the [Raspberry Pi Foundation](https://www.raspber
 This firmware is still in development stage, meaning that it comes with the
 following __major__ limitations:
 
-- USB may only work in pre-OS phase at this stage due to nonstandard ECAM,
-  missing/incomplete ACPI tables as well as other factors. For Linux, using
-  the `ACPI_BASIC_MODE_ENABLE` build option may help.
+- xHCI USB may only work in pre-OS phase due to nonstandard DMA constraints.
+  For 4 GB models running Linux, limiting the RAM to 3 GB should help.
+  The 3 GB limitation is currently enabled by default in the user settings.
+- Device Tree boot of OSes such as Linux may not work at all.
+  For this reason, the provision of a Device Tree is disabled by default in
+  the user settings, in order to enforce ACPI boot.
 - Serial I/O from the OS may not work due to CPU throttling affecting the
   miniUART baudrate. This can be worked around by using the PL011 UART
   through the device tree overlays.
@@ -27,12 +30,6 @@ following __major__ limitations:
 
 Build instructions from the top level edk2-platforms Readme.md apply.
 
-The following additional build options are also available:
-- `-D ACPI_BASIC_MODE_ENABLE=1`: Limits OS visible memory to 3 GB and forces
-  ACPI (by disabling the Device Tree driver altogether). This may be required
-  to boot Operating Systems such as Linux on account of the current PCIe/xHCI
-  limitations.
-
 # Booting the firmware
 
 1. Format a uSD card as FAT16 or FAT32
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index 7f2c37ac9a7f..25058ccc7783 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -57,8 +57,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0|UINT8|0x00000017
   gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018
+  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|1|UINT32|0x0000001B
   gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
   gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
-
-[PcdsFeatureFlag.common]
-  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x0000001B
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms
  2020-03-03 10:33 [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements Pete Batard
                   ` (3 preceding siblings ...)
  2020-03-03 10:33 ` [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision " Pete Batard
@ 2020-03-03 10:33 ` Pete Batard
  2020-03-03 10:45   ` Ard Biesheuvel
  4 siblings, 1 reply; 11+ messages in thread
From: Pete Batard @ 2020-03-03 10:33 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, philmd, awarkentin

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size
  2020-03-03 10:33 ` [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size Pete Batard
@ 2020-03-03 10:37   ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-03-03 10:37 UTC (permalink / raw)
  To: Pete Batard
  Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
	Andrei Warkentin

On Tue, 3 Mar 2020 at 11:33, Pete Batard <pete@akeo.ie> wrote:
>
> 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).
>
> Use the new call in PlatformSmbiosDxe.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>

Please split this into two patches

> ---
>  Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 10 ++---
>  Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c       | 29 +++++++++++-
>  Platform/RaspberryPi/Include/Protocol/RpiFirmware.h                | 47 +++++++++++---------
>  3 files changed, 59 insertions(+), 27 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;
>
> 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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB
  2020-03-03 10:33 ` [edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB Pete Batard
@ 2020-03-03 10:39   ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-03-03 10:39 UTC (permalink / raw)
  To: Pete Batard
  Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
	Andrei Warkentin

On Tue, 3 Mar 2020 at 11:33, Pete Batard <pete@akeo.ie> wrote:
>
> From: Andrei Warkentin <andrey.warkentin@gmail.com>
>
> This 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>

Please fix the commit log so it is clear what 'this' means

> ---
>  Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf  |  3 --
>  Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 33 ++++++++++++++------
>  2 files changed, 23 insertions(+), 13 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..3f257d4fa528 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;
>
> @@ -78,7 +79,6 @@ ArmPlatformGetVirtualMemoryMap (
>      return;
>    }
>
> -
>    // Firmware Volume
>    VirtualMemoryTable[Index].PhysicalBase    = FixedPcdGet64 (PcdFdBaseAddress);
>    VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;

Drop this hunk please

> @@ -155,13 +155,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 4GB, and the additional 1GB is added by ConfigDxe.
> +  //
> +  OrigMemorySize = SystemMemorySize;
> +  SystemMemorySize = MIN(SystemMemorySize, 3U * SIZE_1GB);
>
>    // If we have RAM above the 1 GB mark, declare it
>    if (SystemMemorySize - SIZE_1GB > 0) {
> @@ -170,7 +170,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 3GB";
> +  }
> +
> +  //
> +  // 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 3GB";
>    }
>
>    // End of Table
> --
> 2.21.0.windows.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice
  2020-03-03 10:33 ` [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice Pete Batard
@ 2020-03-03 10:42   ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-03-03 10:42 UTC (permalink / raw)
  To: Pete Batard
  Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
	Andrei Warkentin

On Tue, 3 Mar 2020 at 11:33, Pete Batard <pete@akeo.ie> wrote:
>
> 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      | 58 ++++++++++++++++----
>  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, 120 insertions(+), 14 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 2f48ca0dd758..451a419a5358 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,6 @@ ApplyVariables (
>    UINT32 CpuClock = PcdGet32 (PcdCpuClock);
>    UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
>    UINT32 Rate = 0;
> -  UINT32 ModelFamily = 0;
>
>    if (CpuClock != 0) {
>      if (CpuClock == 2) {
> @@ -258,15 +279,18 @@ 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) {
> +    Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, BASE_1GB + SIZE_2GB,
> +                    SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS),
> +                    EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB);
> +    ASSERT_EFI_ERROR (Status);
> +    Status = gDS->SetMemorySpaceAttributes (BASE_1GB + SIZE_2GB,
> +                    SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS),
> +                    EFI_MEMORY_WB);
> +    ASSERT_EFI_ERROR (Status);
>    }
>

This looks a bit hacky and ad-hoc to me.

What is

SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS),

supposed to mean? Can we declare it somewhere and document it, rather
than drop it into the middle of a function call?

> -
> -  if (ModelFamily == 3) {
> +  if (mModelFamily == 3) {
>      /*
>       * Pi 3: either Arasan or SdHost goes to SD card.
>       *
> @@ -316,7 +340,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 +376,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 +424,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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision a runtime (BIOS setup) choice
  2020-03-03 10:33 ` [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision " Pete Batard
@ 2020-03-03 10:45   ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-03-03 10:45 UTC (permalink / raw)
  To: Pete Batard
  Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
	Andrei Warkentin

On Tue, 3 Mar 2020 at 11:33, Pete Batard <pete@akeo.ie> wrote:
>
> With this and the previous commit, ACPI_BASIC_MODE_ENABLE becomes
> superfluous so remove it.
>
> New option defaults to enabled on Pi 3, disabled on Pi 4.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>

This touches a lot of different files at the same time, but I guess it
makes sense as a single set, so

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

> ---
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c      |  8 ++++++++
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |  1 +
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  5 +++++
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 21 ++++++++++++++++++++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c            |  5 +++++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf          |  3 +++
>  Platform/RaspberryPi/RPi3/RPi3.dsc                      |  5 +++++
>  Platform/RaspberryPi/RPi4/RPi4.dsc                      | 10 +++++-----
>  Platform/RaspberryPi/RPi4/RPi4.fdf                      |  2 --
>  Platform/RaspberryPi/RPi4/Readme.md                     | 15 ++++++--------
>  Platform/RaspberryPi/RaspberryPi.dec                    |  4 +---
>  11 files changed, 60 insertions(+), 19 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 451a419a5358..31a62094e33e 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -151,6 +151,14 @@ SetupVariables (
>      PcdSet32 (PcdRamLimitTo3GB, 0);
>    }
>
> +  Size = sizeof (UINT32);
> +  Status = gRT->GetVariable (L"OptDeviceTree",
> +                  &gConfigDxeFormSetGuid,
> +                  NULL, &Size, &Var32);
> +  if (EFI_ERROR (Status)) {
> +    PcdSet32 (PcdOptDeviceTree, PcdGet32 (PcdOptDeviceTree));
> +  }
> +
>    Size = sizeof (UINT32);
>    Status = gRT->GetVariable (L"SdIsArasan",
>                    &gConfigDxeFormSetGuid,
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 407aac89c7b3..736d49df562b 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -72,6 +72,7 @@ [Pcd]
>    gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit
>    gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes
>    gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot
> +  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
>    gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
>    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 830533a9dc49..2e79e322e558 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -46,6 +46,11 @@
>  #string STR_ADVANCED_3GB_OFF         #language en-US "Disabled"
>  #string STR_ADVANCED_3GB_ON          #language en-US "Enabled"
>
> +#string STR_ADVANCED_DT_PROMPT       #language en-US "Device Tree"
> +#string STR_ADVANCED_DT_HELP         #language en-US "Disable this option to force OSes such as GNU/Linux to use ACPI"
> +#string STR_ADVANCED_DT_OFF          #language en-US "Disabled (Force ACPI)"
> +#string STR_ADVANCED_DT_ON           #language en-US "Enabled"
> +
>  /*
>   * MMC/SD configuration.
>   */
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index 483edd7459c5..d16058da4926 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -85,6 +85,14 @@ typedef struct {
>    UINT32 Enabled;
>  } ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
>
> +typedef struct {
> +  /*
> +   * 0 - Do not provide a Device Tree to the OS
> +   * 1 - Provide a Device Tree to the OS
> +   */
> +  UINT32 Enabled;
> +} ADVANCED_DEVICE_TREE_VARSTORE_DATA;
> +
>  typedef struct {
>    /*
>     * 0 - Don't disable multi-block.
> @@ -162,6 +170,11 @@ formset
>        name  = RamLimitTo3GB,
>        guid  = CONFIGDXE_FORM_SET_GUID;
>
> +    efivarstore ADVANCED_DEVICE_TREE_VARSTORE_DATA,
> +      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> +      name  = OptDeviceTree,
> +      guid  = CONFIGDXE_FORM_SET_GUID;
> +
>      efivarstore MMC_DISMULTI_VARSTORE_DATA,
>        attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>        name  = MmcDisableMulti,
> @@ -279,6 +292,14 @@ formset
>                option text = STRING_TOKEN(STR_ADVANCED_3GB_ON), value = 1, flags = 0;
>            endoneof;
>          endif;
> +
> +        oneof varid = OptDeviceTree.Enabled,
> +            prompt      = STRING_TOKEN(STR_ADVANCED_DT_PROMPT),
> +            help        = STRING_TOKEN(STR_ADVANCED_DT_HELP),
> +            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> +            option text = STRING_TOKEN(STR_ADVANCED_DT_OFF), value = 0, flags = 0;
> +            option text = STRING_TOKEN(STR_ADVANCED_DT_ON), value = 1, flags = DEFAULT;
> +        endoneof;
>      endform;
>
>      form formid = 0x1003,
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index eb8048930c30..e7143f57b3b6 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -443,6 +443,11 @@ FdtDxeInitialize (
>    UINT32     BoardRevision;
>    BOOLEAN    Internal;
>
> +  if (PcdGet32 (PcdOptDeviceTree) == 0) {
> +    DEBUG ((DEBUG_INFO, "Device Tree disabled per user configuration\n"));
> +    return EFI_SUCCESS;
> +  }
> +
>    Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
>                    (VOID**)&mFwProtocol);
>    ASSERT_EFI_ERROR (Status);
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> index bf9912b4f7d8..fc37353f883f 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> @@ -48,3 +48,6 @@ [Depex]
>
>  [FixedPcd]
>    gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress
> +
> +[Pcd]
> +  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 48e1a32e1d24..91d5738afbc6 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -445,6 +445,11 @@ [PcdsDynamicHii.common.DEFAULT]
>    gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0
>    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0
>
> +  #
> +  # Device Tree
> +  #
> +  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
> +
>    #
>    # Common UEFI ones.
>    #
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 3ce2c3e4d519..79295729f6ee 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -38,7 +38,6 @@ [Defines]
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
>    DEFINE INCLUDE_TFTP_COMMAND    = FALSE
>    DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
> -  DEFINE ACPI_BASIC_MODE_ENABLE  = FALSE
>
>  !ifndef TFA_BUILD_ARTIFACTS
>    #
> @@ -271,8 +270,6 @@ [PcdsFeatureFlag.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>
> -  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|$(ACPI_BASIC_MODE_ENABLE)
> -
>  [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
> @@ -483,6 +480,11 @@ [PcdsDynamicHii.common.DEFAULT]
>    gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0
>    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1
>
> +  #
> +  # Device Tree
> +  #
> +  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
> +
>    #
>    # Common UEFI ones.
>    #
> @@ -575,9 +577,7 @@ [Components.common]
>
>    ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>    Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> -!if $(ACPI_BASIC_MODE_ENABLE) == FALSE
>    Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> -!endif
>    Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>    ArmPkg/Drivers/TimerDxe/TimerDxe.inf
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
> index cb5e8e9ae92e..c38320350213 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.fdf
> +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
> @@ -208,9 +208,7 @@ [FV.FvMain]
>
>    INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>    INF Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> -!if $(ACPI_BASIC_MODE_ENABLE) == FALSE
>    INF Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> -!endif
>    INF Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>    INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
>    INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> diff --git a/Platform/RaspberryPi/RPi4/Readme.md b/Platform/RaspberryPi/RPi4/Readme.md
> index 758d0124a6cf..21c9fd4f0a4a 100644
> --- a/Platform/RaspberryPi/RPi4/Readme.md
> +++ b/Platform/RaspberryPi/RPi4/Readme.md
> @@ -16,9 +16,12 @@ Raspberry Pi is a trademark of the [Raspberry Pi Foundation](https://www.raspber
>  This firmware is still in development stage, meaning that it comes with the
>  following __major__ limitations:
>
> -- USB may only work in pre-OS phase at this stage due to nonstandard ECAM,
> -  missing/incomplete ACPI tables as well as other factors. For Linux, using
> -  the `ACPI_BASIC_MODE_ENABLE` build option may help.
> +- xHCI USB may only work in pre-OS phase due to nonstandard DMA constraints.
> +  For 4 GB models running Linux, limiting the RAM to 3 GB should help.
> +  The 3 GB limitation is currently enabled by default in the user settings.
> +- Device Tree boot of OSes such as Linux may not work at all.
> +  For this reason, the provision of a Device Tree is disabled by default in
> +  the user settings, in order to enforce ACPI boot.
>  - Serial I/O from the OS may not work due to CPU throttling affecting the
>    miniUART baudrate. This can be worked around by using the PL011 UART
>    through the device tree overlays.
> @@ -27,12 +30,6 @@ following __major__ limitations:
>
>  Build instructions from the top level edk2-platforms Readme.md apply.
>
> -The following additional build options are also available:
> -- `-D ACPI_BASIC_MODE_ENABLE=1`: Limits OS visible memory to 3 GB and forces
> -  ACPI (by disabling the Device Tree driver altogether). This may be required
> -  to boot Operating Systems such as Linux on account of the current PCIe/xHCI
> -  limitations.
> -
>  # Booting the firmware
>
>  1. Format a uSD card as FAT16 or FAT32
> diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
> index 7f2c37ac9a7f..25058ccc7783 100644
> --- a/Platform/RaspberryPi/RaspberryPi.dec
> +++ b/Platform/RaspberryPi/RaspberryPi.dec
> @@ -57,8 +57,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>    gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016
>    gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0|UINT8|0x00000017
>    gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018
> +  gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|1|UINT32|0x0000001B
>    gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
>    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
> -
> -[PcdsFeatureFlag.common]
> -  gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x0000001B
> --
> 2.21.0.windows.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms
  2020-03-03 10:33 ` [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms Pete Batard
@ 2020-03-03 10:45   ` Ard Biesheuvel
  0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2020-03-03 10:45 UTC (permalink / raw)
  To: Pete Batard
  Cc: edk2-devel-groups-io, Leif Lindholm, Philippe Mathieu-Daudé,
	Andrei Warkentin

On Tue, 3 Mar 2020 at 11:34, Pete Batard <pete@akeo.ie> wrote:
>
> 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>

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

> ---
>  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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-03-03 10:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-03 10:33 [edk2-devel][PATCH 0/5] Platform/RPi: User config improvements Pete Batard
2020-03-03 10:33 ` [edk2-devel][PATCH 1/5] Platform/RPi: Add firmware call to read installed memory size Pete Batard
2020-03-03 10:37   ` Ard Biesheuvel
2020-03-03 10:33 ` [edk2-devel][PATCH 2/5] Platform/RPi: Separate RAM descriptors between 0-3 GB and 3+ GB Pete Batard
2020-03-03 10:39   ` Ard Biesheuvel
2020-03-03 10:33 ` [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice Pete Batard
2020-03-03 10:42   ` Ard Biesheuvel
2020-03-03 10:33 ` [edk2-devel][PATCH 4/5] Platform/RPi: Make Device Tree provision " Pete Batard
2020-03-03 10:45   ` Ard Biesheuvel
2020-03-03 10:33 ` [edk2-devel][PATCH 5/5] Platform/RPi/ConfigDxe: Improve RPi configuration forms Pete Batard
2020-03-03 10:45   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox