diff mbox

[RFC,for-2.7,02/11] pseries: Split device tree construction from device tree load

Message ID 1461119601-4936-3-git-send-email-david@gibson.dropbear.id.au (mailing list archive)
State New, archived
Headers show

Commit Message

David Gibson April 20, 2016, 2:33 a.m. UTC 
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, so split them apart.
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 | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

Comments

Thomas Huth April 20, 2016, 6:15 p.m. UTC | #1 
On 20.04.2016 04:33, 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, so split them apart.
> 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 | 41 ++++++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index feaab08..26b95ce 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -899,10 +899,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());
>      sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> @@ -988,19 +987,8 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>          _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
>      }
>  
> -    _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)
> @@ -1138,6 +1126,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);
> @@ -1164,14 +1154,27 @@ 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));
> +
>      /* Set up the entry state */
>      first_ppc_cpu = POWERPC_CPU(first_cpu);
>      first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;

Looks good.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Alexey Kardashevskiy April 21, 2016, 5:31 a.m. UTC | #2 
On 04/20/2016 12:33 PM, 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, so split them apart.
> 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: Alexey Kardashevskiy <aik@ozlabs.ru>


> ---
>   hw/ppc/spapr.c | 41 ++++++++++++++++++++++-------------------
>   1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index feaab08..26b95ce 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -899,10 +899,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());
>       sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> @@ -988,19 +987,8 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>           _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
>       }
>
> -    _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)
> @@ -1138,6 +1126,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);
> @@ -1164,14 +1154,27 @@ 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));
> +
>       /* Set up the entry state */
>       first_ppc_cpu = POWERPC_CPU(first_cpu);
>       first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
>
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index feaab08..26b95ce 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -899,10 +899,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());
     sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
@@ -988,19 +987,8 @@  static void spapr_finalize_fdt(sPAPRMachineState *spapr,
         _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
     }
 
-    _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)
@@ -1138,6 +1126,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);
@@ -1164,14 +1154,27 @@  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));
+
     /* Set up the entry state */
     first_ppc_cpu = POWERPC_CPU(first_cpu);
     first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;