[PATCH 0/7] fdt: Move towards using ofnode for devicetree fixups
When large quantities of fixups are needed, using the livetree is more efficient. In any case, the ofnode API is easier to use and read than the lower-level libfdt functions.
This series tweaks the DT-fixup implementation a little, so that all of the non-ofnode fixups happen before the ofnode fixups. This should make it possible to whittle away at the former.
This series is based on Sean's one here:
https://patchwork.ozlabs.org/project/uboot/list/?series=380576&state=*
Simon Glass (7): boot: Drop size parameter from image_setup_libfdt() fdt: Check for a valid fdt in oftree_ensure() fdt: Improve the comment for fdt_shrink_to_minimum() fdt: ppc: Drop extra size for ramdisk boot: Move adding initrd earlier in image_setup_libfdt() fdt: Drop the confusing casts in lmb_free() fdt: Move ft_verify_fdt() before the final fixups
arch/mips/lib/bootm.c | 4 ++-- boot/fdt_support.c | 1 - boot/image-board.c | 2 +- boot/image-fdt.c | 27 ++++++++++++--------------- cmd/bootefi.c | 2 +- drivers/core/ofnode.c | 5 +++++ include/fdt_support.h | 24 ++++++++++++++++++++++-- include/image.h | 3 +-- lib/efi_loader/efi_dt_fixup.c | 2 +- 9 files changed, 45 insertions(+), 25 deletions(-)
The of_size parameter is not used, so remove it.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/mips/lib/bootm.c | 4 ++-- boot/image-board.c | 2 +- boot/image-fdt.c | 3 ++- cmd/bootefi.c | 2 +- include/image.h | 3 +-- lib/efi_loader/efi_dt_fixup.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c index d6d2f7d9d031..f1cff691f4fe 100644 --- a/arch/mips/lib/bootm.c +++ b/arch/mips/lib/bootm.c @@ -246,8 +246,8 @@ static int boot_setup_fdt(struct bootm_headers *images) { images->initrd_start = virt_to_phys((void *)images->initrd_start); images->initrd_end = virt_to_phys((void *)images->initrd_end); - return image_setup_libfdt(images, images->ft_addr, images->ft_len, - &images->lmb); + + return image_setup_libfdt(images, images->ft_addr, &images->lmb); }
static void boot_prep_linux(struct bootm_headers *images) diff --git a/boot/image-board.c b/boot/image-board.c index d500da1b4b91..893eaf935137 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -959,7 +959,7 @@ int image_setup_linux(struct bootm_headers *images) }
if (CONFIG_IS_ENABLED(OF_LIBFDT) && of_size) { - ret = image_setup_libfdt(images, *of_flat_tree, of_size, lmb); + ret = image_setup_libfdt(images, *of_flat_tree, lmb); if (ret) return ret; } diff --git a/boot/image-fdt.c b/boot/image-fdt.c index f10200f64743..54d219da7da1 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -604,12 +604,13 @@ __weak int arch_fixup_fdt(void *blob) }
int image_setup_libfdt(struct bootm_headers *images, void *blob, - int of_size, struct lmb *lmb) + struct lmb *lmb) { ulong *initrd_start = &images->initrd_start; ulong *initrd_end = &images->initrd_end; int ret = -EPERM; int fdt_ret; + int of_size;
if (fdt_root(blob) < 0) { printf("ERROR: root node setup failed\n"); diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 20e5c94a33a4..2809f599011f 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -309,7 +309,7 @@ efi_status_t efi_install_fdt(void *fdt) return EFI_OUT_OF_RESOURCES; }
- if (image_setup_libfdt(&img, fdt, 0, NULL)) { + if (image_setup_libfdt(&img, fdt, NULL)) { log_err("ERROR: failed to process device tree\n"); return EFI_LOAD_ERROR; } diff --git a/include/image.h b/include/image.h index 2e3cf839ee36..c7ec83b4e638 100644 --- a/include/image.h +++ b/include/image.h @@ -953,12 +953,11 @@ int image_decomp(int comp, ulong load, ulong image_start, int type, * * @images: Images information * @blob: FDT to update - * @of_size: Size of the FDT * @lmb: Points to logical memory block structure * Return: 0 if ok, <0 on failure */ int image_setup_libfdt(struct bootm_headers *images, void *blob, - int of_size, struct lmb *lmb); + struct lmb *lmb);
/** * Set up the FDT to use for booting a kernel diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c index 838023c78ff7..a0c889cf9867 100644 --- a/lib/efi_loader/efi_dt_fixup.c +++ b/lib/efi_loader/efi_dt_fixup.c @@ -173,7 +173,7 @@ efi_dt_fixup(struct efi_dt_fixup_protocol *this, void *dtb, }
fdt_set_totalsize(dtb, *buffer_size); - if (image_setup_libfdt(&img, dtb, 0, NULL)) { + if (image_setup_libfdt(&img, dtb, NULL)) { log_err("failed to process device tree\n"); ret = EFI_INVALID_PARAMETER; goto out;
On Sun, Nov 12, 2023 at 08:27:44AM -0700, Simon Glass wrote:
The of_size parameter is not used, so remove it.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
On Sun, Nov 12, 2023 at 08:27:44AM -0700, Simon Glass wrote:
The of_size parameter is not used, so remove it.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
Check the header before starting to use it, since this could provide very confusing later, when ofnode calls start to fail.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/ofnode.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index c3d326831fc6..3956e1999c36 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -83,6 +83,11 @@ static oftree oftree_ensure(void *fdt) if (check_tree_count()) return oftree_null();
+ if (fdt_check_header(fdt)) { + log_err("Invalid device tree blob header\n"); + return oftree_null(); + } + /* register the new tree */ i = oftree_count++; oftree_list[i] = fdt;
Add a bit more detail about what this function does.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/fdt_support.c | 1 - include/fdt_support.h | 12 +++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 09ce5828659e..510ad24a999d 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -722,7 +722,6 @@ int fdt_record_loadable(void *blob, u32 index, const char *name, return node; }
-/* Resize the fdt to its actual size + a bit of padding */ int fdt_shrink_to_minimum(void *blob, uint extrasize) { int i; diff --git a/include/fdt_support.h b/include/fdt_support.h index d967118bedf0..791258ec7eff 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -242,13 +242,23 @@ int ft_system_setup(void *blob, struct bd_info *bd); void set_working_fdt_addr(ulong addr);
/** - * shrink down the given blob to minimum size + some extrasize if required + * fdt_shrink_to_minimum() - shrink FDT while allowing for some margin + * + * Shrink down the given blob to 'minimum' size + some extrasize. + * + * The new size is enough to hold the existing contents plus @extrasize bytes, + * plus 5 memory reservations. Also, the end of the FDT is aligned to a 4KB + * boundary, so it might end up up to 4KB larger than needed. + * + * If there is an existing memory reservation for @blob in the FDT, it is + * updated for the new size. * * @param blob FDT blob to update * @param extrasize additional bytes needed * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_shrink_to_minimum(void *blob, uint extrasize); + int fdt_increase_size(void *fdt, int add_len);
int fdt_delete_disabled_nodes(void *blob);
This code dates from around 2008:
56844a22b76 powerpc: Fix bootm to boot up again with a Ramdisk
Since then we have added FDT relocation which provides enough space for expansion. We have also added all sorts of fixups earlier in image_setup_libfdt() which require more space, with ramdisk being the least of them.
Therefore this extra hack for ramdisk seems unnecessary. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fdt.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 54d219da7da1..408369d489ca 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -24,9 +24,6 @@ #include <dm/ofnode.h> #include <tee/optee.h>
-/* adding a ramdisk needs 0x44 bytes in version 2008.10 */ -#define FDT_RAMDISK_OVERHEAD 0x80 - DECLARE_GLOBAL_DATA_PTR;
static void fdt_error(const char *msg) @@ -692,10 +689,6 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, goto err; of_size = ret;
- if (*initrd_start && *initrd_end) { - of_size += FDT_RAMDISK_OVERHEAD; - fdt_set_totalsize(blob, of_size); - } /* Create a new LMB reservation */ if (lmb) lmb_reserve(lmb, (ulong)blob, of_size);
On Sun, Nov 12, 2023 at 08:27:47AM -0700, Simon Glass wrote:
This code dates from around 2008:
56844a22b76 powerpc: Fix bootm to boot up again with a Ramdisk
Since then we have added FDT relocation which provides enough space for expansion. We have also added all sorts of fixups earlier in image_setup_libfdt() which require more space, with ramdisk being the least of them.
Therefore this extra hack for ramdisk seems unnecessary. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
On Sun, Nov 12, 2023 at 08:27:47AM -0700, Simon Glass wrote:
This code dates from around 2008:
56844a22b76 powerpc: Fix bootm to boot up again with a Ramdisk
Since then we have added FDT relocation which provides enough space for expansion. We have also added all sorts of fixups earlier in image_setup_libfdt() which require more space, with ramdisk being the least of them.
Therefore this extra hack for ramdisk seems unnecessary. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
This may as well happen before the general event is emitted, so move it. This will allow us to use the livetree for the event part, but the flattree for the earlier part.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fdt.c | 5 ++++- include/fdt_support.h | 12 +++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 408369d489ca..ac7dc055cc9c 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -664,6 +664,10 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, goto err; } } + + if (fdt_initrd(blob, *initrd_start, *initrd_end)) + goto err; + if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) { struct event_ft_fixup fixup;
@@ -693,7 +697,6 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, if (lmb) lmb_reserve(lmb, (ulong)blob, of_size);
- fdt_initrd(blob, *initrd_start, *initrd_end); if (!ft_verify_fdt(blob)) goto err;
diff --git a/include/fdt_support.h b/include/fdt_support.h index 791258ec7eff..bc249b045302 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -57,7 +57,17 @@ int fdt_chosen(void *fdt); /** * Add initrd information to the FDT before booting the OS. * - * @param fdt FDT address in memory + * Adds linux,initrd-start and linux,initrd-end properties to the /chosen node, + * creating it if necessary. + * + * A memory reservation for the ramdisk is added to the FDT, or an existing one + * (with matching @initrd_start) updated. + * + * If @initrd_start == @initrd_end this function does nothing and returns 0. + * + * @fdt: Pointer to FDT in memory + * @initrd_start: Start of ramdisk + * @initrd_end: End of ramdisk * Return: 0 if ok, or -FDT_ERR_... on error */ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
This may as well happen before the general event is emitted, so move it. This will allow us to use the livetree for the event part, but the flattree for the earlier part.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fdt.c | 5 ++++- include/fdt_support.h | 12 +++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-)
Applied to u-boot-dm/next, thanks!
Just use map_to_sysmem() instead of all the casting.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fdt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c index ac7dc055cc9c..3e889be9f2b6 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -685,8 +685,7 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
/* Delete the old LMB reservation */ if (lmb) - lmb_free(lmb, (phys_addr_t)(u32)(uintptr_t)blob, - (phys_size_t)fdt_totalsize(blob)); + lmb_free(lmb, map_to_sysmem(blob), fdt_totalsize(blob));
ret = fdt_shrink_to_minimum(blob, 0); if (ret < 0) @@ -695,7 +694,7 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
/* Create a new LMB reservation */ if (lmb) - lmb_reserve(lmb, (ulong)blob, of_size); + lmb_reserve(lmb, map_to_sysmem(blob), of_size);
if (!ft_verify_fdt(blob)) goto err;
Just use map_to_sysmem() instead of all the casting.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fdt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Applied to u-boot-dm/next, thanks!
Move this check before the FDT fixups so that we can use a livetree after this point.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fdt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 3e889be9f2b6..c2b6c5338385 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -668,6 +668,10 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, if (fdt_initrd(blob, *initrd_start, *initrd_end)) goto err;
+ if (!ft_verify_fdt(blob)) + goto err; + + /* after here we are using a livetree */ if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) { struct event_ft_fixup fixup;
@@ -696,9 +700,6 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, if (lmb) lmb_reserve(lmb, map_to_sysmem(blob), of_size);
- if (!ft_verify_fdt(blob)) - goto err; - #if defined(CONFIG_ARCH_KEYSTONE) if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) ft_board_setup_ex(blob, gd->bd);
participants (2)
-
Simon Glass -
Tom Rini