Re: [PATCH] hw/arm/virt: Expose empty NUMA nodes through ACPI

[Gavin Shan]

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