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

[Marcel Apfelbaum]

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.


Reviewed-by: Marcel Apfelbaum <address@hidden>

Thanks,
Marcel