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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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


-- 
Alexey