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

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 30 Jun 2016 20:47:33 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

Thanks for review,
I'll fix up patch according to your comments and post v3 shortly

> On Thu, Jun 30, 2016 at 02:23:06PM +0200, Igor Mammedov wrote:
> > fixes long standing issue where Linux kernel would assing  
> 
> assign?
> 
> > hotplugged CPU to 1st numa node as it discards proximity
> > for hotplugged CPUs after SRAT is parsed.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  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)));
> > +                }
> > +            }  
> 
> this code is duplicated in a couple of other places.
> how about a helper to find the numa node for a CPU?
> 
> 
> > +
> >              aml_append(cpus_dev, dev);
> >          }
> >      }  
> 
> 
> I'd like a comment here explaining why it's needed.
> E.g. /*
>       * Linux guests discard SRAT info for non-present CPUs
>       * as a result _PXM is required for all CPUs which might
>       * be hot-plugged. For simplicity, add it for all CPUs.
>       */
> 
> 
> > -- 
> > 1.8.3.1  
>