Re: [Qemu-devel] [RFC for-2.7 09/11] pseries: Consolidate construction of /chosen device tree node

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 04/20/2016 12:33 PM, David Gibson wrote:
> For historical reasons construction of the /chosen node in the device tree
> (amongst others) is split into several places.  This patch brings these
> pieces back together to make things clearer.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

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

> ---
>   hw/ppc/spapr.c             | 59 ++++++++++++++++++++--------------------------
>   hw/ppc/spapr_vio.c         | 16 +++----------
>   include/hw/ppc/spapr_vio.h |  2 +-
>   3 files changed, 30 insertions(+), 47 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d04d403..a000056 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -707,9 +707,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>       MachineState *machine = MACHINE(spapr);
>       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;
>       char *buf;
> @@ -759,6 +757,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>       /* /chosen */
>       {
>           QDTNode *chosen = qdt_add_subnode(root, "chosen");
> +        char *bootlist;
> +        size_t cb = 0, i;
>
>           /* Set Form1_affinity */
>           qdt_setprop_bytes(chosen, "ibm,architecture-vec-5",
> @@ -782,6 +782,29 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>           qdt_setprop_cells(chosen, "qemu,graphic-width", graphic_width);
>           qdt_setprop_cells(chosen, "qemu,graphic-height", graphic_height);
>           qdt_setprop_cells(chosen, "qemu,graphic-depth", graphic_depth);
> +
> +        bootlist = get_boot_devices_list(&cb, true);
> +        if (cb && bootlist) {
> +            for (i = 0; i < cb; i++) {
> +                if (bootlist[i] == '\n') {
> +                    bootlist[i] = ' ';
> +                }
> +
> +            }
> +            qdt_setprop_string(chosen, "qemu,boot-list", bootlist);
> +        }
> +        g_free(bootlist);
> +
> +        if (boot_device && strlen(boot_device)) {
> +            qdt_setprop_string(chosen, "qemu,boot-device", boot_device);
> +        }
> +
> +        if (!spapr->has_graphics) {
> +            char *stdout = spapr_vio_stdout_path(spapr->vio_bus);
> +
> +            qdt_setprop_string(chosen, "linux,stdout-path", stdout);
> +            g_free(stdout);
> +        }
>       }
>
>       /* RTAS */
> @@ -933,40 +956,10 @@ 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));
>       }
>
> -    g_free(bootlist);
> -
>       /* Build memory reserve map */
>       if (spapr->kernel_size) {
>           _FDT((fdt_add_mem_rsv(fdt, KERNEL_LOAD_ADDR, spapr->kernel_size)));
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 8aa021f..18b07e0 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -679,28 +679,18 @@ 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;
> -    }
> +        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 c9733e7..ea00c3b 100644
> --- a/include/hw/ppc/spapr_vio.h
> +++ b/include/hw/ppc/spapr_vio.h
> @@ -82,7 +82,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);
> +gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus);
>
>   extern int spapr_vio_signal(VIOsPAPRDevice *dev, target_ulong mode);
>
>


-- 
Alexey