[edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: fix FDT handling for RPi4
Pete Batard
pete at akeo.ie
Fri Mar 6 12:46:13 UTC 2020
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>
> [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/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