qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/3]  Device tree cleanups
@ 2013-11-11  8:14 peter.crosthwaite
  2013-11-11  8:14 ` [Qemu-devel] [PATCH v2 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: peter.crosthwaite @ 2013-11-11  8:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, edgar.iglesias, agraf, afaerber, david

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Fix the name stem of the devicetree API (P1 - s/qemu_devtree/qemu_fdt)
and cleanup error report (P3). Trivial patch P2 fixing an arugment name
along the way.

Tested using:

1: Alex's e500 test vector.
2: Xilinx Zynq (tests arm/boot.c).

I have testing using Zynq with Mem > 4gb and a bogus dts (size cells = 1)
to give that particular error path some exercise.

To give some exercise to the error paths, I hacked up my libfdt to throw
errors randomly:

--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -48,6 +48,8 @@
  *     OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
  *     EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+
+#include <stdlib.h>
 #include "libfdt_env.h"

 #include <fdt.h>
@@ -279,6 +281,15 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,

    FDT_RW_CHECK_HEADER(fdt);

+    static int seeded = 0;
+    if (!seeded) {
+        srand(time(NULL));
+        seeded = 1;
+    }
+    if (!(rand() & 0x7)) {
+        return -((rand() & 0x3) + 1);
+    }
+

Some sample outputs from e500 boot (Using the above tainted libfdt):

-----
qemu-system-ppc: qemu_fdt_setprop: Couldn't set /memory/reg: FDT_ERR_BADOFFSET

Aborted
-----
qemu-system-ppc: qemu_fdt_setprop_string: Couldn't set /soc@e0000000/device_type = soc: FDT_ERR_NOSPACE

Aborted
-----
qemu-system-ppc: qemu_fdt_setprop_cell: Couldn't set /soc@e0000000/#address-cells = 0x000001: FDT_ERR_NOSPACE

Aborted
-----


Peter Crosthwaite (3):
  device_tree: s/qemu_devtree/qemu_fdt globally
  device_tree: qemu_fdt_setprop: Rename val_array arg
  device_tree: qemu_fdt_setprop: Fixup error reporting

 device_tree.c                | 230 +++++++++++++++++++++++++++--------------
 hw/arm/boot.c                |  51 ++++-----
 hw/arm/vexpress.c            |  23 +++--
 hw/microblaze/boot.c         |  17 ++-
 hw/ppc/e500.c                | 241 ++++++++++++++++++++++---------------------
 hw/ppc/e500.h                |   3 +-
 hw/ppc/e500plat.c            |   9 +-
 hw/ppc/mpc8544ds.c           |   9 +-
 hw/ppc/ppc440_bamboo.c       |  34 ++----
 hw/ppc/spapr_rtas.c          |  40 ++-----
 hw/ppc/virtex_ml507.c        |   5 +-
 include/sysemu/device_tree.h | 219 ++++++++++++++++++++++++++++++---------
 12 files changed, 519 insertions(+), 362 deletions(-)

-- 
1.8.3.rc1.44.gb387c77.dirty

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH v2 1/3] device_tree: s/qemu_devtree/qemu_fdt globally
  2013-11-11  8:14 [Qemu-devel] [PATCH v2 0/3] Device tree cleanups peter.crosthwaite
@ 2013-11-11  8:14 ` peter.crosthwaite
  2013-11-11  8:15 ` [Qemu-devel] [PATCH v2 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: peter.crosthwaite @ 2013-11-11  8:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, edgar.iglesias, agraf, afaerber, david

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

The qemu_devtree API is a wrapper around the fdt_ set of APIs.
Rename accordingly.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 device_tree.c                |  62 ++++++-------
 hw/arm/boot.c                |  24 ++---
 hw/arm/vexpress.c            |  18 ++--
 hw/microblaze/boot.c         |  12 +--
 hw/ppc/e500.c                | 213 ++++++++++++++++++++++---------------------
 hw/ppc/e500plat.c            |   6 +-
 hw/ppc/mpc8544ds.c           |   6 +-
 hw/ppc/ppc440_bamboo.c       |  24 ++---
 hw/ppc/spapr_rtas.c          |  16 ++--
 hw/ppc/virtex_ml507.c        |   2 +-
 include/sysemu/device_tree.h |  80 ++++++++--------
 11 files changed, 232 insertions(+), 231 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index ffec99a..5a13d41 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -127,8 +127,8 @@ static int findnode_nofail(void *fdt, const char *node_path)
     return offset;
 }
 
-int qemu_devtree_setprop(void *fdt, const char *node_path,
-                         const char *property, const void *val_array, int size)
+int qemu_fdt_setprop(void *fdt, const char *node_path,
+                     const char *property, const void *val_array, int size)
 {
     int r;
 
@@ -142,8 +142,8 @@ int qemu_devtree_setprop(void *fdt, const char *node_path,
     return r;
 }
 
-int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
-                              const char *property, uint32_t val)
+int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
+                          const char *property, uint32_t val)
 {
     int r;
 
@@ -157,15 +157,15 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
     return r;
 }
 
-int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
-                             const char *property, uint64_t val)
+int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+                         const char *property, uint64_t val)
 {
     val = cpu_to_be64(val);
-    return qemu_devtree_setprop(fdt, node_path, property, &val, sizeof(val));
+    return qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val));
 }
 
-int qemu_devtree_setprop_string(void *fdt, const char *node_path,
-                                const char *property, const char *string)
+int qemu_fdt_setprop_string(void *fdt, const char *node_path,
+                            const char *property, const char *string)
 {
     int r;
 
@@ -179,8 +179,8 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
     return r;
 }
 
-const void *qemu_devtree_getprop(void *fdt, const char *node_path,
-                                 const char *property, int *lenp)
+const void *qemu_fdt_getprop(void *fdt, const char *node_path,
+                             const char *property, int *lenp)
 {
     int len;
     const void *r;
@@ -196,11 +196,11 @@ const void *qemu_devtree_getprop(void *fdt, const char *node_path,
     return r;
 }
 
-uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path,
-                                   const char *property)
+uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
+                               const char *property)
 {
     int len;
-    const uint32_t *p = qemu_devtree_getprop(fdt, node_path, property, &len);
+    const uint32_t *p = qemu_fdt_getprop(fdt, node_path, property, &len);
     if (len != 4) {
         fprintf(stderr, "%s: %s/%s not 4 bytes long (not a cell?)\n",
                 __func__, node_path, property);
@@ -209,7 +209,7 @@ uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path,
     return be32_to_cpu(*p);
 }
 
-uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
+uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
 {
     uint32_t r;
 
@@ -223,15 +223,15 @@ uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
     return r;
 }
 
-int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
-                                 const char *property,
-                                 const char *target_node_path)
+int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
+                             const char *property,
+                             const char *target_node_path)
 {
-    uint32_t phandle = qemu_devtree_get_phandle(fdt, target_node_path);
-    return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
+    uint32_t phandle = qemu_fdt_get_phandle(fdt, target_node_path);
+    return qemu_fdt_setprop_cell(fdt, node_path, property, phandle);
 }
 
-uint32_t qemu_devtree_alloc_phandle(void *fdt)
+uint32_t qemu_fdt_alloc_phandle(void *fdt)
 {
     static int phandle = 0x0;
 
@@ -255,7 +255,7 @@ uint32_t qemu_devtree_alloc_phandle(void *fdt)
     return phandle++;
 }
 
-int qemu_devtree_nop_node(void *fdt, const char *node_path)
+int qemu_fdt_nop_node(void *fdt, const char *node_path)
 {
     int r;
 
@@ -269,7 +269,7 @@ int qemu_devtree_nop_node(void *fdt, const char *node_path)
     return r;
 }
 
-int qemu_devtree_add_subnode(void *fdt, const char *name)
+int qemu_fdt_add_subnode(void *fdt, const char *name)
 {
     char *dupname = g_strdup(name);
     char *basename = strrchr(dupname, '/');
@@ -299,7 +299,7 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
     return retval;
 }
 
-void qemu_devtree_dumpdtb(void *fdt, int size)
+void qemu_fdt_dumpdtb(void *fdt, int size)
 {
     const char *dumpdtb = qemu_opt_get(qemu_get_machine_opts(), "dumpdtb");
 
@@ -309,11 +309,11 @@ void qemu_devtree_dumpdtb(void *fdt, int size)
     }
 }
 
-int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
-                                                const char *node_path,
-                                                const char *property,
-                                                int numvalues,
-                                                uint64_t *values)
+int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
+                                            const char *node_path,
+                                            const char *property,
+                                            int numvalues,
+                                            uint64_t *values)
 {
     uint32_t *propcells;
     uint64_t value;
@@ -338,6 +338,6 @@ int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
         propcells[cellnum++] = cpu_to_be32(value);
     }
 
-    return qemu_devtree_setprop(fdt, node_path, property, propcells,
-                                cellnum * sizeof(uint32_t));
+    return qemu_fdt_setprop(fdt, node_path, property, propcells,
+                            cellnum * sizeof(uint32_t));
 }
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 583ec79..1a11574 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -246,8 +246,8 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
     }
     g_free(filename);
 
-    acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
-    scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
+    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells");
+    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells");
     if (acells == 0 || scells == 0) {
         fprintf(stderr, "dtb file invalid (#address-cells or #size-cells 0)\n");
         goto fail;
@@ -262,17 +262,17 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
         goto fail;
     }
 
-    rc = qemu_devtree_setprop_sized_cells(fdt, "/memory", "reg",
-                                          acells, binfo->loader_start,
-                                          scells, binfo->ram_size);
+    rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",
+                                      acells, binfo->loader_start,
+                                      scells, binfo->ram_size);
     if (rc < 0) {
         fprintf(stderr, "couldn't set /memory/reg\n");
         goto fail;
     }
 
     if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
-        rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
-                                          binfo->kernel_cmdline);
+        rc = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+                                     binfo->kernel_cmdline);
         if (rc < 0) {
             fprintf(stderr, "couldn't set /chosen/bootargs\n");
             goto fail;
@@ -280,15 +280,15 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
     }
 
     if (binfo->initrd_size) {
-        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                binfo->initrd_start);
+        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                                   binfo->initrd_start);
         if (rc < 0) {
             fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
             goto fail;
         }
 
-        rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                    binfo->initrd_start + binfo->initrd_size);
+        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                                   binfo->initrd_start + binfo->initrd_size);
         if (rc < 0) {
             fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
             goto fail;
@@ -299,7 +299,7 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
         binfo->modify_dtb(binfo, fdt);
     }
 
-    qemu_devtree_dumpdtb(fdt, size);
+    qemu_fdt_dumpdtb(fdt, size);
 
     cpu_physical_memory_write(addr, fdt, size);
 
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index f48de00..05e6c94 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -419,13 +419,13 @@ static int add_virtio_mmio_node(void *fdt, uint32_t acells, uint32_t scells,
     int rc;
     char *nodename = g_strdup_printf("/virtio_mmio@%" PRIx64, addr);
 
-    rc = qemu_devtree_add_subnode(fdt, nodename);
-    rc |= qemu_devtree_setprop_string(fdt, nodename,
-                                      "compatible", "virtio,mmio");
-    rc |= qemu_devtree_setprop_sized_cells(fdt, nodename, "reg",
-                                           acells, addr, scells, size);
-    qemu_devtree_setprop_cells(fdt, nodename, "interrupt-parent", intc);
-    qemu_devtree_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
+    rc = qemu_fdt_add_subnode(fdt, nodename);
+    rc |= qemu_fdt_setprop_string(fdt, nodename,
+                                  "compatible", "virtio,mmio");
+    rc |= qemu_fdt_setprop_sized_cells(fdt, nodename, "reg",
+                                       acells, addr, scells, size);
+    qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", intc);
+    qemu_fdt_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
     g_free(nodename);
     if (rc) {
         return -1;
@@ -456,8 +456,8 @@ static void vexpress_modify_dtb(const struct arm_boot_info *info, void *fdt)
     uint32_t acells, scells, intc;
     const VEDBoardInfo *daughterboard = (const VEDBoardInfo *)info;
 
-    acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
-    scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
+    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells");
+    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells");
     intc = find_int_controller(fdt);
     if (!intc) {
         /* Not fatal, we just won't provide virtio. This will
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 2a7ea5c..48d9e7a 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -79,19 +79,19 @@ static int microblaze_load_dtb(hwaddr addr,
     }
 
     if (kernel_cmdline) {
-        r = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
-                                                        kernel_cmdline);
+        r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+                                    kernel_cmdline);
         if (r < 0) {
             fprintf(stderr, "couldn't set /chosen/bootargs\n");
         }
     }
 
     if (initrd_start) {
-        qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                  initrd_start);
+        qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                              initrd_start);
 
-        qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                  initrd_end);
+        qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                              initrd_end);
     }
 
     cpu_physical_memory_write(addr, fdt, fdt_size);
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index cfdd84b..b37ce9d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -108,18 +108,18 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
     char ser[128];
 
     snprintf(ser, sizeof(ser), "%s/serial@%llx", soc, offset);
-    qemu_devtree_add_subnode(fdt, ser);
-    qemu_devtree_setprop_string(fdt, ser, "device_type", "serial");
-    qemu_devtree_setprop_string(fdt, ser, "compatible", "ns16550");
-    qemu_devtree_setprop_cells(fdt, ser, "reg", offset, 0x100);
-    qemu_devtree_setprop_cell(fdt, ser, "cell-index", idx);
-    qemu_devtree_setprop_cell(fdt, ser, "clock-frequency", 0);
-    qemu_devtree_setprop_cells(fdt, ser, "interrupts", 42, 2);
-    qemu_devtree_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
-    qemu_devtree_setprop_string(fdt, "/aliases", alias, ser);
+    qemu_fdt_add_subnode(fdt, ser);
+    qemu_fdt_setprop_string(fdt, ser, "device_type", "serial");
+    qemu_fdt_setprop_string(fdt, ser, "compatible", "ns16550");
+    qemu_fdt_setprop_cells(fdt, ser, "reg", offset, 0x100);
+    qemu_fdt_setprop_cell(fdt, ser, "cell-index", idx);
+    qemu_fdt_setprop_cell(fdt, ser, "clock-frequency", 0);
+    qemu_fdt_setprop_cells(fdt, ser, "interrupts", 42, 2);
+    qemu_fdt_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
+    qemu_fdt_setprop_string(fdt, "/aliases", alias, ser);
 
     if (defcon) {
-        qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser);
+        qemu_fdt_setprop_string(fdt, "/chosen", "linux,stdout-path", ser);
     }
 }
 
@@ -183,30 +183,30 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
     }
 
     /* Manipulate device tree in memory. */
-    qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 2);
-    qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 2);
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 2);
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 2);
 
-    qemu_devtree_add_subnode(fdt, "/memory");
-    qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
-    qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
-                         sizeof(mem_reg_property));
+    qemu_fdt_add_subnode(fdt, "/memory");
+    qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
+    qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+                     sizeof(mem_reg_property));
 
-    qemu_devtree_add_subnode(fdt, "/chosen");
+    qemu_fdt_add_subnode(fdt, "/chosen");
     if (initrd_size) {
-        ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                        initrd_base);
+        ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                                    initrd_base);
         if (ret < 0) {
             fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
         }
 
-        ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                        (initrd_base + initrd_size));
+        ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                                    (initrd_base + initrd_size));
         if (ret < 0) {
             fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
         }
     }
 
-    ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
                                       args->kernel_cmdline);
     if (ret < 0)
         fprintf(stderr, "couldn't set /chosen/bootargs\n");
@@ -217,22 +217,22 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
         tb_freq = kvmppc_get_tbfreq();
 
         /* indicate KVM hypercall interface */
-        qemu_devtree_add_subnode(fdt, "/hypervisor");
-        qemu_devtree_setprop_string(fdt, "/hypervisor", "compatible",
-                                    "linux,kvm");
+        qemu_fdt_add_subnode(fdt, "/hypervisor");
+        qemu_fdt_setprop_string(fdt, "/hypervisor", "compatible",
+                                "linux,kvm");
         kvmppc_get_hypercall(env, hypercall, sizeof(hypercall));
-        qemu_devtree_setprop(fdt, "/hypervisor", "hcall-instructions",
-                             hypercall, sizeof(hypercall));
+        qemu_fdt_setprop(fdt, "/hypervisor", "hcall-instructions",
+                         hypercall, sizeof(hypercall));
         /* if KVM supports the idle hcall, set property indicating this */
         if (kvmppc_get_hasidle(env)) {
-            qemu_devtree_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
+            qemu_fdt_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
         }
     }
 
     /* Create CPU nodes */
-    qemu_devtree_add_subnode(fdt, "/cpus");
-    qemu_devtree_setprop_cell(fdt, "/cpus", "#address-cells", 1);
-    qemu_devtree_setprop_cell(fdt, "/cpus", "#size-cells", 0);
+    qemu_fdt_add_subnode(fdt, "/cpus");
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 1);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0);
 
     /* We need to generate the cpu nodes in reverse order, so Linux can pick
        the first node as boot node and be happy */
@@ -249,55 +249,56 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
 
         snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
                  cpu->cpu_index);
-        qemu_devtree_add_subnode(fdt, cpu_name);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
-        qemu_devtree_setprop_string(fdt, cpu_name, "device_type", "cpu");
-        qemu_devtree_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "d-cache-line-size",
-                                  env->dcache_line_size);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "i-cache-line-size",
-                                  env->icache_line_size);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "d-cache-size", 0x8000);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "i-cache-size", 0x8000);
-        qemu_devtree_setprop_cell(fdt, cpu_name, "bus-frequency", 0);
+        qemu_fdt_add_subnode(fdt, cpu_name);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
+        qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
+        qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-line-size",
+                              env->dcache_line_size);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-line-size",
+                              env->icache_line_size);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-size", 0x8000);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-size", 0x8000);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "bus-frequency", 0);
         if (cpu->cpu_index) {
-            qemu_devtree_setprop_string(fdt, cpu_name, "status", "disabled");
-            qemu_devtree_setprop_string(fdt, cpu_name, "enable-method", "spin-table");
-            qemu_devtree_setprop_u64(fdt, cpu_name, "cpu-release-addr",
-                                     cpu_release_addr);
+            qemu_fdt_setprop_string(fdt, cpu_name, "status", "disabled");
+            qemu_fdt_setprop_string(fdt, cpu_name, "enable-method",
+                                    "spin-table");
+            qemu_fdt_setprop_u64(fdt, cpu_name, "cpu-release-addr",
+                                 cpu_release_addr);
         } else {
-            qemu_devtree_setprop_string(fdt, cpu_name, "status", "okay");
+            qemu_fdt_setprop_string(fdt, cpu_name, "status", "okay");
         }
     }
 
-    qemu_devtree_add_subnode(fdt, "/aliases");
+    qemu_fdt_add_subnode(fdt, "/aliases");
     /* XXX These should go into their respective devices' code */
     snprintf(soc, sizeof(soc), "/soc@%llx", MPC8544_CCSRBAR_BASE);
-    qemu_devtree_add_subnode(fdt, soc);
-    qemu_devtree_setprop_string(fdt, soc, "device_type", "soc");
-    qemu_devtree_setprop(fdt, soc, "compatible", compatible_sb,
-                         sizeof(compatible_sb));
-    qemu_devtree_setprop_cell(fdt, soc, "#address-cells", 1);
-    qemu_devtree_setprop_cell(fdt, soc, "#size-cells", 1);
-    qemu_devtree_setprop_cells(fdt, soc, "ranges", 0x0,
-                               MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
-                               MPC8544_CCSRBAR_SIZE);
+    qemu_fdt_add_subnode(fdt, soc);
+    qemu_fdt_setprop_string(fdt, soc, "device_type", "soc");
+    qemu_fdt_setprop(fdt, soc, "compatible", compatible_sb,
+                     sizeof(compatible_sb));
+    qemu_fdt_setprop_cell(fdt, soc, "#address-cells", 1);
+    qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1);
+    qemu_fdt_setprop_cells(fdt, soc, "ranges", 0x0,
+                           MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
+                           MPC8544_CCSRBAR_SIZE);
     /* XXX should contain a reasonable value */
-    qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
+    qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0);
 
     snprintf(mpic, sizeof(mpic), "%s/pic@%llx", soc, MPC8544_MPIC_REGS_OFFSET);
-    qemu_devtree_add_subnode(fdt, mpic);
-    qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
-    qemu_devtree_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
-    qemu_devtree_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
-                               0x40000);
-    qemu_devtree_setprop_cell(fdt, mpic, "#address-cells", 0);
-    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
-    mpic_ph = qemu_devtree_alloc_phandle(fdt);
-    qemu_devtree_setprop_cell(fdt, mpic, "phandle", mpic_ph);
-    qemu_devtree_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
-    qemu_devtree_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
+    qemu_fdt_add_subnode(fdt, mpic);
+    qemu_fdt_setprop_string(fdt, mpic, "device_type", "open-pic");
+    qemu_fdt_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
+    qemu_fdt_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
+                           0x40000);
+    qemu_fdt_setprop_cell(fdt, mpic, "#address-cells", 0);
+    qemu_fdt_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
+    mpic_ph = qemu_fdt_alloc_phandle(fdt);
+    qemu_fdt_setprop_cell(fdt, mpic, "phandle", mpic_ph);
+    qemu_fdt_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
+    qemu_fdt_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
 
     /*
      * We have to generate ser1 first, because Linux takes the first
@@ -311,19 +312,19 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
 
     snprintf(gutil, sizeof(gutil), "%s/global-utilities@%llx", soc,
              MPC8544_UTIL_OFFSET);
-    qemu_devtree_add_subnode(fdt, gutil);
-    qemu_devtree_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
-    qemu_devtree_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
-    qemu_devtree_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
+    qemu_fdt_add_subnode(fdt, gutil);
+    qemu_fdt_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
+    qemu_fdt_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
+    qemu_fdt_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
 
     snprintf(msi, sizeof(msi), "/%s/msi@%llx", soc, MPC8544_MSI_REGS_OFFSET);
-    qemu_devtree_add_subnode(fdt, msi);
-    qemu_devtree_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
-    qemu_devtree_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
-    msi_ph = qemu_devtree_alloc_phandle(fdt);
-    qemu_devtree_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
-    qemu_devtree_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
-    qemu_devtree_setprop_cells(fdt, msi, "interrupts",
+    qemu_fdt_add_subnode(fdt, msi);
+    qemu_fdt_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
+    qemu_fdt_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
+    msi_ph = qemu_fdt_alloc_phandle(fdt);
+    qemu_fdt_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
+    qemu_fdt_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
+    qemu_fdt_setprop_cells(fdt, msi, "interrupts",
         0xe0, 0x0,
         0xe1, 0x0,
         0xe2, 0x0,
@@ -332,46 +333,46 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
         0xe5, 0x0,
         0xe6, 0x0,
         0xe7, 0x0);
-    qemu_devtree_setprop_cell(fdt, msi, "phandle", msi_ph);
-    qemu_devtree_setprop_cell(fdt, msi, "linux,phandle", msi_ph);
+    qemu_fdt_setprop_cell(fdt, msi, "phandle", msi_ph);
+    qemu_fdt_setprop_cell(fdt, msi, "linux,phandle", msi_ph);
 
     snprintf(pci, sizeof(pci), "/pci@%llx", MPC8544_PCI_REGS_BASE);
-    qemu_devtree_add_subnode(fdt, pci);
-    qemu_devtree_setprop_cell(fdt, pci, "cell-index", 0);
-    qemu_devtree_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
-    qemu_devtree_setprop_string(fdt, pci, "device_type", "pci");
-    qemu_devtree_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
-                               0x0, 0x7);
-    pci_map = pci_map_create(fdt, qemu_devtree_get_phandle(fdt, mpic),
+    qemu_fdt_add_subnode(fdt, pci);
+    qemu_fdt_setprop_cell(fdt, pci, "cell-index", 0);
+    qemu_fdt_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
+    qemu_fdt_setprop_string(fdt, pci, "device_type", "pci");
+    qemu_fdt_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
+                           0x0, 0x7);
+    pci_map = pci_map_create(fdt, qemu_fdt_get_phandle(fdt, mpic),
                              params->pci_first_slot, params->pci_nr_slots,
                              &len);
-    qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, len);
-    qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
-    qemu_devtree_setprop_cells(fdt, pci, "interrupts", 24, 2);
-    qemu_devtree_setprop_cells(fdt, pci, "bus-range", 0, 255);
+    qemu_fdt_setprop(fdt, pci, "interrupt-map", pci_map, len);
+    qemu_fdt_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
+    qemu_fdt_setprop_cells(fdt, pci, "interrupts", 24, 2);
+    qemu_fdt_setprop_cells(fdt, pci, "bus-range", 0, 255);
     for (i = 0; i < 14; i++) {
         pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
     }
-    qemu_devtree_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
-    qemu_devtree_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
-    qemu_devtree_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
-                               MPC8544_PCI_REGS_BASE, 0, 0x1000);
-    qemu_devtree_setprop_cell(fdt, pci, "clock-frequency", 66666666);
-    qemu_devtree_setprop_cell(fdt, pci, "#interrupt-cells", 1);
-    qemu_devtree_setprop_cell(fdt, pci, "#size-cells", 2);
-    qemu_devtree_setprop_cell(fdt, pci, "#address-cells", 3);
-    qemu_devtree_setprop_string(fdt, "/aliases", "pci0", pci);
+    qemu_fdt_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
+    qemu_fdt_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
+    qemu_fdt_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
+                           MPC8544_PCI_REGS_BASE, 0, 0x1000);
+    qemu_fdt_setprop_cell(fdt, pci, "clock-frequency", 66666666);
+    qemu_fdt_setprop_cell(fdt, pci, "#interrupt-cells", 1);
+    qemu_fdt_setprop_cell(fdt, pci, "#size-cells", 2);
+    qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3);
+    qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci);
 
     params->fixup_devtree(params, fdt);
 
     if (toplevel_compat) {
-        qemu_devtree_setprop(fdt, "/", "compatible", toplevel_compat,
-                             strlen(toplevel_compat) + 1);
+        qemu_fdt_setprop(fdt, "/", "compatible", toplevel_compat,
+                         strlen(toplevel_compat) + 1);
     }
 
 done:
     if (!dry_run) {
-        qemu_devtree_dumpdtb(fdt, fdt_size);
+        qemu_fdt_dumpdtb(fdt, fdt_size);
         cpu_physical_memory_write(addr, fdt, fdt_size);
     }
     ret = fdt_size;
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index 2e964b2..7d5357e 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -23,9 +23,9 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
     const char model[] = "QEMU ppce500";
     const char compatible[] = "fsl,qemu-e500";
 
-    qemu_devtree_setprop(fdt, "/", "model", model, sizeof(model));
-    qemu_devtree_setprop(fdt, "/", "compatible", compatible,
-                         sizeof(compatible));
+    qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
+    qemu_fdt_setprop(fdt, "/", "compatible", compatible,
+                     sizeof(compatible));
 }
 
 static void e500plat_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index edcc0be..292c709 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -21,9 +21,9 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
     const char model[] = "MPC8544DS";
     const char compatible[] = "MPC8544DS\0MPC85xxDS";
 
-    qemu_devtree_setprop(fdt, "/", "model", model, sizeof(model));
-    qemu_devtree_setprop(fdt, "/", "compatible", compatible,
-                         sizeof(compatible));
+    qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
+    qemu_fdt_setprop(fdt, "/", "compatible", compatible,
+                     sizeof(compatible));
 }
 
 static void mpc8544ds_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 655e499..22e121f 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -77,23 +77,23 @@ static int bamboo_load_device_tree(hwaddr addr,
 
     /* Manipulate device tree in memory. */
 
-    ret = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
-                               sizeof(mem_reg_property));
+    ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+                           sizeof(mem_reg_property));
     if (ret < 0)
         fprintf(stderr, "couldn't set /memory/reg\n");
 
-    ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                    initrd_base);
+    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                                initrd_base);
     if (ret < 0)
         fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
 
-    ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                    (initrd_base + initrd_size));
+    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                                (initrd_base + initrd_size));
     if (ret < 0)
         fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
 
-    ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
-                                      kernel_cmdline);
+    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+                                  kernel_cmdline);
     if (ret < 0)
         fprintf(stderr, "couldn't set /chosen/bootargs\n");
 
@@ -105,10 +105,10 @@ static int bamboo_load_device_tree(hwaddr addr,
         clock_freq = kvmppc_get_clockfreq();
     }
 
-    qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
-                              clock_freq);
-    qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
-                              tb_freq);
+    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
+                          clock_freq);
+    qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
+                          tb_freq);
 
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     g_free(fdt);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index eb542f2..ce8e697 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -291,24 +291,24 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
         return ret;
     }
 
-    ret = qemu_devtree_setprop_cell(fdt, "/rtas", "linux,rtas-base",
-                                    rtas_addr);
+    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
+                                rtas_addr);
     if (ret < 0) {
         fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
                 fdt_strerror(ret));
         return ret;
     }
 
-    ret = qemu_devtree_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
-                                    rtas_addr);
+    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
+                                rtas_addr);
     if (ret < 0) {
         fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
                 fdt_strerror(ret));
         return ret;
     }
 
-    ret = qemu_devtree_setprop_cell(fdt, "/rtas", "rtas-size",
-                                    rtas_size);
+    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
+                                rtas_size);
     if (ret < 0) {
         fprintf(stderr, "Couldn't add rtas-size property: %s\n",
                 fdt_strerror(ret));
@@ -322,8 +322,8 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
             continue;
         }
 
-        ret = qemu_devtree_setprop_cell(fdt, "/rtas", call->name,
-                                        i + TOKEN_BASE);
+        ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
+                                    i + TOKEN_BASE);
         if (ret < 0) {
             fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
                     call->name, fdt_strerror(ret));
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index fcfa678..bdb057e 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -166,7 +166,7 @@ static int xilinx_load_device_tree(hwaddr addr,
     if (!fdt) {
         return 0;
     }
-    r = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
+    r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
     if (r < 0)
         fprintf(stderr, "couldn't set /chosen/bootargs\n");
     cpu_physical_memory_write(addr, fdt, fdt_size);
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 2b58baf..68da97a 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -17,27 +17,27 @@
 void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 
-int qemu_devtree_setprop(void *fdt, const char *node_path,
-                         const char *property, const void *val_array, int size);
-int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
-                              const char *property, uint32_t val);
-int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
-                             const char *property, uint64_t val);
-int qemu_devtree_setprop_string(void *fdt, const char *node_path,
-                                const char *property, const char *string);
-int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
-                                 const char *property,
-                                 const char *target_node_path);
-const void *qemu_devtree_getprop(void *fdt, const char *node_path,
-                                 const char *property, int *lenp);
-uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path,
-                                   const char *property);
-uint32_t qemu_devtree_get_phandle(void *fdt, const char *path);
-uint32_t qemu_devtree_alloc_phandle(void *fdt);
-int qemu_devtree_nop_node(void *fdt, const char *node_path);
-int qemu_devtree_add_subnode(void *fdt, const char *name);
+int qemu_fdt_setprop(void *fdt, const char *node_path,
+                     const char *property, const void *val_array, int size);
+int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
+                          const char *property, uint32_t val);
+int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+                         const char *property, uint64_t val);
+int qemu_fdt_setprop_string(void *fdt, const char *node_path,
+                            const char *property, const char *string);
+int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
+                             const char *property,
+                             const char *target_node_path);
+const void *qemu_fdt_getprop(void *fdt, const char *node_path,
+                             const char *property, int *lenp);
+uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
+                               const char *property);
+uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
+uint32_t qemu_fdt_alloc_phandle(void *fdt);
+int qemu_fdt_nop_node(void *fdt, const char *node_path);
+int qemu_fdt_add_subnode(void *fdt, const char *name);
 
-#define qemu_devtree_setprop_cells(fdt, node_path, property, ...)             \
+#define qemu_fdt_setprop_cells(fdt, node_path, property, ...)                 \
     do {                                                                      \
         uint32_t qdt_tmp[] = { __VA_ARGS__ };                                 \
         int i;                                                                \
@@ -45,14 +45,14 @@ int qemu_devtree_add_subnode(void *fdt, const char *name);
         for (i = 0; i < ARRAY_SIZE(qdt_tmp); i++) {                           \
             qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]);                             \
         }                                                                     \
-        qemu_devtree_setprop(fdt, node_path, property, qdt_tmp,               \
-                             sizeof(qdt_tmp));                                \
+        qemu_fdt_setprop(fdt, node_path, property, qdt_tmp,                   \
+                         sizeof(qdt_tmp));                                    \
     } while (0)
 
-void qemu_devtree_dumpdtb(void *fdt, int size);
+void qemu_fdt_dumpdtb(void *fdt, int size);
 
 /**
- * qemu_devtree_setprop_sized_cells_from_array:
+ * qemu_fdt_setprop_sized_cells_from_array:
  * @fdt: device tree blob
  * @node_path: node to set property on
  * @property: property to set
@@ -72,20 +72,20 @@ void qemu_devtree_dumpdtb(void *fdt, int size);
  * the number of cells used for each element vary depending on the
  * #address-cells and #size-cells properties of their parent node.
  * If you know all your cell elements are one cell wide you can use the
- * simpler qemu_devtree_setprop_cells(). If you're not setting up the
- * array programmatically, qemu_devtree_setprop_sized_cells may be more
+ * simpler qemu_fdt_setprop_cells(). If you're not setting up the
+ * array programmatically, qemu_fdt_setprop_sized_cells may be more
  * convenient.
  *
  * Return value: 0 on success, <0 on error.
  */
-int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
-                                                const char *node_path,
-                                                const char *property,
-                                                int numvalues,
-                                                uint64_t *values);
+int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
+                                            const char *node_path,
+                                            const char *property,
+                                            int numvalues,
+                                            uint64_t *values);
 
 /**
- * qemu_devtree_setprop_sized_cells:
+ * qemu_fdt_setprop_sized_cells:
  * @fdt: device tree blob
  * @node_path: node to set property on
  * @property: property to set
@@ -97,17 +97,17 @@ int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
  * used by this value" and "value".
  *
  * This is a convenience wrapper for the function
- * qemu_devtree_setprop_sized_cells_from_array().
+ * qemu_fdt_setprop_sized_cells_from_array().
  *
  * Return value: 0 on success, <0 on error.
  */
-#define qemu_devtree_setprop_sized_cells(fdt, node_path, property, ...)       \
-    ({                                                                        \
-        uint64_t qdt_tmp[] = { __VA_ARGS__ };                                 \
-        qemu_devtree_setprop_sized_cells_from_array(fdt, node_path,           \
-                                                    property,                 \
-                                                    ARRAY_SIZE(qdt_tmp) / 2,  \
-                                                    qdt_tmp);                 \
+#define qemu_fdt_setprop_sized_cells(fdt, node_path, property, ...)       \
+    ({                                                                    \
+        uint64_t qdt_tmp[] = { __VA_ARGS__ };                             \
+        qemu_fdt_setprop_sized_cells_from_array(fdt, node_path,           \
+                                                property,                 \
+                                                ARRAY_SIZE(qdt_tmp) / 2,  \
+                                                qdt_tmp);                 \
     })
 
 #endif /* __DEVICE_TREE_H__ */
-- 
1.8.3.rc1.44.gb387c77.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH v2 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg
  2013-11-11  8:14 [Qemu-devel] [PATCH v2 0/3] Device tree cleanups peter.crosthwaite
  2013-11-11  8:14 ` [Qemu-devel] [PATCH v2 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
@ 2013-11-11  8:15 ` peter.crosthwaite
  2013-11-11  8:16 ` [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
  2013-11-27 13:10 ` [Qemu-devel] [PATCH v2 0/3] Device tree cleanups Peter Crosthwaite
  3 siblings, 0 replies; 7+ messages in thread
From: peter.crosthwaite @ 2013-11-11  8:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, edgar.iglesias, agraf, afaerber, david

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Looking at the implementation, this doesn't really have a lot to do
with arrays. Its just a pointer to a buffer and is passed through
to the wrapped fn (qemu_fdt_setprop) unchanged. So rename to make it
consistent with libfdt, which in the wrapped function just calls it
"val".

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 device_tree.c                | 4 ++--
 include/sysemu/device_tree.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 5a13d41..1e6bbad 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -128,11 +128,11 @@ static int findnode_nofail(void *fdt, const char *node_path)
 }
 
 int qemu_fdt_setprop(void *fdt, const char *node_path,
-                     const char *property, const void *val_array, int size)
+                     const char *property, const void *val, int size)
 {
     int r;
 
-    r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val_array, size);
+    r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size);
     if (r < 0) {
         fprintf(stderr, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
                 property, fdt_strerror(r));
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 68da97a..899f05c 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -18,7 +18,7 @@ void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 
 int qemu_fdt_setprop(void *fdt, const char *node_path,
-                     const char *property, const void *val_array, int size);
+                     const char *property, const void *val, int size);
 int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
                           const char *property, uint32_t val);
 int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
-- 
1.8.3.rc1.44.gb387c77.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
  2013-11-11  8:14 [Qemu-devel] [PATCH v2 0/3] Device tree cleanups peter.crosthwaite
  2013-11-11  8:14 ` [Qemu-devel] [PATCH v2 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
  2013-11-11  8:15 ` [Qemu-devel] [PATCH v2 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
@ 2013-11-11  8:16 ` peter.crosthwaite
  2013-12-18 13:48   ` Alexander Graf
  2013-11-27 13:10 ` [Qemu-devel] [PATCH v2 0/3] Device tree cleanups Peter Crosthwaite
  3 siblings, 1 reply; 7+ messages in thread
From: peter.crosthwaite @ 2013-11-11  8:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, edgar.iglesias, agraf, afaerber, david

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

There are a mix of usages of the qemu_fdt_* API calls, some which
wish to assert and abort QEMU on failure and some of which wish to do
their own error handling. The latter in more correct, but the former
is in the majority and more pragmatic. However the asserting clients
are usually doing large numbers fdt ops and only want to assert if any
one of them breaks. So the cleanest compromising solution is:

1. Require an Error ** to be passes to all fdt ops.
2. And perform no action if its already non-null (error condition).
3. If no Error ** is given report errors to stderr

Step one allows clients to do their own error handling if they wish
too, which is the most flexible:

Error *err = NULL;
fdt_foo( ... , &err);
if (err) {
    /* handle error my special way */
}

Step two allows you to do a large number of fdt ops then check them
all only the once at the end:

Error *err = NULL;
fdt_foo( ... , &err);
fdt_bar( ... , &err);
fdt_baz( ... , &err);
if (err) {
    /* First encountered error will be in err */
}

Step 3, handles the common case where the error is not an issue, but
just needs to make noise at the user (via stderr). This seems common for
bootloader code that sets chosen and initrd props etc (rather than machine
description).

All error reporting is stylistically udpated to use Error ** instead of
integer return codes and no more exit(1) on failures.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 device_tree.c                | 222 ++++++++++++++++++++++++++++---------------
 hw/arm/boot.c                |  49 ++++------
 hw/arm/vexpress.c            |  23 ++---
 hw/microblaze/boot.c         |  13 +--
 hw/ppc/e500.c                | 214 ++++++++++++++++++++---------------------
 hw/ppc/e500.h                |   3 +-
 hw/ppc/e500plat.c            |   9 +-
 hw/ppc/mpc8544ds.c           |   9 +-
 hw/ppc/ppc440_bamboo.c       |  30 ++----
 hw/ppc/spapr_rtas.c          |  40 ++------
 hw/ppc/virtex_ml507.c        |   5 +-
 include/sysemu/device_tree.h | 193 ++++++++++++++++++++++++++++++-------
 12 files changed, 483 insertions(+), 327 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 1e6bbad..525cdaa 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -4,8 +4,10 @@
  * interface.
  *
  * Copyright 2008 IBM Corporation.
+ * Copyright 2013 Xilinx Inc.
  * Authors: Jerone Young <jyoung5@us.ibm.com>
  *          Hollis Blanchard <hollisb@us.ibm.com>
+ *          Peter Crosthwaite <peter.crosthwaite@xilinx.com>
  *
  * This work is licensed under the GNU GPL license version 2 or later.
  *
@@ -113,122 +115,176 @@ fail:
     return NULL;
 }
 
-static int findnode_nofail(void *fdt, const char *node_path)
+#define error_set_or_print(errp, ...)                               \
+    do {                                                            \
+        if (errp) {                                                 \
+            error_setg(errp, __VA_ARGS__);                          \
+        } else {                                                    \
+            fprintf(stderr, __VA_ARGS__);                           \
+        }                                                           \
+    } while (0)
+
+static int findnode(void *fdt, const char *node_path, Error **errp)
 {
     int offset;
 
     offset = fdt_path_offset(fdt, node_path);
     if (offset < 0) {
-        fprintf(stderr, "%s Couldn't find node %s: %s\n", __func__, node_path,
-                fdt_strerror(offset));
-        exit(1);
+        error_set_or_print(errp, "%s Couldn't find node %s: %s\n", __func__,
+                           node_path, fdt_strerror(offset));
     }
 
     return offset;
 }
 
-int qemu_fdt_setprop(void *fdt, const char *node_path,
-                     const char *property, const void *val, int size)
+void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
+                      const void *val, int size, Error **errp)
 {
     int r;
 
-    r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size);
-    if (r < 0) {
-        fprintf(stderr, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
-                property, fdt_strerror(r));
-        exit(1);
+    if (errp && *errp) {
+        return;
     }
 
-    return r;
+    r = findnode(fdt, node_path, errp);
+    if (r < 0) {
+        return;
+    }
+    r = fdt_setprop(fdt, r, property, val, size);
+    if (r < 0) {
+        error_set_or_print(errp, "%s: Couldn't set %s/%s: %s\n", __func__,
+                           node_path, property, fdt_strerror(r));
+    }
 }
 
-int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
-                          const char *property, uint32_t val)
+void qemu_fdt_setprop_cell(void *fdt, const char *node_path,
+                           const char *property, uint32_t val, Error **errp)
 {
     int r;
 
-    r = fdt_setprop_cell(fdt, findnode_nofail(fdt, node_path), property, val);
-    if (r < 0) {
-        fprintf(stderr, "%s: Couldn't set %s/%s = %#08x: %s\n", __func__,
-                node_path, property, val, fdt_strerror(r));
-        exit(1);
+    if (errp && *errp) {
+        return;
     }
 
-    return r;
+    r = findnode(fdt, node_path, errp);
+    if (r < 0) {
+        return;
+    }
+    r = fdt_setprop_cell(fdt, r, property, val);
+    if (r < 0) {
+        error_set_or_print(errp, "%s: Couldn't set %s/%s = %#08x: %s\n",
+                           __func__, node_path, property, val, fdt_strerror(r));
+    }
 }
 
-int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
-                         const char *property, uint64_t val)
+void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+                          const char *property, uint64_t val, Error **errp)
 {
     val = cpu_to_be64(val);
-    return qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val));
+    qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val), errp);
 }
 
-int qemu_fdt_setprop_string(void *fdt, const char *node_path,
-                            const char *property, const char *string)
+void qemu_fdt_setprop_string(void *fdt, const char *node_path,
+                             const char *property, const char *string,
+                             Error **errp)
 {
     int r;
 
-    r = fdt_setprop_string(fdt, findnode_nofail(fdt, node_path), property, string);
-    if (r < 0) {
-        fprintf(stderr, "%s: Couldn't set %s/%s = %s: %s\n", __func__,
-                node_path, property, string, fdt_strerror(r));
-        exit(1);
+    if (errp && *errp) {
+        return;
     }
 
-    return r;
+    r = findnode(fdt, node_path, errp);
+    if (r < 0) {
+        return;
+    }
+    r = fdt_setprop_string(fdt, r, property, string);
+    if (r < 0) {
+        error_set_or_print(errp, "%s: Couldn't set %s/%s = %s: %s\n", __func__,
+                           node_path, property, string, fdt_strerror(r));
+    }
 }
 
 const void *qemu_fdt_getprop(void *fdt, const char *node_path,
-                             const char *property, int *lenp)
+                             const char *property, int *lenp, Error **errp)
 {
     int len;
+    int node;
     const void *r;
+
+    if (errp && *errp) {
+        return NULL;
+    }
+
     if (!lenp) {
         lenp = &len;
     }
-    r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);
+
+    node = findnode(fdt, node_path, errp);
+    if (node < 0) {
+        return NULL;
+    }
+    r = fdt_getprop(fdt, node, property, lenp);
     if (!r) {
-        fprintf(stderr, "%s: Couldn't get %s/%s: %s\n", __func__,
-                node_path, property, fdt_strerror(*lenp));
-        exit(1);
+        error_set_or_print(errp, "%s: Couldn't get %s/%s: %s\n", __func__,
+                           node_path, property, fdt_strerror(*lenp));
+        return NULL;
     }
     return r;
 }
 
 uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
-                               const char *property)
+                               const char *property, Error **errp)
 {
     int len;
-    const uint32_t *p = qemu_fdt_getprop(fdt, node_path, property, &len);
+    const uint32_t *p;
+
+    if (errp && *errp) {
+        return 0;
+    }
+
+    p = qemu_fdt_getprop(fdt, node_path, property, &len, errp);
+    if (!p) {
+        return 0;
+    }
+
     if (len != 4) {
-        fprintf(stderr, "%s: %s/%s not 4 bytes long (not a cell?)\n",
-                __func__, node_path, property);
-        exit(1);
+        error_set_or_print(errp, "%s: %s/%s not 4 bytes long (not a cell?)\n",
+                           __func__, node_path, property);
+        return 0;
     }
     return be32_to_cpu(*p);
 }
 
-uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
+uint32_t qemu_fdt_get_phandle(void *fdt, const char *path, Error **errp)
 {
+    int node;
     uint32_t r;
 
-    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
+    if (errp && *errp) {
+        return 0;
+    }
+
+    node = findnode(fdt, path, errp);
+    if (node < 0) {
+        return 0;
+    }
+    r = fdt_get_phandle(fdt, node);
     if (r == 0) {
-        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n", __func__,
-                path, fdt_strerror(r));
-        exit(1);
+        error_set_or_print(errp, "%s: Couldn't get phandle for %s: %s\n",
+                           __func__, path, fdt_strerror(r));
+        return 0;
     }
 
     return r;
 }
 
-int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
-                             const char *property,
-                             const char *target_node_path)
+void qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
+                              const char *property,
+                              const char *target_node_path, Error **errp)
 {
-    uint32_t phandle = qemu_fdt_get_phandle(fdt, target_node_path);
-    return qemu_fdt_setprop_cell(fdt, node_path, property, phandle);
+    uint32_t phandle = qemu_fdt_get_phandle(fdt, target_node_path, errp);
+    qemu_fdt_setprop_cell(fdt, node_path, property, phandle, errp);
 }
 
 uint32_t qemu_fdt_alloc_phandle(void *fdt)
@@ -255,48 +311,54 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt)
     return phandle++;
 }
 
-int qemu_fdt_nop_node(void *fdt, const char *node_path)
+void qemu_fdt_nop_node(void *fdt, const char *node_path, Error **errp)
 {
     int r;
 
-    r = fdt_nop_node(fdt, findnode_nofail(fdt, node_path));
-    if (r < 0) {
-        fprintf(stderr, "%s: Couldn't nop node %s: %s\n", __func__, node_path,
-                fdt_strerror(r));
-        exit(1);
+    if (errp && *errp) {
+        return;
     }
 
-    return r;
+    r = findnode(fdt, node_path, errp);
+    if (r < 0) {
+        return;
+    }
+    r = fdt_nop_node(fdt, r);
+    if (r < 0) {
+        error_set_or_print(errp, "%s: Couldn't nop node %s: %s\n", __func__,
+                           node_path, fdt_strerror(r));
+    }
 }
 
-int qemu_fdt_add_subnode(void *fdt, const char *name)
+void qemu_fdt_add_subnode(void *fdt, const char *name, Error **errp)
 {
     char *dupname = g_strdup(name);
     char *basename = strrchr(dupname, '/');
     int retval;
     int parent = 0;
 
-    if (!basename) {
+    if (!basename || (errp && *errp)) {
         g_free(dupname);
-        return -1;
+        return;
     }
 
     basename[0] = '\0';
     basename++;
 
     if (dupname[0]) {
-        parent = findnode_nofail(fdt, dupname);
+        parent = findnode(fdt, dupname, errp);
+        if (parent < 0) {
+            return;
+        }
     }
 
     retval = fdt_add_subnode(fdt, parent, basename);
     if (retval < 0) {
-        fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
-                fdt_strerror(retval));
-        exit(1);
+        error_set_or_print(errp, "FDT: Failed to create subnode %s: %s\n",
+                           name, fdt_strerror(retval));
     }
 
     g_free(dupname);
-    return retval;
 }
 
 void qemu_fdt_dumpdtb(void *fdt, int size)
@@ -309,35 +371,45 @@ void qemu_fdt_dumpdtb(void *fdt, int size)
     }
 }
 
-int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
-                                            const char *node_path,
-                                            const char *property,
-                                            int numvalues,
-                                            uint64_t *values)
+void qemu_fdt_setprop_sized_cells_from_array(void *fdt,
+                                             const char *node_path,
+                                             const char *property,
+                                             int numvalues,
+                                             uint64_t *values, Error **errp)
 {
     uint32_t *propcells;
     uint64_t value;
     int cellnum, vnum, ncells;
     uint32_t hival;
 
+    if (errp && *errp) {
+        return;
+    }
+
     propcells = g_new0(uint32_t, numvalues * 2);
 
     cellnum = 0;
     for (vnum = 0; vnum < numvalues; vnum++) {
         ncells = values[vnum * 2];
         if (ncells != 1 && ncells != 2) {
-            return -1;
+            error_set_or_print(errp, "%s: Setting property %s/%s, element [%d]:"
+                               " Unsupported number of cells:%d", __func__,
+                               node_path, property, vnum, ncells);
+            return;
         }
         value = values[vnum * 2 + 1];
         hival = cpu_to_be32(value >> 32);
         if (ncells > 1) {
             propcells[cellnum++] = hival;
         } else if (hival != 0) {
-            return -1;
+            error_set_or_print(errp, "%s: Setting property %s/%s, element [%d]:"
+                               " Value too large for single cell: %#" PRIx64,
+                               __func__, node_path, property, vnum, value);
+            return;
         }
         propcells[cellnum++] = cpu_to_be32(value);
     }
 
-    return qemu_fdt_setprop(fdt, node_path, property, propcells,
-                            cellnum * sizeof(uint32_t));
+    qemu_fdt_setprop(fdt, node_path, property, propcells,
+                     cellnum * sizeof(uint32_t), errp);
 }
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1a11574..305df4e 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -229,8 +229,9 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
 {
     void *fdt = NULL;
     char *filename;
-    int size, rc;
+    int size;
     uint32_t acells, scells;
+    Error *err = NULL;
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
     if (!filename) {
@@ -246,8 +247,8 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
     }
     g_free(filename);
 
-    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells");
-    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells");
+    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", &err);
+    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", &err);
     if (acells == 0 || scells == 0) {
         fprintf(stderr, "dtb file invalid (#address-cells or #size-cells 0)\n");
         goto fail;
@@ -262,37 +263,24 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
         goto fail;
     }
 
-    rc = qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg",
-                                      acells, binfo->loader_start,
-                                      scells, binfo->ram_size);
-    if (rc < 0) {
-        fprintf(stderr, "couldn't set /memory/reg\n");
-        goto fail;
-    }
+    qemu_fdt_setprop_sized_cells(fdt, "/memory", "reg", &err,
+                                 acells, binfo->loader_start,
+                                 scells, binfo->ram_size);
 
     if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
-        rc = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
-                                     binfo->kernel_cmdline);
-        if (rc < 0) {
-            fprintf(stderr, "couldn't set /chosen/bootargs\n");
-            goto fail;
-        }
+        qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+                                binfo->kernel_cmdline, &err);
     }
 
     if (binfo->initrd_size) {
-        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                   binfo->initrd_start);
-        if (rc < 0) {
-            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
-            goto fail;
-        }
+        qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                              binfo->initrd_start, &err);
+        qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                              binfo->initrd_start + binfo->initrd_size, &err);
+    }
 
-        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                   binfo->initrd_start + binfo->initrd_size);
-        if (rc < 0) {
-            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
-            goto fail;
-        }
+    if (err) {
+        goto fail;
     }
 
     if (binfo->modify_dtb) {
@@ -304,10 +292,13 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
     cpu_physical_memory_write(addr, fdt, size);
 
     g_free(fdt);
-
     return 0;
 
 fail:
+    if (err) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
+    }
     g_free(fdt);
     return -1;
 }
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 05e6c94..d6fda0f 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -416,18 +416,19 @@ static int add_virtio_mmio_node(void *fdt, uint32_t acells, uint32_t scells,
      * interrupt controller that interrupt-parent points to; these are for
      * the ARM GIC and indicate an SPI interrupt, rising-edge-triggered.)
      */
-    int rc;
+    Error *err = NULL;
     char *nodename = g_strdup_printf("/virtio_mmio@%" PRIx64, addr);
 
-    rc = qemu_fdt_add_subnode(fdt, nodename);
-    rc |= qemu_fdt_setprop_string(fdt, nodename,
-                                  "compatible", "virtio,mmio");
-    rc |= qemu_fdt_setprop_sized_cells(fdt, nodename, "reg",
-                                       acells, addr, scells, size);
-    qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", intc);
-    qemu_fdt_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
+    qemu_fdt_add_subnode(fdt, nodename, &err);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "virtio,mmio", &err);
+    qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", &err, acells, addr,
+                                 scells, size);
+    qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", &err, intc);
+    qemu_fdt_setprop_cells(fdt, nodename, "interrupts", &err, 0, irq, 1);
     g_free(nodename);
-    if (rc) {
+    if (err) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
         return -1;
     }
     return 0;
@@ -456,8 +457,8 @@ static void vexpress_modify_dtb(const struct arm_boot_info *info, void *fdt)
     uint32_t acells, scells, intc;
     const VEDBoardInfo *daughterboard = (const VEDBoardInfo *)info;
 
-    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells");
-    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells");
+    acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", NULL);
+    scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", NULL);
     intc = find_int_controller(fdt);
     if (!intc) {
         /* Not fatal, we just won't provide virtio. This will
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 48d9e7a..4b860d2 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -69,7 +69,7 @@ static int microblaze_load_dtb(hwaddr addr,
 {
     int fdt_size;
     void *fdt = NULL;
-    int r;
+    Error *err = NULL;
 
     if (dtb_filename) {
         fdt = load_device_tree(dtb_filename, &fdt_size);
@@ -79,19 +79,16 @@ static int microblaze_load_dtb(hwaddr addr,
     }
 
     if (kernel_cmdline) {
-        r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
-                                    kernel_cmdline);
-        if (r < 0) {
-            fprintf(stderr, "couldn't set /chosen/bootargs\n");
-        }
+        qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+                                kernel_cmdline, &err);
     }
 
     if (initrd_start) {
         qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                              initrd_start);
+                              initrd_start, &err);
 
         qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                              initrd_end);
+                              initrd_end, &err);
     }
 
     cpu_physical_memory_write(addr, fdt, fdt_size);
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index b37ce9d..bc5e9c1 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -103,23 +103,24 @@ static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
 
 static void dt_serial_create(void *fdt, unsigned long long offset,
                              const char *soc, const char *mpic,
-                             const char *alias, int idx, bool defcon)
+                             const char *alias, int idx, bool defcon,
+                             Error **errp)
 {
     char ser[128];
 
     snprintf(ser, sizeof(ser), "%s/serial@%llx", soc, offset);
-    qemu_fdt_add_subnode(fdt, ser);
-    qemu_fdt_setprop_string(fdt, ser, "device_type", "serial");
-    qemu_fdt_setprop_string(fdt, ser, "compatible", "ns16550");
-    qemu_fdt_setprop_cells(fdt, ser, "reg", offset, 0x100);
-    qemu_fdt_setprop_cell(fdt, ser, "cell-index", idx);
-    qemu_fdt_setprop_cell(fdt, ser, "clock-frequency", 0);
-    qemu_fdt_setprop_cells(fdt, ser, "interrupts", 42, 2);
-    qemu_fdt_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
-    qemu_fdt_setprop_string(fdt, "/aliases", alias, ser);
+    qemu_fdt_add_subnode(fdt, ser, errp);
+    qemu_fdt_setprop_string(fdt, ser, "device_type", "serial", errp);
+    qemu_fdt_setprop_string(fdt, ser, "compatible", "ns16550", errp);
+    qemu_fdt_setprop_cells(fdt, ser, "reg", errp, offset, 0x100);
+    qemu_fdt_setprop_cell(fdt, ser, "cell-index", idx, errp);
+    qemu_fdt_setprop_cell(fdt, ser, "clock-frequency", 0, errp);
+    qemu_fdt_setprop_cells(fdt, ser, "interrupts", errp, 42, 2);
+    qemu_fdt_setprop_phandle(fdt, ser, "interrupt-parent", mpic, errp);
+    qemu_fdt_setprop_string(fdt, "/aliases", alias, ser, errp);
 
     if (defcon) {
-        qemu_fdt_setprop_string(fdt, "/chosen", "linux,stdout-path", ser);
+        qemu_fdt_setprop_string(fdt, "/chosen", "linux,stdout-path", ser, errp);
     }
 }
 
@@ -149,6 +150,7 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
     char msi[128];
     uint32_t *pci_map = NULL;
     int len;
+    Error *err = NULL;
     uint32_t pci_ranges[14] =
         {
             0x2000000, 0x0, 0xc0000000,
@@ -183,33 +185,25 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
     }
 
     /* Manipulate device tree in memory. */
-    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 2);
-    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 2);
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 2, &err);
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 2, &err);
 
-    qemu_fdt_add_subnode(fdt, "/memory");
-    qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
+    qemu_fdt_add_subnode(fdt, "/memory", &err);
+    qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory", &err);
     qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
-                     sizeof(mem_reg_property));
+                     sizeof(mem_reg_property), &err);
 
-    qemu_fdt_add_subnode(fdt, "/chosen");
+    qemu_fdt_add_subnode(fdt, "/chosen", &err);
     if (initrd_size) {
-        ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                    initrd_base);
-        if (ret < 0) {
-            fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
-        }
+        qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start", initrd_base,
+                              &err);
 
-        ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                    (initrd_base + initrd_size));
-        if (ret < 0) {
-            fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
-        }
+        qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                              (initrd_base + initrd_size), &err);
     }
 
-    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
-                                      args->kernel_cmdline);
-    if (ret < 0)
-        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", args->kernel_cmdline,
+                            &err);
 
     if (kvm_enabled()) {
         /* Read out host's frequencies */
@@ -217,22 +211,22 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
         tb_freq = kvmppc_get_tbfreq();
 
         /* indicate KVM hypercall interface */
-        qemu_fdt_add_subnode(fdt, "/hypervisor");
-        qemu_fdt_setprop_string(fdt, "/hypervisor", "compatible",
-                                "linux,kvm");
+        qemu_fdt_add_subnode(fdt, "/hypervisor", &err);
+        qemu_fdt_setprop_string(fdt, "/hypervisor", "compatible", "linux,kvm",
+                                &err);
         kvmppc_get_hypercall(env, hypercall, sizeof(hypercall));
-        qemu_fdt_setprop(fdt, "/hypervisor", "hcall-instructions",
-                         hypercall, sizeof(hypercall));
+        qemu_fdt_setprop(fdt, "/hypervisor", "hcall-instructions", hypercall,
+                         sizeof(hypercall), &err);
         /* if KVM supports the idle hcall, set property indicating this */
         if (kvmppc_get_hasidle(env)) {
-            qemu_fdt_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
+            qemu_fdt_setprop(fdt, "/hypervisor", "has-idle", NULL, 0, &err);
         }
     }
 
     /* Create CPU nodes */
-    qemu_fdt_add_subnode(fdt, "/cpus");
-    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 1);
-    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0);
+    qemu_fdt_add_subnode(fdt, "/cpus", &err);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 1, &err);
+    qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0, &err);
 
     /* We need to generate the cpu nodes in reverse order, so Linux can pick
        the first node as boot node and be happy */
@@ -249,56 +243,58 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
 
         snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
                  cpu->cpu_index);
-        qemu_fdt_add_subnode(fdt, cpu_name);
-        qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
-        qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
-        qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
-        qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
+        qemu_fdt_add_subnode(fdt, cpu_name, &err);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq,
+                              &err);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq,
+                              &err);
+        qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu", &err);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index, &err);
         qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-line-size",
-                              env->dcache_line_size);
+                              env->dcache_line_size, &err);
         qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-line-size",
-                              env->icache_line_size);
-        qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-size", 0x8000);
-        qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-size", 0x8000);
-        qemu_fdt_setprop_cell(fdt, cpu_name, "bus-frequency", 0);
+                              env->icache_line_size, &err);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-size", 0x8000, &err);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-size", 0x8000, &err);
+        qemu_fdt_setprop_cell(fdt, cpu_name, "bus-frequency", 0, &err);
         if (cpu->cpu_index) {
-            qemu_fdt_setprop_string(fdt, cpu_name, "status", "disabled");
+            qemu_fdt_setprop_string(fdt, cpu_name, "status", "disabled", &err);
             qemu_fdt_setprop_string(fdt, cpu_name, "enable-method",
-                                    "spin-table");
+                                    "spin-table", &err);
             qemu_fdt_setprop_u64(fdt, cpu_name, "cpu-release-addr",
-                                 cpu_release_addr);
+                                 cpu_release_addr, &err);
         } else {
-            qemu_fdt_setprop_string(fdt, cpu_name, "status", "okay");
+            qemu_fdt_setprop_string(fdt, cpu_name, "status", "okay", &err);
         }
     }
 
-    qemu_fdt_add_subnode(fdt, "/aliases");
+    qemu_fdt_add_subnode(fdt, "/aliases", &err);
     /* XXX These should go into their respective devices' code */
     snprintf(soc, sizeof(soc), "/soc@%llx", MPC8544_CCSRBAR_BASE);
-    qemu_fdt_add_subnode(fdt, soc);
-    qemu_fdt_setprop_string(fdt, soc, "device_type", "soc");
+    qemu_fdt_add_subnode(fdt, soc, &err);
+    qemu_fdt_setprop_string(fdt, soc, "device_type", "soc", &err);
     qemu_fdt_setprop(fdt, soc, "compatible", compatible_sb,
-                     sizeof(compatible_sb));
-    qemu_fdt_setprop_cell(fdt, soc, "#address-cells", 1);
-    qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1);
-    qemu_fdt_setprop_cells(fdt, soc, "ranges", 0x0,
-                           MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
-                           MPC8544_CCSRBAR_SIZE);
+                     sizeof(compatible_sb), &err);
+    qemu_fdt_setprop_cell(fdt, soc, "#address-cells", 1, &err);
+    qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1, &err);
+    qemu_fdt_setprop_cells(fdt, soc, "ranges", &err, 0x0,
+                           MPC8544_CCSRBAR_BASE >> 32,
+                           MPC8544_CCSRBAR_BASE, MPC8544_CCSRBAR_SIZE);
     /* XXX should contain a reasonable value */
-    qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0);
+    qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0, &err);
 
     snprintf(mpic, sizeof(mpic), "%s/pic@%llx", soc, MPC8544_MPIC_REGS_OFFSET);
-    qemu_fdt_add_subnode(fdt, mpic);
-    qemu_fdt_setprop_string(fdt, mpic, "device_type", "open-pic");
-    qemu_fdt_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
-    qemu_fdt_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
+    qemu_fdt_add_subnode(fdt, mpic, &err);
+    qemu_fdt_setprop_string(fdt, mpic, "device_type", "open-pic", &err);
+    qemu_fdt_setprop_string(fdt, mpic, "compatible", "fsl,mpic", &err);
+    qemu_fdt_setprop_cells(fdt, mpic, "reg", &err, MPC8544_MPIC_REGS_OFFSET,
                            0x40000);
-    qemu_fdt_setprop_cell(fdt, mpic, "#address-cells", 0);
-    qemu_fdt_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
+    qemu_fdt_setprop_cell(fdt, mpic, "#address-cells", 0, &err);
+    qemu_fdt_setprop_cell(fdt, mpic, "#interrupt-cells", 2, &err);
     mpic_ph = qemu_fdt_alloc_phandle(fdt);
-    qemu_fdt_setprop_cell(fdt, mpic, "phandle", mpic_ph);
-    qemu_fdt_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
-    qemu_fdt_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, mpic, "phandle", mpic_ph, &err);
+    qemu_fdt_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph, &err);
+    qemu_fdt_setprop(fdt, mpic, "interrupt-controller", NULL, 0, &err);
 
     /*
      * We have to generate ser1 first, because Linux takes the first
@@ -306,25 +302,27 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
      * devices in reverse order to the dt.
      */
     dt_serial_create(fdt, MPC8544_SERIAL1_REGS_OFFSET,
-                     soc, mpic, "serial1", 1, false);
+                     soc, mpic, "serial1", 1, false, &err);
     dt_serial_create(fdt, MPC8544_SERIAL0_REGS_OFFSET,
-                     soc, mpic, "serial0", 0, true);
+                     soc, mpic, "serial0", 0, true, &err);
 
     snprintf(gutil, sizeof(gutil), "%s/global-utilities@%llx", soc,
              MPC8544_UTIL_OFFSET);
-    qemu_fdt_add_subnode(fdt, gutil);
-    qemu_fdt_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
-    qemu_fdt_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
-    qemu_fdt_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
+    qemu_fdt_add_subnode(fdt, gutil, &err);
+    qemu_fdt_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts", &err);
+    qemu_fdt_setprop_cells(fdt, gutil, "reg", &err, MPC8544_UTIL_OFFSET,
+                           0x1000);
+    qemu_fdt_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0, &err);
 
     snprintf(msi, sizeof(msi), "/%s/msi@%llx", soc, MPC8544_MSI_REGS_OFFSET);
-    qemu_fdt_add_subnode(fdt, msi);
-    qemu_fdt_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
-    qemu_fdt_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
+    qemu_fdt_add_subnode(fdt, msi, &err);
+    qemu_fdt_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi", &err);
+    qemu_fdt_setprop_cells(fdt, msi, "reg", &err, MPC8544_MSI_REGS_OFFSET,
+                           0x200);
     msi_ph = qemu_fdt_alloc_phandle(fdt);
-    qemu_fdt_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
-    qemu_fdt_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
-    qemu_fdt_setprop_cells(fdt, msi, "interrupts",
+    qemu_fdt_setprop_cells(fdt, msi, "msi-available-ranges", &err, 0x0, 0x100);
+    qemu_fdt_setprop_phandle(fdt, msi, "interrupt-parent", mpic, &err);
+    qemu_fdt_setprop_cells(fdt, msi, "interrupts", &err,
         0xe0, 0x0,
         0xe1, 0x0,
         0xe2, 0x0,
@@ -333,41 +331,41 @@ static int ppce500_load_device_tree(QEMUMachineInitArgs *args,
         0xe5, 0x0,
         0xe6, 0x0,
         0xe7, 0x0);
-    qemu_fdt_setprop_cell(fdt, msi, "phandle", msi_ph);
-    qemu_fdt_setprop_cell(fdt, msi, "linux,phandle", msi_ph);
+    qemu_fdt_setprop_cell(fdt, msi, "phandle", msi_ph, &err);
+    qemu_fdt_setprop_cell(fdt, msi, "linux,phandle", msi_ph, &err);
 
     snprintf(pci, sizeof(pci), "/pci@%llx", MPC8544_PCI_REGS_BASE);
-    qemu_fdt_add_subnode(fdt, pci);
-    qemu_fdt_setprop_cell(fdt, pci, "cell-index", 0);
-    qemu_fdt_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
-    qemu_fdt_setprop_string(fdt, pci, "device_type", "pci");
-    qemu_fdt_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
+    qemu_fdt_add_subnode(fdt, pci, &err);
+    qemu_fdt_setprop_cell(fdt, pci, "cell-index", 0, &err);
+    qemu_fdt_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci", &err);
+    qemu_fdt_setprop_string(fdt, pci, "device_type", "pci", &err);
+    qemu_fdt_setprop_cells(fdt, pci, "interrupt-map-mask", &err, 0xf800, 0x0,
                            0x0, 0x7);
-    pci_map = pci_map_create(fdt, qemu_fdt_get_phandle(fdt, mpic),
+    pci_map = pci_map_create(fdt, qemu_fdt_get_phandle(fdt, mpic, &err),
                              params->pci_first_slot, params->pci_nr_slots,
                              &len);
-    qemu_fdt_setprop(fdt, pci, "interrupt-map", pci_map, len);
-    qemu_fdt_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
-    qemu_fdt_setprop_cells(fdt, pci, "interrupts", 24, 2);
-    qemu_fdt_setprop_cells(fdt, pci, "bus-range", 0, 255);
+    qemu_fdt_setprop(fdt, pci, "interrupt-map", pci_map, len, &err);
+    qemu_fdt_setprop_phandle(fdt, pci, "interrupt-parent", mpic, &err);
+    qemu_fdt_setprop_cells(fdt, pci, "interrupts", &err, 24, 2);
+    qemu_fdt_setprop_cells(fdt, pci, "bus-range", &err, 0, 255);
     for (i = 0; i < 14; i++) {
         pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
     }
-    qemu_fdt_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
-    qemu_fdt_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
-    qemu_fdt_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
+    qemu_fdt_setprop_cell(fdt, pci, "fsl,msi", msi_ph, &err);
+    qemu_fdt_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges), &err);
+    qemu_fdt_setprop_cells(fdt, pci, "reg", &err, MPC8544_PCI_REGS_BASE >> 32,
                            MPC8544_PCI_REGS_BASE, 0, 0x1000);
-    qemu_fdt_setprop_cell(fdt, pci, "clock-frequency", 66666666);
-    qemu_fdt_setprop_cell(fdt, pci, "#interrupt-cells", 1);
-    qemu_fdt_setprop_cell(fdt, pci, "#size-cells", 2);
-    qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3);
-    qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci);
+    qemu_fdt_setprop_cell(fdt, pci, "clock-frequency", 66666666, &err);
+    qemu_fdt_setprop_cell(fdt, pci, "#interrupt-cells", 1, &err);
+    qemu_fdt_setprop_cell(fdt, pci, "#size-cells", 2, &err);
+    qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3, &err);
+    qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci, &err);
 
-    params->fixup_devtree(params, fdt);
+    params->fixup_devtree(params, fdt, &err);
 
     if (toplevel_compat) {
         qemu_fdt_setprop(fdt, "/", "compatible", toplevel_compat,
-                         strlen(toplevel_compat) + 1);
+                         strlen(toplevel_compat) + 1, &err);
     }
 
 done:
@@ -380,6 +378,8 @@ done:
 out:
     g_free(pci_map);
 
+    assert_no_error(err);
+
     return ret;
 }
 
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index 52726a2..f260af9 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -8,7 +8,8 @@ typedef struct PPCE500Params {
     int pci_nr_slots;
 
     /* required -- must at least add toplevel board compatible */
-    void (*fixup_devtree)(struct PPCE500Params *params, void *fdt);
+    void (*fixup_devtree)(struct PPCE500Params *params, void *fdt,
+          Error **errp);
 
     int mpic_version;
 } PPCE500Params;
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index 7d5357e..d4cf80d 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -18,14 +18,15 @@
 #include "hw/ppc/openpic.h"
 #include "kvm_ppc.h"
 
-static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
+static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt,
+                                   Error **errp)
 {
     const char model[] = "QEMU ppce500";
     const char compatible[] = "fsl,qemu-e500";
 
-    qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
-    qemu_fdt_setprop(fdt, "/", "compatible", compatible,
-                     sizeof(compatible));
+    qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model), errp);
+    qemu_fdt_setprop(fdt, "/", "compatible", compatible, sizeof(compatible),
+                     errp);
 }
 
 static void e500plat_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 292c709..3107e5d 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -16,14 +16,15 @@
 #include "sysemu/device_tree.h"
 #include "hw/ppc/openpic.h"
 
-static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
+static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt,
+                                    Error **errp)
 {
     const char model[] = "MPC8544DS";
     const char compatible[] = "MPC8544DS\0MPC85xxDS";
 
-    qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
-    qemu_fdt_setprop(fdt, "/", "compatible", compatible,
-                     sizeof(compatible));
+    qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model), errp);
+    qemu_fdt_setprop(fdt, "/", "compatible", compatible, sizeof(compatible),
+                     errp);
 }
 
 static void mpc8544ds_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 22e121f..4d7fb9a 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -77,25 +77,13 @@ static int bamboo_load_device_tree(hwaddr addr,
 
     /* Manipulate device tree in memory. */
 
-    ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
-                           sizeof(mem_reg_property));
-    if (ret < 0)
-        fprintf(stderr, "couldn't set /memory/reg\n");
-
-    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                initrd_base);
-    if (ret < 0)
-        fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
-
-    ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                (initrd_base + initrd_size));
-    if (ret < 0)
-        fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
-
-    ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
-                                  kernel_cmdline);
-    if (ret < 0)
-        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+                     sizeof(mem_reg_property), NULL);
+    qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+                          initrd_base, NULL);
+    qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+                          (initrd_base + initrd_size), NULL);
+    qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline, NULL);
 
     /* Copy data from the host device tree into the guest. Since the guest can
      * directly access the timebase without host involvement, we must expose
@@ -106,9 +94,9 @@ static int bamboo_load_device_tree(hwaddr addr,
     }
 
     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
-                          clock_freq);
+                          clock_freq, NULL);
     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
-                          tb_freq);
+                          tb_freq, NULL);
 
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     g_free(fdt);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ce8e697..6b12d6e 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -283,6 +283,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
 {
     int ret;
     int i;
+    Error *err = NULL;
 
     ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
     if (ret < 0) {
@@ -291,29 +292,9 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
         return ret;
     }
 
-    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
-                                rtas_addr);
-    if (ret < 0) {
-        fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
-                fdt_strerror(ret));
-        return ret;
-    }
-
-    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
-                                rtas_addr);
-    if (ret < 0) {
-        fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
-                fdt_strerror(ret));
-        return ret;
-    }
-
-    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
-                                rtas_size);
-    if (ret < 0) {
-        fprintf(stderr, "Couldn't add rtas-size property: %s\n",
-                fdt_strerror(ret));
-        return ret;
-    }
+    qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base", rtas_addr, &err);
+    qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry", rtas_addr, &err);
+    qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size", rtas_size, &err);
 
     for (i = 0; i < TOKEN_MAX; i++) {
         struct rtas_call *call = &rtas_table[i];
@@ -322,14 +303,13 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
             continue;
         }
 
-        ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
-                                    i + TOKEN_BASE);
-        if (ret < 0) {
-            fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
-                    call->name, fdt_strerror(ret));
-            return ret;
-        }
+        qemu_fdt_setprop_cell(fdt, "/rtas", call->name, i + TOKEN_BASE, &err);
+    }
 
+    if (err) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
+        return -1;
     }
     return 0;
 }
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index bdb057e..9f447b6 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -142,7 +142,6 @@ static int xilinx_load_device_tree(hwaddr addr,
     char *path;
     int fdt_size;
     void *fdt = NULL;
-    int r;
     const char *dtb_filename;
 
     dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
@@ -166,9 +165,7 @@ static int xilinx_load_device_tree(hwaddr addr,
     if (!fdt) {
         return 0;
     }
-    r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
-    if (r < 0)
-        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline, NULL);
     cpu_physical_memory_write(addr, fdt, fdt_size);
     return fdt_size;
 }
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 899f05c..6dcdff4 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -4,8 +4,10 @@
  * interface.
  *
  * Copyright 2008 IBM Corporation.
+ * Copyright 2013 Xilinx Inc.
  * Authors: Jerone Young <jyoung5@us.ibm.com>
  *          Hollis Blanchard <hollisb@us.ibm.com>
+ *          Peter Crosthwaite <peter.crosthwaite@xilinx.com>
  *
  * This work is licensed under the GNU GPL license version 2 or later.
  *
@@ -14,30 +16,116 @@
 #ifndef __DEVICE_TREE_H__
 #define __DEVICE_TREE_H__
 
+#include "qapi/qmp/qerror.h"
+
 void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 
-int qemu_fdt_setprop(void *fdt, const char *node_path,
-                     const char *property, const void *val, int size);
-int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
-                          const char *property, uint32_t val);
-int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
-                         const char *property, uint64_t val);
-int qemu_fdt_setprop_string(void *fdt, const char *node_path,
-                            const char *property, const char *string);
-int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
-                             const char *property,
-                             const char *target_node_path);
-const void *qemu_fdt_getprop(void *fdt, const char *node_path,
-                             const char *property, int *lenp);
-uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
-                               const char *property);
-uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
 uint32_t qemu_fdt_alloc_phandle(void *fdt);
-int qemu_fdt_nop_node(void *fdt, const char *node_path);
-int qemu_fdt_add_subnode(void *fdt, const char *name);
+void qemu_fdt_nop_node(void *fdt, const char *node_path, Error **errp);
+void qemu_fdt_add_subnode(void *fdt, const char *name, Error **errp);
+
+void qemu_fdt_dumpdtb(void *fdt, int size);
+
+/**
+ * qemu_fdt_setprop - create or change a property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @val: pointer to data to set the property value to
+ * @size: length of the property value
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop() sets the value of the named property in the given
+ * node to the given value and length, creating the property if it
+ * does not already exist.
+ */
+
+void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
+                      const void *val, int size, Error **errp);
+
+/**
+ * qemu_fdt_setprop_cell - create or change a cell property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @val: value to set
+ * @size: length of the property value
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop_cell() sets the value of the named property in the given
+ * node to the given 32-bit (single fdt cell) value, creating the property if it
+ * does not already exist.
+ */
 
-#define qemu_fdt_setprop_cells(fdt, node_path, property, ...)                 \
+void qemu_fdt_setprop_cell(void *fdt, const char *node_path,
+                           const char *property, uint32_t val, Error **errp);
+
+/**
+ * qemu_fdt_setprop_u64 - create or change a 64bit int property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @val: value to set
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop_u64() sets the value of the named property in the given
+ * node to the given uint64_t value. The value is converted to big endian
+ * format as per device tree formatting.
+ */
+
+void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+                          const char *property, uint64_t val, Error **errp);
+
+/**
+ * qemu_fdt_setprop_string - create or change a string property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the phandle property to change
+ * @string: string to set
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop_string() sets the value of the named property in the given
+ * node to a the string @string. The property is created if it does not already
+ * exist.
+ */
+
+void qemu_fdt_setprop_string(void *fdt, const char *node_path,
+                             const char *property, const char *string,
+                             Error **errp);
+
+/**
+ * qemu_fdt_setprop_phandle - create or change a phandle property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the phandle property to change
+ * @target_path: node path of node for phandle to point to
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop_phandle() sets the value of the named property in the given
+ * node to a phandle which will reference the node described by @target_path.
+ * The property is created if it does not already exist.
+ */
+
+void qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
+                              const char *property,
+                              const char *target_node_path, Error **errp);
+
+/**
+ * qemu_fdt_setprop_cells - create or change a multi-cell property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @errp: Error to populate incase of error
+ * @...: varargs list of cells to write to property
+ *
+ * qemu_fdt_setprop_cells() sets the value of the named property in the given
+ * node to a list of cells. The varargs are converted to an appropriate length
+ * uint32_t array and converted to big endian. The array is then written as
+ * the property value.
+ */
+
+#define qemu_fdt_setprop_cells(fdt, node_path, property, errp, ...)           \
     do {                                                                      \
         uint32_t qdt_tmp[] = { __VA_ARGS__ };                                 \
         int i;                                                                \
@@ -46,11 +134,9 @@ int qemu_fdt_add_subnode(void *fdt, const char *name);
             qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]);                             \
         }                                                                     \
         qemu_fdt_setprop(fdt, node_path, property, qdt_tmp,                   \
-                         sizeof(qdt_tmp));                                    \
+                         sizeof(qdt_tmp), errp);                              \
     } while (0)
 
-void qemu_fdt_dumpdtb(void *fdt, int size);
-
 /**
  * qemu_fdt_setprop_sized_cells_from_array:
  * @fdt: device tree blob
@@ -58,6 +144,7 @@ void qemu_fdt_dumpdtb(void *fdt, int size);
  * @property: property to set
  * @numvalues: number of values
  * @values: array of number-of-cells, value pairs
+ * @errp: Error to populate incase of error
  *
  * Set the specified property on the specified node in the device tree
  * to be an array of cells. The values of the cells are specified via
@@ -75,14 +162,12 @@ void qemu_fdt_dumpdtb(void *fdt, int size);
  * simpler qemu_fdt_setprop_cells(). If you're not setting up the
  * array programmatically, qemu_fdt_setprop_sized_cells may be more
  * convenient.
- *
- * Return value: 0 on success, <0 on error.
  */
-int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
-                                            const char *node_path,
-                                            const char *property,
-                                            int numvalues,
-                                            uint64_t *values);
+void qemu_fdt_setprop_sized_cells_from_array(void *fdt,
+                                             const char *node_path,
+                                             const char *property,
+                                             int numvalues,
+                                             uint64_t *values, Error **errp);
 
 /**
  * qemu_fdt_setprop_sized_cells:
@@ -98,16 +183,58 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
  *
  * This is a convenience wrapper for the function
  * qemu_fdt_setprop_sized_cells_from_array().
- *
- * Return value: 0 on success, <0 on error.
  */
-#define qemu_fdt_setprop_sized_cells(fdt, node_path, property, ...)       \
+#define qemu_fdt_setprop_sized_cells(fdt, node_path, property, errp, ...) \
     ({                                                                    \
         uint64_t qdt_tmp[] = { __VA_ARGS__ };                             \
         qemu_fdt_setprop_sized_cells_from_array(fdt, node_path,           \
                                                 property,                 \
                                                 ARRAY_SIZE(qdt_tmp) / 2,  \
-                                                qdt_tmp);                 \
+                                                qdt_tmp, errp);           \
     })
 
+/**
+ * qemu_fdt_get_phandle - get the FDT phandle for a specified node
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of node to get phandle for
+ * @errp: Error to populate incase of error
+ *
+ * Gets the phandle for a node path. Return the phandle value on sucess or
+ * 0 or error. errp in populated in case of error.
+ */
+
+uint32_t qemu_fdt_get_phandle(void *fdt, const char *node_path, Error **errp);
+
+/**
+ * qemu_fdt_getprop - get an FDT property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to get
+ * @property: name of the property to get
+ * @lenp: Pointer to store length of property
+ * @errp: Error to populate incase of error
+ *
+ * Get an FDT property. On success, returns a pointer to the requested
+ * property and populates lenp with the length of the property (in bytes).
+ * On failure, returns NULL and populates errp with the specific error.
+ */
+
+const void *qemu_fdt_getprop(void *fdt, const char *node_path,
+                             const char *property, int *lenp, Error **errp);
+
+/**
+ * qemu_fdt_getprop - get an FDT cell property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to get
+ * @property: name of the property to get
+ * @errp: Error to populate incase of error
+ *
+ * Get an FDT property assuming its a cell. On success, returns the cell
+ * value.
+ * On failure, returns 0 and populates errp with the specific error.
+ * The cell value is coverted to host endianess as needed.
+ */
+
+uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
+                               const char *property, Error **errp);
+
 #endif /* __DEVICE_TREE_H__ */
-- 
1.8.3.rc1.44.gb387c77.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/3] Device tree cleanups
  2013-11-11  8:14 [Qemu-devel] [PATCH v2 0/3] Device tree cleanups peter.crosthwaite
                   ` (2 preceding siblings ...)
  2013-11-11  8:16 ` [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
@ 2013-11-27 13:10 ` Peter Crosthwaite
  3 siblings, 0 replies; 7+ messages in thread
From: Peter Crosthwaite @ 2013-11-27 13:10 UTC (permalink / raw)
  To: qemu-devel@nongnu.org Developers
  Cc: Peter Maydell, Edgar E. Iglesias, Alexander Graf,
	Andreas Färber, David Gibson

Ping!

On Mon, Nov 11, 2013 at 6:14 PM,  <peter.crosthwaite@xilinx.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Fix the name stem of the devicetree API (P1 - s/qemu_devtree/qemu_fdt)
> and cleanup error report (P3). Trivial patch P2 fixing an arugment name
> along the way.
>
> Tested using:
>
> 1: Alex's e500 test vector.
> 2: Xilinx Zynq (tests arm/boot.c).
>
> I have testing using Zynq with Mem > 4gb and a bogus dts (size cells = 1)
> to give that particular error path some exercise.
>
> To give some exercise to the error paths, I hacked up my libfdt to throw
> errors randomly:
>
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -48,6 +48,8 @@
>   *     OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
>   *     EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
> +
> +#include <stdlib.h>
>  #include "libfdt_env.h"
>
>  #include <fdt.h>
> @@ -279,6 +281,15 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>
>     FDT_RW_CHECK_HEADER(fdt);
>
> +    static int seeded = 0;
> +    if (!seeded) {
> +        srand(time(NULL));
> +        seeded = 1;
> +    }
> +    if (!(rand() & 0x7)) {
> +        return -((rand() & 0x3) + 1);
> +    }
> +
>
> Some sample outputs from e500 boot (Using the above tainted libfdt):
>
> -----
> qemu-system-ppc: qemu_fdt_setprop: Couldn't set /memory/reg: FDT_ERR_BADOFFSET
>
> Aborted
> -----
> qemu-system-ppc: qemu_fdt_setprop_string: Couldn't set /soc@e0000000/device_type = soc: FDT_ERR_NOSPACE
>
> Aborted
> -----
> qemu-system-ppc: qemu_fdt_setprop_cell: Couldn't set /soc@e0000000/#address-cells = 0x000001: FDT_ERR_NOSPACE
>
> Aborted
> -----
>
>
> Peter Crosthwaite (3):
>   device_tree: s/qemu_devtree/qemu_fdt globally
>   device_tree: qemu_fdt_setprop: Rename val_array arg
>   device_tree: qemu_fdt_setprop: Fixup error reporting
>
>  device_tree.c                | 230 +++++++++++++++++++++++++++--------------
>  hw/arm/boot.c                |  51 ++++-----
>  hw/arm/vexpress.c            |  23 +++--
>  hw/microblaze/boot.c         |  17 ++-
>  hw/ppc/e500.c                | 241 ++++++++++++++++++++++---------------------
>  hw/ppc/e500.h                |   3 +-
>  hw/ppc/e500plat.c            |   9 +-
>  hw/ppc/mpc8544ds.c           |   9 +-
>  hw/ppc/ppc440_bamboo.c       |  34 ++----
>  hw/ppc/spapr_rtas.c          |  40 ++-----
>  hw/ppc/virtex_ml507.c        |   5 +-
>  include/sysemu/device_tree.h | 219 ++++++++++++++++++++++++++++++---------
>  12 files changed, 519 insertions(+), 362 deletions(-)
>
> --
> 1.8.3.rc1.44.gb387c77.dirty
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
  2013-11-11  8:16 ` [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
@ 2013-12-18 13:48   ` Alexander Graf
  2013-12-18 23:51     ` Peter Crosthwaite
  0 siblings, 1 reply; 7+ messages in thread
From: Alexander Graf @ 2013-12-18 13:48 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Peter Maydell, Edgar E. Iglesias, QEMU Developers,
	Andreas Färber, David Gibson


On 11.11.2013, at 09:16, peter.crosthwaite@xilinx.com wrote:

> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> There are a mix of usages of the qemu_fdt_* API calls, some which
> wish to assert and abort QEMU on failure and some of which wish to do
> their own error handling. The latter in more correct, but the former
> is in the majority and more pragmatic. However the asserting clients
> are usually doing large numbers fdt ops and only want to assert if any
> one of them breaks. So the cleanest compromising solution is:
> 
> 1. Require an Error ** to be passes to all fdt ops.
> 2. And perform no action if its already non-null (error condition).
> 3. If no Error ** is given report errors to stderr
> 
> Step one allows clients to do their own error handling if they wish
> too, which is the most flexible:
> 
> Error *err = NULL;
> fdt_foo( ... , &err);
> if (err) {
>    /* handle error my special way */
> }
> 
> Step two allows you to do a large number of fdt ops then check them
> all only the once at the end:
> 
> Error *err = NULL;
> fdt_foo( ... , &err);
> fdt_bar( ... , &err);
> fdt_baz( ... , &err);
> if (err) {
>    /* First encountered error will be in err */
> }
> 
> Step 3, handles the common case where the error is not an issue, but
> just needs to make noise at the user (via stderr). This seems common for
> bootloader code that sets chosen and initrd props etc (rather than machine
> description).
> 
> All error reporting is stylistically udpated to use Error ** instead of
> integer return codes and no more exit(1) on failures.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Thanks a lot. I've applied the first two patches to ppc-next with some adjustments to also cover hw/arm/virt.c. But this patch is too much work to rebase to current master :).

Please repost a version that also covers virt.c. I'd also like to see an Ack from Peter at least.


Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
  2013-12-18 13:48   ` Alexander Graf
@ 2013-12-18 23:51     ` Peter Crosthwaite
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Crosthwaite @ 2013-12-18 23:51 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Peter Maydell, QEMU Developers, lcapitulino, Edgar E. Iglesias,
	Andreas Färber, David Gibson

On Wed, Dec 18, 2013 at 11:48 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 11.11.2013, at 09:16, peter.crosthwaite@xilinx.com wrote:
>
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> There are a mix of usages of the qemu_fdt_* API calls, some which
>> wish to assert and abort QEMU on failure and some of which wish to do
>> their own error handling. The latter in more correct, but the former
>> is in the majority and more pragmatic. However the asserting clients
>> are usually doing large numbers fdt ops and only want to assert if any
>> one of them breaks. So the cleanest compromising solution is:
>>
>> 1. Require an Error ** to be passes to all fdt ops.
>> 2. And perform no action if its already non-null (error condition).
>> 3. If no Error ** is given report errors to stderr
>>
>> Step one allows clients to do their own error handling if they wish
>> too, which is the most flexible:
>>
>> Error *err = NULL;
>> fdt_foo( ... , &err);
>> if (err) {
>>    /* handle error my special way */
>> }
>>
>> Step two allows you to do a large number of fdt ops then check them
>> all only the once at the end:
>>
>> Error *err = NULL;
>> fdt_foo( ... , &err);
>> fdt_bar( ... , &err);
>> fdt_baz( ... , &err);
>> if (err) {
>>    /* First encountered error will be in err */
>> }
>>
>> Step 3, handles the common case where the error is not an issue, but
>> just needs to make noise at the user (via stderr). This seems common for
>> bootloader code that sets chosen and initrd props etc (rather than machine
>> description).
>>
>> All error reporting is stylistically udpated to use Error ** instead of
>> integer return codes and no more exit(1) on failures.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Thanks a lot. I've applied the first two patches to ppc-next with some adjustments to also cover hw/arm/virt.c.

Thanks,

That make my respin of this one much easier.

> But this patch is too much work to rebase to current master :).

This patch is going to see a major change pattern once the
&error_abort series goes through, as the assert_no_error scheme is now
gone. This would have conflicted with the qmp queue that holds those
patches (I probably should have put a note on list to retract this
patch).

>
> Please repost a version that also covers virt.c. I'd also like to see an Ack from Peter at least.
>

I'll wait for both yours and Luiz' queues to flush and spin it.

Regards,
Peter

>
> Alex
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-12-18 23:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-11  8:14 [Qemu-devel] [PATCH v2 0/3] Device tree cleanups peter.crosthwaite
2013-11-11  8:14 ` [Qemu-devel] [PATCH v2 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
2013-11-11  8:15 ` [Qemu-devel] [PATCH v2 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
2013-11-11  8:16 ` [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
2013-12-18 13:48   ` Alexander Graf
2013-12-18 23:51     ` Peter Crosthwaite
2013-11-27 13:10 ` [Qemu-devel] [PATCH v2 0/3] Device tree cleanups Peter Crosthwaite

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).