On Fri, 6 Mar 2020 at 13:46, Pete Batard <[email protected]> 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 <[email protected]> > > --- > > 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 <[email protected]> > Tested-by: Pete Batard <[email protected]> >
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/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
