Hi,
On 6/19/25 18:22, Raymond Mao wrote:
Hi Michal,
On Thu, 19 Jun 2025 at 07:39, Michal Simek <[email protected]> wrote:
On 6/18/25 16:59, Raymond Mao wrote:
During FDT setup, apply all existing DT overlays from the bloblist
to the base FDT if bloblist is being used for handoff from previous
boot stage.
More technical details would be good to have here.
Especially that the way what you are using is to expand DT
Agree. I can add more details in the next post.
Signed-off-by: Raymond Mao <[email protected]>
---
lib/fdtdec.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index c38738b48c7..3a218d8c94a 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1687,6 +1687,60 @@ void fdtdec_setup_embed(void)
gd->fdt_src = FDTSRC_EMBED;
}
+static int fdtdec_apply_dto_from_blob(void **blob)
+{
+ int ret;
+ struct fdt_header *live_fdt;
+ size_t new_fdt_size, blob_size;
+ int expand_by;
+
+ if (!CONFIG_IS_ENABLED(OF_LIBFDT_OVERLAY) ||
+ !CONFIG_IS_ENABLED(BLOBLIST))
+ return -EPERM;
+
+ live_fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
+ ret = fdt_check_header(live_fdt);
+ if (ret)
+ return ret;
Why is this needed? You already have gd->fdt_blob which has been checked.
You are right, these can be skipped.
+
+ ret = fdt_check_header(*blob);
+ if (ret)
+ return ret;
+
+ blob_size = fdt_totalsize(*blob);
+
+ /* Expand the FDT for spare spaces to apply the overlay */
+ new_fdt_size = fdt_totalsize(live_fdt) + blob_size + 0x400;
Some notes about this 0x400 would be good. Do you see issue if you don't use
this value? Is file DTB after expansion actually bigger then origin+overlay that
you need to add 0x400 magic value here?
The value 0x400 is arbitrary and it really depends on the DTOs.
When applying a DTO, it may create new entries in:
The structure block (nodes, properties)
The strings block (all unique strings, including property names and values)
These new entries require extra space, not just equal to the overlay
size - because:
Some parts will bring in new strings, which get appended to the string table.
Also alignment padding might require more bytes.
What I am concerned about is that 0x400 might be not sufficient for
those complex DTOs.
Maybe more bytes are required.
Valid concern but it should be described in commit message to also say that it
is just a number based on your experiments but maybe it is not enough.
+ ret = bloblist_resize(BLOBLISTT_CONTROL_FDT, new_fdt_size, &expand_by);
+ if (ret)
+ return ret;
You do resize inside tl list.. That should be described in commit message
because my exception was that this location can be read only from NS world.
I can update the commit message with more details but why should it be
read-only? At least I don't recall if the spec says the TL is not
expected to be updated in BL33.
It is not about if should be read only but about it can be because it is
exported from secure world. I understand why you are doing it but please
describe it to make it clear.
Spec is saying
"TL relocation typically happens in later phases of the boot when there is
more memory available, which is needed for adding larger entries."
I didn't run this on HW and likely I should do it to understand this better
but isn't this moving CONTROL_FDT entry to different location which will have
bigger space for applying overlay?
Do you mean to move it to a predefined address like existing
CONFIG_BLOBLIST_ADDR with more space reserved? I am open to it.
I don't want this because I don't want to define address where it should be.
I want to have one universal bootloader which can run from different memory
locations and relocate based on information in DT/TL.
I just want to make sure that description declare that properly how it is
supposed to be used.
But what I have to say is that the entry memory can only be marked
with void, no reclaim memory mechanisms. This leads to duplicated
memory ranges.
Other than that, I think dynamically increasing/shrinking the size is
more flexible.
I have to take a look how exactly this is working. I though that you can reclaim
that memory back. We plan to have limited space (6MB) for TL. There are going to
be multiple DT overlays inside. It means I don't want to be any close to 6MB limit.
Isn't it also worth to check if existing CONTROL_FDT has enough space inside
that new overlay can fit there before resizing it?
By default each entry only contains the actual size since bloblist
(aka transfer list) is designed as a compact data structure and there
should not be a waiver for CONTROL_FDT to carry additional spare
spaces.
Right but you can build dtb via dtc -S/-p to add additional padding. It means
dtb will be bigger which goes to TL and should be possible to fit overlay there.
+
+ /* The blob is shifted if it was following the FDT */
+ if (*blob > (void *)live_fdt)
+ *blob += expand_by;
+
+ ret = fdt_open_into(live_fdt, live_fdt, new_fdt_size);
+ if (ret)
+ return ret;
+
+ ret = fdt_overlay_apply_verbose(live_fdt, *blob);
+ if (ret)
+ return ret;
+
+ /* Shrink the excessive spaces */
+ fdt_pack(live_fdt);
+ new_fdt_size = fdt_totalsize(live_fdt);
+ ret = bloblist_resize(BLOBLISTT_CONTROL_FDT, new_fdt_size, &expand_by);
+ if (ret)
+ return ret;
I didn't try to run on HW but don't you need to also adjust gd->fdt_blob to
point to current location where fdt is?
gd->fdt_blob is already pointed to the one inside TL, it remains the
same, only the size is changed.
I though that you are moving base DTB inside TL list. Isn't this the case?
I have to do closer look and debug your code to understand it better.
Thanks,
Michal
