[PATCH 0/7] fdt: Move towards using ofnode for devicetree fixups

[PATCH 0/7] fdt: Move towards using ofnode for devicetree fixups

[Simon Glass]

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(-)

-- 
2.42.0.869.gea05f2083d-goog

[PATCH 1/7] boot: Drop size parameter from image_setup_libfdt()

[Simon Glass]

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;
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 2/7] fdt: Check for a valid fdt in oftree_ensure()

[Simon Glass]

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;
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 3/7] fdt: Improve the comment for fdt_shrink_to_minimum()

[Simon Glass]

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);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 4/7] fdt: ppc: Drop extra size for ramdisk

[Simon Glass]

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);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 5/7] boot: Move adding initrd earlier in image_setup_libfdt()

[Simon Glass]

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);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 6/7] fdt: Drop the confusing casts in lmb_free()

[Simon Glass]

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;
-- 
2.42.0.869.gea05f2083d-goog

[PATCH 7/7] fdt: Move ft_verify_fdt() before the final fixups

[Simon Glass]

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);
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH 4/7] fdt: ppc: Drop extra size for ramdisk

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/7] boot: Drop size parameter from image_setup_libfdt()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 6/7] fdt: Drop the confusing casts in lmb_free()

[Simon Glass]

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!

Re: [PATCH 5/7] boot: Move adding initrd earlier in image_setup_libfdt()

[Simon Glass]

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!

Re: [PATCH 4/7] fdt: ppc: Drop extra size for ramdisk

[Simon Glass]

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>

-- 
Tom

Applied to u-boot-dm/next, thanks!

Re: [PATCH 1/7] boot: Drop size parameter from image_setup_libfdt()

[Simon Glass]

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>

-- 
Tom

Applied to u-boot-dm/next, thanks!