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

[Bharata B Rao]

On Thu, Feb 12, 2015 at 04:19:36PM +1100, David Gibson wrote:
> > 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).

ok.

> 
> >  };
> >  
> >  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.

I am basing this on x86 memory hotplug and that's how 1GB is coming. It
is not PAPR specified.

> 
> > +        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+?

Arbitrary max, not defined by PAPR.

Regards,
Bharata.