Please see one note at the end:

On 2020.03.06 05:53, Andrei Warkentin wrote:
A rev-up of start4.elf VPU firmware meant that
the previous scheme of loading the DTB over top
of RPI_EFI.FD no longer works - the DT is now
loaded way before the armstub, so any overlap
means the DT is overridden.

This change re-arranges a few items in the FD,
allowing the DTB to loaded directly after the
FD in physical memory.

This moves UEFI image down by 0x10000, and reduces
the FD image size by 0x10000, leaving space for
a DTB to be loaded by config.txt at 0x1f0000.

You need a matching "rev RPi4 TF-A for DTB fix" patch
to edk2-non-osi, as it requires a TF-A build with these
options:

PRELOADED_BL33_BASE=0x20000 RPI3_PRELOADED_DTB_BASE=0x1f0000

Note: the same problem still affects the Pi 3, and will be
fixed in a separate change.

Signed-off-by: Andrei Warkentin <andrey.warken...@gmail.com>
---
  Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf  |  2 ++
  Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 13 ++++++++++++-
  Platform/RaspberryPi/RPi4/RPi4.dsc                        | 10 +++++++---
  Platform/RaspberryPi/RPi4/RPi4.fdf                        | 20 
+++++++-------------
  Platform/RaspberryPi/RPi4/Readme.md                       | 10 +++++-----
  Platform/RaspberryPi/RaspberryPi.dec                      | 15 ++++++++-------
  6 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf 
b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
index 77cdbe39..3aac6a98 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
@@ -44,6 +44,8 @@
  [FixedPcd]
    gArmTokenSpaceGuid.PcdFdBaseAddress
    gArmTokenSpaceGuid.PcdFvBaseAddress
+  gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress
+  gRaspberryPiTokenSpaceGuid.PcdFdtSize
    gArmPlatformTokenSpaceGuid.PcdCoreCount
    gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
    gArmTokenSpaceGuid.PcdArmPrimaryCore
diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c 
b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
index e795a885..dec8e09d 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
+++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
@@ -94,7 +94,18 @@ ArmPlatformGetVirtualMemoryMap (
    VirtualMemoryInfo[Index].Type             = RPI_MEM_RUNTIME_REGION;
    VirtualMemoryInfo[Index++].Name           = L"FD Variables";
- if (BCM2711_SOC_REGISTERS == 0) {
+  if (BCM2711_SOC_REGISTERS != 0) {
+     //
+     // Only the Pi 4 firmware today expects the DTB to directly follow the
+     // FD instead of overlapping the FD.
+     //
+     VirtualMemoryTable[Index].PhysicalBase    = FixedPcdGet32 
(PcdFdtBaseAddress);
+     VirtualMemoryTable[Index].VirtualBase     = 
VirtualMemoryTable[Index].PhysicalBase;
+     VirtualMemoryTable[Index].Length          = FixedPcdGet32 (PcdFdtSize);;
+     VirtualMemoryTable[Index].Attributes      = 
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+     VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION;
+     VirtualMemoryInfo[Index++].Name           = L"Flattened Device Tree";
+  } else {
       //
       // TF-A reserved RAM only exists for the Pi 3 TF-A.
       //
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc 
b/Platform/RaspberryPi/RPi4/RPi4.dsc
index da62dc5b..2e98c3e1 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -279,7 +279,10 @@
    gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|1
    gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
    gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
-  gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress|0x20000
+  #
+  # Follows right after the FD image.
+  #
+  gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress|0x001f0000
# DEBUG_ASSERT_ENABLED 0x01
    # DEBUG_PRINT_ENABLED        0x02
@@ -393,8 +396,9 @@
    # Size of the region used by UEFI in permanent memory (Reserved 64MB)
    gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000
    #
-  # This matches PcdFvBaseAddress, since everything less is ATF, and
-  # will be reserved away.
+  # 0x00000000 - 0x001F0000  FD (PcdFdBaseAddress, PcdFdSize)
+  # 0x001F0000 - 0x00200000 DTB (PcdFdtBaseAddress, PcdFdtSize)
+  # 0x00200000 - ...        RAM (PcdSystemMemoryBase, PcdSystemMemorySize)
    #
    gArmTokenSpaceGuid.PcdSystemMemoryBase|0x00200000
    gArmTokenSpaceGuid.PcdSystemMemorySize|0x3fe00000
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf 
b/Platform/RaspberryPi/RPi4/RPi4.fdf
index c3832035..a59d3b60 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -25,11 +25,11 @@
[FD.RPI_EFI]
  BaseAddress   = 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress
-Size          = 0x00200000|gArmTokenSpaceGuid.PcdFdSize
+Size          = 0x001f0000|gArmTokenSpaceGuid.PcdFdSize
  ErasePolarity = 1
BlockSize = 0x00001000|gRaspberryPiTokenSpaceGuid.PcdFirmwareBlockSize
-NumBlocks     = 0x200
+NumBlocks     = 0x1f0
################################################################################
  #
@@ -53,16 +53,10 @@ NumBlocks     = 0x200
  0x00000000|0x00020000
  FILE = $(TFA_BUILD_BL31)
-#
-# DTB.
-#
-0x00020000|0x00010000
-DATA = { 0x00 }
-
  #
  # UEFI image
  #
-0x00030000|0x001b0000
+0x00020000|0x001b0000
  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
  FV = FVMAIN_COMPACT
@@ -76,7 +70,7 @@ FV = FVMAIN_COMPACT
  #
# NV_VARIABLE_STORE
-0x001e0000|0x0000e000
+0x001d0000|0x0000e000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
DATA = {
@@ -119,11 +113,11 @@ DATA = {
  }
# NV_EVENT_LOG
-0x001ee000|0x00001000
+0x001de000|0x00001000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogBase|gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize
# NV_FTW_WORKING header
-0x001ef000|0x00001000
+0x001df000|0x00001000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
DATA = {
@@ -138,7 +132,7 @@ DATA = {
  }
# NV_FTW_WORKING data
-0x001f0000|0x00010000
+0x001e0000|0x00010000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
################################################################################
diff --git a/Platform/RaspberryPi/RPi4/Readme.md 
b/Platform/RaspberryPi/RPi4/Readme.md
index 21c9fd4f..e2f0d698 100644
--- a/Platform/RaspberryPi/RPi4/Readme.md
+++ b/Platform/RaspberryPi/RPi4/Readme.md
@@ -49,8 +49,8 @@ Build instructions from the top level edk2-platforms 
Readme.md apply.
      ```
      Additionally, if you want to use PL011 instead of the miniUART, you can 
add the lines:
      ```
-    device_tree_address=0x20000
-    device_tree_end=0x30000
+    device_tree_address=0x1f0000
+    device_tree_end=0x200000
      device_tree=bcm2711-rpi-4-b.dtb
      dtoverlay=miniuart-bt
      ```
@@ -80,12 +80,12 @@ You can pass a custom Device Tree and overlays using the 
following:
  ```
  (...)
  disable_commandline_tags=2
-device_tree_address=0x20000
-device_tree_end=0x30000
+device_tree_address=0x1f0000
+device_tree_end=0x200000
  device_tree=bcm2711-rpi-4-b.dtb
  ```
-Note: the address range **must** be `[0x20000:0x30000]`.
+Note: the address range **must** be `[0x1f0000:0x200000]`.
  `dtoverlay` and `dtparam` parameters are also supported **when** providing a 
Device Tree`.
## Custom `bootargs`
diff --git a/Platform/RaspberryPi/RaspberryPi.dec 
b/Platform/RaspberryPi/RaspberryPi.dec
index 25058ccc..3ebb83d9 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -36,13 +36,14 @@
[PcdsFixedAtBuild.common]
    gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress|0x10000|UINT32|0x00000001
-  gRaspberryPiTokenSpaceGuid.PcdFirmwareBlockSize|0x0|UINT32|0x00000002
-  gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogBase|0x0|UINT32|0x00000003
-  gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize|0x0|UINT32|0x00000004
-  gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|0x0|UINT32|0x00000005
-  gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|0x0|UINT32|0x00000006
-  gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|0x0|UINT32|0x00000007
-  gRaspberryPiTokenSpaceGuid.PcdExtendedMemoryBase|0x0|UINT32|0x00000008
+  gRaspberryPiTokenSpaceGuid.PcdFdtSize|0x10000|UINT32|0x00000002
+  gRaspberryPiTokenSpaceGuid.PcdFirmwareBlockSize|0x0|UINT32|0x00000003
+  gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogBase|0x0|UINT32|0x00000004
+  gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize|0x0|UINT32|0x00000005
+  gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|0x0|UINT32|0x00000006
+  gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|0x0|UINT32|0x00000007
+  gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|0x0|UINT32|0x00000008
+  gRaspberryPiTokenSpaceGuid.PcdExtendedMemoryBase|0x0|UINT32|0x00000009

Since I got the same remark last time I went for something similar, I'm just going to point out that you only want to change/add the ids for the Pcds you are explicitly modifying or adding, and leave the rest as is even if it breaks sequentiality.

In other words, you could just have inserted a:

gRaspberryPiTokenSpaceGuid.PcdFdtSize|0x10000|UINT32|0x00000009

either after gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress or preferably at the end, since I would assert that we care less about grouping the PCDs in a logical order here than we care about finding out the next free PCD id to use.

I'm hoping that this can be fixed by a maintainer at integration, rather than require a v2, since that's the only comment I have on this patchset.

With this:

Reviewed-by: Pete Batard <p...@akeo.ie>
Tested-by: Pete Batard <p...@akeo.ie>

  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
    gRaspberryPiTokenSpaceGuid.PcdCpuClock|0|UINT32|0x0000000d




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55597): https://edk2.groups.io/g/devel/message/55597
Mute This Topic: https://groups.io/mt/71767123/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to