[PATCH for-7.2 v3 00/20] QMP/HMP: add 'dumpdtb' and 'info fdt' commands

[PATCH for-7.2 v3 00/20] QMP/HMP: add 'dumpdtb' and 'info fdt' commands

[Daniel Henrique Barboza]

Hi,

In this new version the most notable changes are:

- removed fdt_pack() from machine specific code. As discussed in the previous
version, the proper use of fdt_pack() would require more work/thought and,
since it's not required for the work we're doing here, it was removed;

- we're now handling string arrays. The previous version was interpreting
all string properties as a single, plain string. We're now dealing with string
arrays instead;

- changed the output format to be more in line with the dts format.

Other small changes were made based on the feeback of the previous version.

Changes from v2:
- patches 1-8:
  - remove fdt_pack() to shrink the FDT before assigning it to ms->fdt
- patch 9:
  - call g_free(ms->fdt) to avoid leaking an old fdt during reset
- patch 10:
  - added a commit msg note about why we're not eliminating spapr->fdt_blob
for machine->fdt at this moment
- patches 11, 12:
  - remove fdt_pack() to shrink the FDT before assigning it to ms->fdt
  - added Alistair's r-b
- patch 13:
  - remove fdt_pack() to shrink the FDT before assigning it to ms->fdt
- patch 14:
  - added a commit msg note about BQL
- patch 15:
  - added a commit msg note about BQL
- patch 16:
  - renamed fdt_prop_is_string to fdt_prop_is_string_array. \0 characters
in the middle of the data array is now legal
  - added a new fdt_prop_format_string_array() to format the string array
  - added a semicolon at the end of the string array
- patch 17:
  - added semicolon at the end of properties
  - use %02x instead of %x to format vals in [] notation

- v2 link: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00937.html


Daniel Henrique Barboza (20):
  hw/arm: do not free machine->fdt in arm_load_dtb()
  hw/microblaze: set machine->fdt in microblaze_load_dtb()
  hw/nios2: set machine->fdt in nios2_load_dtb()
  hw/ppc: set machine->fdt in ppce500_load_device_tree()
  hw/ppc: set machine->fdt in bamboo_load_device_tree()
  hw/ppc: set machine->fdt in sam460ex_load_device_tree()
  hw/ppc: set machine->fdt in xilinx_load_device_tree()
  hw/ppc: set machine->fdt in pegasos2_machine_reset()
  hw/ppc: set machine->fdt in pnv_reset()
  hw/ppc: set machine->fdt in spapr machine
  hw/riscv: set machine->fdt in sifive_u_machine_init()
  hw/riscv: set machine->fdt in spike_board_init()
  hw/xtensa: set machine->fdt in xtfpga_init()
  qmp/hmp, device_tree.c: introduce dumpdtb
  qmp/hmp, device_tree.c: introduce 'info fdt' command
  device_tree.c: support string array prop in fdt_format_node()
  device_tree.c: support remaining FDT prop types
  device_node.c: enable 'info fdt' to print subnodes
  device_tree.c: add fdt_format_property() helper
  hmp, device_tree.c: add 'info fdt <property>' support

 hmp-commands-info.hx         |  14 +++
 hmp-commands.hx              |  13 +++
 hw/arm/boot.c                |   6 +-
 hw/microblaze/boot.c         |  11 +-
 hw/microblaze/meson.build    |   2 +-
 hw/nios2/boot.c              |  11 +-
 hw/nios2/meson.build         |   2 +-
 hw/ppc/e500.c                |  13 ++-
 hw/ppc/pegasos2.c            |   7 ++
 hw/ppc/pnv.c                 |   8 +-
 hw/ppc/ppc440_bamboo.c       |  11 +-
 hw/ppc/sam460ex.c            |   8 +-
 hw/ppc/spapr.c               |   6 +
 hw/ppc/spapr_hcall.c         |   8 ++
 hw/ppc/virtex_ml507.c        |  11 +-
 hw/riscv/sifive_u.c          |   6 +
 hw/riscv/spike.c             |   9 ++
 hw/xtensa/meson.build        |   2 +-
 hw/xtensa/xtfpga.c           |   9 +-
 include/monitor/hmp.h        |   2 +
 include/sysemu/device_tree.h |   7 ++
 monitor/hmp-cmds.c           |  28 +++++
 monitor/qmp-cmds.c           |  27 +++++
 qapi/machine.json            |  38 ++++++
 softmmu/device_tree.c        | 219 +++++++++++++++++++++++++++++++++++
 25 files changed, 466 insertions(+), 12 deletions(-)

-- 
2.37.2

[PATCH for-7.2 v3 01/20] hw/arm: do not free machine->fdt in arm_load_dtb()

[Daniel Henrique Barboza]

At this moment, arm_load_dtb() can free machine->fdt when
binfo->dtb_filename is NULL. If there's no 'dtb_filename', 'fdt' will be
retrieved by binfo->get_dtb(). If get_dtb() returns machine->fdt, as is
the case of machvirt_dtb() from hw/arm/virt.c, fdt now has a pointer to
machine->fdt. And, in that case, the existing g_free(fdt) at the end of
arm_load_dtb() will make machine->fdt point to an invalid memory region.

This is not an issue right now because there's no code that access
machine->fdt after arm_load_dtb(), but we're going to add a couple do
FDT HMP commands that will rely on machine->fdt being valid.

Instead of freeing 'fdt' at the end of arm_load_dtb(), assign it to
machine->fdt. This will allow the FDT of ARM machines that relies on
arm_load_dtb() to be accessed later on.

Since all ARM machines allocates the FDT only once, we don't need to
worry about leaking the existing FDT during a machine reset (which is
something that other machines have to look after, e.g. the ppc64 pSeries
machine).

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/arm/boot.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..669a978157 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -684,7 +684,11 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
      */
     rom_add_blob_fixed_as("dtb", fdt, size, addr, as);
 
-    g_free(fdt);
+    /*
+     * Update the ms->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    ms->fdt = fdt;
 
     return size;
 
-- 
2.37.2

[PATCH for-7.2 v3 02/20] hw/microblaze: set machine->fdt in microblaze_load_dtb()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
all microblaze machines that uses microblaze_load_dtb().

Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/microblaze/boot.c      | 11 ++++++++++-
 hw/microblaze/meson.build |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 8b92a9801a..e9ebc04381 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -39,6 +39,8 @@
 
 #include "boot.h"
 
+#include <libfdt.h>
+
 static struct
 {
     void (*machine_cpu_reset)(MicroBlazeCPU *);
@@ -72,6 +74,7 @@ static int microblaze_load_dtb(hwaddr addr,
                                const char *kernel_cmdline,
                                const char *dtb_filename)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
     int fdt_size;
     void *fdt = NULL;
     int r;
@@ -100,7 +103,13 @@ static int microblaze_load_dtb(hwaddr addr,
     }
 
     cpu_physical_memory_write(addr, fdt, fdt_size);
-    g_free(fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
+
     return fdt_size;
 }
 
diff --git a/hw/microblaze/meson.build b/hw/microblaze/meson.build
index bb9e4eb8f4..a38a397872 100644
--- a/hw/microblaze/meson.build
+++ b/hw/microblaze/meson.build
@@ -1,5 +1,5 @@
 microblaze_ss = ss.source_set()
-microblaze_ss.add(files('boot.c'))
+microblaze_ss.add(files('boot.c'), fdt)
 microblaze_ss.add(when: 'CONFIG_PETALOGIX_S3ADSP1800', if_true: files('petalogix_s3adsp1800_mmu.c'))
 microblaze_ss.add(when: 'CONFIG_PETALOGIX_ML605', if_true: files('petalogix_ml605_mmu.c'))
 microblaze_ss.add(when: 'CONFIG_XLNX_ZYNQMP_PMU', if_true: files('xlnx-zynqmp-pmu.c'))
-- 
2.37.2

[PATCH for-7.2 v3 03/20] hw/nios2: set machine->fdt in nios2_load_dtb()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
all nios2 machines that uses nios2_load_dtb().

Cc: Chris Wulff <crwulff@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/nios2/boot.c      | 11 ++++++++++-
 hw/nios2/meson.build |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 21cbffff47..db3b21fea6 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -43,6 +43,8 @@
 
 #include "boot.h"
 
+#include <libfdt.h>
+
 #define NIOS2_MAGIC    0x534f494e
 
 static struct nios2_boot_info {
@@ -81,6 +83,7 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
 static int nios2_load_dtb(struct nios2_boot_info bi, const uint32_t ramsize,
                           const char *kernel_cmdline, const char *dtb_filename)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
     int fdt_size;
     void *fdt = NULL;
     int r;
@@ -113,7 +116,13 @@ static int nios2_load_dtb(struct nios2_boot_info bi, const uint32_t ramsize,
     }
 
     cpu_physical_memory_write(bi.fdt, fdt, fdt_size);
-    g_free(fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
+
     return fdt_size;
 }
 
diff --git a/hw/nios2/meson.build b/hw/nios2/meson.build
index 6c58e8082b..22277bd6c5 100644
--- a/hw/nios2/meson.build
+++ b/hw/nios2/meson.build
@@ -1,5 +1,5 @@
 nios2_ss = ss.source_set()
-nios2_ss.add(files('boot.c'))
+nios2_ss.add(files('boot.c'), fdt)
 nios2_ss.add(when: 'CONFIG_NIOS2_10M50', if_true: files('10m50_devboard.c'))
 nios2_ss.add(when: 'CONFIG_NIOS2_GENERIC_NOMMU', if_true: files('generic_nommu.c'))
 
-- 
2.37.2

[PATCH for-7.2 v3 04/20] hw/ppc: set machine->fdt in ppce500_load_device_tree()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
the e500 machine.

Cc: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/e500.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 32495d0123..6197a905b8 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -47,6 +47,8 @@
 #include "hw/i2c/i2c.h"
 #include "hw/irq.h"
 
+#include <libfdt.h>
+
 #define EPAPR_MAGIC                (0x45504150)
 #define DTC_LOAD_PAD               0x1800000
 #define DTC_PAD_MASK               0xFFFFF
@@ -600,7 +602,16 @@ done:
         cpu_physical_memory_write(addr, fdt, fdt_size);
     }
     ret = fdt_size;
-    g_free(fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     *
+     * The FDT is re-created during reset, so free machine->fdt
+     * to avoid leaking the old FDT.
+     */
+    g_free(machine->fdt);
+    machine->fdt = fdt;
 
 out:
     g_free(pci_map);
-- 
2.37.2

[PATCH for-7.2 v3 05/20] hw/ppc: set machine->fdt in bamboo_load_device_tree()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
the bamboo machine.

Cc: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/ppc440_bamboo.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 873f930c77..9c89b23c08 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -34,6 +34,8 @@
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
 
+#include <libfdt.h>
+
 #define BINARY_DEVICE_TREE_FILE "bamboo.dtb"
 
 /* from u-boot */
@@ -62,6 +64,7 @@ static int bamboo_load_device_tree(hwaddr addr,
                                      hwaddr initrd_size,
                                      const char *kernel_cmdline)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
     int ret = -1;
     uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
     char *filename;
@@ -116,7 +119,13 @@ static int bamboo_load_device_tree(hwaddr addr,
                           tb_freq);
 
     rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-    g_free(fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
+
     return 0;
 }
 
-- 
2.37.2

[PATCH for-7.2 v3 06/20] hw/ppc: set machine->fdt in sam460ex_load_device_tree()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
the sam460ex machine.

Cc: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/sam460ex.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0357ee077f..413a425d37 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -138,6 +138,7 @@ static int sam460ex_load_device_tree(hwaddr addr,
                                      hwaddr initrd_size,
                                      const char *kernel_cmdline)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
     uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
     char *filename;
     int fdt_size;
@@ -209,7 +210,12 @@ static int sam460ex_load_device_tree(hwaddr addr,
                               EBC_FREQ);
 
     rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-    g_free(fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
 
     return fdt_size;
 }
-- 
2.37.2

[PATCH for-7.2 v3 07/20] hw/ppc: set machine->fdt in xilinx_load_device_tree()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
the virtex_ml507 machine.

Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/virtex_ml507.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 53b126ff48..9f4c5d85a4 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -45,6 +45,8 @@
 #include "hw/qdev-properties.h"
 #include "ppc405.h"
 
+#include <libfdt.h>
+
 #define EPAPR_MAGIC    (0x45504150)
 #define FLASH_SIZE     (16 * MiB)
 
@@ -153,6 +155,7 @@ static int xilinx_load_device_tree(hwaddr addr,
                                       hwaddr initrd_size,
                                       const char *kernel_cmdline)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
     char *path;
     int fdt_size;
     void *fdt = NULL;
@@ -197,7 +200,13 @@ static int xilinx_load_device_tree(hwaddr addr,
     if (r < 0)
         fprintf(stderr, "couldn't set /chosen/bootargs\n");
     cpu_physical_memory_write(addr, fdt, fdt_size);
-    g_free(fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
+
     return fdt_size;
 }
 
-- 
2.37.2

[PATCH for-7.2 v3 08/20] hw/ppc: set machine->fdt in pegasos2_machine_reset()

[Daniel Henrique Barboza]

We'll introduce QMP/HMP commands that requires machine->fdt to be set
properly.

Cc: BALATON Zoltan <balaton@eik.bme.hu>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/pegasos2.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..624036d88b 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -331,6 +331,13 @@ static void pegasos2_machine_reset(MachineState *machine)
 
     vof_build_dt(fdt, pm->vof);
     vof_client_open_store(fdt, pm->vof, "/chosen", "stdout", "/failsafe");
+
+    /*
+     * Set the common machine->fdt pointer to enable support
+     * for 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
+
     pm->cpu->vhyp = PPC_VIRTUAL_HYPERVISOR(machine);
 }
 
-- 
2.37.2

[PATCH for-7.2 v3 09/20] hw/ppc: set machine->fdt in pnv_reset()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
all powernv machines.

Cc: Cédric Le Goater <clg@kaod.org>
Cc: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/pnv.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d3f77c8367..296995a600 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -608,7 +608,13 @@ static void pnv_reset(MachineState *machine)
     qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
     cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
 
-    g_free(fdt);
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' commands. Free the existing
+     * machine->fdt to avoid leaking it during a reset.
+     */
+    g_free(machine->fdt);
+    machine->fdt = fdt;
 }
 
 static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
-- 
2.37.2

[PATCH for-7.2 v3 10/20] hw/ppc: set machine->fdt in spapr machine

[Daniel Henrique Barboza]

The pSeries machine never bothered with the common machine->fdt
attribute. We do all the FDT related work using spapr->fdt_blob.

We're going to introduce HMP commands to read and save the FDT, which
will rely on setting machine->fdt properly to work across all machine
archs/types.

Let's set machine->fdt in the two places where we manipulate the FDT:
spapr_machine_reset() and CAS.

spapr->fdt_blob is left untouched for now. To replace it with
machine->fdt, since we're migrating spapr->fdt_blob, we would need to
migrate machine->fdt as well. This is something that we would like to to
do keep our code simpler but it's a work we'll do another day.

Cc: Cédric Le Goater <clg@kaod.org>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr.c       | 6 ++++++
 hw/ppc/spapr_hcall.c | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bc9ba6e6dc..7031cf964a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1713,6 +1713,12 @@ static void spapr_machine_reset(MachineState *machine)
     spapr->fdt_initial_size = spapr->fdt_size;
     spapr->fdt_blob = fdt;
 
+    /*
+     * Set the common machine->fdt pointer to enable support
+     * for 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = fdt;
+
     /* Set up the entry state */
     first_ppc_cpu->env.gpr[5] = 0;
 
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index a8d4a6bcf0..a53bfd76f4 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1256,6 +1256,14 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu,
     spapr->fdt_initial_size = spapr->fdt_size;
     spapr->fdt_blob = fdt;
 
+    /*
+     * Set the machine->fdt pointer again since we just freed
+     * it above (by freeing spapr->fdt_blob). We set this
+     * pointer to enable support for 'dumpdtb' and 'info fdt'
+     * QMP/HMP commands.
+     */
+    MACHINE(spapr)->fdt = fdt;
+
     return H_SUCCESS;
 }
 
-- 
2.37.2

[PATCH for-7.2 v3 11/20] hw/riscv: set machine->fdt in sifive_u_machine_init()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
the sifive_u machine.

Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Bin Meng <bin.meng@windriver.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/riscv/sifive_u.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..f14d8411df 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -634,6 +634,12 @@ static void sifive_u_machine_init(MachineState *machine)
         start_addr_hi32 = (uint64_t)start_addr >> 32;
     }
 
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = s->fdt;
+
     /* reset vector */
     uint32_t reset_vec[12] = {
         s->msel,                       /* MSEL pin state */
-- 
2.37.2

[PATCH for-7.2 v3 12/20] hw/riscv: set machine->fdt in spike_board_init()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
the spike machine.

Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <alistair.francis@wdc.com>
Cc: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/riscv/spike.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e41b6aa9f0..17f517bfa6 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -40,6 +40,8 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/sysemu.h"
 
+#include <libfdt.h>
+
 static const MemMapEntry spike_memmap[] = {
     [SPIKE_MROM] =     {     0x1000,     0xf000 },
     [SPIKE_HTIF] =     {  0x1000000,     0x1000 },
@@ -304,6 +306,13 @@ static void spike_board_init(MachineState *machine)
     /* Compute the fdt load address in dram */
     fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
                                    machine->ram_size, s->fdt);
+
+    /*
+     * Update the machine->fdt pointer to enable support for
+     * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+     */
+    machine->fdt = s->fdt;
+
     /* load the reset vector */
     riscv_setup_rom_reset_vec(machine, &s->soc[0], memmap[SPIKE_DRAM].base,
                               memmap[SPIKE_MROM].base,
-- 
2.37.2

[PATCH for-7.2 v3 13/20] hw/xtensa: set machine->fdt in xtfpga_init()

[Daniel Henrique Barboza]

This will enable support for 'dumpdtb' and 'info fdt' HMP commands for
all xtensa machines that uses a FDT.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/xtensa/meson.build | 2 +-
 hw/xtensa/xtfpga.c    | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/xtensa/meson.build b/hw/xtensa/meson.build
index 1d5835df4b..ebba51cc74 100644
--- a/hw/xtensa/meson.build
+++ b/hw/xtensa/meson.build
@@ -6,6 +6,6 @@ xtensa_ss.add(files(
 ))
 xtensa_ss.add(when: 'CONFIG_XTENSA_SIM', if_true: files('sim.c'))
 xtensa_ss.add(when: 'CONFIG_XTENSA_VIRT', if_true: files('virt.c'))
-xtensa_ss.add(when: 'CONFIG_XTENSA_XTFPGA', if_true: files('xtfpga.c'))
+xtensa_ss.add(when: 'CONFIG_XTENSA_XTFPGA', if_true: [files('xtfpga.c'), fdt])
 
 hw_arch += {'xtensa': xtensa_ss}
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 2a5556a35f..9e2f911caa 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -50,6 +50,8 @@
 #include "hw/xtensa/mx_pic.h"
 #include "migration/vmstate.h"
 
+#include <libfdt.h>
+
 typedef struct XtfpgaFlashDesc {
     hwaddr base;
     size_t size;
@@ -377,7 +379,12 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
             cur_tagptr = put_tag(cur_tagptr, BP_TAG_FDT,
                                  sizeof(dtb_addr), &dtb_addr);
             cur_lowmem = QEMU_ALIGN_UP(cur_lowmem + fdt_size, 4 * KiB);
-            g_free(fdt);
+
+            /*
+             * Update the machine->fdt pointer to enable support for
+             * 'dumpdtb' and 'info fdt' QMP/HMP commands.
+             */
+            machine->fdt = fdt;
         }
 #else
         if (dtb_filename) {
-- 
2.37.2

[PATCH for-7.2 v3 14/20] qmp/hmp, device_tree.c: introduce dumpdtb

[Daniel Henrique Barboza]

To save the FDT blob we have the '-machine dumpdtb=<file>' property.
With this property set, the machine saves the FDT in <file> and exit.
The created file can then be converted to plain text dts format using
'dtc'.

There's nothing particularly sophisticated into saving the FDT that
can't be done with the machine at any state, as long as the machine has
a valid FDT to be saved.

The 'dumpdtb' command receives a 'filename' paramenter and, if a valid
FDT is available, it'll save it in a file 'filename'. In short, this is
a '-machine dumpdtb' that can be fired on demand via QMP/HMP.

A valid FDT consists of a FDT that was created using libfdt being
retrieved via 'current_machine->fdt' in device_tree.c. This condition is
met by most FDT users in QEMU.

This command will always be executed in-band (i.e. holding BQL),
avoiding potential race conditions with machines that might change the
FDT during runtime (e.g. PowerPC 'pseries' machine).

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hmp-commands.hx              | 13 +++++++++++++
 include/monitor/hmp.h        |  1 +
 include/sysemu/device_tree.h |  1 +
 monitor/hmp-cmds.c           | 12 ++++++++++++
 monitor/qmp-cmds.c           | 13 +++++++++++++
 qapi/machine.json            | 17 +++++++++++++++++
 softmmu/device_tree.c        | 18 ++++++++++++++++++
 7 files changed, 75 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 182e639d14..d2554e9701 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1800,3 +1800,16 @@ ERST
                       "\n\t\t\t\t\t limit on a specified virtual cpu",
         .cmd        = hmp_cancel_vcpu_dirty_limit,
     },
+
+SRST
+``dumpdtb`` *filename*
+  Save the FDT in the 'filename' file to be decoded using dtc.
+  Requires 'libfdt' support.
+ERST
+    {
+        .name       = "dumpdtb",
+        .args_type  = "filename:s",
+        .params     = "[filename] file to save the FDT",
+        .help       = "save the FDT in the 'filename' file to be decoded using dtc",
+        .cmd        = hmp_dumpdtb,
+    },
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index a618eb1e4e..d7f324da59 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -134,6 +134,7 @@ void hmp_calc_dirty_rate(Monitor *mon, const QDict *qdict);
 void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
 void hmp_cancel_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
 void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
+void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
 void hmp_human_readable_text_helper(Monitor *mon,
                                     HumanReadableText *(*qmp_handler)(Error **));
 void hmp_info_stats(Monitor *mon, const QDict *qdict);
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..bf7684e4ed 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -136,6 +136,7 @@ int qemu_fdt_add_path(void *fdt, const char *path);
     } while (0)
 
 void qemu_fdt_dumpdtb(void *fdt, int size);
+void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp);
 
 /**
  * qemu_fdt_setprop_sized_cells_from_array:
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index c6cd6f91dd..d23ec85f9d 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -2472,3 +2472,15 @@ exit:
 exit_no_print:
     error_free(err);
 }
+
+void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
+{
+    const char *filename = qdict_get_str(qdict, "filename");
+    Error *local_err = NULL;
+
+    qmp_dumpdtb(filename, &local_err);
+
+    if (local_err) {
+        error_report_err(local_err);
+    }
+}
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 7314cd813d..8415aca08c 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -45,6 +45,7 @@
 #include "hw/intc/intc.h"
 #include "hw/rdma/rdma.h"
 #include "monitor/stats.h"
+#include "sysemu/device_tree.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -596,3 +597,15 @@ bool apply_str_list_filter(const char *string, strList *list)
     }
     return false;
 }
+
+#ifdef CONFIG_FDT
+void qmp_dumpdtb(const char *filename, Error **errp)
+{
+    return qemu_fdt_qmp_dumpdtb(filename, errp);
+}
+#else
+void qmp_dumpdtb(const char *filename, Error **errp)
+{
+    error_setg(errp, "dumpdtb requires libfdt");
+}
+#endif
diff --git a/qapi/machine.json b/qapi/machine.json
index 6afd1936b0..aeb013f3dd 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1664,3 +1664,20 @@
      '*size': 'size',
      '*max-size': 'size',
      '*slots': 'uint64' } }
+
+##
+# @dumpdtb:
+#
+# Save the FDT in dtb format. Requires 'libfdt' support.
+#
+# @filename: name of the FDT file to be created
+#
+# Since: 7.2
+#
+# Example:
+#   {"execute": "dumpdtb"}
+#    "arguments": { "filename": "/tmp/fdt.dtb" } }
+#
+##
+{ 'command': 'dumpdtb',
+  'data': { 'filename': 'str' } }
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 6ca3fad285..cd487ddd4d 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -643,3 +643,21 @@ out:
     g_free(propcells);
     return ret;
 }
+
+void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
+{
+    int size;
+
+    if (!current_machine->fdt) {
+        error_setg(errp, "Unable to find the machine FDT");
+        return;
+    }
+
+    size = fdt_totalsize(current_machine->fdt);
+
+    if (g_file_set_contents(filename, current_machine->fdt, size, NULL)) {
+        return;
+    }
+
+    error_setg(errp, "Error when saving machine FDT to file %s", filename);
+}
-- 
2.37.2

[PATCH for-7.2 v3 15/20] qmp/hmp, device_tree.c: introduce 'info fdt' command

[Daniel Henrique Barboza]

Reading the FDT requires that the user saves the fdt_blob and then use
'dtc' to read the contents. Saving the file and using 'dtc' is a strong
use case when we need to compare two FDTs, but it's a lot of steps if
you want to do quick check on a certain node or property.

'info fdt' retrieves FDT nodes (and properties, later on) and print it
to the user. This can be used to check the FDT on running machines
without having to save the blob and use 'dtc'.

The implementation is based on the premise that the machine thas a FDT
created using libfdt and pointed by 'machine->fdt'. As long as this
pre-requisite is met the machine should be able to support it.

For now we're going to add the required QMP/HMP boilerplate and the
capability of printing the name of the properties of a given node. Next
patches will extend 'info fdt' to be able to print nodes recursively,
and then individual properties.

This command will always be executed in-band (i.e. holding BQL),
avoiding potential race conditions with machines that might change the
FDT during runtime (e.g. PowerPC 'pseries' machine).

'info fdt' is not something that we expect to be used aside from debugging,
so we're implementing it in QMP as 'x-query-fdt'.

This is an example of 'info fdt' fetching the '/chosen' node of the
pSeries machine:

(qemu) info fdt /chosen
chosen {
    ibm,architecture-vec-5;
    rng-seed;
    ibm,arch-vec-5-platform-support;
    linux,pci-probe-only;
    stdout-path;
    linux,stdout-path;
    qemu,graphic-depth;
    qemu,graphic-height;
    qemu,graphic-width;
}

And the same node for the aarch64 'virt' machine:

(qemu) info fdt /chosen
chosen {
    stdout-path;
    rng-seed;
    kaslr-seed;
}

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hmp-commands-info.hx         | 13 ++++++++++
 include/monitor/hmp.h        |  1 +
 include/sysemu/device_tree.h |  4 +++
 monitor/hmp-cmds.c           | 13 ++++++++++
 monitor/qmp-cmds.c           | 12 +++++++++
 qapi/machine.json            | 19 +++++++++++++++
 softmmu/device_tree.c        | 47 ++++++++++++++++++++++++++++++++++++
 7 files changed, 109 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 188d9ece3b..743b48865d 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -921,3 +921,16 @@ SRST
   ``stats``
     Show runtime-collected statistics
 ERST
+
+    {
+        .name       = "fdt",
+        .args_type  = "nodepath:s",
+        .params     = "nodepath",
+        .help       = "show firmware device tree node given its path",
+        .cmd        = hmp_info_fdt,
+    },
+
+SRST
+  ``info fdt``
+    Show a firmware device tree node given its path. Requires libfdt.
+ERST
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index d7f324da59..c0883dd1e3 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -135,6 +135,7 @@ void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
 void hmp_cancel_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
 void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
 void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
+void hmp_info_fdt(Monitor *mon, const QDict *qdict);
 void hmp_human_readable_text_helper(Monitor *mon,
                                     HumanReadableText *(*qmp_handler)(Error **));
 void hmp_info_stats(Monitor *mon, const QDict *qdict);
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index bf7684e4ed..057d13e397 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -14,6 +14,8 @@
 #ifndef DEVICE_TREE_H
 #define DEVICE_TREE_H
 
+#include "qapi/qapi-types-common.h"
+
 void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 #ifdef CONFIG_LINUX
@@ -137,6 +139,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,
+                                          Error **errp);
 
 /**
  * qemu_fdt_setprop_sized_cells_from_array:
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index d23ec85f9d..accde90380 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -2484,3 +2484,16 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
         error_report_err(local_err);
     }
 }
+
+void hmp_info_fdt(Monitor *mon, const QDict *qdict)
+{
+    const char *nodepath = qdict_get_str(qdict, "nodepath");
+    Error *err = NULL;
+    g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err);
+
+    if (hmp_handle_error(mon, err)) {
+        return;
+    }
+
+    monitor_printf(mon, "%s", info->human_readable_text);
+}
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 8415aca08c..db2c6aa7da 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -603,9 +603,21 @@ 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)
+{
+    return qemu_fdt_qmp_query_fdt(nodepath, errp);
+}
 #else
 void qmp_dumpdtb(const char *filename, Error **errp)
 {
     error_setg(errp, "dumpdtb requires libfdt");
 }
+
+HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp)
+{
+    error_setg(errp, "this command requires libfdt");
+
+    return NULL;
+}
 #endif
diff --git a/qapi/machine.json b/qapi/machine.json
index aeb013f3dd..96cff541ca 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1681,3 +1681,22 @@
 ##
 { 'command': 'dumpdtb',
   'data': { 'filename': 'str' } }
+
+##
+# @x-query-fdt:
+#
+# Query for FDT element (node or property). Requires 'libfdt'.
+#
+# @nodepath: the path of the FDT node to be retrieved
+#
+# Features:
+# @unstable: This command is meant for debugging.
+#
+# Returns: FDT node
+#
+# Since: 7.2
+##
+{ 'command': 'x-query-fdt',
+  'data': { 'nodepath': 'str' },
+  'returns': 'HumanReadableText',
+  'features': [ 'unstable' ]  }
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index cd487ddd4d..3fb07b537f 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -18,6 +18,7 @@
 #endif
 
 #include "qapi/error.h"
+#include "qapi/type-helpers.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include "qemu/bswap.h"
@@ -661,3 +662,49 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
 
     error_setg(errp, "Error when saving machine FDT to file %s", filename);
 }
+
+static void fdt_format_node(GString *buf, int node, int depth)
+{
+    const struct fdt_property *prop = NULL;
+    const char *propname = NULL;
+    void *fdt = current_machine->fdt;
+    int padding = depth * 4;
+    int property = 0;
+    int prop_size;
+
+    g_string_append_printf(buf, "%*s%s {\n", padding, "",
+                           fdt_get_name(fdt, node, NULL));
+
+    padding += 4;
+
+    fdt_for_each_property_offset(property, fdt, node) {
+        prop = fdt_get_property_by_offset(fdt, property, &prop_size);
+        propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+
+        g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+    }
+
+    padding -= 4;
+    g_string_append_printf(buf, "%*s}\n", padding, "");
+}
+
+HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp)
+{
+    g_autoptr(GString) buf = g_string_new("");
+    int node;
+
+    if (!current_machine->fdt) {
+        error_setg(errp, "Unable to find the machine FDT");
+        return NULL;
+    }
+
+    node = fdt_path_offset(current_machine->fdt, nodepath);
+    if (node < 0) {
+        error_setg(errp, "node '%s' not found in FDT", nodepath);
+        return NULL;
+    }
+
+    fdt_format_node(buf, node, 0);
+
+    return human_readable_text_from_str(buf);
+}
-- 
2.37.2

[PATCH for-7.2 v3 16/20] device_tree.c: support string array prop in fdt_format_node()

[Daniel Henrique Barboza]

To support printing string properties in 'info fdt' we need to determine
whether a void data might contain a string array.

We do that by casting the void data to a string array and:

- check if the array finishes with a null character
- check if there's no empty string in the middle of the array (i.e.
consecutive \0\0 characters)
- check if all characters of each substring are printable

If all conditions are met, we'll consider it to be a string array data
type and print it accordingly. After this change, 'info fdt' is now able
to print string arrays. Here's an example of string arrays we're able to
print in the /rtas node of the ppc64 pSeries machine:

(qemu) info fdt /rtas
rtas {
    (...)
    qemu,hypertas-functions = "hcall-memop1";
    ibm,hypertas-functions = "hcall-pft","hcall-term","hcall-dabr",
"hcall-interrupt","hcall-tce","hcall-vio","hcall-splpar","hcall-join",
"hcall-bulk","hcall-set-mode","hcall-sprg0","hcall-copy","hcall-debug",
"hcall-vphn","hcall-multi-tce","hcall-hpt-resize","hcall-watchdog";
}

'qemu,hypertas-functions' is a property with a single string while
'ibm,hypertas-functions' is a string array.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 softmmu/device_tree.c | 64 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 3fb07b537f..d32d6856da 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -663,6 +663,63 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
     error_setg(errp, "Error when saving machine FDT to file %s", filename);
 }
 
+static bool fdt_prop_is_string_array(const void *data, int size)
+{
+    const char *str_arr, *str;
+    int i, str_len;
+
+    str_arr = str = data;
+
+    if (size <= 0 || str_arr[size - 1] != '\0') {
+        return false;
+    }
+
+    while (str < str_arr + size) {
+        str_len = strlen(str);
+
+        /*
+         * Do not consider empty strings (consecutives \0\0)
+         * as valid.
+         */
+        if (str_len == 0) {
+            return false;
+        }
+
+        for (i = 0; i < str_len; i++) {
+            if (!g_ascii_isprint(str[i])) {
+                return false;
+            }
+        }
+
+        str += str_len + 1;
+    }
+
+    return true;
+}
+
+static void fdt_prop_format_string_array(GString *buf,
+                                         const char *propname,
+                                         const char *data,
+                                         int prop_size, int padding)
+{
+    const char *str = data;
+
+    g_string_append_printf(buf, "%*s%s = ", padding, "", propname);
+
+    while (str < data + prop_size) {
+        /* appends up to the next '\0' */
+        g_string_append_printf(buf, "\"%s\"", str);
+
+        str += strlen(str) + 1;
+        if (str < data + prop_size) {
+            /* add a comma separator for the next string */
+            g_string_append_printf(buf, ",");
+        }
+    }
+
+    g_string_append_printf(buf, ";\n");
+}
+
 static void fdt_format_node(GString *buf, int node, int depth)
 {
     const struct fdt_property *prop = NULL;
@@ -681,7 +738,12 @@ static void fdt_format_node(GString *buf, int node, int depth)
         prop = fdt_get_property_by_offset(fdt, property, &prop_size);
         propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
 
-        g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+        if (fdt_prop_is_string_array(prop->data, prop_size)) {
+            fdt_prop_format_string_array(buf, propname, prop->data,
+                                         prop_size, padding);
+        } else {
+            g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+        }
     }
 
     padding -= 4;
-- 
2.37.2

[PATCH for-7.2 v3 17/20] device_tree.c: support remaining FDT prop types

[Daniel Henrique Barboza]

When printing a blob with 'dtc' using the '-O dts' option there are 3
distinct data types being printed: strings, arrays of uint32s and
regular byte arrays.

Previous patch added support to print strings. Let's add the remaining
formats. We want to resemble the format that 'dtc -O dts' uses, so every
uint32 array uses angle brackets (<>), and regular byte array uses square
brackets ([]). For properties that has no values we keep printing just
its name.

The /chosen FDT node from the pSeris machine gives an example of all
property types 'info fdt' is now able to display:

(qemu) info fdt /chosen
chosen {
    ibm,architecture-vec-5 = [00 00];
    rng-seed = <0x9cf5071b 0xf8804213 0xbe797764 0xad3d955 0xe0c9637 0x1f99c61e 0xe9243741 0xe800f17d>;
    ibm,arch-vec-5-platform-support = <0x178018c0 0x19001a40>;
    linux,pci-probe-only = <0x0>;
    stdout-path = "/vdevice/vty@71000000";
    linux,stdout-path = "/vdevice/vty@71000000";
    qemu,graphic-depth = <0x20>;
    qemu,graphic-height = <0x258>;
    qemu,graphic-width = <0x320>;
}

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 softmmu/device_tree.c | 57 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index d32d6856da..43f96e371b 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -720,6 +720,52 @@ static void fdt_prop_format_string_array(GString *buf,
     g_string_append_printf(buf, ";\n");
 }
 
+static bool fdt_prop_is_uint32_array(int size)
+{
+    return size % 4 == 0;
+}
+
+static void fdt_prop_format_uint32_array(GString *buf,
+                                         const char *propname,
+                                         const void *data,
+                                         int prop_size, int padding)
+{
+    const fdt32_t *array = data;
+    int array_len = prop_size / 4;
+    int i;
+
+    g_string_append_printf(buf, "%*s%s = <", padding, "", propname);
+
+    for (i = 0; i < array_len; i++) {
+        g_string_append_printf(buf, "0x%" PRIx32, fdt32_to_cpu(array[i]));
+
+        if (i < array_len - 1) {
+            g_string_append_printf(buf, " ");
+        }
+    }
+
+    g_string_append_printf(buf, ">;\n");
+}
+
+static void fdt_prop_format_val(GString *buf, const char *propname,
+                                const void *data, int prop_size,
+                                int padding)
+{
+    const char *val = data;
+    int i;
+
+    g_string_append_printf(buf, "%*s%s = [", padding, "", propname);
+
+    for (i = 0; i < prop_size; i++) {
+        g_string_append_printf(buf, "%02x", val[i]);
+        if (i < prop_size - 1) {
+            g_string_append_printf(buf, " ");
+        }
+    }
+
+    g_string_append_printf(buf, "];\n");
+}
+
 static void fdt_format_node(GString *buf, int node, int depth)
 {
     const struct fdt_property *prop = NULL;
@@ -738,11 +784,20 @@ static void fdt_format_node(GString *buf, int node, int depth)
         prop = fdt_get_property_by_offset(fdt, property, &prop_size);
         propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
 
+        if (prop_size == 0) {
+            g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+            continue;
+        }
+
         if (fdt_prop_is_string_array(prop->data, prop_size)) {
             fdt_prop_format_string_array(buf, propname, prop->data,
                                          prop_size, padding);
+        } else if (fdt_prop_is_uint32_array(prop_size)) {
+            fdt_prop_format_uint32_array(buf, propname, prop->data, prop_size,
+                                         padding);
         } else {
-            g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+            fdt_prop_format_val(buf, propname, prop->data,
+                                prop_size, padding);
         }
     }
 
-- 
2.37.2

[PATCH for-7.2 v3 18/20] device_node.c: enable 'info fdt' to print subnodes

[Daniel Henrique Barboza]

Printing subnodes of a given node will allow us to show a whole subtree,
which the additional perk of 'info fdt /' being able to print the whole
FDT.

Since we're now printing more than one subnode, change 'fdt_info' to
print the full path of the first node. This small tweak helps
identifying which node or subnode are being displayed.

To demostrate this capability without printing the whole FDT, the
'/cpus/cpu-map' node from the ARM 'virt' machine has a lot of subnodes:

(qemu) info fdt /cpus/cpu-map
/cpus/cpu-map {
    socket0 {
        cluster0 {
            core0 {
                cpu = <0x8001>
            }
        }
    }
}

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 softmmu/device_tree.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 43f96e371b..a6bfbc0617 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -766,17 +766,26 @@ static void fdt_prop_format_val(GString *buf, const char *propname,
     g_string_append_printf(buf, "];\n");
 }
 
-static void fdt_format_node(GString *buf, int node, int depth)
+
+static void fdt_format_node(GString *buf, int node, int depth,
+                            const char *fullpath)
 {
     const struct fdt_property *prop = NULL;
+    const char *nodename = NULL;
     const char *propname = NULL;
     void *fdt = current_machine->fdt;
     int padding = depth * 4;
     int property = 0;
+    int parent = node;
     int prop_size;
 
-    g_string_append_printf(buf, "%*s%s {\n", padding, "",
-                           fdt_get_name(fdt, node, NULL));
+    if (fullpath != NULL) {
+        nodename = fullpath;
+    } else {
+        nodename = fdt_get_name(fdt, node, NULL);
+    }
+
+    g_string_append_printf(buf, "%*s%s {\n", padding, "", nodename);
 
     padding += 4;
 
@@ -801,6 +810,10 @@ static void fdt_format_node(GString *buf, int node, int depth)
         }
     }
 
+    fdt_for_each_subnode(node, fdt, parent) {
+        fdt_format_node(buf, node, depth + 1, NULL);
+    }
+
     padding -= 4;
     g_string_append_printf(buf, "%*s}\n", padding, "");
 }
@@ -821,7 +834,7 @@ HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp)
         return NULL;
     }
 
-    fdt_format_node(buf, node, 0);
+    fdt_format_node(buf, node, 0, nodepath);
 
     return human_readable_text_from_str(buf);
 }
-- 
2.37.2

[PATCH for-7.2 v3 19/20] device_tree.c: add fdt_format_property() helper

[Daniel Henrique Barboza]

We want to be able to also print properties with 'info fdt'.

Create a helper to format properties based on the already existing code
from fdt_format_node().

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 softmmu/device_tree.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index a6bfbc0617..9e681739bd 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -766,6 +766,25 @@ static void fdt_prop_format_val(GString *buf, const char *propname,
     g_string_append_printf(buf, "];\n");
 }
 
+static void fdt_format_property(GString *buf, const char *propname,
+                                const void *data, int prop_size,
+                                int padding)
+{
+    if (prop_size == 0) {
+        g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+        return;
+    }
+
+    if (fdt_prop_is_string_array(data, prop_size)) {
+        fdt_prop_format_string_array(buf, propname, data, prop_size,
+                                     padding);
+    } else if (fdt_prop_is_uint32_array(prop_size)) {
+        fdt_prop_format_uint32_array(buf, propname, data, prop_size,
+                                     padding);
+    } else {
+        fdt_prop_format_val(buf, propname, data, prop_size, padding);
+    }
+}
 
 static void fdt_format_node(GString *buf, int node, int depth,
                             const char *fullpath)
@@ -793,21 +812,7 @@ static void fdt_format_node(GString *buf, int node, int depth,
         prop = fdt_get_property_by_offset(fdt, property, &prop_size);
         propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
 
-        if (prop_size == 0) {
-            g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
-            continue;
-        }
-
-        if (fdt_prop_is_string_array(prop->data, prop_size)) {
-            fdt_prop_format_string_array(buf, propname, prop->data,
-                                         prop_size, padding);
-        } else if (fdt_prop_is_uint32_array(prop_size)) {
-            fdt_prop_format_uint32_array(buf, propname, prop->data, prop_size,
-                                         padding);
-        } else {
-            fdt_prop_format_val(buf, propname, prop->data,
-                                prop_size, padding);
-        }
+        fdt_format_property(buf, propname, prop->data, prop_size, padding);
     }
 
     fdt_for_each_subnode(node, fdt, parent) {
-- 
2.37.2

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

[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@71000003 ibm,my-dma-window
/vdevice/v-scsi@71000003/ibm,my-dma-window = <0x71000003 0x0 0x0 0x0 0x10000000>

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);
 }
-- 
2.37.2

Re: [PATCH for-7.2 v3 16/20] device_tree.c: support string array prop in fdt_format_node()

[David Gibson]

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

On Tue, Aug 16, 2022 at 02:34:24PM -0300, Daniel Henrique Barboza wrote:
> To support printing string properties in 'info fdt' we need to determine
> whether a void data might contain a string array.
> 
> We do that by casting the void data to a string array and:
> 
> - check if the array finishes with a null character
> - check if there's no empty string in the middle of the array (i.e.
> consecutive \0\0 characters)
> - check if all characters of each substring are printable
> 
> If all conditions are met, we'll consider it to be a string array data
> type and print it accordingly. After this change, 'info fdt' is now able
> to print string arrays. Here's an example of string arrays we're able to
> print in the /rtas node of the ppc64 pSeries machine:
> 
> (qemu) info fdt /rtas
> rtas {
>     (...)
>     qemu,hypertas-functions = "hcall-memop1";
>     ibm,hypertas-functions = "hcall-pft","hcall-term","hcall-dabr",
> "hcall-interrupt","hcall-tce","hcall-vio","hcall-splpar","hcall-join",
> "hcall-bulk","hcall-set-mode","hcall-sprg0","hcall-copy","hcall-debug",
> "hcall-vphn","hcall-multi-tce","hcall-hpt-resize","hcall-watchdog";

nit: typical dts style would have extra spaces:
	prop = "foo", "bar";
i.e. separated by ", " not just ",".

> }
> 
> 'qemu,hypertas-functions' is a property with a single string while
> 'ibm,hypertas-functions' is a string array.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  softmmu/device_tree.c | 64 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 3fb07b537f..d32d6856da 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -663,6 +663,63 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
>      error_setg(errp, "Error when saving machine FDT to file %s", filename);
>  }
>  
> +static bool fdt_prop_is_string_array(const void *data, int size)
> +{
> +    const char *str_arr, *str;
> +    int i, str_len;
> +
> +    str_arr = str = data;
> +
> +    if (size <= 0 || str_arr[size - 1] != '\0') {
> +        return false;
> +    }
> +
> +    while (str < str_arr + size) {
> +        str_len = strlen(str);
> +
> +        /*
> +         * Do not consider empty strings (consecutives \0\0)
> +         * as valid.
> +         */
> +        if (str_len == 0) {
> +            return false;
> +        }
> +
> +        for (i = 0; i < str_len; i++) {
> +            if (!g_ascii_isprint(str[i])) {
> +                return false;
> +            }
> +        }
> +
> +        str += str_len + 1;
> +    }
> +
> +    return true;
> +}
> +
> +static void fdt_prop_format_string_array(GString *buf,
> +                                         const char *propname,
> +                                         const char *data,
> +                                         int prop_size, int padding)
> +{
> +    const char *str = data;
> +
> +    g_string_append_printf(buf, "%*s%s = ", padding, "", propname);
> +
> +    while (str < data + prop_size) {
> +        /* appends up to the next '\0' */
> +        g_string_append_printf(buf, "\"%s\"", str);
> +
> +        str += strlen(str) + 1;
> +        if (str < data + prop_size) {
> +            /* add a comma separator for the next string */
> +            g_string_append_printf(buf, ",");
> +        }
> +    }
> +
> +    g_string_append_printf(buf, ";\n");
> +}
> +
>  static void fdt_format_node(GString *buf, int node, int depth)
>  {
>      const struct fdt_property *prop = NULL;
> @@ -681,7 +738,12 @@ static void fdt_format_node(GString *buf, int node, int depth)
>          prop = fdt_get_property_by_offset(fdt, property, &prop_size);
>          propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
>  
> -        g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
> +        if (fdt_prop_is_string_array(prop->data, prop_size)) {
> +            fdt_prop_format_string_array(buf, propname, prop->data,
> +                                         prop_size, padding);
> +        } else {
> +            g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
> +        }
>      }
>  
>      padding -= 4;

-- 
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 --]

Re: [PATCH for-7.2 v3 18/20] device_node.c: enable 'info fdt' to print subnodes

[David Gibson]

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

On Tue, Aug 16, 2022 at 02:34:26PM -0300, Daniel Henrique Barboza wrote:
> Printing subnodes of a given node will allow us to show a whole subtree,
> which the additional perk of 'info fdt /' being able to print the whole
> FDT.
> 
> Since we're now printing more than one subnode, change 'fdt_info' to
> print the full path of the first node. This small tweak helps
> identifying which node or subnode are being displayed.
> 
> To demostrate this capability without printing the whole FDT, the
> '/cpus/cpu-map' node from the ARM 'virt' machine has a lot of subnodes:
> 
> (qemu) info fdt /cpus/cpu-map
> /cpus/cpu-map {
>     socket0 {
>         cluster0 {
>             core0 {
>                 cpu = <0x8001>
>             }
>         }
>     }
> }

nit: dts format requires a ; after each closing }
	foo {
		bar {
		};
	};

> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  softmmu/device_tree.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 43f96e371b..a6bfbc0617 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -766,17 +766,26 @@ static void fdt_prop_format_val(GString *buf, const char *propname,
>      g_string_append_printf(buf, "];\n");
>  }
>  
> -static void fdt_format_node(GString *buf, int node, int depth)
> +
> +static void fdt_format_node(GString *buf, int node, int depth,
> +                            const char *fullpath)
>  {
>      const struct fdt_property *prop = NULL;
> +    const char *nodename = NULL;
>      const char *propname = NULL;
>      void *fdt = current_machine->fdt;
>      int padding = depth * 4;
>      int property = 0;
> +    int parent = node;
>      int prop_size;
>  
> -    g_string_append_printf(buf, "%*s%s {\n", padding, "",
> -                           fdt_get_name(fdt, node, NULL));
> +    if (fullpath != NULL) {
> +        nodename = fullpath;
> +    } else {
> +        nodename = fdt_get_name(fdt, node, NULL);
> +    }
> +
> +    g_string_append_printf(buf, "%*s%s {\n", padding, "", nodename);
>  
>      padding += 4;
>  
> @@ -801,6 +810,10 @@ static void fdt_format_node(GString *buf, int node, int depth)
>          }
>      }
>  
> +    fdt_for_each_subnode(node, fdt, parent) {
> +        fdt_format_node(buf, node, depth + 1, NULL);
> +    }
> +
>      padding -= 4;
>      g_string_append_printf(buf, "%*s}\n", padding, "");
>  }
> @@ -821,7 +834,7 @@ HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp)
>          return NULL;
>      }
>  
> -    fdt_format_node(buf, node, 0);
> +    fdt_format_node(buf, node, 0, nodepath);
>  
>      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 --]

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

[David Gibson]

[-- 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 --]

Re: [PATCH for-7.2 v3 10/20] hw/ppc: set machine->fdt in spapr machine

[David Gibson]

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

On Tue, Aug 16, 2022 at 02:34:18PM -0300, Daniel Henrique Barboza wrote:
> The pSeries machine never bothered with the common machine->fdt
> attribute. We do all the FDT related work using spapr->fdt_blob.
> 
> We're going to introduce HMP commands to read and save the FDT, which
> will rely on setting machine->fdt properly to work across all machine
> archs/types.
> 
> Let's set machine->fdt in the two places where we manipulate the FDT:
> spapr_machine_reset() and CAS.

So, there's a third place where fdt_blob is updated, in h_update_dt();
that happens because SLOF can make some updates to the DT that qemu
needs to be aware of.  It's kinda ugly, and is a consequence of the
fact that qemu and SLOF kind of share the role of "platform firmware"
for spapr.

But.. it's worse than that.  Those are the only 3 places we actually
alter fdt_blob, but not the only places we logically update the device
tree.  Up until now there wasn't a way to introspect the DT, and so we
didn't bother keeping spapr->fdt_blob update.  Essentially, we
considered maintaining the DT image the job of the guest after CAS.

Specifically, every dynamic reconfiguration event (hotplug or unplug)
alters the device tree.  We generate an fdt fragment for the new
device then stream that as an update to the guest using the PAPR
specified interface (rtas_ibm_configure_connector).  As noted we
currently don't update qemu's global fdt image based on that.  On hot
unplug logically we need to revert those changes, which is actually
pretty tricky, but currently the guest's job.


Really, the trouble is that just dumping or viewing the dt is only
simple in an "embedded" style environment where the fdt is generate
then spit into the guest.  In an actual open firmware environment like
spapr, the DT is logically a dynamic thing maintained by firmware -
but because "firmware"'s responsibility is split between SLOF and
RTAS/qemu, keeping track of that is pretty nasty.  For an environment
like this, the flat tree format isn't really suited either - we'd want
a dynamic representation of the tree.  We get away with flat trees for
now (barely) only because we mostly delegate the responsibility for
managing the tree to SLOF and/or the OS kernel, both of which do use
non-flat representations of the tree.

> spapr->fdt_blob is left untouched for now. To replace it with
> machine->fdt, since we're migrating spapr->fdt_blob, we would need to
> migrate machine->fdt as well. This is something that we would like to to
> do keep our code simpler but it's a work we'll do another day.
> 
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr.c       | 6 ++++++
>  hw/ppc/spapr_hcall.c | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bc9ba6e6dc..7031cf964a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1713,6 +1713,12 @@ static void spapr_machine_reset(MachineState *machine)
>      spapr->fdt_initial_size = spapr->fdt_size;
>      spapr->fdt_blob = fdt;
>  
> +    /*
> +     * Set the common machine->fdt pointer to enable support
> +     * for 'dumpdtb' and 'info fdt' QMP/HMP commands.
> +     */
> +    machine->fdt = fdt;
> +
>      /* Set up the entry state */
>      first_ppc_cpu->env.gpr[5] = 0;
>  
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index a8d4a6bcf0..a53bfd76f4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1256,6 +1256,14 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu,
>      spapr->fdt_initial_size = spapr->fdt_size;
>      spapr->fdt_blob = fdt;
>  
> +    /*
> +     * Set the machine->fdt pointer again since we just freed
> +     * it above (by freeing spapr->fdt_blob). We set this
> +     * pointer to enable support for 'dumpdtb' and 'info fdt'
> +     * QMP/HMP commands.
> +     */
> +    MACHINE(spapr)->fdt = fdt;
> +
>      return H_SUCCESS;
>  }
>  

-- 
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 --]

Re: [PATCH for-7.2 v3 14/20] qmp/hmp, device_tree.c: introduce dumpdtb

[Dr. David Alan Gilbert]

* Daniel Henrique Barboza (danielhb413@gmail.com) wrote:
> To save the FDT blob we have the '-machine dumpdtb=<file>' property.
> With this property set, the machine saves the FDT in <file> and exit.
> The created file can then be converted to plain text dts format using
> 'dtc'.
> 
> There's nothing particularly sophisticated into saving the FDT that
> can't be done with the machine at any state, as long as the machine has
> a valid FDT to be saved.
> 
> The 'dumpdtb' command receives a 'filename' paramenter and, if a valid
> FDT is available, it'll save it in a file 'filename'. In short, this is
> a '-machine dumpdtb' that can be fired on demand via QMP/HMP.
> 
> A valid FDT consists of a FDT that was created using libfdt being
> retrieved via 'current_machine->fdt' in device_tree.c. This condition is
> met by most FDT users in QEMU.
> 
> This command will always be executed in-band (i.e. holding BQL),
> avoiding potential race conditions with machines that might change the
> FDT during runtime (e.g. PowerPC 'pseries' machine).
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hmp-commands.hx              | 13 +++++++++++++
>  include/monitor/hmp.h        |  1 +
>  include/sysemu/device_tree.h |  1 +
>  monitor/hmp-cmds.c           | 12 ++++++++++++
>  monitor/qmp-cmds.c           | 13 +++++++++++++
>  qapi/machine.json            | 17 +++++++++++++++++
>  softmmu/device_tree.c        | 18 ++++++++++++++++++
>  7 files changed, 75 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 182e639d14..d2554e9701 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1800,3 +1800,16 @@ ERST
>                        "\n\t\t\t\t\t limit on a specified virtual cpu",
>          .cmd        = hmp_cancel_vcpu_dirty_limit,
>      },
> +
> +SRST
> +``dumpdtb`` *filename*
> +  Save the FDT in the 'filename' file to be decoded using dtc.
> +  Requires 'libfdt' support.
> +ERST
> +    {
> +        .name       = "dumpdtb",
> +        .args_type  = "filename:s",

I think you want :F there for a flename
(you still use qdict_get_str but it'll set it up for filename
completion.)

> +        .params     = "[filename] file to save the FDT",
> +        .help       = "save the FDT in the 'filename' file to be decoded using dtc",
> +        .cmd        = hmp_dumpdtb,
> +    },
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index a618eb1e4e..d7f324da59 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -134,6 +134,7 @@ void hmp_calc_dirty_rate(Monitor *mon, const QDict *qdict);
>  void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>  void hmp_cancel_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>  void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
> +void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
>  void hmp_human_readable_text_helper(Monitor *mon,
>                                      HumanReadableText *(*qmp_handler)(Error **));
>  void hmp_info_stats(Monitor *mon, const QDict *qdict);
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index ef060a9759..bf7684e4ed 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -136,6 +136,7 @@ int qemu_fdt_add_path(void *fdt, const char *path);
>      } while (0)
>  
>  void qemu_fdt_dumpdtb(void *fdt, int size);
> +void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp);
>  
>  /**
>   * qemu_fdt_setprop_sized_cells_from_array:
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index c6cd6f91dd..d23ec85f9d 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -2472,3 +2472,15 @@ exit:
>  exit_no_print:
>      error_free(err);
>  }
> +
> +void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
> +{
> +    const char *filename = qdict_get_str(qdict, "filename");
> +    Error *local_err = NULL;
> +
> +    qmp_dumpdtb(filename, &local_err);
> +
> +    if (local_err) {
> +        error_report_err(local_err);
> +    }

That should be hmp_handle_error(

Dave

> +}
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 7314cd813d..8415aca08c 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -45,6 +45,7 @@
>  #include "hw/intc/intc.h"
>  #include "hw/rdma/rdma.h"
>  #include "monitor/stats.h"
> +#include "sysemu/device_tree.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -596,3 +597,15 @@ bool apply_str_list_filter(const char *string, strList *list)
>      }
>      return false;
>  }
> +
> +#ifdef CONFIG_FDT
> +void qmp_dumpdtb(const char *filename, Error **errp)
> +{
> +    return qemu_fdt_qmp_dumpdtb(filename, errp);
> +}
> +#else
> +void qmp_dumpdtb(const char *filename, Error **errp)
> +{
> +    error_setg(errp, "dumpdtb requires libfdt");
> +}
> +#endif
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 6afd1936b0..aeb013f3dd 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1664,3 +1664,20 @@
>       '*size': 'size',
>       '*max-size': 'size',
>       '*slots': 'uint64' } }
> +
> +##
> +# @dumpdtb:
> +#
> +# Save the FDT in dtb format. Requires 'libfdt' support.
> +#
> +# @filename: name of the FDT file to be created
> +#
> +# Since: 7.2
> +#
> +# Example:
> +#   {"execute": "dumpdtb"}
> +#    "arguments": { "filename": "/tmp/fdt.dtb" } }
> +#
> +##
> +{ 'command': 'dumpdtb',
> +  'data': { 'filename': 'str' } }
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 6ca3fad285..cd487ddd4d 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -643,3 +643,21 @@ out:
>      g_free(propcells);
>      return ret;
>  }
> +
> +void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
> +{
> +    int size;
> +
> +    if (!current_machine->fdt) {
> +        error_setg(errp, "Unable to find the machine FDT");
> +        return;
> +    }
> +
> +    size = fdt_totalsize(current_machine->fdt);
> +
> +    if (g_file_set_contents(filename, current_machine->fdt, size, NULL)) {
> +        return;
> +    }
> +
> +    error_setg(errp, "Error when saving machine FDT to file %s", filename);
> +}
> -- 
> 2.37.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH for-7.2 v3 15/20] qmp/hmp, device_tree.c: introduce 'info fdt' command

[Dr. David Alan Gilbert]

* Daniel Henrique Barboza (danielhb413@gmail.com) wrote:
> Reading the FDT requires that the user saves the fdt_blob and then use
> 'dtc' to read the contents. Saving the file and using 'dtc' is a strong
> use case when we need to compare two FDTs, but it's a lot of steps if
> you want to do quick check on a certain node or property.
> 
> 'info fdt' retrieves FDT nodes (and properties, later on) and print it
> to the user. This can be used to check the FDT on running machines
> without having to save the blob and use 'dtc'.
> 
> The implementation is based on the premise that the machine thas a FDT
> created using libfdt and pointed by 'machine->fdt'. As long as this
> pre-requisite is met the machine should be able to support it.
> 
> For now we're going to add the required QMP/HMP boilerplate and the
> capability of printing the name of the properties of a given node. Next
> patches will extend 'info fdt' to be able to print nodes recursively,
> and then individual properties.
> 
> This command will always be executed in-band (i.e. holding BQL),
> avoiding potential race conditions with machines that might change the
> FDT during runtime (e.g. PowerPC 'pseries' machine).
> 
> 'info fdt' is not something that we expect to be used aside from debugging,
> so we're implementing it in QMP as 'x-query-fdt'.
> 
> This is an example of 'info fdt' fetching the '/chosen' node of the
> pSeries machine:
> 
> (qemu) info fdt /chosen
> chosen {
>     ibm,architecture-vec-5;
>     rng-seed;
>     ibm,arch-vec-5-platform-support;
>     linux,pci-probe-only;
>     stdout-path;
>     linux,stdout-path;
>     qemu,graphic-depth;
>     qemu,graphic-height;
>     qemu,graphic-width;
> }
> 
> And the same node for the aarch64 'virt' machine:
> 
> (qemu) info fdt /chosen
> chosen {
>     stdout-path;
>     rng-seed;
>     kaslr-seed;
> }
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

For HMP:
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  hmp-commands-info.hx         | 13 ++++++++++
>  include/monitor/hmp.h        |  1 +
>  include/sysemu/device_tree.h |  4 +++
>  monitor/hmp-cmds.c           | 13 ++++++++++
>  monitor/qmp-cmds.c           | 12 +++++++++
>  qapi/machine.json            | 19 +++++++++++++++
>  softmmu/device_tree.c        | 47 ++++++++++++++++++++++++++++++++++++
>  7 files changed, 109 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 188d9ece3b..743b48865d 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -921,3 +921,16 @@ SRST
>    ``stats``
>      Show runtime-collected statistics
>  ERST
> +
> +    {
> +        .name       = "fdt",
> +        .args_type  = "nodepath:s",
> +        .params     = "nodepath",
> +        .help       = "show firmware device tree node given its path",
> +        .cmd        = hmp_info_fdt,
> +    },
> +
> +SRST
> +  ``info fdt``
> +    Show a firmware device tree node given its path. Requires libfdt.
> +ERST
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index d7f324da59..c0883dd1e3 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -135,6 +135,7 @@ void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>  void hmp_cancel_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>  void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>  void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
> +void hmp_info_fdt(Monitor *mon, const QDict *qdict);
>  void hmp_human_readable_text_helper(Monitor *mon,
>                                      HumanReadableText *(*qmp_handler)(Error **));
>  void hmp_info_stats(Monitor *mon, const QDict *qdict);
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index bf7684e4ed..057d13e397 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -14,6 +14,8 @@
>  #ifndef DEVICE_TREE_H
>  #define DEVICE_TREE_H
>  
> +#include "qapi/qapi-types-common.h"
> +
>  void *create_device_tree(int *sizep);
>  void *load_device_tree(const char *filename_path, int *sizep);
>  #ifdef CONFIG_LINUX
> @@ -137,6 +139,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,
> +                                          Error **errp);
>  
>  /**
>   * qemu_fdt_setprop_sized_cells_from_array:
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index d23ec85f9d..accde90380 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -2484,3 +2484,16 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
>          error_report_err(local_err);
>      }
>  }
> +
> +void hmp_info_fdt(Monitor *mon, const QDict *qdict)
> +{
> +    const char *nodepath = qdict_get_str(qdict, "nodepath");
> +    Error *err = NULL;
> +    g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err);
> +
> +    if (hmp_handle_error(mon, err)) {
> +        return;
> +    }
> +
> +    monitor_printf(mon, "%s", info->human_readable_text);
> +}
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 8415aca08c..db2c6aa7da 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -603,9 +603,21 @@ 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)
> +{
> +    return qemu_fdt_qmp_query_fdt(nodepath, errp);
> +}
>  #else
>  void qmp_dumpdtb(const char *filename, Error **errp)
>  {
>      error_setg(errp, "dumpdtb requires libfdt");
>  }
> +
> +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp)
> +{
> +    error_setg(errp, "this command requires libfdt");
> +
> +    return NULL;
> +}
>  #endif
> diff --git a/qapi/machine.json b/qapi/machine.json
> index aeb013f3dd..96cff541ca 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1681,3 +1681,22 @@
>  ##
>  { 'command': 'dumpdtb',
>    'data': { 'filename': 'str' } }
> +
> +##
> +# @x-query-fdt:
> +#
> +# Query for FDT element (node or property). Requires 'libfdt'.
> +#
> +# @nodepath: the path of the FDT node to be retrieved
> +#
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
> +# Returns: FDT node
> +#
> +# Since: 7.2
> +##
> +{ 'command': 'x-query-fdt',
> +  'data': { 'nodepath': 'str' },
> +  'returns': 'HumanReadableText',
> +  'features': [ 'unstable' ]  }
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index cd487ddd4d..3fb07b537f 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -18,6 +18,7 @@
>  #endif
>  
>  #include "qapi/error.h"
> +#include "qapi/type-helpers.h"
>  #include "qemu/error-report.h"
>  #include "qemu/option.h"
>  #include "qemu/bswap.h"
> @@ -661,3 +662,49 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
>  
>      error_setg(errp, "Error when saving machine FDT to file %s", filename);
>  }
> +
> +static void fdt_format_node(GString *buf, int node, int depth)
> +{
> +    const struct fdt_property *prop = NULL;
> +    const char *propname = NULL;
> +    void *fdt = current_machine->fdt;
> +    int padding = depth * 4;
> +    int property = 0;
> +    int prop_size;
> +
> +    g_string_append_printf(buf, "%*s%s {\n", padding, "",
> +                           fdt_get_name(fdt, node, NULL));
> +
> +    padding += 4;
> +
> +    fdt_for_each_property_offset(property, fdt, node) {
> +        prop = fdt_get_property_by_offset(fdt, property, &prop_size);
> +        propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
> +
> +        g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
> +    }
> +
> +    padding -= 4;
> +    g_string_append_printf(buf, "%*s}\n", padding, "");
> +}
> +
> +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp)
> +{
> +    g_autoptr(GString) buf = g_string_new("");
> +    int node;
> +
> +    if (!current_machine->fdt) {
> +        error_setg(errp, "Unable to find the machine FDT");
> +        return NULL;
> +    }
> +
> +    node = fdt_path_offset(current_machine->fdt, nodepath);
> +    if (node < 0) {
> +        error_setg(errp, "node '%s' not found in FDT", nodepath);
> +        return NULL;
> +    }
> +
> +    fdt_format_node(buf, node, 0);
> +
> +    return human_readable_text_from_str(buf);
> +}
> -- 
> 2.37.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH for-7.2 v3 10/20] hw/ppc: set machine->fdt in spapr machine

[Daniel Henrique Barboza]

On 8/17/22 23:07, David Gibson wrote:
> On Tue, Aug 16, 2022 at 02:34:18PM -0300, Daniel Henrique Barboza wrote:
>> The pSeries machine never bothered with the common machine->fdt
>> attribute. We do all the FDT related work using spapr->fdt_blob.
>>
>> We're going to introduce HMP commands to read and save the FDT, which
>> will rely on setting machine->fdt properly to work across all machine
>> archs/types.
>>
>> Let's set machine->fdt in the two places where we manipulate the FDT:
>> spapr_machine_reset() and CAS.
> 
> So, there's a third place where fdt_blob is updated, in h_update_dt();
> that happens because SLOF can make some updates to the DT that qemu
> needs to be aware of.  It's kinda ugly, and is a consequence of the
> fact that qemu and SLOF kind of share the role of "platform firmware"
> for spapr.
> 
> But.. it's worse than that.  Those are the only 3 places we actually
> alter fdt_blob, but not the only places we logically update the device
> tree.  Up until now there wasn't a way to introspect the DT, and so we
> didn't bother keeping spapr->fdt_blob update.  Essentially, we
> considered maintaining the DT image the job of the guest after CAS.
> 
> Specifically, every dynamic reconfiguration event (hotplug or unplug)
> alters the device tree.  We generate an fdt fragment for the new
> device then stream that as an update to the guest using the PAPR
> specified interface (rtas_ibm_configure_connector).  As noted we
> currently don't update qemu's global fdt image based on that.  On hot
> unplug logically we need to revert those changes, which is actually
> pretty tricky, but currently the guest's job.
> 
> 
> Really, the trouble is that just dumping or viewing the dt is only
> simple in an "embedded" style environment where the fdt is generate
> then spit into the guest.  In an actual open firmware environment like
> spapr, the DT is logically a dynamic thing maintained by firmware -
> but because "firmware"'s responsibility is split between SLOF and
> RTAS/qemu, keeping track of that is pretty nasty.  For an environment
> like this, the flat tree format isn't really suited either - we'd want
> a dynamic representation of the tree.  We get away with flat trees for
> now (barely) only because we mostly delegate the responsibility for
> managing the tree to SLOF and/or the OS kernel, both of which do use
> non-flat representations of the tree.

Thanks for the explanation, but I'm not sure what to do with this patch now.
Should I amend the commit msg to reflect what you explained, mentioning that
we're missing a handful of places where the FDT is updated? We can then come
back at a later time and update ms->fdt in those places as well. This would
be a good follow-up to do after we get rid of spapr->fdt_blob and use ms->fdt
only.

Another alternative is to drop this patch and do spapr in separate later on.
I can live with that, but I'd rather have something for spapr even under the
disclaimer that "this might be not the most up to date FDT the guest is
using".


Thanks,


Daniel

> 
>> spapr->fdt_blob is left untouched for now. To replace it with
>> machine->fdt, since we're migrating spapr->fdt_blob, we would need to
>> migrate machine->fdt as well. This is something that we would like to to
>> do keep our code simpler but it's a work we'll do another day.
>>
>> Cc: Cédric Le Goater <clg@kaod.org>
>> Cc: qemu-ppc@nongnu.org
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   hw/ppc/spapr.c       | 6 ++++++
>>   hw/ppc/spapr_hcall.c | 8 ++++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index bc9ba6e6dc..7031cf964a 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1713,6 +1713,12 @@ static void spapr_machine_reset(MachineState *machine)
>>       spapr->fdt_initial_size = spapr->fdt_size;
>>       spapr->fdt_blob = fdt;
>>   
>> +    /*
>> +     * Set the common machine->fdt pointer to enable support
>> +     * for 'dumpdtb' and 'info fdt' QMP/HMP commands.
>> +     */
>> +    machine->fdt = fdt;
>> +
>>       /* Set up the entry state */
>>       first_ppc_cpu->env.gpr[5] = 0;
>>   
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index a8d4a6bcf0..a53bfd76f4 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1256,6 +1256,14 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu,
>>       spapr->fdt_initial_size = spapr->fdt_size;
>>       spapr->fdt_blob = fdt;
>>   
>> +    /*
>> +     * Set the machine->fdt pointer again since we just freed
>> +     * it above (by freeing spapr->fdt_blob). We set this
>> +     * pointer to enable support for 'dumpdtb' and 'info fdt'
>> +     * QMP/HMP commands.
>> +     */
>> +    MACHINE(spapr)->fdt = fdt;
>> +
>>       return H_SUCCESS;
>>   }
>>   
> 

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

[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@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";


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

(qemu) info fdt /vdevice/v-scsi@71000003 ibm,my-dma-window
/vdevice/v-scsi@71000003/ibm,my-dma-window = <0x71000003 0x0 0x0 0x0 0x10000000>;
(qemu)


I'll update it in v4.

Thanks,

Daniel

> 
>> 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);
>>   }
>