diff mbox

hw/acpi-build: Add a check for non-memory NUMA nodes.

Message ID 20180705021038.14036-1-douly.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dou Liyang July 5, 2018, 2:10 a.m. UTC 
Currently, Qemu ACPI builder doesn't consider the non-memory NUMA nodes, eg:

  -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 <douly.fnst@cn.fujitsu.com>
---
Have done a bootup test in Linux and window 10, 7
---
 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/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

Comments

Michael S. Tsirkin July 5, 2018, 1:51 p.m. UTC | #1 
On Thu, Jul 05, 2018 at 10:10:38AM +0800, Dou Liyang wrote:
> Currently, Qemu ACPI builder doesn't consider the non-memory NUMA nodes, eg:
> 
>   -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 <douly.fnst@cn.fujitsu.com>

Igor, can you review pls?

> ---
> Have done a bootup test in Linux and window 10, 7
> ---
>  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++) {
> 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
> 
> -- 
> 2.14.3
> 
>
Igor Mammedov July 10, 2018, 7:52 a.m. UTC | #2 
On Thu, 5 Jul 2018 10:10:38 +0800
Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:

> Currently, Qemu ACPI builder doesn't consider the non-memory NUMA nodes, eg:
s/non-memory/memory-less/ throughout subj/commit message


>   -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 <douly.fnst@cn.fujitsu.com>
> ---
> 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 <imammedo@redhat.com>

> ---
>  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.

> 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
>
Dou Liyang July 10, 2018, 8:31 a.m. UTC | #3 
Hi Igor,

At 07/10/2018 03:52 PM, Igor Mammedov wrote:
> On Thu, 5 Jul 2018 10:10:38 +0800
> Dou Liyang <douly.fnst@cn.fujitsu.com> 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 <douly.fnst@cn.fujitsu.com>
>> ---
>> 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 <imammedo@redhat.com>
> 
>> ---
>>   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
>>
> 
> 
> 
>
diff mbox

Patch

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++) {