On Wed, 27 Oct 2021 16:20:30 +1100
Gavin Shan <gs...@redhat.com> wrote:
> On 10/26/21 8:47 PM, Igor Mammedov wrote:
> > On Tue, 26 Oct 2021 07:41:01 +0800
> > Gavin Shan <gs...@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 <gs...@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
>
