hw/arm/virt-acpi-build.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
The empty NUMA nodes, where no memory resides, aren't exposed
through ACPI SRAT table. It's not user preferred behaviour because
the corresponding memory node devices are missed from the guest
kernel as the following example shows, and memory can't be hot
added to these empty NUMA nodes at later point.
/home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
-accel kvm -machine virt,gic-version=host \
-cpu host -smp 4,sockets=2,cores=2,threads=1 \
-m 1024M,slots=16,maxmem=64G \
-object memory-backend-ram,id=mem0,size=512M \
-object memory-backend-ram,id=mem1,size=512M \
-numa node,nodeid=0,cpus=0-1,memdev=mem0 \
-numa node,nodeid=1,cpus=2-3,memdev=mem1 \
-numa node,nodeid=2 \
-numa node,nodeid=3 \
:
guest# ls /sys/devices/system/node | grep node
node0
node1
node2
This exposes these empty NUMA nodes through ACPI SRAT table. With
this applied, the corresponding memory node devices can be found
from the guest. Note that the hotpluggable capability is explicitly
given to these empty NUMA nodes for sake of completeness.
guest# ls /sys/devices/system/node | grep node
node0
node1
node2
node3
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
hw/arm/virt-acpi-build.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 674f902652..a4c95b2f64 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -526,6 +526,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(ms);
AcpiTable table = { .sig = "SRAT", .rev = 3, .oem_id = vms->oem_id,
.oem_table_id = vms->oem_table_id };
+ MemoryAffinityFlags flags;
acpi_table_begin(&table, table_data);
build_append_int_noprefix(table_data, 1, 4); /* Reserved */
@@ -547,12 +548,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
mem_base = vms->memmap[VIRT_MEM].base;
for (i = 0; i < ms->numa_state->num_nodes; ++i) {
- if (ms->numa_state->nodes[i].node_mem > 0) {
- build_srat_memory(table_data, mem_base,
- ms->numa_state->nodes[i].node_mem, i,
- MEM_AFFINITY_ENABLED);
- mem_base += ms->numa_state->nodes[i].node_mem;
+ if (ms->numa_state->nodes[i].node_mem) {
+ flags = MEM_AFFINITY_ENABLED;
+ } else {
+ flags = MEM_AFFINITY_ENABLED | MEM_AFFINITY_HOTPLUGGABLE;
}
+
+ build_srat_memory(table_data, mem_base,
+ ms->numa_state->nodes[i].node_mem, i, flags);
+ mem_base += ms->numa_state->nodes[i].node_mem;
}
if (ms->nvdimms_state->is_enabled) {
--
2.23.0
On Tue, Oct 26, 2021 at 07:41:01AM +0800, Gavin Shan wrote:
> The empty NUMA nodes, where no memory resides, aren't exposed
> through ACPI SRAT table. It's not user preferred behaviour because
> the corresponding memory node devices are missed from the guest
> kernel as the following example shows, and memory can't be hot
> added to these empty NUMA nodes at later point.
>
> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> -accel kvm -machine virt,gic-version=host \
> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
> -m 1024M,slots=16,maxmem=64G \
> -object memory-backend-ram,id=mem0,size=512M \
> -object memory-backend-ram,id=mem1,size=512M \
> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
> -numa node,nodeid=2 \
> -numa node,nodeid=3 \
> :
> guest# ls /sys/devices/system/node | grep node
> node0
> node1
> node2
node2 shouldn't be in this list, should it?
>
> This exposes these empty NUMA nodes through ACPI SRAT table. With
> this applied, the corresponding memory node devices can be found
> from the guest. Note that the hotpluggable capability is explicitly
> given to these empty NUMA nodes for sake of completeness.
>
> guest# ls /sys/devices/system/node | grep node
> node0
> node1
> node2
> node3
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> hw/arm/virt-acpi-build.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 674f902652..a4c95b2f64 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -526,6 +526,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(ms);
> AcpiTable table = { .sig = "SRAT", .rev = 3, .oem_id = vms->oem_id,
> .oem_table_id = vms->oem_table_id };
> + MemoryAffinityFlags flags;
>
> acpi_table_begin(&table, table_data);
> build_append_int_noprefix(table_data, 1, 4); /* Reserved */
> @@ -547,12 +548,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>
> mem_base = vms->memmap[VIRT_MEM].base;
> for (i = 0; i < ms->numa_state->num_nodes; ++i) {
> - if (ms->numa_state->nodes[i].node_mem > 0) {
> - build_srat_memory(table_data, mem_base,
> - ms->numa_state->nodes[i].node_mem, i,
> - MEM_AFFINITY_ENABLED);
> - mem_base += ms->numa_state->nodes[i].node_mem;
> + if (ms->numa_state->nodes[i].node_mem) {
> + flags = MEM_AFFINITY_ENABLED;
> + } else {
> + flags = MEM_AFFINITY_ENABLED | MEM_AFFINITY_HOTPLUGGABLE;
> }
> +
> + build_srat_memory(table_data, mem_base,
> + ms->numa_state->nodes[i].node_mem, i, flags);
> + mem_base += ms->numa_state->nodes[i].node_mem;
> }
>
> if (ms->nvdimms_state->is_enabled) {
> --
> 2.23.0
>
Besides the possible issue with the commit message,
Reviewed-by: Andrew Jones <drjones@redhat.com>
Thanks,
drew
On 10/26/21 5:25 PM, Andrew Jones wrote:
> On Tue, Oct 26, 2021 at 07:41:01AM +0800, Gavin Shan wrote:
>> The empty NUMA nodes, where no memory resides, aren't exposed
>> through ACPI SRAT table. It's not user preferred behaviour because
>> the corresponding memory node devices are missed from the guest
>> kernel as the following example shows, and memory can't be hot
>> added to these empty NUMA nodes at later point.
>>
>> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
>> -accel kvm -machine virt,gic-version=host \
>> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
>> -m 1024M,slots=16,maxmem=64G \
>> -object memory-backend-ram,id=mem0,size=512M \
>> -object memory-backend-ram,id=mem1,size=512M \
>> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
>> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
>> -numa node,nodeid=2 \
>> -numa node,nodeid=3 \
>> :
>> guest# ls /sys/devices/system/node | grep node
>> node0
>> node1
>> node2
>
> node2 shouldn't be in this list, should it?
>
Yes, the list shouldn't include node2. I'll amend the commit
log in v2.
>>
>> This exposes these empty NUMA nodes through ACPI SRAT table. With
>> this applied, the corresponding memory node devices can be found
>> from the guest. Note that the hotpluggable capability is explicitly
>> given to these empty NUMA nodes for sake of completeness.
>>
>> guest# ls /sys/devices/system/node | grep node
>> node0
>> node1
>> node2
>> node3
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> hw/arm/virt-acpi-build.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 674f902652..a4c95b2f64 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -526,6 +526,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(ms);
>> AcpiTable table = { .sig = "SRAT", .rev = 3, .oem_id = vms->oem_id,
>> .oem_table_id = vms->oem_table_id };
>> + MemoryAffinityFlags flags;
>>
>> acpi_table_begin(&table, table_data);
>> build_append_int_noprefix(table_data, 1, 4); /* Reserved */
>> @@ -547,12 +548,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>
>> mem_base = vms->memmap[VIRT_MEM].base;
>> for (i = 0; i < ms->numa_state->num_nodes; ++i) {
>> - if (ms->numa_state->nodes[i].node_mem > 0) {
>> - build_srat_memory(table_data, mem_base,
>> - ms->numa_state->nodes[i].node_mem, i,
>> - MEM_AFFINITY_ENABLED);
>> - mem_base += ms->numa_state->nodes[i].node_mem;
>> + if (ms->numa_state->nodes[i].node_mem) {
>> + flags = MEM_AFFINITY_ENABLED;
>> + } else {
>> + flags = MEM_AFFINITY_ENABLED | MEM_AFFINITY_HOTPLUGGABLE;
>> }
>> +
>> + build_srat_memory(table_data, mem_base,
>> + ms->numa_state->nodes[i].node_mem, i, flags);
>> + mem_base += ms->numa_state->nodes[i].node_mem;
>> }
>>
>> if (ms->nvdimms_state->is_enabled) {
>> --
>> 2.23.0
>>
>
> Besides the possible issue with the commit message,
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
Thanks,
Gavin
On Tue, 26 Oct 2021 07:41:01 +0800
Gavin Shan <gshan@redhat.com> wrote:
> The empty NUMA nodes, where no memory resides, aren't exposed
> through ACPI SRAT table. It's not user preferred behaviour because
> the corresponding memory node devices are missed from the guest
> kernel as the following example shows, and memory can't be hot
> added to these empty NUMA nodes at later point.
a error message one gets would be useful here.
btw:
memory hotplug seems to work for x86 without adding empty nodes.
So it beg a question, if absence of empty nodes is the issue here.
>
> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> -accel kvm -machine virt,gic-version=host \
> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
> -m 1024M,slots=16,maxmem=64G \
> -object memory-backend-ram,id=mem0,size=512M \
> -object memory-backend-ram,id=mem1,size=512M \
> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
> -numa node,nodeid=2 \
> -numa node,nodeid=3 \
> :
> guest# ls /sys/devices/system/node | grep node
> node0
> node1
> node2
>
> This exposes these empty NUMA nodes through ACPI SRAT table. With
> this applied, the corresponding memory node devices can be found
> from the guest. Note that the hotpluggable capability is explicitly
> given to these empty NUMA nodes for sake of completeness.
>
> guest# ls /sys/devices/system/node | grep node
> node0
> node1
> node2
> node3
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
> hw/arm/virt-acpi-build.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 674f902652..a4c95b2f64 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -526,6 +526,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(ms);
> AcpiTable table = { .sig = "SRAT", .rev = 3, .oem_id = vms->oem_id,
> .oem_table_id = vms->oem_table_id };
> + MemoryAffinityFlags flags;
>
> acpi_table_begin(&table, table_data);
> build_append_int_noprefix(table_data, 1, 4); /* Reserved */
> @@ -547,12 +548,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>
> mem_base = vms->memmap[VIRT_MEM].base;
> for (i = 0; i < ms->numa_state->num_nodes; ++i) {
> - if (ms->numa_state->nodes[i].node_mem > 0) {
> - build_srat_memory(table_data, mem_base,
> - ms->numa_state->nodes[i].node_mem, i,
> - MEM_AFFINITY_ENABLED);
> - mem_base += ms->numa_state->nodes[i].node_mem;
> + if (ms->numa_state->nodes[i].node_mem) {
> + flags = MEM_AFFINITY_ENABLED;
> + } else {
> + flags = MEM_AFFINITY_ENABLED | MEM_AFFINITY_HOTPLUGGABLE;
> }
> +
> + build_srat_memory(table_data, mem_base,
> + ms->numa_state->nodes[i].node_mem, i, flags);
> + mem_base += ms->numa_state->nodes[i].node_mem;
> }
>
> if (ms->nvdimms_state->is_enabled) {
On 10/26/21 8:47 PM, Igor Mammedov wrote:
> On Tue, 26 Oct 2021 07:41:01 +0800
> Gavin Shan <gshan@redhat.com> wrote:
>
>> The empty NUMA nodes, where no memory resides, aren't exposed
>> through ACPI SRAT table. It's not user preferred behaviour because
>> the corresponding memory node devices are missed from the guest
>> kernel as the following example shows, and memory can't be hot
>> added to these empty NUMA nodes at later point.
>
> a error message one gets would be useful here.
>
> btw:
> memory hotplug seems to work for x86 without adding empty nodes.
> So it beg a question, if absence of empty nodes is the issue here.
>
Yes, the memory can be still hot added even the empty NUMA nodes
aren't exposed. However, we still need to expose them so that
the guest kernel has the information as the users specifies.
I will make the commit log more precise in v2.
>>
>> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
>> -accel kvm -machine virt,gic-version=host \
>> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
>> -m 1024M,slots=16,maxmem=64G \
>> -object memory-backend-ram,id=mem0,size=512M \
>> -object memory-backend-ram,id=mem1,size=512M \
>> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
>> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
>> -numa node,nodeid=2 \
>> -numa node,nodeid=3 \
>> :
>> guest# ls /sys/devices/system/node | grep node
>> node0
>> node1
>> node2
>>
>> This exposes these empty NUMA nodes through ACPI SRAT table. With
>> this applied, the corresponding memory node devices can be found
>> from the guest. Note that the hotpluggable capability is explicitly
>> given to these empty NUMA nodes for sake of completeness.
>>
>> guest# ls /sys/devices/system/node | grep node
>> node0
>> node1
>> node2
>> node3
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>> hw/arm/virt-acpi-build.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 674f902652..a4c95b2f64 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -526,6 +526,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(ms);
>> AcpiTable table = { .sig = "SRAT", .rev = 3, .oem_id = vms->oem_id,
>> .oem_table_id = vms->oem_table_id };
>> + MemoryAffinityFlags flags;
>>
>> acpi_table_begin(&table, table_data);
>> build_append_int_noprefix(table_data, 1, 4); /* Reserved */
>> @@ -547,12 +548,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>
>> mem_base = vms->memmap[VIRT_MEM].base;
>> for (i = 0; i < ms->numa_state->num_nodes; ++i) {
>> - if (ms->numa_state->nodes[i].node_mem > 0) {
>> - build_srat_memory(table_data, mem_base,
>> - ms->numa_state->nodes[i].node_mem, i,
>> - MEM_AFFINITY_ENABLED);
>> - mem_base += ms->numa_state->nodes[i].node_mem;
>> + if (ms->numa_state->nodes[i].node_mem) {
>> + flags = MEM_AFFINITY_ENABLED;
>> + } else {
>> + flags = MEM_AFFINITY_ENABLED | MEM_AFFINITY_HOTPLUGGABLE;
>> }
>> +
>> + build_srat_memory(table_data, mem_base,
>> + ms->numa_state->nodes[i].node_mem, i, flags);
>> + mem_base += ms->numa_state->nodes[i].node_mem;
>> }
>>
>> if (ms->nvdimms_state->is_enabled) {
>
Thanks,
Gavin
On Wed, 27 Oct 2021 16:20:30 +1100
Gavin Shan <gshan@redhat.com> wrote:
> On 10/26/21 8:47 PM, Igor Mammedov wrote:
> > On Tue, 26 Oct 2021 07:41:01 +0800
> > Gavin Shan <gshan@redhat.com> wrote:
> >
> >> The empty NUMA nodes, where no memory resides, aren't exposed
> >> through ACPI SRAT table. It's not user preferred behaviour because
> >> the corresponding memory node devices are missed from the guest
> >> kernel as the following example shows, and memory can't be hot
> >> added to these empty NUMA nodes at later point.
> >
> > a error message one gets would be useful here.
> >
> > btw:
> > memory hotplug seems to work for x86 without adding empty nodes.
> > So it beg a question, if absence of empty nodes is the issue here.
> >
>
> Yes, the memory can be still hot added even the empty NUMA nodes
> aren't exposed. However, we still need to expose them so that
> the guest kernel has the information as the users specifies.
commit message says that memory can't be hotplugged though ...
so what doesn't work/is broken currently.
Question is why do we need to expose empty nodes
that aren't warranted by any present hardware (cpu/mem)?
(so far I see it as extra burden on qemu without any gain)
SRAT is typically used to describe startup configuration,
any changes to topology later during runtime are
made using _PXM objects.
>
> I will make the commit log more precise in v2.
>
> >>
> >> /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
> >> -accel kvm -machine virt,gic-version=host \
> >> -cpu host -smp 4,sockets=2,cores=2,threads=1 \
> >> -m 1024M,slots=16,maxmem=64G \
> >> -object memory-backend-ram,id=mem0,size=512M \
> >> -object memory-backend-ram,id=mem1,size=512M \
> >> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
> >> -numa node,nodeid=1,cpus=2-3,memdev=mem1 \
> >> -numa node,nodeid=2 \
> >> -numa node,nodeid=3 \
> >> :
> >> guest# ls /sys/devices/system/node | grep node
> >> node0
> >> node1
> >> node2
> >>
> >> This exposes these empty NUMA nodes through ACPI SRAT table. With
> >> this applied, the corresponding memory node devices can be found
> >> from the guest. Note that the hotpluggable capability is explicitly
> >> given to these empty NUMA nodes for sake of completeness.
> >>
> >> guest# ls /sys/devices/system/node | grep node
> >> node0
> >> node1
> >> node2
> >> node3
> >>
> >> Signed-off-by: Gavin Shan <gshan@redhat.com>
> >> ---
> >> hw/arm/virt-acpi-build.c | 14 +++++++++-----
> >> 1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index 674f902652..a4c95b2f64 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -526,6 +526,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >> const CPUArchIdList *cpu_list = mc->possible_cpu_arch_ids(ms);
> >> AcpiTable table = { .sig = "SRAT", .rev = 3, .oem_id = vms->oem_id,
> >> .oem_table_id = vms->oem_table_id };
> >> + MemoryAffinityFlags flags;
> >>
> >> acpi_table_begin(&table, table_data);
> >> build_append_int_noprefix(table_data, 1, 4); /* Reserved */
> >> @@ -547,12 +548,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >>
> >> mem_base = vms->memmap[VIRT_MEM].base;
> >> for (i = 0; i < ms->numa_state->num_nodes; ++i) {
> >> - if (ms->numa_state->nodes[i].node_mem > 0) {
> >> - build_srat_memory(table_data, mem_base,
> >> - ms->numa_state->nodes[i].node_mem, i,
> >> - MEM_AFFINITY_ENABLED);
> >> - mem_base += ms->numa_state->nodes[i].node_mem;
> >> + if (ms->numa_state->nodes[i].node_mem) {
> >> + flags = MEM_AFFINITY_ENABLED;
> >> + } else {
> >> + flags = MEM_AFFINITY_ENABLED | MEM_AFFINITY_HOTPLUGGABLE;
> >> }
> >> +
> >> + build_srat_memory(table_data, mem_base,
> >> + ms->numa_state->nodes[i].node_mem, i, flags);
> >> + mem_base += ms->numa_state->nodes[i].node_mem;
> >> }
> >>
> >> if (ms->nvdimms_state->is_enabled) {
> >
>
> Thanks,
> Gavin
>
© 2016 - 2024 Red Hat, Inc.