Re: [Qemu-devel] [RFC PATCH v1 05/13] spapr: Support ibm, lrdr-capacity device tree property

[Michael Roth]

Quoting Bharata B Rao (2015-01-08 00:10:12)
> Add support for ibm,lrdr-capacity since this is needed by the guest
> kernel to know about the possible hot-pluggable CPUs and Memory.
> 
> Define minimum hotpluggable memory size as 256MB and start storing maximum
> possible memory for the guest in sPAPREnvironment.
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/ppc/spapr.c         |  3 ++-
>  hw/ppc/spapr_rtas.c    | 28 ++++++++++++++++++++++++++--
>  include/hw/ppc/spapr.h |  6 ++++--
>  3 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f49b0fa..515d770 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -775,7 +775,7 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
>      }
> 
>      /* RTAS */
> -    ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
> +    ret = spapr_rtas_device_tree_setup(spapr, fdt, rtas_addr, rtas_size);
>      if (ret < 0) {
>          fprintf(stderr, "Couldn't set up RTAS device tree properties\n");
>      }
> @@ -1473,6 +1473,7 @@ static void ppc_spapr_init(MachineState *machine)
> 
>      /* allocate RAM */
>      spapr->ram_limit = ram_size;
> +    spapr->maxram_limit = machine->maxram_size;
>      memory_region_allocate_system_memory(ram, NULL, "ppc_spapr.ram",
>                                           spapr->ram_limit);
>      memory_region_add_subregion(sysmem, 0, ram);
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index d847f45..e8a0f21 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -29,6 +29,7 @@
>  #include "sysemu/char.h"
>  #include "hw/qdev.h"
>  #include "sysemu/device_tree.h"
> +#include "sysemu/cpus.h"
> 
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> @@ -551,11 +552,12 @@ 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)
> +int spapr_rtas_device_tree_setup(sPAPREnvironment *spapr, void *fdt,
> +                                 hwaddr rtas_addr, hwaddr rtas_size)
>  {
>      int ret;
>      int i;
> +    uint32_t lrdr_capacity[5];
> 
>      ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
>      if (ret < 0) {
> @@ -604,6 +606,28 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
> rtas_addr,
>          }
> 
>      }
> +
> +    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "#address-cells", 0x2);
> +    if (ret < 0) {
> +        fprintf(stderr, "Couldn't add #address-cells rtas property\n");
> +    }
> +
> +    ret = qemu_fdt_setprop_cell(fdt, "/rtas", "#size-cells", 0x2);
> +    if (ret < 0) {
> +        fprintf(stderr, "Couldn't add #size-cells rtas property\n");
> +    }
> +
> +    lrdr_capacity[0] = cpu_to_be32(spapr->maxram_limit >> 32);
> +    lrdr_capacity[1] = cpu_to_be32(spapr->maxram_limit & 0xffffffff);
> +    lrdr_capacity[2] = 0;
> +    lrdr_capacity[3] = cpu_to_be32(SPAPR_MIN_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");
> +    }
> +

The property seems simple enough, but would be worthwhile to add a description
of how/when it's used in a new section of docs/specs/ppc-spapr-hotplug.txt to
keep the documentation complete

>      return 0;
>  }
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 831db6b..ae8b4e1 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -18,6 +18,7 @@ typedef struct sPAPREnvironment {
>      XICSState *icp;
> 
>      hwaddr ram_limit;
> +    hwaddr maxram_limit;
>      void *htab;
>      uint32_t htab_shift;
>      hwaddr rma_size;
> @@ -444,8 +445,8 @@ void spapr_rtas_register(int token, const char *name, 
> spapr_rtas_fn fn);
>  target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>                               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);
> +int spapr_rtas_device_tree_setup(sPAPREnvironment *spapr, void *fdt,
> +                                 hwaddr rtas_addr, hwaddr rtas_size);
> 
>  #define SPAPR_TCE_PAGE_SHIFT   12
>  #define SPAPR_TCE_PAGE_SIZE    (1ULL << SPAPR_TCE_PAGE_SHIFT)
> @@ -479,6 +480,7 @@ struct sPAPRTCETable {
>  };
> 
>  #define TIMEBASE_FREQ           512000000ULL
> +#define SPAPR_MIN_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */

Is this actually the min, or a set increment size? Documentation suggests
the latter, in which case the naming is a little confusing.

> 
>  void spapr_events_init(sPAPREnvironment *spapr);
>  void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
> -- 
> 2.1.0