Re: [Qemu-devel] [RFC PATCH v4 1/5] spapr: Initialize hotplug memory address space

[Bharata B Rao]

On Tue, Jun 23, 2015 at 11:33:58AM +1000, David Gibson wrote:
> On Fri, Jun 19, 2015 at 03:47:53PM +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                    | 28 ++++++++++++++++++++++++++++
> >  include/hw/ppc/spapr.h            | 12 ++++++++++++
> >  3 files changed, 41 insertions(+)
> > 
> > diff --git a/default-configs/ppc64-softmmu.mak 
> > b/default-configs/ppc64-softmmu.mak
> > index ab62cc7..e77cb1a 100644
> > --- a/default-configs/ppc64-softmmu.mak
> > +++ b/default-configs/ppc64-softmmu.mak
> > @@ -52,3 +52,4 @@ CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
> >  # For PReP
> >  CONFIG_MC146818RTC=y
> >  CONFIG_ISA_TESTDEV=y
> > +CONFIG_MEM_HOTPLUG=y
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5ca817c..87a29dc 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1549,6 +1549,34 @@ static void ppc_spapr_init(MachineState *machine)
> >          memory_region_add_subregion(sysmem, 0, rma_region);
> >      }
> >  
> > +    /* initialize hotplug memory address space */
> > +    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);
> > +        }
> > +
> > +        spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
> > +                                              SPAPR_HOTPLUG_MEM_ALIGN);
> > +
> > +        hotplug_mem_size += SPAPR_HOTPLUG_MEM_ALIGN * machine->ram_slots;
> 
> I'm not sure what this adjustment is about.  Are you putting a gap of
> size SPAPR_HOTPLUG_MEM_ALIGN between each memory slot?  That doesn't
> see to match the DRC initialization code in the next patch which
> assigns all the LMBs in the hotplug area contiguous addresses.

Right, this is some leftover alignment bits from x86 implementation which
to me looks like not needed in Power. Let me clean this up in the next
iteration.

Regards,
Bharata.