Re: [Qemu-devel] [PATCH v3 1/2] memhp: extend address auto assignment to support gaps

[Bharata B Rao]

On Thu, Oct 08, 2015 at 07:01:09PM +0530, Bharata B Rao wrote:
> > @@ -333,13 +334,15 @@ uint64_t pc_dimm_get_free_addr(uint64_t 
> > address_space_start,
> >              goto out;
> >          }
> > 
> > -        if (ranges_overlap(dimm->addr, dimm_size, new_addr, size)) {
> > +        if (ranges_overlap(dimm->addr, dimm_size, new_addr,
> > +                           size + (gap ? 1 : 0))) {
> >              if (hint) {
> >                  DeviceState *d = DEVICE(dimm);
> >                  error_setg(errp, "address range conflicts with '%s'", 
> > d->id);
> >                  goto out;
> >              }
> > -            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size, align);
> > +            new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size + (gap ? 1 : 
> > 0),
> > +                                     align);
> 
> This change of adding 1 byte gap will break PowerPC memory hotplug in
> its current form.

Sorry should have been explicit in saying that this will break PowerPC
memory hotplug only if gap is enabled like I attempted here:

https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg00749.html

> 
> Currently we divide hotpluggable memory region into chuncks of 256M
> and there is a DR connector object for each such chunk. The DR connector
> object maintains/controls the state transitions of that memory chunk
> as per PAPR specifications.
> 
> Now after this 1 byte gap, we end up having a 64K alignment (default
> page size) for the DIMM address and end up having an address for which
> there is no DR connector object. I will have to revisit the parts of
> the code in PowerPC that creates DR connector objects and looks them up
> by address.
> 
> Regards,
> Bharata.