Re: [Qemu-devel] [PATCH] hw/acpi-build: Add a check for non-memory NUMA nodes.

[Dou Liyang]

Hi Igor,

At 07/10/2018 03:52 PM, Igor Mammedov wrote:
> On Thu, 5 Jul 2018 10:10:38 +0800
> Dou Liyang <address@hidden> wrote:
>
> > Currently, Qemu ACPI builder doesn't consider the non-memory NUMA nodes, eg:
> s/non-memory/memory-less/ throughout subj/commit message

Yes, I will do it.

> >    -m 4G,slots=4,maxmem=8G \
> >    -numa node,nodeid=0 \
> >    -numa node,nodeid=1,mem=2G \
> >    -numa node,nodeid=2,mem=2G \
> >    -numa node,nodeid=3\
> >
> > Guest Linux will report
> >
> >    [    0.000000] ACPI: SRAT: Node 0 PXM 0 [mem 0x00000000-0xffffffffffffffff]
> >    [    0.000000] ACPI: SRAT: Node 1 PXM 1 [mem 0x00000000-0x0009ffff]
> >    [    0.000000] ACPI: SRAT: Node 1 PXM 1 [mem 0x00100000-0x7fffffff]
> >    [    0.000000] ACPI: SRAT: Node 2 PXM 2 [mem 0x80000000-0xbfffffff]
> >    [    0.000000] ACPI: SRAT: Node 2 PXM 2 [mem 0x100000000-0x13fffffff]
> >    [    0.000000] ACPI: SRAT: Node 3 PXM 3 [mem 0x140000000-0x13fffffff]
> >    [    0.000000] ACPI: SRAT: Node 3 PXM 3 [mem 0x140000000-0x33fffffff] hotplug
> >
> > [mem 0x00000000-0xffffffffffffffff] and [mem 0x140000000-0x13fffffff] are bogus.
> >
> > Add a check to avoid building srat memory for non-memory NUMA nodes, also update
> > the test file. Now the info in guest linux will be
> >
> >    [    0.000000] ACPI: SRAT: Node 1 PXM 1 [mem 0x00000000-0x0009ffff]
> >    [    0.000000] ACPI: SRAT: Node 1 PXM 1 [mem 0x00100000-0x7fffffff]
> >    [    0.000000] ACPI: SRAT: Node 2 PXM 2 [mem 0x80000000-0xbfffffff]
> >    [    0.000000] ACPI: SRAT: Node 2 PXM 2 [mem 0x100000000-0x13fffffff]
> >    [    0.000000] ACPI: SRAT: Node 3 PXM 3 [mem 0x140000000-0x33fffffff] hotplug
> >
> > Signed-off-by: Dou Liyang <address@hidden>
> > ---
> > Have done a bootup test in Linux and window 10, 7
>
> add here":
> note to maintainer: update ACPI tables test blobs on commit.
>
> with patch amended
>
> Reviewed-by: Igor Mammedov <address@hidden>
>
> > ---
> >   hw/i386/acpi-build.c                  |   9 ++++++---
> >   tests/acpi-test-data/pc/SRAT.numamem  | Bin 224 -> 224 bytes
> >   tests/acpi-test-data/q35/SRAT.numamem | Bin 224 -> 224 bytes
> >   3 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 9e8350c55d..c584642e4e 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2392,9 +2392,12 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> > MachineState *machine)
> >               mem_len = next_base - pcms->below_4g_mem_size;
> >               next_base = mem_base + mem_len;
> >           }
> > -        numamem = acpi_data_push(table_data, sizeof *numamem);
> > -        build_srat_memory(numamem, mem_base, mem_len, i - 1,
> > -                          MEM_AFFINITY_ENABLED);
> > +
> > +        if (mem_len > 0) {
> > +            numamem = acpi_data_push(table_data, sizeof *numamem);
> > +            build_srat_memory(numamem, mem_base, mem_len, i - 1,
> > +                              MEM_AFFINITY_ENABLED);
> > +        }
> >       }
> >       slots = (table_data->len - numa_start) / sizeof *numamem;
> >       for (; slots < pcms->numa_nodes + 2; slots++) {
>
> Drop binary blobs from patch (for reviewer convenience we post
> blobs as separate patch with DO NOT APPLY tag in subject).
>
> Michael will update test blobs manually when merging your patch.
Thank you for your explanation. I see. will split them out in v2.

Thanks,
        dou

> > diff --git a/tests/acpi-test-data/pc/SRAT.numamem 
> > b/tests/acpi-test-data/pc/SRAT.numamem
> > index 
> > dbc595d9cb85d3fcb5a4243153f42bb431c9de8f..119922f4973f621602047d1dc160519f810922a3
> >  100644
> > GIT binary patch
> > delta 24
> > gcmaFB_<)fsILI;N0RsaA<JXB?78A3|ChpJx0A~{jk^lez
> >
> > delta 24
> > gcmaFB_<)fsILI;N0RsaA<ClqC787?#O*Cl%0A_s%T>t<8
> >
> > diff --git a/tests/acpi-test-data/q35/SRAT.numamem 
> > b/tests/acpi-test-data/q35/SRAT.numamem
> > index 
> > dbc595d9cb85d3fcb5a4243153f42bb431c9de8f..119922f4973f621602047d1dc160519f810922a3
> >  100644
> > GIT binary patch
> > delta 24
> > gcmaFB_<)fsILI;N0RsaA<JXB?78A3|ChpJx0A~{jk^lez
> >
> > delta 24
> > gcmaFB_<)fsILI;N0RsaA<ClqC787?#O*Cl%0A_s%T>t<8