[Qemu-devel] [PATCH v5 0/4] fix numa node mapping for hotplugged CPUs

[Qemu-devel] [PATCH v5 0/4] fix numa node mapping for hotplugged CPUs

[Igor Mammedov]

Changelog:
v4->v5:
  - rebase on top of current master, practicaaly resend of reviewed v4
    which got lost during 2.7 merge window, so I've kept all Reviewed-by-s
     * 'tests: acpi: extend cphp testcase with numa check'
          couldn't be applied due context change
          so I had to fix it manually
  - split out expected tables blobs into separate patch

v3->v4:
  - return -1 on failure from numa_get_node_for_cpu()
  - replace SRAT table blob in 5/5 with note to maintainer to add it manually
v1->v3:
  - fix commit message for 4/5
  - add numa_get_node_for_cpu() helper
  - add comment in code explaining why _PXM is being added

Series makes hotplugged CPUs assigned to correct numa nodes
for Linux guests and extends CPU hotplug test with numa options

NOTE TO MAINTAINER:
After running rebuild-expected-aml.sh
blobs to be updated
   tests/acpi-test-data/pc/DSDT.cphp
   tests/acpi-test-data/q35/DSDT.cphp
new blobs to be added
   tests/acpi-test-data/pc/SRAT.cphp
   tests/acpi-test-data/q35/SRAT.cphp


Igor Mammedov (4):
  numa: reduce code duplication by adding helper numa_get_node_for_cpu()
  acpi: provide _PXM method for CPU devices if QEMU is started numa
    enabled
  tests: acpi: extend cphp testcase with numa check
  DO NOT APPLY: acpi tables expected blobs update

 include/sysemu/numa.h              |   3 +++
 hw/acpi/cpu.c                      |  12 ++++++++++++
 hw/arm/virt-acpi-build.c           |   6 ++----
 hw/arm/virt.c                      |   7 +++----
 hw/i386/acpi-build.c               |   7 ++-----
 hw/i386/pc.c                       |   8 +++-----
 hw/ppc/spapr_cpu_core.c            |   6 ++----
 numa.c                             |  12 ++++++++++++
 tests/acpi-test-data/pc/DSDT.cphp  | Bin 6435 -> 6471 bytes
 tests/acpi-test-data/pc/SRAT.cphp  | Bin 0 -> 304 bytes
 tests/acpi-test-data/q35/DSDT.cphp | Bin 9197 -> 9233 bytes
 tests/acpi-test-data/q35/SRAT.cphp | Bin 0 -> 304 bytes
 tests/bios-tables-test.c           |   6 ++++--
 13 files changed, 43 insertions(+), 24 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/SRAT.cphp
 create mode 100644 tests/acpi-test-data/q35/SRAT.cphp

-- 
2.7.4

[Qemu-devel] [PATCH v5 1/4] numa: reduce code duplication by adding helper numa_get_node_for_cpu()

[Igor Mammedov]

Replace repeated pattern

    for (i = 0; i < nb_numa_nodes; i++) {
        if (test_bit(idx, numa_info[i].node_cpu)) {
           ...
           break;

with a helper function to lookup numa node index for cpu.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 include/sysemu/numa.h    |  3 +++
 hw/arm/virt-acpi-build.c |  6 ++----
 hw/arm/virt.c            |  7 +++----
 hw/i386/acpi-build.c     |  7 ++-----
 hw/i386/pc.c             |  8 +++-----
 hw/ppc/spapr_cpu_core.c  |  6 ++----
 numa.c                   | 12 ++++++++++++
 7 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index bb184c9..4da808a 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -32,4 +32,7 @@ void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
 void numa_unset_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
 uint32_t numa_get_node(ram_addr_t addr, Error **errp);
 
+/* on success returns node index in numa_info,
+ * on failure returns nb_numa_nodes */
+int numa_get_node_for_cpu(int idx);
 #endif
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7b39b1d..c77525d 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -427,11 +427,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
     uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof(uint32_t));
 
     for (i = 0; i < guest_info->smp_cpus; i++) {
-        for (j = 0; j < nb_numa_nodes; j++) {
-            if (test_bit(i, numa_info[j].node_cpu)) {
+        j = numa_get_node_for_cpu(i);
+        if (j < nb_numa_nodes) {
                 cpu_node[i] = j;
-                break;
-            }
         }
     }
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0f6305d..795740d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -413,10 +413,9 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
                                   armcpu->mp_affinity);
         }
 
-        for (i = 0; i < nb_numa_nodes; i++) {
-            if (test_bit(cpu, numa_info[i].node_cpu)) {
-                qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i);
-            }
+        i = numa_get_node_for_cpu(cpu);
+        if (i < nb_numa_nodes) {
+            qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i);
         }
 
         g_free(nodename);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c20bc71..e999654 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2410,18 +2410,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     srat->reserved1 = cpu_to_le32(1);
 
     for (i = 0; i < apic_ids->len; i++) {
-        int j;
+        int j = numa_get_node_for_cpu(i);
         int apic_id = apic_ids->cpus[i].arch_id;
 
         core = acpi_data_push(table_data, sizeof *core);
         core->type = ACPI_SRAT_PROCESSOR_APIC;
         core->length = sizeof(*core);
         core->local_apic_id = apic_id;
-        for (j = 0; j < nb_numa_nodes; j++) {
-            if (test_bit(i, numa_info[j].node_cpu)) {
+        if (j < nb_numa_nodes) {
                 core->proximity_lo = j;
-                break;
-            }
         }
         memset(core->proximity_hi, 0, 3);
         core->local_sapic_eid = 0;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2d6d792..93ff49c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -779,11 +779,9 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
     for (i = 0; i < max_cpus; i++) {
         unsigned int apic_id = x86_cpu_apic_id_from_index(i);
         assert(apic_id < pcms->apic_id_limit);
-        for (j = 0; j < nb_numa_nodes; j++) {
-            if (test_bit(i, numa_info[j].node_cpu)) {
-                numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
-                break;
-            }
+        j = numa_get_node_for_cpu(i);
+        if (j < nb_numa_nodes) {
+            numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
         }
     }
     for (i = 0; i < nb_numa_nodes; i++) {
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 6f0533c..82637c6 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -69,11 +69,9 @@ void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
     }
 
     /* Set NUMA node for the added CPUs  */
-    for (i = 0; i < nb_numa_nodes; i++) {
-        if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) {
+    i = numa_get_node_for_cpu(cs->cpu_index);
+    if (i < nb_numa_nodes) {
             cs->numa_node = i;
-            break;
-        }
     }
 
     xics_cpu_setup(spapr->xics, cpu);
diff --git a/numa.c b/numa.c
index 6289f46..9c09e45 100644
--- a/numa.c
+++ b/numa.c
@@ -550,3 +550,15 @@ MemdevList *qmp_query_memdev(Error **errp)
     object_child_foreach(obj, query_memdev, &list);
     return list;
 }
+
+int numa_get_node_for_cpu(int idx)
+{
+    int i;
+
+    for (i = 0; i < nb_numa_nodes; i++) {
+        if (test_bit(idx, numa_info[i].node_cpu)) {
+            break;
+        }
+    }
+    return i;
+}
-- 
2.7.4

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

[Igor Mammedov]

Workaround for long standing issue where Linux kernel
assigns hotplugged CPU to 1st numa node as it discards
proximity for possible CPUs from SRAT after it's parsed.

_PXM method allows linux query proximity directly from
hotplugged CPU object, which allows Linux to assing CPU
to the correct numa node.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/acpi/cpu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index c13b65c..902f5c9 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,16 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
                           aml_arg(1), aml_arg(2))
             );
             aml_append(dev, method);
+
+            /* 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.
+             */
+            j = numa_get_node_for_cpu(i);
+            if (j < nb_numa_nodes) {
+                aml_append(dev, aml_name_decl("_PXM", aml_int(j)));
+            }
+
             aml_append(cpus_dev, dev);
         }
     }
-- 
2.7.4

[Qemu-devel] [PATCH v5 3/4] tests: acpi: extend cphp testcase with numa check

[Igor Mammedov]

so it would be possible to verify _PXM generation in
DSDT and SRAT tables.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
NOTE to maintainer:

After running rebuild-expected-aml.sh
following blobs should be updated
   tests/acpi-test-data/pc/DSDT.cphp
   tests/acpi-test-data/q35/DSDT.cphp
following new blobs should be added
   tests/acpi-test-data/pc/SRAT.cphp
   tests/acpi-test-data/q35/SRAT.cphp
---
 tests/bios-tables-test.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 7e27ea9..6ea2b6d 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -811,7 +811,8 @@ static void test_acpi_piix4_tcg_cphp(void)
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_PC;
     data.variant = ".cphp";
-    test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6",
+    test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6"
+                  " -numa node -numa node",
                   &data);
     free_test_data(&data);
 }
@@ -823,7 +824,8 @@ static void test_acpi_q35_tcg_cphp(void)
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_Q35;
     data.variant = ".cphp";
-    test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6",
+    test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6"
+                  " -numa node -numa node",
                   &data);
     free_test_data(&data);
 }
-- 
2.7.4

[Qemu-devel] [PATCH v5 4/4] tests: acpi tables expected blobs update

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
fill free to discard if pull req contains other changes to
expected blobs

---
 tests/acpi-test-data/pc/DSDT.cphp  | Bin 6435 -> 6471 bytes
 tests/acpi-test-data/pc/SRAT.cphp  | Bin 0 -> 304 bytes
 tests/acpi-test-data/q35/DSDT.cphp | Bin 9197 -> 9233 bytes
 tests/acpi-test-data/q35/SRAT.cphp | Bin 0 -> 304 bytes
 4 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/SRAT.cphp
 create mode 100644 tests/acpi-test-data/q35/SRAT.cphp

diff --git a/tests/acpi-test-data/pc/DSDT.cphp b/tests/acpi-test-data/pc/DSDT.cphp
index e8b146208eb3877e1cde2cc361c5afd270d194c6..9f405cfd83d39a8e06bc08428e160a0192fc9704 100644
GIT binary patch
delta 122
zcmZ2%blix`CD<jzU6O%;F?AzXB|DQx$mCY`)lA-=n~gXQGBJ8j{=((L$q^qA;mZ)+
u>^<3<8^QCN+{En;m-Cx^2F7EIZuXlj#sifD^AdR6*}$eSZeGq)!vg?MeIj%K

delta 85
zcmX?ZwAhHtCD<iIS(1T)@ySN6N_HlfpvkT5tC`&0Hyd#rWMXuk{DsSfIl9?(vLiQ$
a_L$tq?GC2zL1{f62)~ZUee-Fa8Xf>!Wg5-^

diff --git a/tests/acpi-test-data/pc/SRAT.cphp b/tests/acpi-test-data/pc/SRAT.cphp
new file mode 100644
index 0000000000000000000000000000000000000000..ff2137642f488ec70b85207ed6c20e7351d61e98
GIT binary patch
literal 304
zcmWFzattwGWME)4bMklg2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}yOvY_!<(fDl0
pd`1npO;83GTmZW|po75R12aq^syaB21u74tQT&BzFU&Ml8UVWm2>}2A

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/DSDT.cphp b/tests/acpi-test-data/q35/DSDT.cphp
index 6cc28c6daec2b331030ab0600a4d79034c1dfc40..a0ce6b3264c69999c6e82a8ae7bab49338e4819b 100644
GIT binary patch
delta 122
zcmaFsKGB2ACD<iIP=$ekaoa|&MoA`*kjcH0tC_q#H(N;^WMcH5{71%xlOsML!j~bs
u*?Y3HEQ04Zxl7g^F6TG-3XI1X-Rw76P7W#u=4Hsavw=-v+`L}Sjuim7cO#ep

delta 85
zcmbQ}@z$NoCD<k8tug}xquEBTMoA`@pvk?GtC`&0H(N;^WMXuk{71%xIl9?(vZpMF
a_Lw|P)*VbggVJVl5PqAS`{v7XcB}xQ78~*a

diff --git a/tests/acpi-test-data/q35/SRAT.cphp b/tests/acpi-test-data/q35/SRAT.cphp
new file mode 100644
index 0000000000000000000000000000000000000000..ff2137642f488ec70b85207ed6c20e7351d61e98
GIT binary patch
literal 304
zcmWFzattwGWME)4bMklg2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}yOvY_!<(fDl0
pd`1npO;83GTmZW|po75R12aq^syaB21u74tQT&BzFU&Ml8UVWm2>}2A

literal 0
HcmV?d00001

-- 
2.7.4