| Message ID | 20211025234101.224705-1-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt: Expose empty NUMA nodes through ACPI | expand |
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 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 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 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 >
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) {
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(-)