On 16. 04. 20 19:21, Stephen Warren wrote:
On 4/16/20 11:06 AM, Tom Rini wrote:
On Thu, Apr 16, 2020 at 05:47:41PM +0200, Michal Simek wrote:
On 16. 04. 20 17:39, Tom Rini wrote:
On Mon, Apr 13, 2020 at 10:03:04AM +0200, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
FDT memory is aligned by 4KB. This is hardcoded in common/board_f.c. Add Kconfig option, assign default value of 0x1000 and enable option to change this value.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
common/board_f.c | 3 ++- dts/Kconfig | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 82a164752aa3..928874e03555 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -546,7 +546,8 @@ static int reserve_fdt(void) * will be relocated with other data. */ if (gd->fdt_blob) {
gd->fdt_size = ALIGN(fdt_totalsize(gd->fdt_blob) + 0x1000, 32);
gd->fdt_size = ALIGN(fdt_totalsize(gd->fdt_blob) +CONFIG_FDT_MEM_ALIGN_SIZE, 32);gd->start_addr_sp -= gd->fdt_size; gd->new_fdt = map_sysmem(gd->start_addr_sp, gd->fdt_size);
diff --git a/dts/Kconfig b/dts/Kconfig index 046a54a17366..696c0b71afaf 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -121,6 +121,13 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config FDT_MEM_ALIGN_SIZE
- hex "FDT memory alignment size"
- default 0x1000
- help
This option is used to set the default alignment when reserving memoryfor fdt.I'm not sure this is clear enough of a description. At first I thought this was about where the start address needs to be, and that is 8 bytes and non-configurable (both arm32 and arm64 require 64bit aligned). What I don't see is where the size of the overall blob needs to be aligned. We need to point at that requirement somewhere in the help so that we don't have people using bad values and then working around it without realizing the problem. Thanks!
Definitely description should be better as we agreed in second review. But the point is do we really need such a big FDT alignment here? It was added by Simon long time ago without any explanation as the part of bigger patch. But are we allowing resizing FDT that we need additional "4k" alignment?
So, I stumbled on an even older thread asking about this where it seems like part of the reason we do something here is so that there's room to add bootargs, etc to /chosen. The start address must be 64bit-aligned, but the size seems like it just needs to be big enough for everything we could have in it, and at this point in the code we don't know just how much that is?
Adding space for DT modifications certainly makes sense. This isn't alignment though, it's just adding space.
yes but 4k additional space pointed by $fdtcontroladdr. As Heinrich pointed out UEFI is doing own copy. I tried to run booti 80000 - $fdtcontroladdr (bootm should be the same) and DT is copied to different location anyway (based on fdt_high) which is reported by "Loading Device Tree to 000000000fff3000, end 000000000fffffff ... OK" And on this copy image_setup_libfdt() is called.
And 4k is not enough for playing games with DT overlays anyway.
That's why my question remains. Do we use this additional 4k space in GD structure for any purpose?
Looking back at the original patch, there was no change to alignment of the start or end of the DT, just the ability to configure the amount of extra space added to the DT. Nothing should refer to that as alignment, since it's not.
That description will be fixed when we finish this discussion.
Thanks, Michal