Re: [Qemu-ppc] [RFC for-2.7 10/11] pseries: Consolidate construction of /rtas device tree node

[Alexey Kardashevskiy]

On 04/20/2016 12:33 PM, David Gibson wrote:
> 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_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.  Most of it
> goes inline (but together) into spapr_build_fdt(), and
> spapr_rtas_device_tree_setup() is renamed to spapr_rtas_dt_tokens() and
> now only adds the token properties.
>
> Signed-off-by: David Gibson <address@hidden>

Reviewed-by: Alexey Kardashevskiy <address@hidden>

> ---
>   hw/ppc/spapr.c         | 18 +++++++++++-------
>   hw/ppc/spapr_rtas.c    | 32 ++------------------------------
>   include/hw/ppc/spapr.h |  4 ++--
>   3 files changed, 15 insertions(+), 39 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a000056..f52add7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -807,11 +807,13 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>           }
>       }
>
> -    /* RTAS */
> +    /* /rtas */
>       {
>           QDTNode *rtas = qdt_add_subnode(root, "rtas");
>           GString *hypertas = g_string_sized_new(256);
>           GString *qemu_hypertas = g_string_sized_new(256);
> +        uint64_t max_hotplug_addr = spapr->hotplug_memory.base
> +            + memory_region_size(&spapr->hotplug_memory.mr);
>
>           add_str(hypertas, "hcall-pft");
>           add_str(hypertas, "hcall-term");
> @@ -851,6 +853,14 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>            * that rtas call return will always occur. Set this property.
>            */
>           qdt_setprop_empty(rtas, "ibm,extended-os-term");
> +
> +        qdt_setprop_cells(rtas, "ibm,lrdr-capacity",
> +                          max_hotplug_addr >> 32,
> +                          max_hotplug_addr & 0xffffffff,
> +                          0, SPAPR_MEMORY_BLOCK_SIZE,
> +                          max_cpus / smp_threads);
> +
> +        spapr_rtas_dt_tokens(rtas);
>       }
>
>       /* /interrupt controller */
> @@ -947,12 +957,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>           exit(1);
>       }
>
> -    /* RTAS */
> -    ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
> -    if (ret < 0) {
> -        fprintf(stderr, "Couldn't set up RTAS device tree properties\n");
> -    }
> -
>       /* cpus */
>       spapr_populate_cpus_dt_node(fdt, spapr);
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index c0cb41e..806a7b0 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -677,16 +677,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_rtas_dt_tokens(QDTNode *node)
>   {
> -    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];
> @@ -695,29 +688,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) {
> -            fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
> -                    call->name, fdt_strerror(ret));
> -            return ret;
> -        }
> -
> +        qdt_setprop_cells(node, 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) {
> -        fprintf(stderr, "Couldn't add ibm,lrdr-capacity rtas property\n");
> -        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 40d3724..1e609a0 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -6,6 +6,7 @@
>   #include "hw/ppc/xics.h"
>   #include "hw/ppc/spapr_drc.h"
>   #include "hw/mem/pc-dimm.h"
> +#include "qemu/qdt.h"
>
>   struct VIOsPAPRBus;
>   struct sPAPRPHBState;
> @@ -510,8 +511,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_rtas_dt_tokens(QDTNode *node);
>   void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr);
>
>   #define SPAPR_TCE_PAGE_SHIFT   12


--
Alexey