Re: [Qemu-devel] [PATCH 1/9] spapr: Enable dynamic change of the supported hypercalls list

[Alexander Graf]

On 21.05.14 16:21, Alexey Kardashevskiy wrote:
> At the moment the "ibm,hypertas-functions" list is fixed. However some
> calls should be listed there if they are supported by QEMU or the host
> kernel.
>
> This enables hyperrtas_prop to grow on stack by adding
> a SPAPR_HYPERRTAS_ADD macro.
>
> The first user of this is going to be a "multi-tce" property.
>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
>   hw/ppc/spapr.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0a61246..e174e04 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -306,8 +306,6 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>       CPUState *cs;
>       uint32_t start_prop = cpu_to_be32(initrd_base);
>       uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
> -    char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
> -        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-set-mode";
>       char qemu_hypertas_prop[] = "hcall-memop1";
>       uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
>       uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
> @@ -316,6 +314,24 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>       QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
>       unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
>       uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
> +    char *hypertas_prop = NULL;
> +    int hypertas_prop_len = 0;
> +
> +#define SPAPR_HYPERRTAS_ADD(prop)                                   \
> +    do {                                                            \
> +        const char proptmp[] = prop;                                \
> +        char *httmp = alloca(hypertas_prop_len + sizeof(proptmp));  \
> +        if (hypertas_prop_len) {                                    \
> +            memcpy(httmp, hypertas_prop, hypertas_prop_len);        \
> +        }                                                           \
> +        memcpy(httmp + hypertas_prop_len, proptmp, sizeof(proptmp));\
> +        hypertas_prop_len += sizeof(proptmp);                       \
> +        hypertas_prop = httmp;                                      \
> +    } while (0)

Please make this an inline function. Also while I appreciate your 
attempt to speed up memory allocation with alloca, I don't think alloca 
is available on Windows hosts, so we can't use it.

Isn't there a gstring type we can use to grow dynamically and maintain 
content and length at the same time?

> +
> +    SPAPR_HYPERRTAS_ADD(
> +        "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
> +        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-set-mode");

Add these individually please :).


Alex

>   
>       fdt = g_malloc0(FDT_MAX_SIZE);
>       _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
> @@ -485,7 +501,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>       _FDT((fdt_begin_node(fdt, "rtas")));
>   
>       _FDT((fdt_property(fdt, "ibm,hypertas-functions", hypertas_prop,
> -                       sizeof(hypertas_prop))));
> +                       hypertas_prop_len)));
>       _FDT((fdt_property(fdt, "qemu,hypertas-functions", qemu_hypertas_prop,
>                          sizeof(qemu_hypertas_prop))));
>