Re: [Qemu-devel] [RFC PATCH v1 11/13] spapr: Initialize hotplug memory address space

[David Gibson]

On Thu, Jan 08, 2015 at 11:40:18AM +0530, Bharata B Rao wrote:
> Initialize a hotplug memory region under which all the hotplugged
> memory is accommodated. Also enable memory hotplug by setting
> CONFIG_MEM_HOTPLUG.
> 
> Modelled on i386 memory hotplug.
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  default-configs/ppc64-softmmu.mak |  1 +
>  hw/ppc/spapr.c                    | 26 ++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h            |  3 +++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/default-configs/ppc64-softmmu.mak 
> b/default-configs/ppc64-softmmu.mak
> index bd30d69..03210de 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -60,3 +60,4 @@ CONFIG_I82374=y
>  CONFIG_I8257=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
> +CONFIG_MEM_HOTPLUG=y
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 44405b2..9ff08ff 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -120,6 +120,8 @@ struct sPAPRMachineState {
>  
>      /*< public >*/
>      char *kvm_type;
> +    ram_addr_t hotplug_memory_base;
> +    MemoryRegion hotplug_memory;

We should really unify sPAPRMachineState with sPAPREnvironment at some
point (I realise that doesn't reasonably fit within the scope of this
series).

>  };
>  
>  sPAPREnvironment *spapr;
> @@ -1403,6 +1405,7 @@ static void ppc_spapr_init(MachineState *machine)
>      bool kernel_le = false;
>      char *filename;
>      int smt = kvmppc_smt_threads();
> +    sPAPRMachineState *ms = SPAPR_MACHINE(machine);
>  
>      msi_supported = true;
>  
> @@ -1492,6 +1495,29 @@ static void ppc_spapr_init(MachineState *machine)
>          memory_region_add_subregion(sysmem, 0, rma_region);
>      }
>  
> +    if (machine->ram_size < machine->maxram_size) {
> +        ram_addr_t hotplug_mem_size = machine->maxram_size - 
> machine->ram_size;
> +
> +        if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
> +            error_report("unsupported amount of memory slots: %"PRIu64,
> +                         machine->ram_slots);
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        ms->hotplug_memory_base = ROUND_UP(machine->ram_size, 1ULL << 30);

Is there a particular significance to the 1GiB alignment?  Is it just
a conveniently large alignment, or is that value specified in PAPR
somewhere?  Using a named constant would probably help to clarify that.

> +        if ((ms->hotplug_memory_base + hotplug_mem_size) < hotplug_mem_size) 
> {
> +            error_report("unsupported amount of maximum memory: " 
> RAM_ADDR_FMT,
> +                         machine->maxram_size);
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        memory_region_init(&ms->hotplug_memory, OBJECT(ms),
> +                           "hotplug-memory", hotplug_mem_size);
> +        memory_region_add_subregion(sysmem, ms->hotplug_memory_base,
> +                                    &ms->hotplug_memory);
> +    }
> +
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
>      spapr->rtas_size = get_image_size(filename);
>      spapr->rtas_blob = g_malloc(spapr->rtas_size);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ae8b4e1..64681c4 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -482,6 +482,9 @@ struct sPAPRTCETable {
>  #define TIMEBASE_FREQ           512000000ULL
>  #define SPAPR_MIN_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
>  
> +/* Support a min of 1TB hotplug memory assuming 256MB per slot */
> +#define SPAPR_MAX_RAM_SLOTS     (1ULL << 12)

Is this constraint arbitrary, or does it come from something in PAPR+?

>  void spapr_events_init(sPAPREnvironment *spapr);
>  void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
>  int spapr_h_cas_compose_response(target_ulong addr, target_ulong size);

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgpN8nTGtSO8R.pgp
Description: PGP signature