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

[David Gibson <david@gibson.dropbear.id.au>]

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

On Tue, Aug 16, 2022 at 02:34:28PM -0300, Daniel Henrique Barboza 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.
> 
> Add a new optional 'propname' parameter to x-query-fdt to specify the
> property of a given node. If it's present, we'll proceed to find the
> node as usual but, instead of printing the node, we'll attempt to find
> the property and print it standalone.
> 
> 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
> /cpus/cpu-map/socket0/cluster0/core0/cpu = <0x8001>
> 
> 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
> /vdevice/v-scsi@71000003/ibm,my-dma-window = <0x71000003 0x0 0x0 0x0 0x10000000>

nit: dts property definitions also include a terminating ';'
	prop = "foobar";

> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hmp-commands-info.hx         |  9 +++++----
>  include/sysemu/device_tree.h |  2 ++
>  monitor/hmp-cmds.c           |  5 ++++-
>  monitor/qmp-cmds.c           |  8 +++++---
>  qapi/machine.json            |  4 +++-
>  softmmu/device_tree.c        | 29 ++++++++++++++++++++++++-----
>  6 files changed, 43 insertions(+), 14 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 743b48865d..17d6ee4d30 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -924,13 +924,14 @@ ERST
>  
>      {
>          .name       = "fdt",
> -        .args_type  = "nodepath:s",
> -        .params     = "nodepath",
> -        .help       = "show firmware device tree node given its path",
> +        .args_type  = "nodepath:s,propname:s?",
> +        .params     = "nodepath [propname]",
> +        .help       = "show firmware device tree node or property given its path",
>          .cmd        = hmp_info_fdt,
>      },
>  
>  SRST
>    ``info fdt``
> -    Show a firmware device tree node given its path. Requires libfdt.
> +    Show a firmware device tree node or property given its path.
> +    Requires libfdt.
>  ERST
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index 057d13e397..551a02dee2 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -140,6 +140,8 @@ int qemu_fdt_add_path(void *fdt, const char *path);
>  void qemu_fdt_dumpdtb(void *fdt, int size);
>  void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp);
>  HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath,
> +                                          bool has_propname,
> +                                          const char *propname,
>                                            Error **errp);
>  
>  /**
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index accde90380..df8493adc5 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -2488,8 +2488,11 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
>  void hmp_info_fdt(Monitor *mon, const QDict *qdict)
>  {
>      const char *nodepath = qdict_get_str(qdict, "nodepath");
> +    const char *propname = qdict_get_try_str(qdict, "propname");
>      Error *err = NULL;
> -    g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err);
> +    g_autoptr(HumanReadableText) info = NULL;
> +
> +    info = qmp_x_query_fdt(nodepath, propname != NULL, propname, &err);
>  
>      if (hmp_handle_error(mon, err)) {
>          return;
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index db2c6aa7da..ca2a96cdf7 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -604,9 +604,10 @@ void qmp_dumpdtb(const char *filename, Error **errp)
>      return qemu_fdt_qmp_dumpdtb(filename, errp);
>  }
>  
> -HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp)
> +HumanReadableText *qmp_x_query_fdt(const char *nodepath, bool has_propname,
> +                                   const char *propname, Error **errp)
>  {
> -    return qemu_fdt_qmp_query_fdt(nodepath, errp);
> +    return qemu_fdt_qmp_query_fdt(nodepath, has_propname, propname, errp);
>  }
>  #else
>  void qmp_dumpdtb(const char *filename, Error **errp)
> @@ -614,7 +615,8 @@ void qmp_dumpdtb(const char *filename, Error **errp)
>      error_setg(errp, "dumpdtb requires libfdt");
>  }
>  
> -HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp)
> +HumanReadableText *qmp_x_query_fdt(const char *nodepath, bool has_propname,
> +                                   const char *propname, Error **errp)
>  {
>      error_setg(errp, "this command requires libfdt");
>  
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 96cff541ca..c15ce60f46 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1688,6 +1688,7 @@
>  # Query for FDT element (node or property). Requires 'libfdt'.
>  #
>  # @nodepath: the path of the FDT node to be retrieved
> +# @propname: name of the property inside the node
>  #
>  # Features:
>  # @unstable: This command is meant for debugging.
> @@ -1697,6 +1698,7 @@
>  # Since: 7.2
>  ##
>  { 'command': 'x-query-fdt',
> -  'data': { 'nodepath': 'str' },
> +  'data': { 'nodepath': 'str',
> +            '*propname': 'str' },
>    'returns': 'HumanReadableText',
>    'features': [ 'unstable' ]  }
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 9e681739bd..523c9b8d4d 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -823,23 +823,42 @@ static void fdt_format_node(GString *buf, int node, int depth,
>      g_string_append_printf(buf, "%*s}\n", padding, "");
>  }
>  
> -HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp)
> +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath,
> +                                          bool has_propname,
> +                                          const char *propname,
> +                                          Error **errp)
>  {
>      g_autoptr(GString) buf = g_string_new("");
> -    int node;
> +    const struct fdt_property *prop = NULL;
> +    void *fdt = current_machine->fdt;
> +    int node, prop_size;
>  
> -    if (!current_machine->fdt) {
> +    if (!fdt) {
>          error_setg(errp, "Unable to find the machine FDT");
>          return NULL;
>      }
>  
> -    node = fdt_path_offset(current_machine->fdt, nodepath);
> +    node = fdt_path_offset(fdt, nodepath);
>      if (node < 0) {
>          error_setg(errp, "node '%s' not found in FDT", nodepath);
>          return NULL;
>      }
>  
> -    fdt_format_node(buf, node, 0, nodepath);
> +    if (!has_propname) {
> +        fdt_format_node(buf, node, 0, nodepath);
> +    } else {
> +        g_autofree char *proppath = g_strdup_printf("%s/%s", nodepath,
> +                                                    propname);
> +
> +        prop = fdt_get_property(fdt, node, propname, &prop_size);
> +        if (!prop) {
> +            error_setg(errp, "property '%s' not found in node '%s' in FDT",
> +                       propname, nodepath);
> +            return NULL;
> +        }
> +
> +        fdt_format_property(buf, proppath, prop->data, prop_size, 0);
> +    }
>  
>      return human_readable_text_from_str(buf);
>  }

-- 
David Gibson			| 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]