On 28 July 2015 at 11:41, Ryan Harkin <ryan.har...@linaro.org> wrote: > > > On 28 July 2015 at 10:26, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: >> >> On 28 July 2015 at 11:01, Ryan Harkin <ryan.har...@linaro.org> wrote: >> > [+ Tixy as he's interested in making sure UEFI follows the Linux >> > requirements] >> > >> > On 28 July 2015 at 07:39, Ard Biesheuvel <ard.biesheu...@linaro.org> >> > wrote: >> >> >> >> >> On 27 July 2015 at 22:42, Ryan Harkin <ryan.har...@linaro.org> wrote: >> >> > Device tree files in recent kernels (eg. Linux 4.2) can be >16KB. >> >> > >> >> > The max offset of 0x4000 meant that the device tree would be >> >> > allocated >> >> > at a "random address", which more often than not was above the >> >> > recommended 128MiB boundary. >> >> > >> >> >> >> From Documentation/arm/Booting: >> >> >> >> """ >> >> 4b. Setup the device tree >> >> ... >> >> A safe location is just above the 128MiB boundary from start of RAM. >> >> """ >> >> >> >> i.e., the documented protocol explicitly suggests using an address > >> >> 128 >> >> MB. >> >> So what exactly goes wrong if you load it at a random address? >> > >> > >> > For some reason, I've been reading this as "before" 128MiB. How strange >> > of >> > me. >> > >> > The advice I was following was from the bottom of the email thread where >> > Nicolas Mitre says: >> > >> > "You can therefore load everything (zImage, initrd, DTB) sequentially >> > from the 32MB mark in RAM and be safe." >> > >> > But mostly, I was trying to keep the bottom 32MB unused. >> > >> >> Yes, I think that is the primary requirement here. >> >> > We don't have the option to load sequentially from 32MB unless I change >> > the >> > code a lot more. I've already tried a hack that placed the 3 files >> > sequentially from 0x82000000 on TC2 and it works well, although it's >> > technically wrong because I didn't explicity reserve memory in those >> > areas >> > to stop UEFI from using it. >> > >> > I'll try changing the max offsets to be all above 128MiB and see if it >> > still >> > works. As Tixy pointed out to me, the problem I had with the Linux >> > Loader's >> > "random address" for the DTB is that it was always in the same region >> > that >> > Linux reserves for CMA. And I think that starts before the DTB is >> > finished >> > with. >> > >> >> I think we could simply raise your max address limit to 132 MB: by the >> looks of it, that is the maximum size the current kernel code will >> ever be able to support since it uses two adjacent sections to map the >> FDT, and sections are 2 MB at most when using long descriptors. > > > I've tested it with a patch to change the max offsets to 0x84000000 and it > works well. My hacked in debug shows: > > linux: address 0x87FD2000 > linux: length 0x42D248
Hmm, this doesn't look right. The zImage should be below 128 MB, since it infers the base of DRAM by rounding down its own address to a multiple of 128 MB. I seem to have missed the part before where the PCD in question also affects the placement of the zImage and not only the FDT. > fdt: address 0x87E6E000 > fdt: length 0x3FFC > initrd: address 0x87E73000 > initrd: length 0x15E600 > So the rules say: - zImage between 32 MB and 128 MB - FDT preferably at the next 128 MB boundary - initrd preferably right above the FDT I guess your v1 was best after all: even if the FDT and initrd end up below 128 MB, it is the best we can do with just this PCD -- Ard. > From this patch: > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec > index b30de91..d5b930a 100644 > --- a/ArmPkg/ArmPkg.dec > +++ b/ArmPkg/ArmPkg.dec > @@ -117,7 +117,7 @@ > # > gArmTokenSpaceGuid.PcdArmMachineType|0|UINT32|0x0000001E > # The compressed Linux kernel is expected to be under 128MB from the > beginning of the System Memory > - > gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x08000000|UINT32|0x0000001F > + > gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x08400000|UINT32|0x0000001F^M > # Maximum file size for TFTP servers that do not support 'tsize' > extension > gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x01000000|UINT32|0x00000000 > > @@ -159,7 +159,7 @@ > # If the fixed FDT address is not available, then it should be loaded > below the kernel. > # The recommendation from the Linux kernel is to have the FDT below 16KB. > # (see the kernel doc: Documentation/arm/Booting) > - gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x4000|UINT32|0x00000023 > + gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x08400000|UINT32|0x00000023^M > # The FDT blob must be loaded at a 64bit aligned address. > gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x00000026 > > > Of course, the comments will have to change too: "under 128MB" => "above > 128MiB" and there is no recommendation for the device tree to be < 16KB. > > Unless I get further comment, I'll submit a v2 patch as above with the > comments updated. > > >> >> >> BTW it seems odd for the LinuxLoader - which is now a separate EFI >> application - to use fixed PCDs to parametrise its behavior. I think >> it would be justified to hardcode the recommended behavior as per the >> Linux/ARM boot protocol right into the LinuxLoader binary. >> >> -- >> Ard. >> >> > >> > >> >> >> >> -- >> >> Ard. >> >> >> >> >> >> > This email thread explains that the device tree should be placed >> >> > higher >> >> > in RAM: >> >> > >> >> > >> >> > >> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/187476.html >> >> > >> >> > It also explaines that the kernel may use memory in the 16-32KB range >> >> > and that a zImage will by default be uncompressed to an offset of >> >> > 32KB. >> >> > >> >> > Setting the FDT max offset at 128MiB will allow UEFI to place it >> >> > higher >> >> > up in memory, thus avoiding the problems with it being loaded to a >> >> > random address. >> >> > >> >> > With this patch, by using AllocateMaxAdress, where possible the Linux >> >> > Loader will place the FDT, initrd and kernel at the top of the 128MiB >> >> > range, which also allows for more efficient zImage uncompression, as >> >> > per >> >> > the above thread. >> >> > >> >> > Contributed-under: TianoCore Contribution Agreement 1.0 >> >> > Signed-off-by: Ryan Harkin <ryan.har...@linaro.org> >> >> > --- >> >> > ArmPkg/ArmPkg.dec | 2 +- >> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> > >> >> > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec >> >> > index da0c8a9..67c8cc9 100644 >> >> > --- a/ArmPkg/ArmPkg.dec >> >> > +++ b/ArmPkg/ArmPkg.dec >> >> > @@ -158,7 +158,7 @@ >> >> > # If the fixed FDT address is not available, then it should be >> >> > loaded >> >> > below the kernel. >> >> > # The recommendation from the Linux kernel is to have the FDT >> >> > below >> >> > 16KB. >> >> > # (see the kernel doc: Documentation/arm/Booting) >> >> > - >> >> > gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x4000|UINT32|0x00000023 >> >> > + >> >> > >> >> > gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x08000000|UINT32|0x00000023 >> >> > # The FDT blob must be loaded at a 64bit aligned address. >> >> > gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x00000026 >> >> > >> >> > -- >> >> > 2.1.0 >> >> > >> > >> > > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
