Re: [Qemu-devel] [PATCH] pc: ACPI BIOS: use highest NUMA node for hotplug mem hole SRAT entry

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 24 May 2017 11:16:14 +0200
Ladi Prosek <lprosek@redhat.com> wrote:

> On Wed, May 24, 2017 at 11:07 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> > On 05/24/17 10:09, Ladi Prosek wrote:  
> >> For reasons unknown, Windows won't online all memory, both at command
> >> line and hot-plugged later, unless the hotplug mem hole SRAT entry
> >> specifies a node greater or equal to the ones where memory is added.  
> >
> > s/greater or equal to/greater *than* or equal to/
> >
> > Thanks,
> > Laszlo
> > (always finding the important issues! ;) )  
> 
> Haha, thanks! Igor, please let me know if you'd like me to send a
> corrected version.
if you are going to respin see for nit below

also could you check that 'make V=1 check' is happy,
since patch changes SRAT table, bios tables test may fail
if that happen then please add to commit message
you can verify that updating reference blobs fixes make check with
tests/acpi-test-data/rebuild-expected-aml.sh

---
note to maintainer:
tests/acpi-test-data need to be updated due to change in SRAT table

> 
> >>
> >> Using the highest node on the machine makes recent versions of Windows
> >> happy.
> >>
> >> With this example command line:
> >>   ... \
> >>   -m 1024,slots=4,maxmem=32G \
> >>   -numa node,nodeid=0 \
> >>   -numa node,nodeid=1 \
> >>   -numa node,nodeid=2 \
> >>   -numa node,nodeid=3 \
> >>   -object memory-backend-ram,size=1G,id=mem-mem1 \
> >>   -device pc-dimm,id=dimm-mem1,memdev=mem-mem1,node=1
> >>
> >> Windows reports a total of 1G of RAM without this commit and the expected
> >> 2G with this commit.
> >>
> >> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> >> ---
> >>  hw/i386/acpi-build.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index afcadac..9653583 100644
> >> --- a/hw/i386/acpi-build.c
> >> +++ b/hw/i386/acpi-build.c
> >> @@ -2404,14 +2404,17 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
> >>      }
> >>
> >>      /*
> >> -     * Entry is required for Windows to enable memory hotplug in OS.
> >> +     * Entry is required for Windows to enable memory hotplug in OS
> >> +     * and for Linux to enable SWIOTLB even if booted with less than
s/even if/when/

> >> +     * 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) {
> >>          numamem = acpi_data_push(table_data, sizeof *numamem);
> >>          build_srat_memory(numamem, pcms->hotplug_memory.base,
> >> -                          hotplugabble_address_space_size, 0,
> >> +                          hotplugabble_address_space_size, pcms->numa_nodes - 1,
> >>                            MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
> >>      }
> >>
> >>  
> >