Apologies for top-posting, I think this conversation should drop edk2-devel and move to linaro-uefi (added to cc), until there is consensus/conclusion. Could the next person replying to this thread delete edk2-devel from the recipient list please?
/ Leif On Tue, Jul 28, 2015 at 10:41:29AM +0100, Ryan Harkin 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 > fdt: address 0x87E6E000 > fdt: length 0x3FFC > initrd: address 0x87E73000 > initrd: length 0x15E600 > > 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