Re: [Qemu-devel] [PATCH 3/4] acpi: provide _PXM method for CPU devices if QEMU is started numa enabled

[Igor Mammedov]

On Thu, 30 Jun 2016 15:48:54 +0300
Marcel Apfelbaum <address@hidden> wrote:

> On 06/30/2016 03:23 PM, Igor Mammedov wrote:
> > fixes long standing issue where Linux kernel would assing
> > hotplugged CPU to 1st numa node as it discards proximity
> > for hotplugged CPUs after SRAT is parsed.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >   hw/acpi/cpu.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > index c13b65c..d9cf3ee 100644
> > --- a/hw/acpi/cpu.c
> > +++ b/hw/acpi/cpu.c
> > @@ -4,6 +4,7 @@
> >   #include "qapi/error.h"
> >   #include "qapi-event.h"
> >   #include "trace.h"
> > +#include "sysemu/numa.h"
> >
> >   #define ACPI_CPU_HOTPLUG_REG_LEN 12
> >   #define ACPI_CPU_SELECTOR_OFFSET_WR 0
> > @@ -503,6 +504,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
> > CPUHotplugFeatures opts,
> >
> >           /* build Processor object for each processor */
> >           for (i = 0; i < arch_ids->len; i++) {
> > +            int j;
> >               Aml *dev;
> >               Aml *uid = aml_int(i);
> >               GArray *madt_buf = g_array_new(0, 1, 1);
> > @@ -546,6 +548,13 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
> > CPUHotplugFeatures opts,
> >                             aml_arg(1), aml_arg(2))
> >               );
> >               aml_append(dev, method);
> > +
> > +            for (j = 0; j < nb_numa_nodes; j++) {
> > +                if (test_bit(i, numa_info[j].node_cpu)) {
> > +                    aml_append(dev, aml_name_decl("_PXM", aml_int(j)));
> > +                }
> > +            }
> > +
> >               aml_append(cpus_dev, dev);
> >           }
> >       }
> >  
> 
> I would add, at least in the commit message, a pointer to the ACPI spec:
> 
> ACPI 5.0 (6.2.13)
> -----------------
> If the Local APIC ID / Local SAPIC ID / Local x2APIC ID of a dynamically 
> added processor is not
> present in the System Resource Affinity Table (SRAT), a _PXM object must 
> exist for the
> processor’s device or one of its ancestors in the ACPI Namespace.
> 
> 
> I suppose we don't have the APIC id in SRAT for all possible CPUs, so it OK.
we have entries for possible CPUs in SRAT and commit says that Linux discards 
it,
hence we need to add _PXM to CPU objects. So broken linux handling would put
hotplugged CPUs into corrected nodes.

To fix it on linux side, ACPI part probably would need to be refactored to store
parsed tables info somewhere else, so it would be available past boot time
(not a small undertaking) I'd say.
While fixing it on QEMU side is easy and works well even for currently released
kernels.


> 
> 
> Reviewed-by: Marcel Apfelbaum <address@hidden>
Thanks!

> 
> Thanks,
> Marcel