Re: [edk2] [PATCH] ArmPkg: Move FDT offset higher in RAM

2015-07-28 Thread Leif Lindholm
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 0x8200 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 0x8400 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|0x001E
# The compressed Linux kernel is expected to be under 128MB from the
 beginning of the System Memory
 -
 gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x0800|UINT32|0x001F
 +
 gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x0840|UINT32|0x001F^M
# Maximum file size for TFTP servers that do not support 'tsize'
 extension
gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x0100|UINT32|0x
 
 @@ -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|0x0023
 +  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x0840|UINT32|0x0023^M
# The FDT blob must be loaded at a 64bit aligned address.
gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x0026
 
 
 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 

Re: [edk2] [PATCH] ArmPkg: Move FDT offset higher in RAM

2015-07-28 Thread Ard Biesheuvel
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 0x8200 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 0x8400 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|0x001E
# The compressed Linux kernel is expected to be under 128MB from the
 beginning of the System Memory
 -
 gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x0800|UINT32|0x001F
 +
 gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x0840|UINT32|0x001F^M
# Maximum file size for TFTP servers that do not support 'tsize'
 extension
gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x0100|UINT32|0x

 @@ -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|0x0023
 +  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x0840|UINT32|0x0023^M
# The FDT blob must be loaded at a 64bit aligned address.
gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x0026


 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
   

Re: [edk2] [PATCH] ArmPkg: Move FDT offset higher in RAM

2015-07-28 Thread Ryan Harkin
[+ 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.

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 0x8200 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.




 --
 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|0x0023
  +
 gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x0800|UINT32|0x0023
 # The FDT blob must be loaded at a 64bit aligned address.
 gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x0026
 
  --
  2.1.0
 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] ArmPkg: Move FDT offset higher in RAM

2015-07-28 Thread Ryan Harkin
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 0x8200 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 0x8400 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|0x001E
   # The compressed Linux kernel is expected to be under 128MB from the
beginning of the System Memory
-
gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x0800|UINT32|0x001F
+
gArmTokenSpaceGuid.PcdArmLinuxKernelMaxOffset|0x0840|UINT32|0x001F^M
   # Maximum file size for TFTP servers that do not support 'tsize'
extension
   gArmTokenSpaceGuid.PcdMaxTftpFileSize|0x0100|UINT32|0x

@@ -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|0x0023
+  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x0840|UINT32|0x0023^M
   # The FDT blob must be loaded at a 64bit aligned address.
   gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x0026


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.
  
   

Re: [edk2] [PATCH] ArmPkg: Move FDT offset higher in RAM

2015-07-28 Thread Ard Biesheuvel
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 0x8200 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.

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|0x0023
  +
  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x0800|UINT32|0x0023
 # The FDT blob must be loaded at a 64bit aligned address.
 gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x0026
 
  --
  2.1.0
 


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel