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

Reply via email to