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

[Michael S. Tsirkin]

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 <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)));
> +                }
> +            }

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