Add uboot "fdt_high" enviroment variable
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
From: David A. Long <[email protected]>
Add a new "fdt_high" enviroment variable. This can be used to control (or prevent) the relocation of the flattened device tree on boot. It can be used to prevent relocation of the fdt into highmem. The variable behaves similarly to the existing "initrd_high" variable.
--- README | 9 ++++++++ common/image.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 11 deletions(-)
README +9 -0
@@ -3281,6 +3281,15 @@ List of environment variables (most likely not complete): This can be used to load and uncompress arbitrary data. + fdt_high - if set this restricts the maximum address that the + flattened device tree will be copied into upon boot. + If this is set to the special value 0xFFFFFFFF then + the fdt will not be copied at all on boot. For this + to work it must reside in writable memory, have + sufficient padding on the end of it for u-boot to + add the information it needs into it, and the memory + must be accessible by the kernel. + i2cfast - (PPC405GP|PPC405EP only) if set to 'y' configures Linux I2C driver for fast mode (400kHZ). This environment variable is used in
common/image.c +49 -11
83 lines not on this page · raw message
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Hi Dave,
This looks reasonable, with one minor nit...
Quoted 18 lines · David A. Long
From: David A. Long<[email protected]>
Add a new "
fdt_high" enviroment variable. This can be used to control (or prevent) the relocation of the flattened device tree on boot. It can be used to prevent relocation of the fdt into highmem. The variable behaves similarly to the existing "initrd_high" variable.Signed-off-by David A. Long<[email protected]>--- README | 9 ++++++++ common/image.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 11 deletions(-)README +0
-0@@ -3281,6 +3281,15 @@ List of environment variables (most likely not complete):
[snip]
common/image.c int boot_relocate_fdt (struct lmb *lmb, char **of_flat_tree, ulong *of_size) · line 1240
Quoted 12 lines
common/image.c +2
-0@@ -1234,8 +1234,10 @@ int boot_relocate_fdt (struct lmb *lmb, char **of_flat_tree, ulong *of_size) {void *fdt_blob = *of_flat_tree;void *of_start =0 ; +char *fdt_high;ulong of_len =0 ;int err; +int disable_relocation=0 ;
Need spaces around the "="
I will add the spaces before applying the patch unless you send an updated patch.
Thanks,
gvb
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 7 lines · Jerry Van Baren
Hi Dave,
This looks reasonable, with one minor nit...
Quoted 5 lines
Need spaces around the "="
I will add the spaces before applying the patch unless you send an updated patch.
OK, thanks much. Someone else recently pointed that out to me too. I can generate another update if you wish, but I'll let you handle that unless you say otherwise.
FYI: In case it wasn't clear, this all came about because: 1) the Pandaboard has 1GB of RAM; 2) the presence of an fdt causes u-boot to relocate the fdt and ramdisk to the end of ram by default; 3) and the Linux kernel does not like having to access memory beyond about 3/4G (HIGHMEM) during early boot.
Thanks again,
-dl
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 27 lines · David Long
On Thu, 2011-07-14 at 09:10 -0400, Jerry Van Baren wrote:
Hi Dave,
This looks reasonable, with one minor nit...
Need spaces around the "="
I will add the spaces before applying the patch unless you send an updated patch.
Quoted 4 lines
OK, thanks much.? Someone else recently pointed that out to me too.? I can generate another update if you wish, but I'll let you handle that unless you say otherwise.
FYI:? In case it wasn't clear, this all came about because: 1) the Pandaboard has 1GB of RAM;? 2) the presence of an fdt causes u-boot to relocate the fdt and ramdisk to the end of ram by default;? 3) and the Linux kernel does not like having to access memory beyond about 3/4G (HIGHMEM) during early boot.
Regardless of this patch, the pandaboard uboot still needs to be fixed. Setting an fdt_high variable is useful for debug, but it is not a fix.
g.
Quoted 5 lines
-dl
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 4 lines · Grant Likely
Regardless of this patch, the pandaboard uboot still needs to be fixed. Setting an
fdt_highvariable is useful for debug, but it is not a fix.
Then someone needs to own the issue of stopping the current u-boot default behavior of relocating the initrd and fdt to the end of RAM when an fdt is present. This is an issue for any Linux ARM system with more
than 3/4GB of RAM, and probably for other 32-bit architectures. The
logic that causes the problem is in architecture-independent code, and I'm not sure I'm necessarily the right guy to go rummaging around in there. There are too many implications for any other currently supported targets that use u-boot and fdt.
This new u-boot environment variable stands on it's own as potentially useful, and allows us to avoid the problem (admittedly by specifying environment variable values that really should be the default in this case).
-dl
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 9 lines · On Thu, 14 Jul 2011 15:12:25 -0400 David Long <[email protected]> wrote:
Quoted 4 lines · Grant Likely
Regardless of this patch, the pandaboard uboot still needs to be fixed. Setting an
fdt_highvariable is useful for debug, but it is not a fix.
Quoted 8 lines
Then someone needs to own the issue of stopping the current u-boot default behavior of relocating the initrd and fdt to the end of RAM when an fdt is present. This is an issue for any Linux ARM system with more
than 3/4GB of RAM, and probably for other 32-bit architectures. The
logic that causes the problem is in architecture-independent code, and I'm not sure I'm necessarily the right guy to go rummaging around in there. There are too many implications for any other currently supported targets that use u-boot and fdt.
You need to use lmb_reserve() to exclude any memory regions that are not suitable for boot images -- see powerpc's arch_lmb_reserve() and get_effective_memsize()/CONFIG_SYS_LINUX_LOWMEM_MAX_SIZE.
-scott
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 3 lines · Scott Wood
You need to use
lmb_reserve()to exclude any memory regions that are not suitable for boot images -- see powerpc'sarch_lmb_reserve()and get_effective_memsize()/CONFIG_SYS_LINUX_LOWMEM_MAX_SIZE.
If one excludes HIGHMEM from the area u-boot is allowed to relocate the fdt/initrd to, then it will put it at the end of the 3/4GB boundary (can one exclude all memory above the kernel start address?). This splits memory into three, instead of two regions in the kernel. I don't think that split ever goes away. Then there's the additional region we already
have to create for the Ducati memory. That's at least five memory
regions total. There are only eight regions currently allowed by default. I don't have a feel for the implications of this, but it seems unnecessary.
Again, I don't think the problem is where u-boot relocates this data TOO, but the fact that the new default is to relocate it at all. Is there a reason for relocating this stuff? The initrd always used to be happy left where it was.
-dl
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 11 lines · On Thu, 14 Jul 2011 16:09:16 -0400 David Long <[email protected]> wrote:
Quoted 3 lines · Scott Wood
You need to use
lmb_reserve()to exclude any memory regions that are not suitable for boot images -- see powerpc'sarch_lmb_reserve()and get_effective_memsize()/CONFIG_SYS_LINUX_LOWMEM_MAX_SIZE.If one excludes HIGHMEM from the area u-boot is allowed to relocate the fdt/initrd to, then it will put it at the end of the 3/4GB boundary (can one exclude all memory above the kernel start address?).
You have memory below where the kernel is loaded?
Quoted 7 lines
This splits
memory into three, instead of two regions in the kernel. I don't think that split ever goes away. Then there's the additional region we already
have to create for the Ducati memory. That's at least five memory
regions total. There are only eight regions currently allowed by default. I don't have a feel for the implications of this, but it seems unnecessary.
What do you mean by a "region" here, and why can there only be eight of them?
Quoted 4 lines
Again, I don't think the problem is where u-boot relocates this data TOO, but the fact that the new default is to relocate it at all. Is there a reason for relocating this stuff? The initrd always used to be happy left where it was.
The user specified address might be in flash.
-Scott
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 1 line · Scott Wood
You have memory below where the kernel is loaded?
Our boot script loads the kernel 2MB into physical RAM. It loads the initrd and fdt from the same NAND flash file system into RAM below that. When we boot without specifying an FDT, u-boot does not relocate the initrd. When we specify an FDT address in RAM, u-boot relocates both. We do not need that relocation (in this case at least).
Quoted 10 lines
Quoted 7 lines
This splits
memory into three, instead of two regions in the kernel. I don't think that split ever goes away. Then there's the additional region we already
have to create for the Ducati memory. That's at least five memory
regions total. There are only eight regions currently allowed by default. I don't have a feel for the implications of this, but it seems unnecessary.What do you mean by a "region" here, and why can there only be eight of them?
Functionally identical and contiguous sections of RAM recorded in the Linux global meminfo data structure, and (mostly) operated on in code found in arch/arm/mm/. There's a define that sets the size of this array to 8. Again, I don't know the implications, if any, of having several versus a couple of these banks/regions. It just seems a bad idea to create more holes in the middle of physical RAM unless we really have to. Plus, we have to inform the kernel about them somehow. I don't have a clear idea how we would do that in a clean way. Seems to me it'd be uglier than the fdt_high approach. Maybe I'm missing something though. I'm certainly not a VM expert.
Quoted 6 lines
Quoted 4 lines
Again, I don't think the problem is where u-boot relocates this data TOO, but the fact that the new default is to relocate it at all. Is there a reason for relocating this stuff? The initrd always used to be happy left where it was.
The user specified address might be in flash.
But in our case it is not. And we're now being relocated when we did not get relocated prior to FDT functionality.
-dl
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 3 lines · On Thu, 14 Jul 2011 17:20:53 -0400 David Long <[email protected]> wrote:
When we boot without specifying an FDT, u-boot does not relocate the initrd. When we specify an FDT address in RAM, u-boot relocates both. We do not need that relocation (in this case at least).
Well, that does sound strange. I'd think it would be based on whether you define CONFIG_SYS_BOOT_RAMDISK_HIGH, and whether initrd_high is set to 0xffffffff.
Or were you just not telling bootm about the ramdisk before, and letting the kernel know about the address through some other means?
Quoted 8 lines
Quoted 2 lines
What do you mean by a "region" here, and why can there only be eight of them?
Functionally identical and contiguous sections of RAM recorded in the Linux global meminfo data structure, and (mostly) operated on in code found in
arch/arm/mm/. There's a define that sets the size of this array to 8. Again, I don't know the implications, if any, of having several versus a couple of these banks/regions.
I don't think we have this kind of thing on powerpc. We track unusable regions with lmb, based on things like the dtb memreserve list.
Quoted 2 lines
It just seems a bad idea to create more holes in the middle of physical RAM unless we really have to.
So add a mechanism for the user to override if you can justify a use for it, but the default should be allocated from an lmb that has had unusable addresses excluded.
Quoted 2 lines
Plus, we have to inform the kernel about them somehow. I don't have a clear idea how we would do that in a clean way.
I don't know how ARM does it, but on powerpc the kernel is told about the initrd/initramfs address range through the device tree, and the device tree address is in r3 on entry. The device tree blob is also marked reserved in the dtb memreserve list.
The initrd/initramfs doesn't appear to be marked reserved, probably since the kernel had ways of avoiding that region since before flat device trees and memreserve came along.
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Again, I don't think the problem is where u-boot relocates this data TOO, but the fact that the new default is to relocate it at all. Is there a reason for relocating this stuff? The initrd always used to be happy left where it was.
The user specified address might be in flash.
But in our case it is not.
It still needs to be supported.
-Scott
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 6 lines · Grant Likely
You should have everything you need to fix it. If
CONFIG_SYS_BOOTMAPSZis defined, then U-Boot will not use memory larger that that for the dtb or atags.Right now
CONFIG_SYS_BOOTMAPSZis not set by default, but we could default it to a sane value for ARM platforms.
Which makes more sense, setting this or Scott's suggestion of reserving highmem to prevent it's use as is done in the powerpc's arch_lmb_reserve()? The latter would affect all ARM. Wouldn't BOOTMAP need to be set in each boards config file?
Quoted 3 lines · Scott Wood
Well, that does sound strange. I'd think it would be based on whether you define
CONFIG_SYS_BOOT_RAMDISK_HIGH, and whetherinitrd_highis set to0xffffffff.
Same kernel, same u-boot, and initrd_high not set at all. This is the crux of the problem. The default changes when any FDT is provided. I think the difference may come from bootm_linux_fdt().
Rereading your earlier mail and looking at the code, we should probably do as you suggest and add logic to arch_lmb_reserve() to reserve highmem like powerpc does. I think the fdt_high patch is still a good idea though. It allows us to continue booting using lowest memory for the initrd and other startup data.
Quoted 2 lines
Or were you just not telling bootm about the ramdisk before, and letting the kernel know about the address through some other means?
We always provide a ramdisk address to bootm. The problem occurs when we add an FDT address.
Quoted 3 lines
So add a mechanism for the user to override if you can justify a use for it, but the default should be allocated from an lmb that has had unusable addresses excluded.
Well, I do find it troubling I'm having to go to so much trouble to convince u-boot to continue to use this data directly from where we put it in the first place.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
The user specified address might be in flash.
But in our case it is not.
It still needs to be supported.
I can appreciate that, but the default behavior has changed. It's
surprising. For the moment it breaks things (granted that it breaks only if you use the fdt feature).
-dl
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
In reply to David LongQuoted 9 lines · David Long
Quoted 4 lines · Grant Likely
Regardless of this patch, the pandaboard uboot still needs to be fixed. Setting an
fdt_highvariable is useful for debug, but it is not a fix.
Quoted 8 lines
Then someone needs to own the issue of stopping the current u-boot default behavior of relocating the initrd and fdt to the end of RAM when an fdt is present. This is an issue for any Linux ARM system with more
than 3/4GB of RAM, and probably for other 32-bit architectures. The
logic that causes the problem is in architecture-independent code, and I'm not sure I'm necessarily the right guy to go rummaging around in there. There are too many implications for any other currently supported targets that use u-boot and fdt.
You should have everything you need to fix it. If CONFIG_SYS_BOOTMAPSZ is defined, then U-Boot will not use memory larger that that for the dtb or atags.
Right now CONFIG_SYS_BOOTMAPSZ is not set by default, but we could default it to a sane value for ARM platforms.
A better solution in the long term may be to figure out the actual memory footprint required, and not make things any larger than that, but that requires U-Boot to be given more data about the kernel, or the zImage wrapper to be more intelligent about dtb and initrd ranges when uncompressing
g.
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
In reply to David A. LongQuoted 8 lines · David A. Long
From: David A. Long<[email protected]>
Add a new "
fdt_high" enviroment variable. This can be used to control (or prevent) the relocation of the flattened device tree on boot. It can be used to prevent relocation of the fdt into highmem. The variable behaves similarly to the existing "initrd_high" variable.Signed-off-by David A. Long<[email protected]>
While I agree in principle with Scott's point that it would be good to understand and fix/improve the ARM bootm_linux_fdt() function, I have bought into Dave's argument that this is a useful environment variable and is symmetric with "initrd_high".
Consequently, I've added this patch to the u-boot-fdt repo. If anyone has strong objects, now is the time to object strongly. ;-)
Thanks,
gvb
PATCH v2 1/1 Add uboot "fdt_high" enviroment variable
Quoted 10 lines · Jerry Van Baren
Quoted 8 lines · David A. Long
From: David A. Long<[email protected]>
Add a new "
fdt_high" enviroment variable. This can be used to control (or prevent) the relocation of the flattened device tree on boot. It can be used to prevent relocation of the fdt into highmem. The variable behaves similarly to the existing "initrd_high" variable.Signed-off-by David A. Long<[email protected]>
Quoted 7 lines
While I agree in principle with Scott's point that it would be good to understand and fix/improve the ARM
bootm_linux_fdt()function, I have bought into Dave's argument that this is a useful environment variable and is symmetric with "initrd_high".Consequently, I've added this patch to the u-boot-fdt repo. If anyone has strong objects, now is the time to object strongly. ;-)
No, I certainly don't object. My point is simply that it isn't actually a fix for the bug. Either the panda, or the generic arm config needs to have a safe CONFIG_SYS_BOOTMAPSZ set.
g.
Quoted 4 lines
Thanks,
gvb