[- CC edk2-devel + CC linaro-uefi]
On 28 July 2015 at 11:02, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 28 July 2015 at 11:41, Ryan Harkin ryan.harkin@linaro.org wrote:
On 28 July 2015 at 10:26, Ard Biesheuvel ard.biesheuvel@linaro.org
wrote:
On 28 July 2015 at 11:01, Ryan Harkin ryan.harkin@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.biesheuvel@linaro.org wrote:
On 27 July 2015 at 22:42, Ryan Harkin ryan.harkin@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
OK, I can fix this, but it will take a few more lines of patch.
Instead of loading the initrd to LINUX_MAX_OFFSET (ie. gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset) we can load it to LINUX_FDT_MAX_OFFSET ( ie. gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset).
I don't want to create another PCD for the initrd. Which brings me to a comment i missed earlier:
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.
I am trying to keep as little churn as possible, but yes, I agree. It's wrong. Ideally, I'd like to delete the Linux Loader completely, or at least stop using it. But while I'm supporting non-EFI stubbed kernels, I'm stuck.
-- 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.harkin@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
-
Ryan Harkin