On Mon, Jan 26, 2026 at 06:42:48PM +0100, Ruslan Ruslichenko wrote:
> From: Ruslan Ruslichenko <[email protected]>
> 
> The patch adds few utility functions for parsing FDT nodes.
> The helpers are required for upcoming Hardware device tree
> feature.
> 
> Signed-off-by: Ruslan Ruslichenko <[email protected]>
> ---
>  include/system/device_tree.h |  30 ++++++
>  system/device_tree.c         | 203 +++++++++++++++++++++++++++++++++++
>  2 files changed, 233 insertions(+)
> 
> diff --git a/include/system/device_tree.h b/include/system/device_tree.h
> index f34b8b7ef9..458dbb74b4 100644
> --- a/include/system/device_tree.h
> +++ b/include/system/device_tree.h
> @@ -90,6 +90,13 @@ int qemu_fdt_setprop_string_array(void *fdt, const char 
> *node_path,
>  int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
>                               const char *property,
>                               const char *target_node_path);
> +
> +uint64_t qemu_fdt_getprop_sized_cell(void *fdt, const char *node_path,
> +                                     const char *property, int offset,
> +                                     int size, Error **errp);
> +const char *qemu_fdt_getprop_string(void *fdt, const char*node_path,
> +                                    const char *property, int cell,
> +                                    bool inherit, Error **errp);
>  /**
>   * qemu_fdt_getprop: retrieve the value of a given property
>   * @fdt: pointer to the device tree blob
> @@ -193,9 +200,32 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
>                                                  qdt_tmp);                 \
>      })
>  
> +typedef struct QEMUDevtreeProp {
> +    char *name;
> +    int len;
> +    void *value;
> +} QEMUDevtreeProp;
> +
> +/* node queries */
> +
> +char *qemu_devtree_get_node_name(void *fdt, const char *node_path);
> +int qemu_devtree_get_num_children(void *fdt, const char *node_path, int 
> depth);
> +char **qemu_devtree_get_children(void *fdt, const char *node_path, int 
> depth);
> +int qemu_devtree_num_props(void *fdt, const char *node_path);
> +QEMUDevtreeProp *qemu_devtree_get_props(void *fdt, const char *node_path);
>  
> +/* node getters */
> +
> +int qemu_devtree_get_node_by_name(void *fdt, char *node_path,
> +                                  const char *cmpname);
> +int qemu_devtree_get_node_by_phandle(void *fdt, char *node_path, int 
> phandle);
>  int qemu_devtree_getparent(void *fdt, char *node_path,
>                             const char *current);
> +int qemu_devtree_get_root_node(void *fdt, char *node_path);
> +
> +/* misc */
> +
> +int devtree_get_num_nodes(void *fdt);
>  
>  #define DT_PATH_LENGTH 1024
>  
> diff --git a/system/device_tree.c b/system/device_tree.c
> index 41bde0ba5a..7091a4928e 100644
> --- a/system/device_tree.c
> +++ b/system/device_tree.c
> @@ -451,6 +451,50 @@ const void *qemu_fdt_getprop(void *fdt, const char 
> *node_path,
>      return r;
>  }
>  
> +const char *qemu_fdt_getprop_string(void *fdt, const char*node_path,
> +                                    const char *property, int cell,
> +                                    bool inherit, Error **errp)
> +{
> +    int len;
> +    const void *prop;
> +    Error *err = NULL;
> +
> +    if (!errp) {
> +        errp = &err;
> +    }
> +
> +    prop = qemu_fdt_getprop(fdt, node_path, property, &len, inherit, errp);
> +    if (*errp) {
> +        return NULL;
> +    }
> +    while (cell) {

In DT context, "cell" refers specifically to a 32-bit integer value.
It's not the right word for which string in a stringlist you retrieve.

Also, most of this is re-implementing fdt_stringlist_get() from libfdt.

> +        void *term = memchr(prop, '\0', len);
> +        size_t diff;
> +
> +        if (!term) {
> +            error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
> +                      node_path, property, fdt_strerror(len));
> +            return NULL;
> +        }
> +        diff = term - prop + 1;
> +        len -= diff;
> +        assert(len >= 0);
> +        prop += diff;
> +        cell--;
> +    }
> +
> +    if (!len) {
> +        return NULL;
> +    }
> +
> +    if (!*(char *)prop) {
> +        error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
> +                  node_path, property, fdt_strerror(len));
> +        return NULL;
> +    }
> +    return prop;
> +}
> +
>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
>                                 const char *property, int offset,
>                                 bool inherit, Error **errp)
> @@ -471,6 +515,22 @@ uint32_t qemu_fdt_getprop_cell(void *fdt, const char 
> *node_path,
>      return be32_to_cpu(p[offset]);
>  }
>  
> +uint64_t qemu_fdt_getprop_sized_cell(void *fdt, const char *node_path,
> +                                     const char *property, int offset,
> +                                     int size, Error **errp)
> +{

Again, "cell" is 32-bits in DT context, you need a different word.

> +    uint64_t ret = 0;

Since ret is a u64, 'size' must be either 1 or 2.  So you might as
well just have a get_u64() helper (getprop_cell already does u32).

> +    for (; size; size--) {
> +        ret <<= 32;
> +        ret |= qemu_fdt_getprop_cell(fdt, node_path, property, offset++, 
> false,
> +                                     errp);
> +        if (errp && *errp) {
> +            return 0;
> +        }
> +    }
> +    return ret;
> +}
> +
>  uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
>  {
>      uint32_t r;
> @@ -638,6 +698,117 @@ out:
>      return ret;
>  }
>  
> +char *qemu_devtree_get_node_name(void *fdt, const char *node_path)
> +{
> +    const char *ret = fdt_get_name(fdt, fdt_path_offset(fdt, node_path), 
> NULL);

This does a full scan of the fdt, which may be unexpected.

> +    return ret ? strdup(ret) : NULL;
> +}
> +
> +int qemu_devtree_num_props(void *fdt, const char *node_path)
> +{
> +    int offset = fdt_path_offset(fdt, node_path);
> +    int ret = 0;
> +
> +    for (offset = fdt_first_property_offset(fdt, offset);
> +            offset != -FDT_ERR_NOTFOUND;
> +            offset = fdt_next_property_offset(fdt, offset)) {
> +        ret++;
> +    }
> +    return ret;
> +}
> +
> +QEMUDevtreeProp *qemu_devtree_get_props(void *fdt, const char *node_path)
> +{
> +    QEMUDevtreeProp *ret = g_new0(QEMUDevtreeProp,
> +                                    qemu_devtree_num_props(fdt, node_path) + 
> 1);
> +    int offset = fdt_path_offset(fdt, node_path);
> +    int i = 0;
> +
> +    for (offset = fdt_first_property_offset(fdt, offset);
> +            offset != -FDT_ERR_NOTFOUND;
> +            offset = fdt_next_property_offset(fdt, offset)) {
> +        const char *propname;
> +        const void *val = fdt_getprop_by_offset(fdt, offset, &propname,
> +                                                    &ret[i].len);
> +
> +        ret[i].name = g_strdup(propname);
> +        ret[i].value = g_memdup2(val, ret[i].len);
> +        i++;
> +    }
> +    return ret;

How does the caller free the structure?  Using g_free() will leak all
the copied names and values.

> +}
> +
> +static void qemu_devtree_children_info(void *fdt, const char *node_path,
> +        int depth, int *num, char **returned_paths) {
> +    int offset = fdt_path_offset(fdt, node_path);
> +    int root_depth = fdt_node_depth(fdt, offset);

"root" implies the root of the whole tree, not of the node where you
started.  Also, two full scans of the tree above.

> +    int cur_depth = root_depth;
> +
> +    if (num) {
> +        *num = 0;
> +    }
> +    for (;;) {
> +        offset = fdt_next_node(fdt, offset, &cur_depth);
> +        if (cur_depth <= root_depth) {
> +            break;
> +        }
> +        if (cur_depth <= root_depth + depth || depth == 0) {
> +            if (returned_paths) {
> +                returned_paths[*num] = g_malloc0(DT_PATH_LENGTH);

No test to avoid overrunning the length of the returned_paths[] array
here.

> +                fdt_get_path(fdt, offset, returned_paths[*num], 
> DT_PATH_LENGTH);

Another full tree scan for every entry here.

> +            }
> +            if (num) {
> +                (*num)++;
> +            }
> +        }
> +    }
> +}
> +
> +char **qemu_devtree_get_children(void *fdt, const char *node_path, int depth)
> +{
> +    int num_children = qemu_devtree_get_num_children(fdt, node_path, depth);
> +    char **ret = g_malloc0(sizeof(*ret) * num_children);
> +
> +    qemu_devtree_children_info(fdt, node_path, depth, &num_children, ret);
> +    return ret;
> +}
> +
> +int qemu_devtree_get_num_children(void *fdt, const char *node_path, int 
> depth)
> +{
> +    int ret;
> +
> +    qemu_devtree_children_info(fdt, node_path, depth, &ret, NULL);
> +    return ret;
> +}
> +
> +int qemu_devtree_get_node_by_name(void *fdt, char *node_path,
> +        const char *cmpname) {

The semantics of this make no sense to me.  It finds the first node
with the name after the given node, whether it's a child, sibling or
unrelated node.   I'm guessing you want something based on
fdt_subnode_offset() instead.

> +    int offset = 0;
> +    char *name = NULL;
> +
> +    do {
> +        char *at;
> +
> +        offset = fdt_next_node(fdt, offset, NULL);

No test for error here, before using 'offset'.

> +        name = (void *)fdt_get_name(fdt, offset, NULL);
> +        if (!name) {
> +            continue;

The only errors from fdt_get_name() indicate either a corrupted DT, or
bad parameters.  continuing at this point will not do anything useful.

> +        }
> +        at = memchr(name, '@', strlen(name));
> +        if (!strncmp(name, cmpname, at ? at - name : strlen(name))) {
> +            break;
> +        }
> +    } while (offset > 0);
> +    return offset > 0 ?
> +        fdt_get_path(fdt, offset, node_path, DT_PATH_LENGTH) : 1;
> +}
> +
> +int qemu_devtree_get_node_by_phandle(void *fdt, char *node_path, int phandle)
> +{
> +    return fdt_get_path(fdt, fdt_node_offset_by_phandle(fdt, phandle),
> +                            node_path, DT_PATH_LENGTH);

Two full tree scans.

> +}
> +
>  int qemu_devtree_getparent(void *fdt, char *node_path, const char *current)
>  {
>      int offset = fdt_path_offset(fdt, current);
> @@ -648,6 +819,38 @@ int qemu_devtree_getparent(void *fdt, char *node_path, 
> const char *current)
>          fdt_get_path(fdt, parent_offset, node_path, DT_PATH_LENGTH) : 1;
>  }
>  
> +int qemu_devtree_get_root_node(void *fdt, char *node_path)
> +{
> +    return fdt_get_path(fdt, 0, node_path, DT_PATH_LENGTH);

The root node's path is "/" by definition, you don't need this
function.

> +}
> +
> +static void devtree_scan(void *fdt, int *num_nodes)

"scan" is vague.  The name should reflect what this actually does.

> +{
> +    int depth = 0, offset = 0;
> +
> +    if (num_nodes) {
> +        *num_nodes = 0;
> +    }
> +    for (;;) {
> +        offset = fdt_next_node(fdt, offset, &depth);
> +        if (num_nodes) {
> +            (*num_nodes)++;
> +        }
> +        if (offset <= 0 || depth <= 0) {
> +            break;
> +        }
> +    }
> +}
> +
> +int devtree_get_num_nodes(void *fdt)
> +{
> +    int ret;
> +
> +    devtree_scan(fdt, &ret);
> +    return ret;
> +}
> +
> +
>  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

Attachment: signature.asc
Description: PGP signature

Reply via email to