[Qemu-devel] [PATCHv3 00/12] pseries: Consolidate guest device tree construction

[Qemu-devel] [PATCHv3 00/12] pseries: Consolidate guest device tree construction

[David Gibson]

For historical reasons construction of the guest device tree in spapr
is divided between spapr_create_fdt_skel() which is called at init
time, and spapr_build_fdt() which runs at reset time.  Over time, more
and more things have needed to be moved to reset time.

This series consolidates all the device tree construction to reset
time, with some minor cleanups along the way.  This will help to make
it more maintainable in future.

Changes since v2:
  * Removed a leftover reference to a variable called 'stdout',
    shadowing the standard library stdout
Changes since v1:
  * Fixed a memory leak introduced by 1/12 (spotted by Thomas Huth)
  * Removed one patch that's already merged in ppc-for-2.8

David Gibson (12):
  pseries: Split device tree construction from device tree load
  pseries: Remove rtas_addr and fdt_addr fields from machinestate
  pseries: Make spapr_create_fdt_skel() get information from machine
    state
  pseries: Move adding of fdt reserve map entries
  pseries: Consolidate RTAS loading
  pseries: Move construction of /interrupt-controller fdt node
  pseries: Consolidate construction of /chosen device tree node
  pseries: Consolidate construction of /rtas device tree node
  pseries: Move /event-sources construction to spapr_build_fdt()
  pseries: Move /hypervisor node construction to fdt_build_fdt()
  pseries: Consolidate construction of /vdevice device tree node
  pseries: Remove spapr_create_fdt_skel()

 hw/intc/xics_spapr.c       |  22 ++
 hw/ppc/spapr.c             | 560 +++++++++++++++++++++------------------------
 hw/ppc/spapr_events.c      |  21 +-
 hw/ppc/spapr_rtas.c        |  91 +++-----
 hw/ppc/spapr_vio.c         |  40 ++--
 include/hw/ppc/spapr.h     |  12 +-
 include/hw/ppc/spapr_vio.h |   4 +-
 include/hw/ppc/xics.h      |   1 +
 8 files changed, 361 insertions(+), 390 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCHv3 01/12] pseries: Split device tree construction from device tree load

[David Gibson]

spapr_finalize_fdt() both finishes building the device tree for the guest
and loads it into guest memory.  For future cleanups, it's going to be
more convenient to do these two things separately.  The loading portion is
pretty trivial, so we move it inline into the caller, ppc_spapr_reset().

We also rename spapr_finalize_fdt(), because the current name is going to
become inaccurate.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ddb7438..0864411 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -900,10 +900,9 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
     return 0;
 }
 
-static void spapr_finalize_fdt(sPAPRMachineState *spapr,
-                               hwaddr fdt_addr,
-                               hwaddr rtas_addr,
-                               hwaddr rtas_size)
+static void *spapr_build_fdt(sPAPRMachineState *spapr,
+                             hwaddr rtas_addr,
+                             hwaddr rtas_size)
 {
     MachineState *machine = MACHINE(qdev_get_machine());
     MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -999,19 +998,8 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
         }
     }
 
-    _FDT((fdt_pack(fdt)));
-
-    if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
-        error_report("FDT too big ! 0x%x bytes (max is 0x%x)",
-                     fdt_totalsize(fdt), FDT_MAX_SIZE);
-        exit(1);
-    }
-
-    qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
-    cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
-
     g_free(bootlist);
-    g_free(fdt);
+    return fdt;
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
@@ -1147,6 +1135,8 @@ static void ppc_spapr_reset(void)
     sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
     PowerPCCPU *first_ppc_cpu;
     uint32_t rtas_limit;
+    void *fdt;
+    int rc;
 
     /* Check for unknown sysbus devices */
     foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
@@ -1173,14 +1163,28 @@ static void ppc_spapr_reset(void)
     spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
     spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
 
-    /* Load the fdt */
-    spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
-                       spapr->rtas_size);
+    fdt = spapr_build_fdt(spapr, spapr->rtas_addr, spapr->rtas_size);
 
     /* Copy RTAS over */
     cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob,
                               spapr->rtas_size);
 
+    rc = fdt_pack(fdt);
+
+    /* Should only fail if we've built a corrupted tree */
+    assert(rc == 0);
+
+    if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
+        error_report("FDT too big ! 0x%x bytes (max is 0x%x)",
+                     fdt_totalsize(fdt), FDT_MAX_SIZE);
+        exit(1);
+    }
+
+    /* Load the fdt */
+    qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
+    cpu_physical_memory_write(spapr->fdt_addr, fdt, fdt_totalsize(fdt));
+    g_free(fdt);
+
     /* Set up the entry state */
     first_ppc_cpu = POWERPC_CPU(first_cpu);
     first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
-- 
2.7.4

Re: [Qemu-devel] [PATCHv3 01/12] pseries: Split device tree construction from device tree load

[Michael Roth]

Quoting David Gibson (2016-10-24 00:04:32)
> spapr_finalize_fdt() both finishes building the device tree for the guest
> and loads it into guest memory.  For future cleanups, it's going to be
> more convenient to do these two things separately.  The loading portion is
> pretty trivial, so we move it inline into the caller, ppc_spapr_reset().
> 
> We also rename spapr_finalize_fdt(), because the current name is going to
> become inaccurate.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  hw/ppc/spapr.c | 42 +++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ddb7438..0864411 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -900,10 +900,9 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
>      return 0;
>  }
> 
> -static void spapr_finalize_fdt(sPAPRMachineState *spapr,
> -                               hwaddr fdt_addr,
> -                               hwaddr rtas_addr,
> -                               hwaddr rtas_size)
> +static void *spapr_build_fdt(sPAPRMachineState *spapr,
> +                             hwaddr rtas_addr,
> +                             hwaddr rtas_size)
>  {
>      MachineState *machine = MACHINE(qdev_get_machine());
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> @@ -999,19 +998,8 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>          }
>      }
> 
> -    _FDT((fdt_pack(fdt)));
> -
> -    if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
> -        error_report("FDT too big ! 0x%x bytes (max is 0x%x)",
> -                     fdt_totalsize(fdt), FDT_MAX_SIZE);
> -        exit(1);
> -    }
> -
> -    qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> -    cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> -
>      g_free(bootlist);
> -    g_free(fdt);
> +    return fdt;
>  }
> 
>  static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
> @@ -1147,6 +1135,8 @@ static void ppc_spapr_reset(void)
>      sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>      PowerPCCPU *first_ppc_cpu;
>      uint32_t rtas_limit;
> +    void *fdt;
> +    int rc;
> 
>      /* Check for unknown sysbus devices */
>      foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
> @@ -1173,14 +1163,28 @@ static void ppc_spapr_reset(void)
>      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
>      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
> 
> -    /* Load the fdt */
> -    spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
> -                       spapr->rtas_size);
> +    fdt = spapr_build_fdt(spapr, spapr->rtas_addr, spapr->rtas_size);
> 
>      /* Copy RTAS over */
>      cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob,
>                                spapr->rtas_size);
> 
> +    rc = fdt_pack(fdt);
> +
> +    /* Should only fail if we've built a corrupted tree */
> +    assert(rc == 0);
> +
> +    if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
> +        error_report("FDT too big ! 0x%x bytes (max is 0x%x)",
> +                     fdt_totalsize(fdt), FDT_MAX_SIZE);
> +        exit(1);
> +    }
> +
> +    /* Load the fdt */
> +    qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> +    cpu_physical_memory_write(spapr->fdt_addr, fdt, fdt_totalsize(fdt));
> +    g_free(fdt);
> +
>      /* Set up the entry state */
>      first_ppc_cpu = POWERPC_CPU(first_cpu);
>      first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
> -- 
> 2.7.4
> 

Re: [Qemu-devel] [PATCHv3 01/12] pseries: Split device tree construction from device tree load

[Alexey Kardashevskiy]

On 24/10/16 16:04, David Gibson wrote:
> spapr_finalize_fdt() both finishes building the device tree for the guest
> and loads it into guest memory.  For future cleanups, it's going to be
> more convenient to do these two things separately.  The loading portion is
> pretty trivial, so we move it inline into the caller, ppc_spapr_reset().
> 
> We also rename spapr_finalize_fdt(), because the current name is going to
> become inaccurate.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


I did reply with "rb" v2 of this with a comment, somehow it was lost.


Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

with a small nit, grep finds "spapr_finalize_fdt" in a comment:

hw/ppc/spapr_cpu_core.c:187:     * coldplugged CPUs DT entries are setup in
spapr_finalize_fdt().


> ---
>  hw/ppc/spapr.c | 42 +++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ddb7438..0864411 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -900,10 +900,9 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
>      return 0;
>  }
>  
> -static void spapr_finalize_fdt(sPAPRMachineState *spapr,
> -                               hwaddr fdt_addr,
> -                               hwaddr rtas_addr,
> -                               hwaddr rtas_size)
> +static void *spapr_build_fdt(sPAPRMachineState *spapr,
> +                             hwaddr rtas_addr,
> +                             hwaddr rtas_size)
>  {
>      MachineState *machine = MACHINE(qdev_get_machine());
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> @@ -999,19 +998,8 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>          }
>      }
>  
> -    _FDT((fdt_pack(fdt)));
> -
> -    if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
> -        error_report("FDT too big ! 0x%x bytes (max is 0x%x)",
> -                     fdt_totalsize(fdt), FDT_MAX_SIZE);
> -        exit(1);
> -    }
> -
> -    qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> -    cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> -
>      g_free(bootlist);
> -    g_free(fdt);
> +    return fdt;
>  }
>  
>  static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
> @@ -1147,6 +1135,8 @@ static void ppc_spapr_reset(void)
>      sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>      PowerPCCPU *first_ppc_cpu;
>      uint32_t rtas_limit;
> +    void *fdt;
> +    int rc;
>  
>      /* Check for unknown sysbus devices */
>      foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
> @@ -1173,14 +1163,28 @@ static void ppc_spapr_reset(void)
>      spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
>      spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
>  
> -    /* Load the fdt */
> -    spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
> -                       spapr->rtas_size);
> +    fdt = spapr_build_fdt(spapr, spapr->rtas_addr, spapr->rtas_size);
>  
>      /* Copy RTAS over */
>      cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob,
>                                spapr->rtas_size);
>  
> +    rc = fdt_pack(fdt);
> +
> +    /* Should only fail if we've built a corrupted tree */
> +    assert(rc == 0);
> +
> +    if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
> +        error_report("FDT too big ! 0x%x bytes (max is 0x%x)",
> +                     fdt_totalsize(fdt), FDT_MAX_SIZE);
> +        exit(1);
> +    }
> +
> +    /* Load the fdt */
> +    qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> +    cpu_physical_memory_write(spapr->fdt_addr, fdt, fdt_totalsize(fdt));
> +    g_free(fdt);
> +
>      /* Set up the entry state */
>      first_ppc_cpu = POWERPC_CPU(first_cpu);
>      first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
> 


-- 
Alexey

[Qemu-devel] [PATCHv3 02/12] pseries: Remove rtas_addr and fdt_addr fields from machinestate

[David Gibson]

These values are used only within ppc_spapr_reset(), so just change them
to local variables.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c         | 14 +++++++-------
 include/hw/ppc/spapr.h |  1 -
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0864411..2c1c7ff 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1135,6 +1135,7 @@ static void ppc_spapr_reset(void)
     sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
     PowerPCCPU *first_ppc_cpu;
     uint32_t rtas_limit;
+    hwaddr rtas_addr, fdt_addr;
     void *fdt;
     int rc;
 
@@ -1160,14 +1161,13 @@ static void ppc_spapr_reset(void)
      * processed with 32-bit real mode code if necessary
      */
     rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR);
-    spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
-    spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
+    rtas_addr = rtas_limit - RTAS_MAX_SIZE;
+    fdt_addr = rtas_addr - FDT_MAX_SIZE;
 
-    fdt = spapr_build_fdt(spapr, spapr->rtas_addr, spapr->rtas_size);
+    fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
 
     /* Copy RTAS over */
-    cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob,
-                              spapr->rtas_size);
+    cpu_physical_memory_write(rtas_addr, spapr->rtas_blob, spapr->rtas_size);
 
     rc = fdt_pack(fdt);
 
@@ -1182,12 +1182,12 @@ static void ppc_spapr_reset(void)
 
     /* Load the fdt */
     qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
-    cpu_physical_memory_write(spapr->fdt_addr, fdt, fdt_totalsize(fdt));
+    cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
     g_free(fdt);
 
     /* Set up the entry state */
     first_ppc_cpu = POWERPC_CPU(first_cpu);
-    first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
+    first_ppc_cpu->env.gpr[3] = fdt_addr;
     first_ppc_cpu->env.gpr[5] = 0;
     first_cpu->halted = 0;
     first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index aeaba3e..1174741 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -63,7 +63,6 @@ struct sPAPRMachineState {
     uint32_t htab_shift;
     hwaddr rma_size;
     int vrma_adjust;
-    hwaddr fdt_addr, rtas_addr;
     ssize_t rtas_size;
     void *rtas_blob;
     void *fdt_skel;
-- 
2.7.4

Re: [Qemu-devel] [PATCHv3 02/12] pseries: Remove rtas_addr and fdt_addr fields from machinestate

[Michael Roth]

Quoting David Gibson (2016-10-24 00:04:33)
> These values are used only within ppc_spapr_reset(), so just change them
> to local variables.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  hw/ppc/spapr.c         | 14 +++++++-------
>  include/hw/ppc/spapr.h |  1 -
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0864411..2c1c7ff 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1135,6 +1135,7 @@ static void ppc_spapr_reset(void)
>      sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>      PowerPCCPU *first_ppc_cpu;
>      uint32_t rtas_limit;
> +    hwaddr rtas_addr, fdt_addr;
>      void *fdt;
>      int rc;
> 
> @@ -1160,14 +1161,13 @@ static void ppc_spapr_reset(void)
>       * processed with 32-bit real mode code if necessary
>       */
>      rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR);
> -    spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
> -    spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
> +    rtas_addr = rtas_limit - RTAS_MAX_SIZE;
> +    fdt_addr = rtas_addr - FDT_MAX_SIZE;
> 
> -    fdt = spapr_build_fdt(spapr, spapr->rtas_addr, spapr->rtas_size);
> +    fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
> 
>      /* Copy RTAS over */
> -    cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob,
> -                              spapr->rtas_size);
> +    cpu_physical_memory_write(rtas_addr, spapr->rtas_blob, spapr->rtas_size);
> 
>      rc = fdt_pack(fdt);
> 
> @@ -1182,12 +1182,12 @@ static void ppc_spapr_reset(void)
> 
>      /* Load the fdt */
>      qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> -    cpu_physical_memory_write(spapr->fdt_addr, fdt, fdt_totalsize(fdt));
> +    cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
>      g_free(fdt);
> 
>      /* Set up the entry state */
>      first_ppc_cpu = POWERPC_CPU(first_cpu);
> -    first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
> +    first_ppc_cpu->env.gpr[3] = fdt_addr;
>      first_ppc_cpu->env.gpr[5] = 0;
>      first_cpu->halted = 0;
>      first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index aeaba3e..1174741 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -63,7 +63,6 @@ struct sPAPRMachineState {
>      uint32_t htab_shift;
>      hwaddr rma_size;
>      int vrma_adjust;
> -    hwaddr fdt_addr, rtas_addr;
>      ssize_t rtas_size;
>      void *rtas_blob;
>      void *fdt_skel;
> -- 
> 2.7.4
> 

[Qemu-devel] [PATCHv3 03/12] pseries: Make spapr_create_fdt_skel() get information from machine state

[David Gibson]

Currently spapr_create_fdt_skel() takes a bunch of individual parameters
for various things it will put in the device tree.  Some of these can
already be taken directly from sPAPRMachineState.  This patch alters it so
that all of them can be taken from there, which will allow this code to
be moved away from its current caller in future.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c         | 81 ++++++++++++++++++++++----------------------------
 include/hw/ppc/spapr.h |  4 +++
 2 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2c1c7ff..b4c4353 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -271,16 +271,12 @@ static void add_str(GString *s, const gchar *s1)
     g_string_append_len(s, s1, strlen(s1) + 1);
 }
 
-static void *spapr_create_fdt_skel(hwaddr initrd_base,
-                                   hwaddr initrd_size,
-                                   hwaddr kernel_size,
-                                   bool little_endian,
-                                   const char *kernel_cmdline,
-                                   uint32_t epow_irq)
+static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
 {
+    MachineState *machine = MACHINE(spapr);
     void *fdt;
-    uint32_t start_prop = cpu_to_be32(initrd_base);
-    uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
+    uint32_t start_prop = cpu_to_be32(spapr->initrd_base);
+    uint32_t end_prop = cpu_to_be32(spapr->initrd_base + spapr->initrd_size);
     GString *hypertas = g_string_sized_new(256);
     GString *qemu_hypertas = g_string_sized_new(256);
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
@@ -305,11 +301,13 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     fdt = g_malloc0(FDT_MAX_SIZE);
     _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
 
-    if (kernel_size) {
-        _FDT((fdt_add_reservemap_entry(fdt, KERNEL_LOAD_ADDR, kernel_size)));
+    if (spapr->kernel_size) {
+        _FDT((fdt_add_reservemap_entry(fdt, KERNEL_LOAD_ADDR,
+                                       spapr->kernel_size)));
     }
-    if (initrd_size) {
-        _FDT((fdt_add_reservemap_entry(fdt, initrd_base, initrd_size)));
+    if (spapr->initrd_size) {
+        _FDT((fdt_add_reservemap_entry(fdt, spapr->initrd_base,
+                                       spapr->initrd_size)));
     }
     _FDT((fdt_finish_reservemap(fdt)));
 
@@ -354,17 +352,17 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     /* Set Form1_affinity */
     _FDT((fdt_property(fdt, "ibm,architecture-vec-5", vec5, sizeof(vec5))));
 
-    _FDT((fdt_property_string(fdt, "bootargs", kernel_cmdline)));
+    _FDT((fdt_property_string(fdt, "bootargs", machine->kernel_cmdline)));
     _FDT((fdt_property(fdt, "linux,initrd-start",
                        &start_prop, sizeof(start_prop))));
     _FDT((fdt_property(fdt, "linux,initrd-end",
                        &end_prop, sizeof(end_prop))));
-    if (kernel_size) {
+    if (spapr->kernel_size) {
         uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
-                              cpu_to_be64(kernel_size) };
+                              cpu_to_be64(spapr->kernel_size) };
 
         _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop))));
-        if (little_endian) {
+        if (spapr->kernel_le) {
             _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
         }
     }
@@ -441,7 +439,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     _FDT((fdt_end_node(fdt)));
 
     /* event-sources */
-    spapr_events_fdt_skel(fdt, epow_irq);
+    spapr_events_fdt_skel(fdt, spapr->check_exception_irq);
 
     /* /hypervisor node */
     if (kvm_enabled()) {
@@ -1686,7 +1684,6 @@ static void ppc_spapr_init(MachineState *machine)
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
     const char *kernel_filename = machine->kernel_filename;
-    const char *kernel_cmdline = machine->kernel_cmdline;
     const char *initrd_filename = machine->initrd_filename;
     PCIHostState *phb;
     int i;
@@ -1696,10 +1693,7 @@ static void ppc_spapr_init(MachineState *machine)
     void *rma = NULL;
     hwaddr rma_alloc_size;
     hwaddr node0_size = spapr_node0_size();
-    uint32_t initrd_base = 0;
-    long kernel_size = 0, initrd_size = 0;
     long load_limit, fw_size;
-    bool kernel_le = false;
     char *filename;
     int smt = kvmppc_smt_threads();
     int spapr_cores = smp_cpus / smp_threads;
@@ -1972,19 +1966,19 @@ static void ppc_spapr_init(MachineState *machine)
     if (kernel_filename) {
         uint64_t lowaddr = 0;
 
-        kernel_size = load_elf(kernel_filename, translate_kernel_address, NULL,
-                               NULL, &lowaddr, NULL, 1, PPC_ELF_MACHINE,
-                               0, 0);
-        if (kernel_size == ELF_LOAD_WRONG_ENDIAN) {
-            kernel_size = load_elf(kernel_filename,
-                                   translate_kernel_address, NULL,
-                                   NULL, &lowaddr, NULL, 0, PPC_ELF_MACHINE,
-                                   0, 0);
-            kernel_le = kernel_size > 0;
-        }
-        if (kernel_size < 0) {
-            error_report("error loading %s: %s",
-                         kernel_filename, load_elf_strerror(kernel_size));
+        spapr->kernel_size = load_elf(kernel_filename, translate_kernel_address,
+                                      NULL, NULL, &lowaddr, NULL, 1,
+                                      PPC_ELF_MACHINE, 0, 0);
+        if (spapr->kernel_size == ELF_LOAD_WRONG_ENDIAN) {
+            spapr->kernel_size = load_elf(kernel_filename,
+                                          translate_kernel_address, NULL, NULL,
+                                          &lowaddr, NULL, 0, PPC_ELF_MACHINE,
+                                          0, 0);
+            spapr->kernel_le = spapr->kernel_size > 0;
+        }
+        if (spapr->kernel_size < 0) {
+            error_report("error loading %s: %s", kernel_filename,
+                         load_elf_strerror(spapr->kernel_size));
             exit(1);
         }
 
@@ -1993,17 +1987,17 @@ static void ppc_spapr_init(MachineState *machine)
             /* Try to locate the initrd in the gap between the kernel
              * and the firmware. Add a bit of space just in case
              */
-            initrd_base = (KERNEL_LOAD_ADDR + kernel_size + 0x1ffff) & ~0xffff;
-            initrd_size = load_image_targphys(initrd_filename, initrd_base,
-                                              load_limit - initrd_base);
-            if (initrd_size < 0) {
+            spapr->initrd_base = (KERNEL_LOAD_ADDR + spapr->kernel_size
+                                  + 0x1ffff) & ~0xffff;
+            spapr->initrd_size = load_image_targphys(initrd_filename,
+                                                     spapr->initrd_base,
+                                                     load_limit
+                                                     - spapr->initrd_base);
+            if (spapr->initrd_size < 0) {
                 error_report("could not load initial ram disk '%s'",
                              initrd_filename);
                 exit(1);
             }
-        } else {
-            initrd_base = 0;
-            initrd_size = 0;
         }
     }
 
@@ -2030,10 +2024,7 @@ static void ppc_spapr_init(MachineState *machine)
                          &savevm_htab_handlers, spapr);
 
     /* Prepare the device tree */
-    spapr->fdt_skel = spapr_create_fdt_skel(initrd_base, initrd_size,
-                                            kernel_size, kernel_le,
-                                            kernel_cmdline,
-                                            spapr->check_exception_irq);
+    spapr->fdt_skel = spapr_create_fdt_skel(spapr);
     assert(spapr->fdt_skel != NULL);
 
     /* used by RTAS */
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1174741..a0b4bf8 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -65,6 +65,10 @@ struct sPAPRMachineState {
     int vrma_adjust;
     ssize_t rtas_size;
     void *rtas_blob;
+    long kernel_size;
+    bool kernel_le;
+    uint32_t initrd_base;
+    long initrd_size;
     void *fdt_skel;
     uint64_t rtc_offset; /* Now used only during incoming migration */
     struct PPCTimebase tb;
-- 
2.7.4

[Qemu-devel] [PATCHv3 04/12] pseries: Move adding of fdt reserve map entries

[David Gibson]

The flattened device tree passed to pseries guests contains a list of
reserved memory areas.  Currently we construct this list early in
spapr_create_fdt_skel() as we sequentially write the fdt.

This will be inconvenient for upcoming cleanups, so this patch moves
the reserve map changes to the end of fdt construction.  This changes
fdt_add_reservemap_entry() calls - which work when writing the fdt
sequentially to fdt_add_mem_rsv() calls used when altering the fdt in
random access mode.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b4c4353..1eddd1b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -301,14 +301,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
     fdt = g_malloc0(FDT_MAX_SIZE);
     _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
 
-    if (spapr->kernel_size) {
-        _FDT((fdt_add_reservemap_entry(fdt, KERNEL_LOAD_ADDR,
-                                       spapr->kernel_size)));
-    }
-    if (spapr->initrd_size) {
-        _FDT((fdt_add_reservemap_entry(fdt, spapr->initrd_base,
-                                       spapr->initrd_size)));
-    }
     _FDT((fdt_finish_reservemap(fdt)));
 
     /* Root node */
@@ -997,6 +989,15 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     }
 
     g_free(bootlist);
+
+    /* Build memory reserve map */
+    if (spapr->kernel_size) {
+        _FDT((fdt_add_mem_rsv(fdt, KERNEL_LOAD_ADDR, spapr->kernel_size)));
+    }
+    if (spapr->initrd_size) {
+        _FDT((fdt_add_mem_rsv(fdt, spapr->initrd_base, spapr->initrd_size)));
+    }
+
     return fdt;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCHv3 05/12] pseries: Consolidate RTAS loading

[David Gibson]

At each system reset, the pseries machine needs to load RTAS (the runtime
portion of the guest firmware) into the VM.  This means copying
the actual RTAS code into guest memory, and also updating the device
tree so that the guest OS and boot firmware can locate it.

For historical reasons the copy and update to the device tree were in
different parts of the code.  This cleanup brings them both together in
an spapr_load_rtas() function.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c         |  3 +--
 hw/ppc/spapr_rtas.c    | 72 ++++++++++++++++++++++++++++----------------------
 include/hw/ppc/spapr.h |  1 +
 3 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1eddd1b..cb6b1a3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1165,8 +1165,7 @@ static void ppc_spapr_reset(void)
 
     fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
 
-    /* Copy RTAS over */
-    cpu_physical_memory_write(rtas_addr, spapr->rtas_blob, spapr->rtas_size);
+    spapr_load_rtas(spapr, fdt, rtas_addr);
 
     rc = fdt_pack(fdt);
 
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 0db84c8..54b4ad3 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -721,37 +721,6 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
     uint64_t max_hotplug_addr = spapr->hotplug_memory.base +
                                 memory_region_size(&spapr->hotplug_memory.mr);
 
-    ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
-    if (ret < 0) {
-        error_report("Couldn't add RTAS reserve entry: %s",
-                fdt_strerror(ret));
-        return ret;
-    }
-
-    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
-                                rtas_addr);
-    if (ret < 0) {
-        error_report("Couldn't add linux,rtas-base property: %s",
-                fdt_strerror(ret));
-        return ret;
-    }
-
-    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
-                                rtas_addr);
-    if (ret < 0) {
-        error_report("Couldn't add linux,rtas-entry property: %s",
-                fdt_strerror(ret));
-        return ret;
-    }
-
-    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
-                                rtas_size);
-    if (ret < 0) {
-        error_report("Couldn't add rtas-size property: %s",
-                fdt_strerror(ret));
-        return ret;
-    }
-
     for (i = 0; i < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; i++) {
         struct rtas_call *call = &rtas_table[i];
 
@@ -784,6 +753,47 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
     return 0;
 }
 
+void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr)
+{
+    int rtas_node;
+    int ret;
+
+    /* Copy RTAS blob into guest RAM */
+    cpu_physical_memory_write(addr, spapr->rtas_blob, spapr->rtas_size);
+
+    ret = fdt_add_mem_rsv(fdt, addr, spapr->rtas_size);
+    if (ret < 0) {
+        error_report("Couldn't add RTAS reserve entry: %s",
+                     fdt_strerror(ret));
+        exit(1);
+    }
+
+    /* Update the device tree with the blob's location */
+    rtas_node = fdt_path_offset(fdt, "/rtas");
+    assert(rtas_node >= 0);
+
+    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-base", addr);
+    if (ret < 0) {
+        error_report("Couldn't add linux,rtas-base property: %s",
+                     fdt_strerror(ret));
+        exit(1);
+    }
+
+    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-entry", addr);
+    if (ret < 0) {
+        error_report("Couldn't add linux,rtas-entry property: %s",
+                     fdt_strerror(ret));
+        exit(1);
+    }
+
+    ret = fdt_setprop_cell(fdt, rtas_node, "rtas-size", spapr->rtas_size);
+    if (ret < 0) {
+        error_report("Couldn't add rtas-size property: %s",
+                     fdt_strerror(ret));
+        exit(1);
+    }
+}
+
 static void core_rtas_register_types(void)
 {
     spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a0b4bf8..847ae88 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -532,6 +532,7 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPRMachineState *sm,
                              uint32_t nret, target_ulong rets);
 int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
                                  hwaddr rtas_size);
+void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr);
 
 #define SPAPR_TCE_PAGE_SHIFT   12
 #define SPAPR_TCE_PAGE_SIZE    (1ULL << SPAPR_TCE_PAGE_SHIFT)
-- 
2.7.4

[Qemu-devel] [PATCHv3 06/12] pseries: Move construction of /interrupt-controller fdt node

[David Gibson]

Currently the device tree node for the XICS interrupt controller is in
spapr_create_fdt_skel().  As part of consolidating device tree construction
to reset time, this moves it to a function called from spapr_build_fdt().

In addition we move the actual code into hw/intc/xics_spapr.c with the
rest of the PAPR specific interrupt controller code.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/intc/xics_spapr.c  | 22 ++++++++++++++++++++++
 hw/ppc/spapr.c        | 20 +++-----------------
 include/hw/ppc/xics.h |  1 +
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index b4e5501..2e3f1c5 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -32,6 +32,7 @@
 #include "qemu/timer.h"
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/xics.h"
+#include "hw/ppc/fdt.h"
 #include "qapi/visitor.h"
 #include "qapi/error.h"
 
@@ -449,6 +450,27 @@ void xics_spapr_free(XICSState *xics, int irq, int num)
     }
 }
 
+void spapr_dt_xics(XICSState *xics, void *fdt, uint32_t phandle)
+{
+    uint32_t interrupt_server_ranges_prop[] = {
+        0, cpu_to_be32(xics->nr_servers),
+    };
+    int node;
+
+    _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
+
+    _FDT(fdt_setprop_string(fdt, node, "device_type",
+                            "PowerPC-External-Interrupt-Presentation"));
+    _FDT(fdt_setprop_string(fdt, node, "compatible", "IBM,ppc-xicp"));
+    _FDT(fdt_setprop(fdt, node, "interrupt-controller", NULL, 0));
+    _FDT(fdt_setprop(fdt, node, "ibm,interrupt-server-ranges",
+                     interrupt_server_ranges_prop,
+                     sizeof(interrupt_server_ranges_prop)));
+    _FDT(fdt_setprop_cell(fdt, node, "#interrupt-cells", 2));
+    _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
+    _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
+}
+
 static void xics_spapr_register_types(void)
 {
     type_register_static(&xics_spapr_info);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cb6b1a3..2989c9d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -280,7 +280,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
     GString *hypertas = g_string_sized_new(256);
     GString *qemu_hypertas = g_string_sized_new(256);
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
-    uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(max_cpus)};
     unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
     char *buf;
 
@@ -402,22 +401,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
 
     _FDT((fdt_end_node(fdt)));
 
-    /* interrupt controller */
-    _FDT((fdt_begin_node(fdt, "interrupt-controller")));
-
-    _FDT((fdt_property_string(fdt, "device_type",
-                              "PowerPC-External-Interrupt-Presentation")));
-    _FDT((fdt_property_string(fdt, "compatible", "IBM,ppc-xicp")));
-    _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0)));
-    _FDT((fdt_property(fdt, "ibm,interrupt-server-ranges",
-                       interrupt_server_ranges_prop,
-                       sizeof(interrupt_server_ranges_prop))));
-    _FDT((fdt_property_cell(fdt, "#interrupt-cells", 2)));
-    _FDT((fdt_property_cell(fdt, "linux,phandle", PHANDLE_XICP)));
-    _FDT((fdt_property_cell(fdt, "phandle", PHANDLE_XICP)));
-
-    _FDT((fdt_end_node(fdt)));
-
     /* vdevice */
     _FDT((fdt_begin_node(fdt, "vdevice")));
 
@@ -909,6 +892,9 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     /* open out the base tree into a temp buffer for the final tweaks */
     _FDT((fdt_open_into(spapr->fdt_skel, fdt, FDT_MAX_SIZE)));
 
+    /* /interrupt controller */
+    spapr_dt_xics(spapr->xics, fdt, PHANDLE_XICP);
+
     ret = spapr_populate_memory(spapr, fdt);
     if (ret < 0) {
         error_report("couldn't setup memory nodes in fdt");
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 6e5a113..3f0c316 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -187,6 +187,7 @@ int xics_spapr_alloc(XICSState *icp, int irq_hint, bool lsi, Error **errp);
 int xics_spapr_alloc_block(XICSState *icp, int num, bool lsi, bool align,
                            Error **errp);
 void xics_spapr_free(XICSState *icp, int irq, int num);
+void spapr_dt_xics(XICSState *xics, void *fdt, uint32_t phandle);
 
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
 void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
-- 
2.7.4

[Qemu-devel] [PATCHv3 07/12] pseries: Consolidate construction of /chosen device tree node

[David Gibson]

For historical reasons, building the /chosen node in the guest device tree
is split across several places and includes both parts which write the DT
sequentially and others which use random access functions.

This patch consolidates construction of the node into one place, using
random access functions throughout.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr.c             | 131 ++++++++++++++++++++++-----------------------
 hw/ppc/spapr_vio.c         |  17 ++----
 include/hw/ppc/spapr_vio.h |   2 +-
 3 files changed, 70 insertions(+), 80 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2989c9d..8e71f1e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -273,14 +273,10 @@ static void add_str(GString *s, const gchar *s1)
 
 static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
 {
-    MachineState *machine = MACHINE(spapr);
     void *fdt;
-    uint32_t start_prop = cpu_to_be32(spapr->initrd_base);
-    uint32_t end_prop = cpu_to_be32(spapr->initrd_base + spapr->initrd_size);
     GString *hypertas = g_string_sized_new(256);
     GString *qemu_hypertas = g_string_sized_new(256);
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
-    unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
     char *buf;
 
     add_str(hypertas, "hcall-pft");
@@ -337,35 +333,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
     _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
     _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
 
-    /* /chosen */
-    _FDT((fdt_begin_node(fdt, "chosen")));
-
-    /* Set Form1_affinity */
-    _FDT((fdt_property(fdt, "ibm,architecture-vec-5", vec5, sizeof(vec5))));
-
-    _FDT((fdt_property_string(fdt, "bootargs", machine->kernel_cmdline)));
-    _FDT((fdt_property(fdt, "linux,initrd-start",
-                       &start_prop, sizeof(start_prop))));
-    _FDT((fdt_property(fdt, "linux,initrd-end",
-                       &end_prop, sizeof(end_prop))));
-    if (spapr->kernel_size) {
-        uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
-                              cpu_to_be64(spapr->kernel_size) };
-
-        _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop))));
-        if (spapr->kernel_le) {
-            _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
-        }
-    }
-    if (boot_menu) {
-        _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
-    }
-    _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width)));
-    _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height)));
-    _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth)));
-
-    _FDT((fdt_end_node(fdt)));
-
     /* RTAS */
     _FDT((fdt_begin_node(fdt, "rtas")));
 
@@ -873,6 +840,68 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
     return 0;
 }
 
+static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
+{
+    MachineState *machine = MACHINE(spapr);
+    int chosen;
+    const char *boot_device = machine->boot_order;
+    char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus);
+    size_t cb = 0;
+    char *bootlist = get_boot_devices_list(&cb, true);
+    unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
+
+    _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
+
+    /* Set Form1_affinity */
+    _FDT(fdt_setprop(fdt, chosen, "ibm,architecture-vec-5",
+                     vec5, sizeof(vec5)));
+
+    _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
+    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
+                          spapr->initrd_base));
+    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
+                          spapr->initrd_base + spapr->initrd_size));
+
+    if (spapr->kernel_size) {
+        uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
+                              cpu_to_be64(spapr->kernel_size) };
+
+        _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel",
+                         &kprop, sizeof(kprop)));
+        if (spapr->kernel_le) {
+            _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel-le", NULL, 0));
+        }
+    }
+    if (boot_menu) {
+        _FDT((fdt_setprop_cell(fdt, chosen, "qemu,boot-menu", boot_menu)));
+    }
+    _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-width", graphic_width));
+    _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-height", graphic_height));
+    _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-depth", graphic_depth));
+
+    if (cb && bootlist) {
+        int i;
+
+        for (i = 0; i < cb; i++) {
+            if (bootlist[i] == '\n') {
+                bootlist[i] = ' ';
+            }
+        }
+        _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-list", bootlist));
+    }
+
+    if (boot_device && strlen(boot_device)) {
+        _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-device", boot_device));
+    }
+
+    if (!spapr->has_graphics && stdout_path) {
+        _FDT(fdt_setprop_string(fdt, chosen, "linux,stdout-path", stdout_path));
+    }
+
+    g_free(stdout_path);
+    g_free(bootlist);
+}
+
 static void *spapr_build_fdt(sPAPRMachineState *spapr,
                              hwaddr rtas_addr,
                              hwaddr rtas_size)
@@ -880,10 +909,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     MachineState *machine = MACHINE(qdev_get_machine());
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
-    const char *boot_device = machine->boot_order;
-    int ret, i;
-    size_t cb = 0;
-    char *bootlist;
+    int ret;
     void *fdt;
     sPAPRPHBState *phb;
 
@@ -932,34 +958,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     /* cpus */
     spapr_populate_cpus_dt_node(fdt, spapr);
 
-    bootlist = get_boot_devices_list(&cb, true);
-    if (cb && bootlist) {
-        int offset = fdt_path_offset(fdt, "/chosen");
-        if (offset < 0) {
-            exit(1);
-        }
-        for (i = 0; i < cb; i++) {
-            if (bootlist[i] == '\n') {
-                bootlist[i] = ' ';
-            }
-
-        }
-        ret = fdt_setprop_string(fdt, offset, "qemu,boot-list", bootlist);
-    }
-
-    if (boot_device && strlen(boot_device)) {
-        int offset = fdt_path_offset(fdt, "/chosen");
-
-        if (offset < 0) {
-            exit(1);
-        }
-        fdt_setprop_string(fdt, offset, "qemu,boot-device", boot_device);
-    }
-
-    if (!spapr->has_graphics) {
-        spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
-    }
-
     if (smc->dr_lmb_enabled) {
         _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
     }
@@ -974,7 +972,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         }
     }
 
-    g_free(bootlist);
+    /* /chosen */
+    spapr_dt_chosen(spapr, fdt);
 
     /* Build memory reserve map */
     if (spapr->kernel_size) {
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 3648aa5..2b67df0 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -665,28 +665,19 @@ out:
     return ret;
 }
 
-int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus *bus)
+gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus)
 {
     VIOsPAPRDevice *dev;
     char *name, *path;
-    int ret, offset;
 
     dev = spapr_vty_get_default(bus);
-    if (!dev)
-        return 0;
-
-    offset = fdt_path_offset(fdt, "/chosen");
-    if (offset < 0) {
-        return offset;
+    if (!dev) {
+        return NULL;
     }
 
     name = spapr_vio_get_dev_name(DEVICE(dev));
     path = g_strdup_printf("/vdevice/%s", name);
 
-    ret = fdt_setprop_string(fdt, offset, "linux,stdout-path", path);
-
     g_free(name);
-    g_free(path);
-
-    return ret;
+    return path;
 }
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index 0b025fd..a0e7542 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -81,7 +81,7 @@ struct VIOsPAPRBus {
 extern VIOsPAPRBus *spapr_vio_bus_init(void);
 extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg);
 extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt);
-extern int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus *bus);
+extern gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus);
 
 static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev)
 {
-- 
2.7.4

Re: [Qemu-devel] [PATCHv3 07/12] pseries: Consolidate construction of /chosen device tree node

[Michael Roth]

Quoting David Gibson (2016-10-24 00:04:38)
> For historical reasons, building the /chosen node in the guest device tree
> is split across several places and includes both parts which write the DT
> sequentially and others which use random access functions.
> 
> This patch consolidates construction of the node into one place, using
> random access functions throughout.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  hw/ppc/spapr.c             | 131 ++++++++++++++++++++++-----------------------
>  hw/ppc/spapr_vio.c         |  17 ++----
>  include/hw/ppc/spapr_vio.h |   2 +-
>  3 files changed, 70 insertions(+), 80 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2989c9d..8e71f1e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -273,14 +273,10 @@ static void add_str(GString *s, const gchar *s1)
> 
>  static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
>  {
> -    MachineState *machine = MACHINE(spapr);
>      void *fdt;
> -    uint32_t start_prop = cpu_to_be32(spapr->initrd_base);
> -    uint32_t end_prop = cpu_to_be32(spapr->initrd_base + spapr->initrd_size);
>      GString *hypertas = g_string_sized_new(256);
>      GString *qemu_hypertas = g_string_sized_new(256);
>      uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
> -    unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
>      char *buf;
> 
>      add_str(hypertas, "hcall-pft");
> @@ -337,35 +333,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
>      _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>      _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
> 
> -    /* /chosen */
> -    _FDT((fdt_begin_node(fdt, "chosen")));
> -
> -    /* Set Form1_affinity */
> -    _FDT((fdt_property(fdt, "ibm,architecture-vec-5", vec5, sizeof(vec5))));
> -
> -    _FDT((fdt_property_string(fdt, "bootargs", machine->kernel_cmdline)));
> -    _FDT((fdt_property(fdt, "linux,initrd-start",
> -                       &start_prop, sizeof(start_prop))));
> -    _FDT((fdt_property(fdt, "linux,initrd-end",
> -                       &end_prop, sizeof(end_prop))));
> -    if (spapr->kernel_size) {
> -        uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
> -                              cpu_to_be64(spapr->kernel_size) };
> -
> -        _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop))));
> -        if (spapr->kernel_le) {
> -            _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
> -        }
> -    }
> -    if (boot_menu) {
> -        _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
> -    }
> -    _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width)));
> -    _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height)));
> -    _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth)));
> -
> -    _FDT((fdt_end_node(fdt)));
> -
>      /* RTAS */
>      _FDT((fdt_begin_node(fdt, "rtas")));
> 
> @@ -873,6 +840,68 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
>      return 0;
>  }
> 
> +static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
> +{
> +    MachineState *machine = MACHINE(spapr);
> +    int chosen;
> +    const char *boot_device = machine->boot_order;
> +    char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus);
> +    size_t cb = 0;
> +    char *bootlist = get_boot_devices_list(&cb, true);
> +    unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
> +
> +    _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
> +
> +    /* Set Form1_affinity */
> +    _FDT(fdt_setprop(fdt, chosen, "ibm,architecture-vec-5",
> +                     vec5, sizeof(vec5)));
> +
> +    _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
> +    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
> +                          spapr->initrd_base));
> +    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
> +                          spapr->initrd_base + spapr->initrd_size));
> +
> +    if (spapr->kernel_size) {
> +        uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
> +                              cpu_to_be64(spapr->kernel_size) };
> +
> +        _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel",
> +                         &kprop, sizeof(kprop)));
> +        if (spapr->kernel_le) {
> +            _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel-le", NULL, 0));
> +        }
> +    }
> +    if (boot_menu) {
> +        _FDT((fdt_setprop_cell(fdt, chosen, "qemu,boot-menu", boot_menu)));
> +    }
> +    _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-width", graphic_width));
> +    _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-height", graphic_height));
> +    _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-depth", graphic_depth));
> +
> +    if (cb && bootlist) {
> +        int i;
> +
> +        for (i = 0; i < cb; i++) {
> +            if (bootlist[i] == '\n') {
> +                bootlist[i] = ' ';
> +            }
> +        }
> +        _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-list", bootlist));
> +    }
> +
> +    if (boot_device && strlen(boot_device)) {
> +        _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-device", boot_device));
> +    }
> +
> +    if (!spapr->has_graphics && stdout_path) {
> +        _FDT(fdt_setprop_string(fdt, chosen, "linux,stdout-path", stdout_path));
> +    }
> +
> +    g_free(stdout_path);
> +    g_free(bootlist);
> +}
> +
>  static void *spapr_build_fdt(sPAPRMachineState *spapr,
>                               hwaddr rtas_addr,
>                               hwaddr rtas_size)
> @@ -880,10 +909,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      MachineState *machine = MACHINE(qdev_get_machine());
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> -    const char *boot_device = machine->boot_order;
> -    int ret, i;
> -    size_t cb = 0;
> -    char *bootlist;
> +    int ret;
>      void *fdt;
>      sPAPRPHBState *phb;
> 
> @@ -932,34 +958,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      /* cpus */
>      spapr_populate_cpus_dt_node(fdt, spapr);
> 
> -    bootlist = get_boot_devices_list(&cb, true);
> -    if (cb && bootlist) {
> -        int offset = fdt_path_offset(fdt, "/chosen");
> -        if (offset < 0) {
> -            exit(1);
> -        }
> -        for (i = 0; i < cb; i++) {
> -            if (bootlist[i] == '\n') {
> -                bootlist[i] = ' ';
> -            }
> -
> -        }
> -        ret = fdt_setprop_string(fdt, offset, "qemu,boot-list", bootlist);
> -    }
> -
> -    if (boot_device && strlen(boot_device)) {
> -        int offset = fdt_path_offset(fdt, "/chosen");
> -
> -        if (offset < 0) {
> -            exit(1);
> -        }
> -        fdt_setprop_string(fdt, offset, "qemu,boot-device", boot_device);
> -    }
> -
> -    if (!spapr->has_graphics) {
> -        spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
> -    }
> -
>      if (smc->dr_lmb_enabled) {
>          _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
>      }
> @@ -974,7 +972,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>          }
>      }
> 
> -    g_free(bootlist);
> +    /* /chosen */
> +    spapr_dt_chosen(spapr, fdt);
> 
>      /* Build memory reserve map */
>      if (spapr->kernel_size) {
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 3648aa5..2b67df0 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -665,28 +665,19 @@ out:
>      return ret;
>  }
> 
> -int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus *bus)
> +gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus)
>  {
>      VIOsPAPRDevice *dev;
>      char *name, *path;
> -    int ret, offset;
> 
>      dev = spapr_vty_get_default(bus);
> -    if (!dev)
> -        return 0;
> -
> -    offset = fdt_path_offset(fdt, "/chosen");
> -    if (offset < 0) {
> -        return offset;
> +    if (!dev) {
> +        return NULL;
>      }
> 
>      name = spapr_vio_get_dev_name(DEVICE(dev));
>      path = g_strdup_printf("/vdevice/%s", name);
> 
> -    ret = fdt_setprop_string(fdt, offset, "linux,stdout-path", path);
> -
>      g_free(name);
> -    g_free(path);
> -
> -    return ret;
> +    return path;
>  }
> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
> index 0b025fd..a0e7542 100644
> --- a/include/hw/ppc/spapr_vio.h
> +++ b/include/hw/ppc/spapr_vio.h
> @@ -81,7 +81,7 @@ struct VIOsPAPRBus {
>  extern VIOsPAPRBus *spapr_vio_bus_init(void);
>  extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg);
>  extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt);
> -extern int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus *bus);
> +extern gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus);
> 
>  static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev)
>  {
> -- 
> 2.7.4
> 

[Qemu-devel] [PATCHv3 08/12] pseries: Consolidate construction of /rtas device tree node

[David Gibson]

For historical reasons construction of the /rtas node in the device
tree (amongst others) is split into several places.  In particular
it's split between spapr_create_fdt_skel(), spapr_build_fdt() and
spapr_rtas_device_tree_setup().

In fact, as well as adding the actual RTAS tokens to the device tree,
spapr_rtas_device_tree_setup() just adds the ibm,lrdr-capacity
property, which despite going in the /rtas node, doesn't have a lot to
do with RTAS.

This patch consolidates the code constructing /rtas together into a new
spapr_dt_rtas() function.  spapr_rtas_device_tree_setup() is renamed to
spapr_dt_rtas_tokens() and now only adds the token properties.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c         | 130 +++++++++++++++++++++++++++----------------------
 hw/ppc/spapr_rtas.c    |  33 ++-----------
 include/hw/ppc/spapr.h |   3 +-
 3 files changed, 76 insertions(+), 90 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8e71f1e..cf2002c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -274,25 +274,8 @@ static void add_str(GString *s, const gchar *s1)
 static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
 {
     void *fdt;
-    GString *hypertas = g_string_sized_new(256);
-    GString *qemu_hypertas = g_string_sized_new(256);
-    uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
     char *buf;
 
-    add_str(hypertas, "hcall-pft");
-    add_str(hypertas, "hcall-term");
-    add_str(hypertas, "hcall-dabr");
-    add_str(hypertas, "hcall-interrupt");
-    add_str(hypertas, "hcall-tce");
-    add_str(hypertas, "hcall-vio");
-    add_str(hypertas, "hcall-splpar");
-    add_str(hypertas, "hcall-bulk");
-    add_str(hypertas, "hcall-set-mode");
-    add_str(hypertas, "hcall-sprg0");
-    add_str(hypertas, "hcall-copy");
-    add_str(hypertas, "hcall-debug");
-    add_str(qemu_hypertas, "hcall-memop1");
-
     fdt = g_malloc0(FDT_MAX_SIZE);
     _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
 
@@ -333,41 +316,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
     _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
     _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
 
-    /* RTAS */
-    _FDT((fdt_begin_node(fdt, "rtas")));
-
-    if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
-        add_str(hypertas, "hcall-multi-tce");
-    }
-    _FDT((fdt_property(fdt, "ibm,hypertas-functions", hypertas->str,
-                       hypertas->len)));
-    g_string_free(hypertas, TRUE);
-    _FDT((fdt_property(fdt, "qemu,hypertas-functions", qemu_hypertas->str,
-                       qemu_hypertas->len)));
-    g_string_free(qemu_hypertas, TRUE);
-
-    _FDT((fdt_property(fdt, "ibm,associativity-reference-points",
-        refpoints, sizeof(refpoints))));
-
-    _FDT((fdt_property_cell(fdt, "rtas-error-log-max", RTAS_ERROR_LOG_MAX)));
-    _FDT((fdt_property_cell(fdt, "rtas-event-scan-rate",
-                            RTAS_EVENT_SCAN_RATE)));
-
-    if (msi_nonbroken) {
-        _FDT((fdt_property(fdt, "ibm,change-msix-capable", NULL, 0)));
-    }
-
-    /*
-     * According to PAPR, rtas ibm,os-term does not guarantee a return
-     * back to the guest cpu.
-     *
-     * While an additional ibm,extended-os-term property indicates that
-     * rtas call return will always occur. Set this property.
-     */
-    _FDT((fdt_property(fdt, "ibm,extended-os-term", NULL, 0)));
-
-    _FDT((fdt_end_node(fdt)));
-
     /* vdevice */
     _FDT((fdt_begin_node(fdt, "vdevice")));
 
@@ -840,6 +788,75 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
     return 0;
 }
 
+static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
+{
+    int rtas;
+    GString *hypertas = g_string_sized_new(256);
+    GString *qemu_hypertas = g_string_sized_new(256);
+    uint32_t refpoints[] = { cpu_to_be32(0x4), cpu_to_be32(0x4) };
+    uint64_t max_hotplug_addr = spapr->hotplug_memory.base +
+        memory_region_size(&spapr->hotplug_memory.mr);
+    uint32_t lrdr_capacity[] = {
+        cpu_to_be32(max_hotplug_addr >> 32),
+        cpu_to_be32(max_hotplug_addr & 0xffffffff),
+        0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
+        cpu_to_be32(max_cpus / smp_threads),
+    };
+
+    _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
+
+    /* hypertas */
+    add_str(hypertas, "hcall-pft");
+    add_str(hypertas, "hcall-term");
+    add_str(hypertas, "hcall-dabr");
+    add_str(hypertas, "hcall-interrupt");
+    add_str(hypertas, "hcall-tce");
+    add_str(hypertas, "hcall-vio");
+    add_str(hypertas, "hcall-splpar");
+    add_str(hypertas, "hcall-bulk");
+    add_str(hypertas, "hcall-set-mode");
+    add_str(hypertas, "hcall-sprg0");
+    add_str(hypertas, "hcall-copy");
+    add_str(hypertas, "hcall-debug");
+    add_str(qemu_hypertas, "hcall-memop1");
+
+    if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
+        add_str(hypertas, "hcall-multi-tce");
+    }
+    _FDT(fdt_setprop(fdt, rtas, "ibm,hypertas-functions",
+                     hypertas->str, hypertas->len));
+    g_string_free(hypertas, TRUE);
+    _FDT(fdt_setprop(fdt, rtas, "qemu,hypertas-functions",
+                     qemu_hypertas->str, qemu_hypertas->len));
+    g_string_free(qemu_hypertas, TRUE);
+
+    _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
+                     refpoints, sizeof(refpoints)));
+
+    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
+                          RTAS_ERROR_LOG_MAX));
+    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",
+                          RTAS_EVENT_SCAN_RATE));
+
+    if (msi_nonbroken) {
+        _FDT(fdt_setprop(fdt, rtas, "ibm,change-msix-capable", NULL, 0));
+    }
+
+    /*
+     * According to PAPR, rtas ibm,os-term does not guarantee a return
+     * back to the guest cpu.
+     *
+     * While an additional ibm,extended-os-term property indicates
+     * that rtas call return will always occur. Set this property.
+     */
+    _FDT(fdt_setprop(fdt, rtas, "ibm,extended-os-term", NULL, 0));
+
+    _FDT(fdt_setprop(fdt, rtas, "ibm,lrdr-capacity",
+                     lrdr_capacity, sizeof(lrdr_capacity)));
+
+    spapr_dt_rtas_tokens(fdt, rtas);
+}
+
 static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
 {
     MachineState *machine = MACHINE(spapr);
@@ -949,12 +966,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         }
     }
 
-    /* RTAS */
-    ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
-    if (ret < 0) {
-        error_report("Couldn't set up RTAS device tree properties");
-    }
-
     /* cpus */
     spapr_populate_cpus_dt_node(fdt, spapr);
 
@@ -972,6 +983,9 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         }
     }
 
+    /* /rtas */
+    spapr_dt_rtas(spapr, fdt);
+
     /* /chosen */
     spapr_dt_chosen(spapr, fdt);
 
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 54b4ad3..bb19944 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -46,6 +46,7 @@
 #include "hw/ppc/spapr_drc.h"
 #include "qemu/cutils.h"
 #include "trace.h"
+#include "hw/ppc/fdt.h"
 
 static sPAPRConfigureConnectorState *spapr_ccs_find(sPAPRMachineState *spapr,
                                                     uint32_t drc_index)
@@ -710,16 +711,9 @@ void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
     rtas_table[token].fn = fn;
 }
 
-int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
-                                 hwaddr rtas_size)
+void spapr_dt_rtas_tokens(void *fdt, int rtas)
 {
-    int ret;
     int i;
-    uint32_t lrdr_capacity[5];
-    MachineState *machine = MACHINE(qdev_get_machine());
-    sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
-    uint64_t max_hotplug_addr = spapr->hotplug_memory.base +
-                                memory_region_size(&spapr->hotplug_memory.mr);
 
     for (i = 0; i < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; i++) {
         struct rtas_call *call = &rtas_table[i];
@@ -728,29 +722,8 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
             continue;
         }
 
-        ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
-                                    i + RTAS_TOKEN_BASE);
-        if (ret < 0) {
-            error_report("Couldn't add rtas token for %s: %s",
-                    call->name, fdt_strerror(ret));
-            return ret;
-        }
-
+        _FDT(fdt_setprop_cell(fdt, rtas, call->name, i + RTAS_TOKEN_BASE));
     }
-
-    lrdr_capacity[0] = cpu_to_be32(max_hotplug_addr >> 32);
-    lrdr_capacity[1] = cpu_to_be32(max_hotplug_addr & 0xffffffff);
-    lrdr_capacity[2] = 0;
-    lrdr_capacity[3] = cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE);
-    lrdr_capacity[4] = cpu_to_be32(max_cpus/smp_threads);
-    ret = qemu_fdt_setprop(fdt, "/rtas", "ibm,lrdr-capacity", lrdr_capacity,
-                     sizeof(lrdr_capacity));
-    if (ret < 0) {
-        error_report("Couldn't add ibm,lrdr-capacity rtas property");
-        return ret;
-    }
-
-    return 0;
 }
 
 void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 847ae88..8b8bb97 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -530,8 +530,7 @@ void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn);
 target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPRMachineState *sm,
                              uint32_t token, uint32_t nargs, target_ulong args,
                              uint32_t nret, target_ulong rets);
-int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
-                                 hwaddr rtas_size);
+void spapr_dt_rtas_tokens(void *fdt, int rtas);
 void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr);
 
 #define SPAPR_TCE_PAGE_SHIFT   12
-- 
2.7.4

[Qemu-devel] [PATCHv3 09/12] pseries: Move /event-sources construction to spapr_build_fdt()

[David Gibson]

The /event-sources device tree node is built from spapr_create_fdt_skel().
As part of consolidating device tree construction to reset time, this moves
it to spapr_build_fdt().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c         |  6 +++---
 hw/ppc/spapr_events.c  | 21 ++++++++++-----------
 include/hw/ppc/spapr.h |  2 +-
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cf2002c..9477c6f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -328,9 +328,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
 
     _FDT((fdt_end_node(fdt)));
 
-    /* event-sources */
-    spapr_events_fdt_skel(fdt, spapr->check_exception_irq);
-
     /* /hypervisor node */
     if (kvm_enabled()) {
         uint8_t hypercall[16];
@@ -983,6 +980,9 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         }
     }
 
+    /* /event-sources */
+    spapr_dt_events(fdt, spapr->check_exception_irq);
+
     /* /rtas */
     spapr_dt_rtas(spapr, fdt);
 
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 6d35345..89aa5a7 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -211,23 +211,22 @@ struct hp_log_full {
 #define EVENT_MASK_HOTPLUG                   0x10000000
 #define EVENT_MASK_IO                        0x08000000
 
-void spapr_events_fdt_skel(void *fdt, uint32_t check_exception_irq)
+void spapr_dt_events(void *fdt, uint32_t check_exception_irq)
 {
+    int event_sources, epow_events;
     uint32_t irq_ranges[] = {cpu_to_be32(check_exception_irq), cpu_to_be32(1)};
     uint32_t interrupts[] = {cpu_to_be32(check_exception_irq), 0};
 
-    _FDT((fdt_begin_node(fdt, "event-sources")));
+    _FDT(event_sources = fdt_add_subnode(fdt, 0, "event-sources"));
 
-    _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0)));
-    _FDT((fdt_property_cell(fdt, "#interrupt-cells", 2)));
-    _FDT((fdt_property(fdt, "interrupt-ranges",
-                       irq_ranges, sizeof(irq_ranges))));
+    _FDT(fdt_setprop(fdt, event_sources, "interrupt-controller", NULL, 0));
+    _FDT(fdt_setprop_cell(fdt, event_sources, "#interrupt-cells", 2));
+    _FDT(fdt_setprop(fdt, event_sources, "interrupt-ranges",
+                     irq_ranges, sizeof(irq_ranges)));
 
-    _FDT((fdt_begin_node(fdt, "epow-events")));
-    _FDT((fdt_property(fdt, "interrupts", interrupts, sizeof(interrupts))));
-    _FDT((fdt_end_node(fdt)));
-
-    _FDT((fdt_end_node(fdt)));
+    _FDT(epow_events = fdt_add_subnode(fdt, event_sources, "epow-events"));
+    _FDT(fdt_setprop(fdt, epow_events, "interrupts",
+                     interrupts, sizeof(interrupts)));
 }
 
 static void rtas_event_log_queue(int log_type, void *data, bool exception)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 8b8bb97..ae7d1d7 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -581,7 +581,7 @@ struct sPAPREventLogEntry {
 };
 
 void spapr_events_init(sPAPRMachineState *sm);
-void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
+void spapr_dt_events(void *fdt, uint32_t check_exception_irq);
 int spapr_h_cas_compose_response(sPAPRMachineState *sm,
                                  target_ulong addr, target_ulong size,
                                  bool cpu_update, bool memory_update);
-- 
2.7.4

[Qemu-devel] [PATCHv3 10/12] pseries: Move /hypervisor node construction to fdt_build_fdt()

[David Gibson]

Currently the /hypervisor device tree node is constructed in
spapr_create_fdt_skel().  As part of consolidating device tree construction
to reset time, move it to a function called from spapr_build_fdt().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 49 ++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9477c6f..0882698 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -328,27 +328,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
 
     _FDT((fdt_end_node(fdt)));
 
-    /* /hypervisor node */
-    if (kvm_enabled()) {
-        uint8_t hypercall[16];
-
-        /* indicate KVM hypercall interface */
-        _FDT((fdt_begin_node(fdt, "hypervisor")));
-        _FDT((fdt_property_string(fdt, "compatible", "linux,kvm")));
-        if (kvmppc_has_cap_fixup_hcalls()) {
-            /*
-             * Older KVM versions with older guest kernels were broken with the
-             * magic page, don't allow the guest to map it.
-             */
-            if (!kvmppc_get_hypercall(first_cpu->env_ptr, hypercall,
-                                      sizeof(hypercall))) {
-                _FDT((fdt_property(fdt, "hcall-instructions", hypercall,
-                                   sizeof(hypercall))));
-            }
-        }
-        _FDT((fdt_end_node(fdt)));
-    }
-
     _FDT((fdt_end_node(fdt))); /* close root node */
     _FDT((fdt_finish(fdt)));
 
@@ -916,6 +895,29 @@ static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt)
     g_free(bootlist);
 }
 
+static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt)
+{
+    /* The /hypervisor node isn't in PAPR - this is a hack to allow PR
+     * KVM to work under pHyp with some guest co-operation */
+    int hypervisor;
+    uint8_t hypercall[16];
+
+    _FDT(hypervisor = fdt_add_subnode(fdt, 0, "hypervisor"));
+    /* indicate KVM hypercall interface */
+    _FDT(fdt_setprop_string(fdt, hypervisor, "compatible", "linux,kvm"));
+    if (kvmppc_has_cap_fixup_hcalls()) {
+        /*
+         * Older KVM versions with older guest kernels were broken
+         * with the magic page, don't allow the guest to map it.
+         */
+        if (!kvmppc_get_hypercall(first_cpu->env_ptr, hypercall,
+                                  sizeof(hypercall))) {
+            _FDT(fdt_setprop(fdt, hypervisor, "hcall-instructions",
+                             hypercall, sizeof(hypercall)));
+        }
+    }
+}
+
 static void *spapr_build_fdt(sPAPRMachineState *spapr,
                              hwaddr rtas_addr,
                              hwaddr rtas_size)
@@ -989,6 +991,11 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     /* /chosen */
     spapr_dt_chosen(spapr, fdt);
 
+    /* /hypervisor */
+    if (kvm_enabled()) {
+        spapr_dt_hypervisor(spapr, fdt);
+    }
+
     /* Build memory reserve map */
     if (spapr->kernel_size) {
         _FDT((fdt_add_mem_rsv(fdt, KERNEL_LOAD_ADDR, spapr->kernel_size)));
-- 
2.7.4

[Qemu-devel] [PATCHv3 11/12] pseries: Consolidate construction of /vdevice device tree node

[David Gibson]

Construction of the /vdevice node (and its children) is divided between
spapr_create_fdt_skel() (at init time), which creates the base node, and
spapr_populate_vdevice() (at reset time) which creates the nodes for each
individual virtual device.

This consolidates both into a single function called from
spapr_build_fdt().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c             | 19 ++-----------------
 hw/ppc/spapr_vio.c         | 23 ++++++++++++++++-------
 include/hw/ppc/spapr_vio.h |  2 +-
 3 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0882698..83d4d05 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -316,18 +316,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
     _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
     _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
 
-    /* vdevice */
-    _FDT((fdt_begin_node(fdt, "vdevice")));
-
-    _FDT((fdt_property_string(fdt, "device_type", "vdevice")));
-    _FDT((fdt_property_string(fdt, "compatible", "IBM,vdevice")));
-    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
-    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
-    _FDT((fdt_property_cell(fdt, "#interrupt-cells", 0x2)));
-    _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0)));
-
-    _FDT((fdt_end_node(fdt)));
-
     _FDT((fdt_end_node(fdt))); /* close root node */
     _FDT((fdt_finish(fdt)));
 
@@ -943,11 +931,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         exit(1);
     }
 
-    ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
-    if (ret < 0) {
-        error_report("couldn't setup vio devices in fdt");
-        exit(1);
-    }
+    /* /vdevice */
+    spapr_dt_vdevice(spapr->vio_bus, fdt);
 
     if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
         ret = spapr_rng_populate_dt(fdt);
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 2b67df0..cc1e09c 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -36,6 +36,7 @@
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
 #include "hw/ppc/xics.h"
+#include "hw/ppc/fdt.h"
 #include "trace.h"
 
 #include <libfdt.h>
@@ -624,11 +625,21 @@ static int compare_reg(const void *p1, const void *p2)
     return 1;
 }
 
-int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt)
+void spapr_dt_vdevice(VIOsPAPRBus *bus, void *fdt)
 {
     DeviceState *qdev, **qdevs;
     BusChild *kid;
     int i, num, ret = 0;
+    int node;
+
+    _FDT(node = fdt_add_subnode(fdt, 0, "vdevice"));
+
+    _FDT(fdt_setprop_string(fdt, node, "device_type", "vdevice"));
+    _FDT(fdt_setprop_string(fdt, node, "compatible", "IBM,vdevice"));
+    _FDT(fdt_setprop_cell(fdt, node, "#address-cells", 1));
+    _FDT(fdt_setprop_cell(fdt, node, "#size-cells", 0));
+    _FDT(fdt_setprop_cell(fdt, node, "#interrupt-cells", 2));
+    _FDT(fdt_setprop(fdt, node, "interrupt-controller", NULL, 0));
 
     /* Count qdevs on the bus list */
     num = 0;
@@ -650,19 +661,17 @@ int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt)
      * to know that will mean they are in forward order in the tree. */
     for (i = num - 1; i >= 0; i--) {
         VIOsPAPRDevice *dev = (VIOsPAPRDevice *)(qdevs[i]);
+        VIOsPAPRDeviceClass *vdc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
 
         ret = vio_make_devnode(dev, fdt);
-
         if (ret < 0) {
-            goto out;
+            error_report("Couldn't create device node /vdevice/%s@%"PRIx32,
+                         vdc->dt_name, dev->reg);
+            exit(1);
         }
     }
 
-    ret = 0;
-out:
     g_free(qdevs);
-
-    return ret;
 }
 
 gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus)
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index a0e7542..14f5022 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -80,7 +80,7 @@ struct VIOsPAPRBus {
 
 extern VIOsPAPRBus *spapr_vio_bus_init(void);
 extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg);
-extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt);
+void spapr_dt_vdevice(VIOsPAPRBus *bus, void *fdt);
 extern gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus);
 
 static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev)
-- 
2.7.4

[Qemu-devel] [PATCHv3 12/12] pseries: Remove spapr_create_fdt_skel()

[David Gibson]

For historical reasons construction of the guest device tree in spapr is
divided between spapr_create_fdt_skel() which is called at init time, and
spapr_build_fdt() which runs at reset time.  Over time, more and more
things have needed to be moved to reset time.

Previous cleanups mean the only things left in spapr_create_fdt_skel() are
the properties of the root node itself.  Finish consolidating these two
parts of device tree construction, by moving this to the start of
spapr_build_fdt(), and removing spapr_create_fdt_skel() entirely.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c         | 94 +++++++++++++++++++-------------------------------
 include/hw/ppc/spapr.h |  1 -
 2 files changed, 36 insertions(+), 59 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 83d4d05..593d437 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -271,57 +271,6 @@ static void add_str(GString *s, const gchar *s1)
     g_string_append_len(s, s1, strlen(s1) + 1);
 }
 
-static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
-{
-    void *fdt;
-    char *buf;
-
-    fdt = g_malloc0(FDT_MAX_SIZE);
-    _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
-
-    _FDT((fdt_finish_reservemap(fdt)));
-
-    /* Root node */
-    _FDT((fdt_begin_node(fdt, "")));
-    _FDT((fdt_property_string(fdt, "device_type", "chrp")));
-    _FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
-    _FDT((fdt_property_string(fdt, "compatible", "qemu,pseries")));
-
-    /*
-     * Add info to guest to indentify which host is it being run on
-     * and what is the uuid of the guest
-     */
-    if (kvmppc_get_host_model(&buf)) {
-        _FDT((fdt_property_string(fdt, "host-model", buf)));
-        g_free(buf);
-    }
-    if (kvmppc_get_host_serial(&buf)) {
-        _FDT((fdt_property_string(fdt, "host-serial", buf)));
-        g_free(buf);
-    }
-
-    buf = qemu_uuid_unparse_strdup(&qemu_uuid);
-
-    _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
-    if (qemu_uuid_set) {
-        _FDT((fdt_property_string(fdt, "system-id", buf)));
-    }
-    g_free(buf);
-
-    if (qemu_get_vm_name()) {
-        _FDT((fdt_property_string(fdt, "ibm,partition-name",
-                                  qemu_get_vm_name())));
-    }
-
-    _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
-    _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
-
-    _FDT((fdt_end_node(fdt))); /* close root node */
-    _FDT((fdt_finish(fdt)));
-
-    return fdt;
-}
-
 static int spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
                                        hwaddr size)
 {
@@ -916,11 +865,44 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     int ret;
     void *fdt;
     sPAPRPHBState *phb;
+    char *buf;
 
-    fdt = g_malloc(FDT_MAX_SIZE);
+    fdt = g_malloc0(FDT_MAX_SIZE);
+    _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
 
-    /* open out the base tree into a temp buffer for the final tweaks */
-    _FDT((fdt_open_into(spapr->fdt_skel, fdt, FDT_MAX_SIZE)));
+    /* Root node */
+    _FDT(fdt_setprop_string(fdt, 0, "device_type", "chrp"));
+    _FDT(fdt_setprop_string(fdt, 0, "model", "IBM pSeries (emulated by qemu)"));
+    _FDT(fdt_setprop_string(fdt, 0, "compatible", "qemu,pseries"));
+
+    /*
+     * Add info to guest to indentify which host is it being run on
+     * and what is the uuid of the guest
+     */
+    if (kvmppc_get_host_model(&buf)) {
+        _FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
+        g_free(buf);
+    }
+    if (kvmppc_get_host_serial(&buf)) {
+        _FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
+        g_free(buf);
+    }
+
+    buf = qemu_uuid_unparse_strdup(&qemu_uuid);
+
+    _FDT(fdt_setprop_string(fdt, 0, "vm,uuid", buf));
+    if (qemu_uuid_set) {
+        _FDT(fdt_setprop_string(fdt, 0, "system-id", buf));
+    }
+    g_free(buf);
+
+    if (qemu_get_vm_name()) {
+        _FDT(fdt_setprop_string(fdt, 0, "ibm,partition-name",
+                                qemu_get_vm_name()));
+    }
+
+    _FDT(fdt_setprop_cell(fdt, 0, "#address-cells", 2));
+    _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
 
     /* /interrupt controller */
     spapr_dt_xics(spapr->xics, fdt, PHANDLE_XICP);
@@ -2014,10 +1996,6 @@ static void ppc_spapr_init(MachineState *machine)
     register_savevm_live(NULL, "spapr/htab", -1, 1,
                          &savevm_htab_handlers, spapr);
 
-    /* Prepare the device tree */
-    spapr->fdt_skel = spapr_create_fdt_skel(spapr);
-    assert(spapr->fdt_skel != NULL);
-
     /* used by RTAS */
     QTAILQ_INIT(&spapr->ccs_list);
     qemu_register_reset(spapr_ccs_reset_hook, spapr);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ae7d1d7..d5d6e57 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -69,7 +69,6 @@ struct sPAPRMachineState {
     bool kernel_le;
     uint32_t initrd_base;
     long initrd_size;
-    void *fdt_skel;
     uint64_t rtc_offset; /* Now used only during incoming migration */
     struct PPCTimebase tb;
     bool has_graphics;
-- 
2.7.4

Re: [Qemu-devel] [PATCHv3 00/12] pseries: Consolidate guest device tree construction

[David Gibson]

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

On Mon, Oct 24, 2016 at 04:04:31PM +1100, David Gibson wrote:
> For historical reasons construction of the guest device tree in spapr
> is divided between spapr_create_fdt_skel() which is called at init
> time, and spapr_build_fdt() which runs at reset time.  Over time, more
> and more things have needed to be moved to reset time.
> 
> This series consolidates all the device tree construction to reset
> time, with some minor cleanups along the way.  This will help to make
> it more maintainable in future.
> 
> Changes since v2:
>   * Removed a leftover reference to a variable called 'stdout',
>     shadowing the standard library stdout
> Changes since v1:
>   * Fixed a memory leak introduced by 1/12 (spotted by Thomas Huth)
>   * Removed one patch that's already merged in ppc-for-2.8

Thanks for the acks, everyone.  I've now merged this series into
ppc-for-2.8, tentatively.

Bharata Rao reported some boot failures caused by the series, but I
haven't been able to reproduce them so far.  Bharata, could you retest
with the latest version (in ppc-for-2.8) and, if the problems are
still present, send me some reproducer steps.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCHv3 00/12] pseries: Consolidate guest device tree construction

[David Gibson]

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

On Tue, Oct 25, 2016 at 01:50:02PM +1100, David Gibson wrote:
> On Mon, Oct 24, 2016 at 04:04:31PM +1100, David Gibson wrote:
> > For historical reasons construction of the guest device tree in spapr
> > is divided between spapr_create_fdt_skel() which is called at init
> > time, and spapr_build_fdt() which runs at reset time.  Over time, more
> > and more things have needed to be moved to reset time.
> > 
> > This series consolidates all the device tree construction to reset
> > time, with some minor cleanups along the way.  This will help to make
> > it more maintainable in future.
> > 
> > Changes since v2:
> >   * Removed a leftover reference to a variable called 'stdout',
> >     shadowing the standard library stdout
> > Changes since v1:
> >   * Fixed a memory leak introduced by 1/12 (spotted by Thomas Huth)
> >   * Removed one patch that's already merged in ppc-for-2.8
> 
> Thanks for the acks, everyone.  I've now merged this series into
> ppc-for-2.8, tentatively.
> 
> Bharata Rao reported some boot failures caused by the series, but I
> haven't been able to reproduce them so far.  Bharata, could you retest
> with the latest version (in ppc-for-2.8) and, if the problems are
> still present, send me some reproducer steps.

Actually, I just realised the bug Bharata reported looks identical to
one someone else spotted.  That was caused by one of Thomas' NVRAM
cleanups, rather than the DT cleanup.  It's already been removed from
ppc-for-2.8 pending debug.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCHv3 00/12] pseries: Consolidate guest device tree construction

[Bharata B Rao]

On Tue, Oct 25, 2016 at 02:23:41PM +1100, David Gibson wrote:
> On Tue, Oct 25, 2016 at 01:50:02PM +1100, David Gibson wrote:
> > On Mon, Oct 24, 2016 at 04:04:31PM +1100, David Gibson wrote:
> > > For historical reasons construction of the guest device tree in spapr
> > > is divided between spapr_create_fdt_skel() which is called at init
> > > time, and spapr_build_fdt() which runs at reset time.  Over time, more
> > > and more things have needed to be moved to reset time.
> > > 
> > > This series consolidates all the device tree construction to reset
> > > time, with some minor cleanups along the way.  This will help to make
> > > it more maintainable in future.
> > > 
> > > Changes since v2:
> > >   * Removed a leftover reference to a variable called 'stdout',
> > >     shadowing the standard library stdout
> > > Changes since v1:
> > >   * Fixed a memory leak introduced by 1/12 (spotted by Thomas Huth)
> > >   * Removed one patch that's already merged in ppc-for-2.8
> > 
> > Thanks for the acks, everyone.  I've now merged this series into
> > ppc-for-2.8, tentatively.
> > 
> > Bharata Rao reported some boot failures caused by the series, but I
> > haven't been able to reproduce them so far.  Bharata, could you retest
> > with the latest version (in ppc-for-2.8) and, if the problems are
> > still present, send me some reproducer steps.
> 
> Actually, I just realised the bug Bharata reported looks identical to
> one someone else spotted.  That was caused by one of Thomas' NVRAM
> cleanups, rather than the DT cleanup.  It's already been removed from
> ppc-for-2.8 pending debug.

David - After I reported the boot failure with this patchset to you, I
figured out that the root cause was in fact the NVRAM patchset which
I later reported separately.

Regards,
Bharata.