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

2022-08-19 Thread Daniel Henrique Barboza




On 8/17/22 22:34, David Gibson wrote:

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@7103 ibm,my-dma-window
/vdevice/v-scsi@7103/ibm,my-dma-window = <0x7103 0x0 0x0 0x0 0x1000>


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



I forgot to update the commit msg. The code is already putting
the semi-colon at the end:

(qemu) info fdt /vdevice/v-scsi@7103 ibm,my-dma-window
/vdevice/v-scsi@7103/ibm,my-dma-window = <0x7103 0x0 0x0 0x0 
0x1000>;
(qemu)


I'll update it in v4.

Thanks,

Daniel




Cc: Dr. David Alan Gilbert 
Acked-by: Dr. David Alan Gilbert 
Signed-off-by: Daniel Henrique Barboza 
---
  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, );
+g_autoptr(HumanReadableText) info = NULL;
+
+info = qmp_x_query_fdt(nodepath, propname != NULL, propname, );
  
  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 

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

2022-08-18 Thread David Gibson
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@7103 ibm,my-dma-window
> /vdevice/v-scsi@7103/ibm,my-dma-window = <0x7103 0x0 0x0 0x0 
> 0x1000>

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

> Cc: Dr. David Alan Gilbert 
> Acked-by: Dr. David Alan Gilbert 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  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, );
> +g_autoptr(HumanReadableText) info = NULL;
> +
> +info = qmp_x_query_fdt(nodepath, propname != NULL, propname, );
>  
>  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 

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

2022-08-16 Thread Daniel Henrique Barboza
'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@7103 ibm,my-dma-window
/vdevice/v-scsi@7103/ibm,my-dma-window = <0x7103 0x0 0x0 0x0 0x1000>

Cc: Dr. David Alan Gilbert 
Acked-by: Dr. David Alan Gilbert 
Signed-off-by: Daniel Henrique Barboza 
---
 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, );
+g_autoptr(HumanReadableText) info = NULL;
+
+info = qmp_x_query_fdt(nodepath, propname != NULL, propname, );
 
 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