On Mon, Jul 30, 2018 at 11:41:41AM +0200, Igor Mammedov wrote: > Commit 848a1cc1e (hw/acpi-build: build SRAT memory affinity structures for > DIMM devices) > broke the first dimm hotplug in following cases: > > 1: there is no coldplugged dimm in the last numa node > but there is a coldplugged dimm in another node > > -m 4096,slots=4,maxmem=32G \ > -object memory-backend-ram,id=m0,size=2G \ > -device pc-dimm,memdev=m0,node=0 \ > -numa node,nodeid=0 \ > -numa node,nodeid=1 > > 2: if order of dimms on CLI is: > 1st plugged dimm in node1 > 2nd plugged dimm in node0 > > -m 4096,slots=4,maxmem=32G \ > -object memory-backend-ram,size=2G,id=m0 \ > -device pc-dimm,memdev=m0,node=1 \ > -object memory-backend-ram,id=m1,size=2G \ > -device pc-dimm,memdev=m1,node=0 \ > -numa node,nodeid=0 \ > -numa node,nodeid=1 > > (qemu) object_add memory-backend-ram,id=m2,size=1G > (qemu) device_add pc-dimm,memdev=m2,node=0 > > the first DIMM hotplug to any node except the last one > fails (Windows is unable to online it). > > Length reduction of stub hotplug memory SRAT entry, > fixes issue for some reason. >
I'm really bothered by the lack of automated testing for all these NUMA/ACPI corner cases. This looks like a good candidate for an avocado_qemu test case. Can you show pseudo-code of how exactly the bug fix could be verified, so we can try to write a test case? > RHBZ: 1609234 I suggest including full URL of bug report[1]. I doubt anybody outside Red Hat knows what "RHBZ" means. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1609234 > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > --- > NOTE TO MAINTAINER: UPDATE REFERENCE APCI TABLE BLOBS > > v2: > fixup examples in commit message > (they were in wrong order and with duplicate memdev=m1) > --- > hw/i386/acpi-build.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 9e8350c..b52fdb2 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2269,7 +2269,16 @@ static void build_srat_hotpluggable_memory(GArray > *table_data, uint64_t base, > numamem = acpi_data_push(table_data, sizeof *numamem); > > if (!info) { > - build_srat_memory(numamem, cur, end - cur, default_node, > + /* > + * Entry is required for Windows to enable memory hotplug in OS > + * and for Linux to enable SWIOTLB when booted with less than > + * 4G of RAM. Windows works better if the entry sets proximity > + * to the highest NUMA node in the machine at the end of the > + * reserved space. > + * Memory devices may override proximity set by this entry, > + * providing _PXM method if necessary. > + */ > + build_srat_memory(numamem, end - 1, 1, default_node, > MEM_AFFINITY_HOTPLUGGABLE | > MEM_AFFINITY_ENABLED); > break; > } > @@ -2402,14 +2411,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, > MachineState *machine) > build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS); > } > > - /* > - * Entry is required for Windows to enable memory hotplug in OS > - * and for Linux to enable SWIOTLB when booted with less than > - * 4G of RAM. Windows works better if the entry sets proximity > - * to the highest NUMA node in the machine. > - * Memory devices may override proximity set by this entry, > - * providing _PXM method if necessary. > - */ > if (hotplugabble_address_space_size) { > build_srat_hotpluggable_memory(table_data, > machine->device_memory->base, > hotplugabble_address_space_size, > -- > 2.7.4 > -- Eduardo
