Re: [PATCH for-7.2 10/10] hmp, device_tree.c: add 'info fdt <property>' support

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Daniel Henrique Barboza (danielhb413@gmail.com) wrote:
> 'info fdt' is only able to print full nodes so far. It would be good to
> be able to also print single properties, since ometimes we just want
> to verify a single value from the FDT.
> 
> libfdt does not have support to find a property given its full path, but
> it does have a way to return a fdt_property given a prop name and its
> subnode.
> 
> This is how we're going to support it:
> 
> - given the same fullpath parameter, assume it's a node. If we have a
> match with an existing node, print it. If not, assume it's a property;
> 
> - in fdt_find_property() we're going to split 'fullpath' into node and
> property. Unfortunately we can't use g_path_get_basename() to helps us
> because, although the device tree path format is similar to Linux, it'll
> not work when trying to run QEMU under Windows where the path format is
> different;
> 
> - after spliiting into node + property, try to find the node in the FDT.
> If we have a match, use fdt_get_property() to retrieve fdt_property.
> Return it if found;
> 
> - using the fdt_print_property() created previously, print the property.

Would it be easier to make 'info fdt' have an optional 2nd parameter
(hmp can do optionals) which if present is the property name, then these
would become:

> After this change, if an user wants to print just the value of 'cpu' inside
> /cpu/cpu-map(...) from an ARM FDT, we can do it:
> 
> (qemu) info fdt /cpus/cpu-map/socket0/cluster0/core0/cpu

info fdt /cpus/cpu-map/socket0/cluster0/core0 cpu

> /cpus/cpu-map/socket0/cluster0/core0/cpu = <0x8001>
> (qemu)
> 
> Or the 'ibm,my-dma-window' from the v-scsi device inside the pSeries
> FDT:
> 
> (qemu) info fdt /vdevice/v-scsi@71000003/ibm,my-dma-window

info fdt /vdevice/v-scsi@71000003 ibm,my-dma-window

it seems more explicit, and seems easier to implement.

Dave

> /vdevice/v-scsi@71000003/ibm,my-dma-window = <0x71000003 0x0 0x0 0x0 0x10000000>
> (qemu)
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hmp-commands-info.hx  |  2 +-
>  softmmu/device_tree.c | 79 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 76 insertions(+), 5 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index abf277be7d..8891c2918a 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -913,7 +913,7 @@ ERST
>          .name       = "fdt",
>          .args_type  = "fullpath:s",
>          .params     = "fullpath",
> -        .help       = "show firmware device tree node given its full path",
> +        .help       = "show firmware device tree node or property given its full path",
>          .cmd        = hmp_info_fdt,
>      },
>  
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index e41894fbef..f6eb060acc 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -774,9 +774,74 @@ static void fdt_print_node(int node, int depth, const char *fullpath)
>      qemu_printf("%*s}\n", padding, "");
>  }
>  
> +static const struct fdt_property *fdt_find_property(const char *fullpath,
> +                                                    int *prop_size,
> +                                                    Error **errp)
> +{
> +    const struct fdt_property *prop = NULL;
> +    void *fdt = current_machine->fdt;
> +    g_autoptr(GString) nodename = NULL;
> +    const char *propname = NULL;
> +    int path_len = strlen(fullpath);
> +    int node = 0; /* default to root node '/' */
> +    int i, idx = -1;
> +
> +    /*
> +     * We'll assume that we're dealing with a property. libfdt
> +     * does not have an API to find a property given the full
> +     * path, but it does have an API to find a property inside
> +     * a node.
> +     */
> +    nodename = g_string_new("");
> +
> +    for (i = path_len - 1; i >= 0; i--) {
> +        if (fullpath[i] == '/') {
> +            idx = i;
> +            break;
> +        }
> +    }
> +
> +    if (idx == -1) {
> +        error_setg(errp, "FDT paths must contain at least one '/' character");
> +        return NULL;
> +    }
> +
> +    if (idx == path_len - 1) {
> +        error_setg(errp, "FDT paths can't end with a '/' character");
> +        return NULL;
> +    }
> +
> +    propname = &fullpath[idx + 1];
> +
> +    if (idx != 0) {
> +        g_string_append_len(nodename, fullpath, idx);
> +
> +        node = fdt_path_offset(fdt, nodename->str);
> +        if (node < 0) {
> +            error_setg(errp, "node '%s' of property '%s' not found in FDT",
> +                       nodename->str, propname);
> +            return NULL;
> +        }
> +    } else {
> +        /* idx = 0 means that it's a property of the root node */
> +        g_string_append(nodename, "/");
> +    }
> +
> +    prop = fdt_get_property(fdt, node, propname, prop_size);
> +    if (!prop) {
> +        error_setg(errp, "property '%s' not found in node '%s' in FDT",
> +                   propname, nodename->str);
> +        return NULL;
> +    }
> +
> +    return prop;
> +}
> +
>  void fdt_info(const char *fullpath, Error **errp)
>  {
> -    int node;
> +    const struct fdt_property *prop = NULL;
> +    Error *local_err = NULL;
> +    int node, prop_size;
>  
>      if (!current_machine->fdt) {
>          error_setg(errp, "Unable to find the machine FDT");
> @@ -784,10 +849,16 @@ void fdt_info(const char *fullpath, Error **errp)
>      }
>  
>      node = fdt_path_offset(current_machine->fdt, fullpath);
> -    if (node < 0) {
> -        error_setg(errp, "node '%s' not found in FDT", fullpath);
> +    if (node >= 0) {
> +        fdt_print_node(node, 0, fullpath);
> +        return;
> +    }
> +
> +    prop = fdt_find_property(fullpath, &prop_size, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
>          return;
>      }
>  
> -    fdt_print_node(node, 0, fullpath);
> +    fdt_print_property(fullpath, prop->data, prop_size, 0);
>  }
> -- 
> 2.36.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK