[edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: fix FDT handling for RPi4
Ard Biesheuvel
ard.biesheuvel at linaro.org
Fri Mar 6 16:48:11 UTC 2020
On Fri, 6 Mar 2020 at 13:46, Pete Batard <pete at akeo.ie> wrote:
>
> 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.warkentin at 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 <pete at akeo.ie>
> Tested-by: Pete Batard <pete at akeo.ie>
>
Agreed
Pushed as 91ed4f904e16..828fcbf96a38
Thanks all.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55609): https://edk2.groups.io/g/devel/message/55609
Mute This Topic: https://groups.io/mt/71767123/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list