From: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>

Update 'qemu_fdt_getprop' and 'qemu_fdt_getprop_cell'
to support property inheritence from parent node,
in case 'inherit' argument is set.

Update 'qemu_fdt_getprop_cell' to allow accessing
specific cells within multi-cell property array.

Introduced 'qemu_devtreedd_getparent' as it is needed
by both internal and external users.

This will be used by hardware device tree parsing logic.

Signed-off-by: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
---
 hw/arm/boot.c                |  8 ++++----
 hw/arm/raspi4b.c             |  8 ++++----
 hw/arm/vexpress.c            |  4 ++--
 hw/arm/xlnx-zcu102.c         |  3 ++-
 include/system/device_tree.h | 32 +++++++++++++++++++-------------
 system/device_tree.c         | 33 ++++++++++++++++++++++++---------
 6 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index c97d4c4e11..829b8ba12f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -509,10 +509,10 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         return 0;
     }
 
-    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells",
-                                   NULL, &error_fatal);
-    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells",
-                                   NULL, &error_fatal);
+    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", 0,
+                                   false, &error_fatal);
+    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", 0,
+                                   false, &error_fatal);
     if (acells == 0 || scells == 0) {
         fprintf(stderr, "dtb file invalid (#address-cells or #size-cells 0)\n");
         goto fail;
diff --git a/hw/arm/raspi4b.c b/hw/arm/raspi4b.c
index 3eeb8f447e..66eba5d667 100644
--- a/hw/arm/raspi4b.c
+++ b/hw/arm/raspi4b.c
@@ -42,10 +42,10 @@ static void raspi_add_memory_node(void *fdt, hwaddr mem_base, hwaddr mem_len)
     uint32_t acells, scells;
     char *nodename = g_strdup_printf("/memory@%" PRIx64, mem_base);
 
-    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells",
-                                   NULL, &error_fatal);
-    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells",
-                                   NULL, &error_fatal);
+    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", 0,
+                                   false, &error_fatal);
+    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", 0,
+                                   false, &error_fatal);
     /* validated by arm_load_dtb */
     g_assert(acells && scells);
 
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index cc6ae7d4c4..823e316f91 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -486,9 +486,9 @@ static void vexpress_modify_dtb(const struct arm_boot_info *info, void *fdt)
     const VEDBoardInfo *daughterboard = (const VEDBoardInfo *)info;
 
     acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells",
-                                   NULL, &error_fatal);
+                                   0, false, &error_fatal);
     scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells",
-                                   NULL, &error_fatal);
+                                   0, false, &error_fatal);
     intc = find_int_controller(fdt);
     if (!intc) {
         /* Not fatal, we just won't provide virtio. This will
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 3ba2736bab..8f67c8be48 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -88,7 +88,8 @@ static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt)
                                        &error_fatal);
 
         for (i = 0; node_path && node_path[i]; i++) {
-            r = qemu_fdt_getprop(fdt, node_path[i], "method", &prop_len, NULL);
+            r = qemu_fdt_getprop(fdt, node_path[i], "method", &prop_len,
+                                 false, NULL);
             method_is_hvc = r && !strcmp("hvc", r);
 
             /* Allow HVC based firmware if EL2 is enabled.  */
diff --git a/include/system/device_tree.h b/include/system/device_tree.h
index 49d8482ed4..f34b8b7ef9 100644
--- a/include/system/device_tree.h
+++ b/include/system/device_tree.h
@@ -96,27 +96,28 @@ int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
  * @node_path: node path
  * @property: name of the property to find
  * @lenp: fdt error if any or length of the property on success
+ * @inherit: if not found in node, look for property in parent
  * @errp: handle to an error object
  *
  * returns a pointer to the property on success and NULL on failure
  */
 const void *qemu_fdt_getprop(void *fdt, const char *node_path,
                              const char *property, int *lenp,
-                             Error **errp);
+                             bool inherit, Error **errp);
 /**
- * qemu_fdt_getprop_cell: retrieve the value of a given 4 byte property
- * @fdt: pointer to the device tree blob
- * @node_path: node path
- * @property: name of the property to find
- * @lenp: fdt error if any or -EINVAL if the property size is different from
- *        4 bytes, or 4 (expected length of the property) upon success.
- * @errp: handle to an error object
- *
- * returns the property value on success
- */
+* qemu_fdt_getprop_cell: retrieve the value of a given 4 byte property
+* @fdt: pointer to the device tree blob
+* @node_path: node path
+* @property: name of the property to find
+* @ofset: the index of 32bit cell to retrive
+* @inherit: if not found in node, look for property in parent
+* @errp: handle to an error object
+*
+* returns the property value on success
+*/
 uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
-                               const char *property, int *lenp,
-                               Error **errp);
+                               const char *property, int offset,
+                               bool inherit, Error **errp);
 uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
 uint32_t qemu_fdt_alloc_phandle(void *fdt);
 int qemu_fdt_nop_node(void *fdt, const char *node_path);
@@ -193,6 +194,11 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
     })
 
 
+int qemu_devtree_getparent(void *fdt, char *node_path,
+                           const char *current);
+
+#define DT_PATH_LENGTH 1024
+
 /**
  * qemu_fdt_randomize_seeds:
  * @fdt: device tree blob
diff --git a/system/device_tree.c b/system/device_tree.c
index 1ea1962984..41bde0ba5a 100644
--- a/system/device_tree.c
+++ b/system/device_tree.c
@@ -429,7 +429,8 @@ int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
 }
 
 const void *qemu_fdt_getprop(void *fdt, const char *node_path,
-                             const char *property, int *lenp, Error **errp)
+                             const char *property, int *lenp,
+                             bool inherit, Error **errp)
 {
     int len;
     const void *r;
@@ -439,31 +440,35 @@ const void *qemu_fdt_getprop(void *fdt, const char *node_path,
     }
     r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);
     if (!r) {
+        char parent[DT_PATH_LENGTH];
+        if (inherit && !qemu_devtree_getparent(fdt, parent, node_path)) {
+            return qemu_fdt_getprop(fdt, parent, property, lenp, true, errp);
+        }
         error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
                   node_path, property, fdt_strerror(*lenp));
+        return NULL;
     }
     return r;
 }
 
 uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
-                               const char *property, int *lenp, Error **errp)
+                               const char *property, int offset,
+                               bool inherit, Error **errp)
 {
     int len;
     const uint32_t *p;
 
-    if (!lenp) {
-        lenp = &len;
-    }
-    p = qemu_fdt_getprop(fdt, node_path, property, lenp, errp);
+    p = qemu_fdt_getprop(fdt, node_path, property, &len,
+                                         inherit, errp);
     if (!p) {
         return 0;
-    } else if (*lenp != 4) {
+    }
+    if (len < (offset + 1) * 4) {
         error_setg(errp, "%s: %s/%s not 4 bytes long (not a cell?)",
                    __func__, node_path, property);
-        *lenp = -EINVAL;
         return 0;
     }
-    return be32_to_cpu(*p);
+    return be32_to_cpu(p[offset]);
 }
 
 uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
@@ -633,6 +638,16 @@ out:
     return ret;
 }
 
+int qemu_devtree_getparent(void *fdt, char *node_path, const char *current)
+{
+    int offset = fdt_path_offset(fdt, current);
+    int parent_offset = fdt_supernode_atdepth_offset(fdt, offset,
+        fdt_node_depth(fdt, offset) - 1, NULL);
+
+    return parent_offset >= 0 ?
+        fdt_get_path(fdt, parent_offset, node_path, DT_PATH_LENGTH) : 1;
+}
+
 void qmp_dumpdtb(const char *filename, Error **errp)
 {
     ERRP_GUARD();
-- 
2.43.0

On Mon, Jan 26, 2026 at 06:42:47PM +0100, Ruslan Ruslichenko wrote:
> From: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> 
> Update 'qemu_fdt_getprop' and 'qemu_fdt_getprop_cell'
> to support property inheritence from parent node,
> in case 'inherit' argument is set.

A bare bool parameter is pretty ugly and requires updating every
existing caller.  I'd suggest adding a new function for the inheriting
version, instead.

> Update 'qemu_fdt_getprop_cell' to allow accessing
> specific cells within multi-cell property array.
> 
> Introduced 'qemu_devtreedd_getparent' as it is needed
> by both internal and external users.
> 
> This will be used by hardware device tree parsing logic.
> 
> Signed-off-by: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> ---
>  hw/arm/boot.c                |  8 ++++----
>  hw/arm/raspi4b.c             |  8 ++++----
>  hw/arm/vexpress.c            |  4 ++--
>  hw/arm/xlnx-zcu102.c         |  3 ++-
>  include/system/device_tree.h | 32 +++++++++++++++++++-------------
>  system/device_tree.c         | 33 ++++++++++++++++++++++++---------
>  6 files changed, 55 insertions(+), 33 deletions(-)
> 
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index c97d4c4e11..829b8ba12f 100644
[snip]
> diff --git a/include/system/device_tree.h b/include/system/device_tree.h
> index 49d8482ed4..f34b8b7ef9 100644
> --- a/include/system/device_tree.h
> +++ b/include/system/device_tree.h
> @@ -96,27 +96,28 @@ int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
>   * @node_path: node path
>   * @property: name of the property to find
>   * @lenp: fdt error if any or length of the property on success
> + * @inherit: if not found in node, look for property in parent
>   * @errp: handle to an error object
>   *
>   * returns a pointer to the property on success and NULL on failure
>   */
>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,
>                               const char *property, int *lenp,
> -                             Error **errp);
> +                             bool inherit, Error **errp);
>  /**
> - * qemu_fdt_getprop_cell: retrieve the value of a given 4 byte property
> - * @fdt: pointer to the device tree blob
> - * @node_path: node path
> - * @property: name of the property to find
> - * @lenp: fdt error if any or -EINVAL if the property size is different from
> - *        4 bytes, or 4 (expected length of the property) upon success.
> - * @errp: handle to an error object
> - *
> - * returns the property value on success
> - */

Spurious change to all the whitespace here.

> +* qemu_fdt_getprop_cell: retrieve the value of a given 4 byte property

This description is no longer accurate after your change.

> +* @fdt: pointer to the device tree blob
> +* @node_path: node path
> +* @property: name of the property to find
> +* @ofset: the index of 32bit cell to retrive

s/ofset/offset/.  But, in any case in libfdt context 'offset' always
refers to a byte offset, so I think this is a potentially misleading
name.

> +* @inherit: if not found in node, look for property in parent
> +* @errp: handle to an error object
> +*
> +* returns the property value on success
> +*/
>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
> -                               const char *property, int *lenp,
> -                               Error **errp);
> +                               const char *property, int offset,
> +                               bool inherit, Error **errp);
>  uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
>  uint32_t qemu_fdt_alloc_phandle(void *fdt);
>  int qemu_fdt_nop_node(void *fdt, const char *node_path);
> @@ -193,6 +194,11 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
>      })
>  
>  
> +int qemu_devtree_getparent(void *fdt, char *node_path,
> +                           const char *current);
> +
> +#define DT_PATH_LENGTH 1024
> +
>  /**
>   * qemu_fdt_randomize_seeds:
>   * @fdt: device tree blob
> diff --git a/system/device_tree.c b/system/device_tree.c
> index 1ea1962984..41bde0ba5a 100644
> --- a/system/device_tree.c
> +++ b/system/device_tree.c
> @@ -429,7 +429,8 @@ int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
>  }
>  
>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,
> -                             const char *property, int *lenp, Error **errp)
> +                             const char *property, int *lenp,
> +                             bool inherit, Error **errp)
>  {
>      int len;
>      const void *r;
> @@ -439,31 +440,35 @@ const void *qemu_fdt_getprop(void *fdt, const char *node_path,
>      }
>      r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);
>      if (!r) {
> +        char parent[DT_PATH_LENGTH];
> +        if (inherit && !qemu_devtree_getparent(fdt, parent, node_path)) {
> +            return qemu_fdt_getprop(fdt, parent, property, lenp, true, errp);
> +        }

This is a pretty horribly expensive way of doing it.  On a flattened
tree, findnode_nofail() and devtree_getparent() each need to scan the
tree from the start, and you do both for each layer of the tree.  You
could instead scan the tree once from the root, updating a pointer to
the deepest copy of the property found so far.

>          error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
>                    node_path, property, fdt_strerror(*lenp));
> +        return NULL;
>      }
>      return r;
>  }
>  
>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
> -                               const char *property, int *lenp, Error **errp)
> +                               const char *property, int offset,
> +                               bool inherit, Error **errp)
>  {
>      int len;
>      const uint32_t *p;
>  
> -    if (!lenp) {
> -        lenp = &len;
> -    }
> -    p = qemu_fdt_getprop(fdt, node_path, property, lenp, errp);
> +    p = qemu_fdt_getprop(fdt, node_path, property, &len,
> +                                         inherit, errp);
>      if (!p) {
>          return 0;
> -    } else if (*lenp != 4) {
> +    }
> +    if (len < (offset + 1) * 4) {
>          error_setg(errp, "%s: %s/%s not 4 bytes long (not a cell?)",
>                     __func__, node_path, property);

Error message is no longer accurate.

> -        *lenp = -EINVAL;
>          return 0;
>      }
> -    return be32_to_cpu(*p);
> +    return be32_to_cpu(p[offset]);
>  }
>  
>  uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
> @@ -633,6 +638,16 @@ out:
>      return ret;
>  }
>  
> +int qemu_devtree_getparent(void *fdt, char *node_path, const char *current)

This inherently needs to scan the tree from the beginning at least
once, but the implementation is much worse than that.

> +{
> +    int offset = fdt_path_offset(fdt, current);

This scans the tree from the start.

> +    int parent_offset = fdt_supernode_atdepth_offset(fdt, offset,
> +        fdt_node_depth(fdt, offset) - 1, NULL);

This does it twice more (one for fdt_node_depth() and one for
fdt_supernode_atdepth_offset()).

> +
> +    return parent_offset >= 0 ?
> +        fdt_get_path(fdt, parent_offset, node_path, DT_PATH_LENGTH) : 1;

This does it a fourth time.

Since you're starting from a path, not an offset, you could instead
chop off the last path component, then do a single fdt_get_path().

> +}
> +
>  void qmp_dumpdtb(const char *filename, Error **errp)
>  {
>      ERRP_GUARD();
> -- 
> 2.43.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson