Re: [Qemu-devel] [PATCH] pc: acpi: revert back to 1 SRAT entry for hotpluggable area

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Aug 24, 2018 at 10:03:05AM +0200, Igor Mammedov wrote:
> On Thu, 23 Aug 2018 14:25:01 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Thu, Aug 23, 2018 at 10:14:06AM +0200, Igor Mammedov wrote:
> > > On Wed, 22 Aug 2018 15:01:12 -0300
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:  
> > [...]
> > > > However, have you considered keeping adding separate entries for
> > > > NVDIMM devices only (so we follow the spec), but add a single
> > > > (numa_nodes-1, MEM_AFFINITY_HOTPLUGGABLE|MEM_AFFINITY_ENABLED)
> > > > entry to the rest?  
> > > Indeed, I did. It doesn't work either.  
> > 
> > When exactly it didn't work?  Did nvdimm + memory hotplug ever
> > worked together on Windows guests?
> before 848a1cc1e QEMU CLI with nvdimm + memory hotplug worked
> as expected for Windows.

OK, so my suggestion would still have a regression.  Nevermind.

> With approach you suggested, it would create several SRAT entries
> X for nvdimm and Y for the rest (1 in the best case or many if
> nvdimms/pc-dimms are interleaved) and that breaks memory hotplug.
>  
> > For all the other cases there should be absolutely no difference:
> > 
> > nvdimm users would still get a spec-compliant SRAT table (like on
> > QEMU 3.0).
> > 
> > Memory hotplug users w/o nvdimm would get the same ACPI table
> > that they would get after applying this patch (i.e. the one we
> > had before commit 848a1cc1e ("hw/acpi-build: build SRAT memory
> > affinity structures for DIMM devices").
> I did consider it. It still would be a regression but a minor one
> (only Windows nvdimm enabled cases will have regressed memory
> hotplug). I even have a patch for it, but it's still a regression,
> that's why I've posted full 848a1cc1e revert.
> 
> If it were a bug in the newest version of windows (assuming it's
> proven Windows bug), I'd be inclined towards being spec compliant
> and let MS fix Windows as it was with CPU hotplug (hijacked ACPI0010
> container) which they eventually fixed, but in this case it affects
> all guests versions and there is no proof that's a Windows bug.
> 
> So my hope here is that Intel has resources to figure out what
> Windows expectations are wrt SRAT/memory layout and memory hotplug.

Agreed.

-- 
Eduardo