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

Dou Liyang posted 1 patch 7 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180705021038.14036-1-douly.fnst@cn.fujitsu.com
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
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(-)
[Qemu-devel] [PATCH] hw/acpi-build: Add a check for non-memory NUMA nodes.
Posted by Dou Liyang 7 years, 4 months ago
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/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




Re: [Qemu-devel] [PATCH] hw/acpi-build: Add a check for non-memory NUMA nodes.
Posted by Michael S. Tsirkin 7 years, 4 months ago
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
> 
> 

Re: [Qemu-devel] [PATCH] hw/acpi-build: Add a check for non-memory NUMA nodes.
Posted by Igor Mammedov 7 years, 3 months ago
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
> 


Re: [Qemu-devel] [PATCH] hw/acpi-build: Add a check for non-memory NUMA nodes.
Posted by Dou Liyang 7 years, 3 months ago
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
>>
> 
> 
> 
>