[PATCH v8 0/5] hw/arm/virt: Fix CPU's default NUMA node ID

[PATCH v8 0/5] hw/arm/virt: Fix CPU's default NUMA node ID

[Gavin Shan]

When the CPU-to-NUMA association isn't provided by user, the default NUMA
node ID for the specific CPU is returned from virt_get_default_cpu_node_id().
Unfortunately, the default NUMA node ID breaks socket boundary and leads to
the broken CPU topology warning message in Linux guest. This series intends
to fix the issue.

  PATCH[1/5] Add cluster-id to CPU instance property
  PATCH[2/5] Fixes test failure in qtest/numa-test/aarch64_numa_cpu()
  PATCH[3/5] Uses SMP configuration to populate CPU topology
  PATCH[4/5] Fixes the broken CPU topology by considering the socket boundary
             when the default NUMA node ID is given
  PATCH[5/5] Uses the populated CPU topology to build PPTT table, instead of
             calculate it again

Changelog
=========
v8:
   * Separate PATCH[v8 2/5] to fix test failure in qtest/
     numa-test/aarch64_numa_cpu()                               (Igor)
   * Improvents to coding style, changelog and comments         (Yanan)
v6/v7:
   * Fixed description for 'cluster-id' and 'core-id'           (Yanan)
   * Remove '% ms->smp.sockets' in socket ID calculation        (Yanan)
   * Fixed tests/qtest/numa-test/aarch64_numa_cpu()             (Yanan)
   * Initialized offset variables in build_pptt()               (Jonathan)
   * Added comments about the expected and sorted layout of
     cpus[n].props.*_id and assert() on the exceptional cases   (Igor)
v4/v5:
   * Split PATCH[v3 1/3] to PATCH[v5 1/4] and PATCH[v5 2/4].
     Verify or dump 'clsuter-id' in various spots               (Yanan)
   * s/within cluster/within cluster\/die/ for 'core-id' in
     qapi/machine.json                                          (Igor)
   * Apply '% ms->smp.{sockets, clusters, cores, threads} in
     virt_possible_cpu_arch_ids() as x86 does                   (Igor)
   * Use [0 - possible_cpus->len] as ACPI processor UID to
     build PPTT table and PATCH[v3 4/4] is dropped              (Igor)
   * Simplified build_pptt() to add all entries in one loop
     on ms->possible_cpus                                       (Igor)
v3:
   * Split PATCH[v2 1/3] to PATCH[v3 1/4] and PATCH[v3 2/4]     (Yanan)
   * Don't take account of die ID in CPU topology population
     and added assert(!mc->smp_props.dies_supported)            (Yanan/Igor)
   * Assign cluster_id and use it when building PPTT table      (Yanan/Igor)
v2:
   * Populate the CPU topology in virt_possible_cpu_arch_ids()
     so that it can be reused in virt_get_default_cpu_node_id() (Igor)
   * Added PATCH[2/3] to use the existing CPU topology when the
     PPTT table is built                                        (Igor)
   * Added PATCH[3/3] to take thread ID as ACPI processor ID
     in MADT and SRAT table                                     (Gavin)

Gavin Shan (5):
  qapi/machine.json: Add cluster-id
  qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()
  hw/arm/virt: Consider SMP configuration in CPU topology
  hw/arm/virt: Fix CPU's default NUMA node ID
  hw/acpi/aml-build: Use existing CPU topology to build PPTT table

 hw/acpi/aml-build.c        | 111 ++++++++++++++++---------------------
 hw/arm/virt.c              |  19 ++++++-
 hw/core/machine-hmp-cmds.c |   4 ++
 hw/core/machine.c          |  16 ++++++
 qapi/machine.json          |   6 +-
 tests/qtest/numa-test.c    |   3 +-
 6 files changed, 91 insertions(+), 68 deletions(-)

-- 
2.23.0

[PATCH v8 1/5] qapi/machine.json: Add cluster-id

[Gavin Shan]

This adds cluster-id in CPU instance properties, which will be used
by arm/virt machine. Besides, the cluster-id is also verified or
dumped in various spots:

  * hw/core/machine.c::machine_set_cpu_numa_node() to associate
    CPU with its NUMA node.

  * hw/core/machine.c::machine_numa_finish_cpu_init() to record
    CPU slots with no NUMA mapping set.

  * hw/core/machine-hmp-cmds.c::hmp_hotpluggable_cpus() to dump
    cluster-id.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
---
 hw/core/machine-hmp-cmds.c |  4 ++++
 hw/core/machine.c          | 16 ++++++++++++++++
 qapi/machine.json          |  6 ++++--
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index 4e2f319aeb..5cb5eecbfc 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -77,6 +77,10 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
         if (c->has_die_id) {
             monitor_printf(mon, "    die-id: \"%" PRIu64 "\"\n", c->die_id);
         }
+        if (c->has_cluster_id) {
+            monitor_printf(mon, "    cluster-id: \"%" PRIu64 "\"\n",
+                           c->cluster_id);
+        }
         if (c->has_core_id) {
             monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n", c->core_id);
         }
diff --git a/hw/core/machine.c b/hw/core/machine.c
index cb9bbc844d..700c1e76b8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -682,6 +682,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
             return;
         }
 
+        if (props->has_cluster_id && !slot->props.has_cluster_id) {
+            error_setg(errp, "cluster-id is not supported");
+            return;
+        }
+
         if (props->has_socket_id && !slot->props.has_socket_id) {
             error_setg(errp, "socket-id is not supported");
             return;
@@ -701,6 +706,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
                 continue;
         }
 
+        if (props->has_cluster_id &&
+            props->cluster_id != slot->props.cluster_id) {
+                continue;
+        }
+
         if (props->has_die_id && props->die_id != slot->props.die_id) {
                 continue;
         }
@@ -995,6 +1005,12 @@ static char *cpu_slot_to_string(const CPUArchId *cpu)
         }
         g_string_append_printf(s, "die-id: %"PRId64, cpu->props.die_id);
     }
+    if (cpu->props.has_cluster_id) {
+        if (s->len) {
+            g_string_append_printf(s, ", ");
+        }
+        g_string_append_printf(s, "cluster-id: %"PRId64, cpu->props.cluster_id);
+    }
     if (cpu->props.has_core_id) {
         if (s->len) {
             g_string_append_printf(s, ", ");
diff --git a/qapi/machine.json b/qapi/machine.json
index d25a481ce4..4c417e32a5 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -868,10 +868,11 @@
 # @node-id: NUMA node ID the CPU belongs to
 # @socket-id: socket number within node/board the CPU belongs to
 # @die-id: die number within socket the CPU belongs to (since 4.1)
-# @core-id: core number within die the CPU belongs to
+# @cluster-id: cluster number within die the CPU belongs to (since 7.1)
+# @core-id: core number within cluster the CPU belongs to
 # @thread-id: thread number within core the CPU belongs to
 #
-# Note: currently there are 5 properties that could be present
+# Note: currently there are 6 properties that could be present
 #       but management should be prepared to pass through other
 #       properties with device_add command to allow for future
 #       interface extension. This also requires the filed names to be kept in
@@ -883,6 +884,7 @@
   'data': { '*node-id': 'int',
             '*socket-id': 'int',
             '*die-id': 'int',
+            '*cluster-id': 'int',
             '*core-id': 'int',
             '*thread-id': 'int'
   }
-- 
2.23.0

[PATCH v8 2/5] qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()

[Gavin Shan]

The CPU topology isn't enabled on arm/virt machine yet, but we're
going to do it in next patch. After the CPU topology is enabled by
next patch, "thrad-id=1" becomes invalid because the CPU core is
preferred on arm/virt machine. It means these two CPUs have 0/1
as their core IDs, but their thread IDs are all 0. It will trigger
test failure as the following message indicates:

  [14/21 qemu:qtest+qtest-aarch64 / qtest-aarch64/numa-test  ERROR
  1.48s   killed by signal 6 SIGABRT
  >>> G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \
      QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon         \
      QTEST_QEMU_BINARY=./qemu-system-aarch64                                       \
      QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=83                                  \
      /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
  ――――――――――――――――――――――――――――――――――――――――――――――
  stderr:
  qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found

This fixes the issue by providing comprehensive SMP configurations
in aarch64_numa_cpu(). The SMP configurations aren't used before
the CPU topology is enabled in next patch.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
---
 tests/qtest/numa-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index 90bf68a5b3..aeda8c774c 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -223,7 +223,8 @@ static void aarch64_numa_cpu(const void *data)
     QTestState *qts;
     g_autofree char *cli = NULL;
 
-    cli = make_cli(data, "-machine smp.cpus=2 "
+    cli = make_cli(data, "-machine "
+        "smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2 "
         "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
         "-numa cpu,node-id=1,thread-id=0 "
         "-numa cpu,node-id=0,thread-id=1");
-- 
2.23.0

[PATCH v8 3/5] hw/arm/virt: Consider SMP configuration in CPU topology

[Gavin Shan]

Currently, the SMP configuration isn't considered when the CPU
topology is populated. In this case, it's impossible to provide
the default CPU-to-NUMA mapping or association based on the socket
ID of the given CPU.

This takes account of SMP configuration when the CPU topology
is populated. The die ID for the given CPU isn't assigned since
it's not supported on arm/virt machine. Besides, the used SMP
configuration in qtest/numa-test/aarch64_numa_cpu() is corrcted
to avoid testing failure

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
---
 hw/arm/virt.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5bdd98e4a1..0fd7f9a6a1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2560,6 +2560,7 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
     int n;
     unsigned int max_cpus = ms->smp.max_cpus;
     VirtMachineState *vms = VIRT_MACHINE(ms);
+    MachineClass *mc = MACHINE_GET_CLASS(vms);
 
     if (ms->possible_cpus) {
         assert(ms->possible_cpus->len == max_cpus);
@@ -2573,8 +2574,20 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
         ms->possible_cpus->cpus[n].type = ms->cpu_type;
         ms->possible_cpus->cpus[n].arch_id =
             virt_cpu_mp_affinity(vms, n);
+
+        assert(!mc->smp_props.dies_supported);
+        ms->possible_cpus->cpus[n].props.has_socket_id = true;
+        ms->possible_cpus->cpus[n].props.socket_id =
+            n / (ms->smp.clusters * ms->smp.cores * ms->smp.threads);
+        ms->possible_cpus->cpus[n].props.has_cluster_id = true;
+        ms->possible_cpus->cpus[n].props.cluster_id =
+            (n / (ms->smp.cores * ms->smp.threads)) % ms->smp.clusters;
+        ms->possible_cpus->cpus[n].props.has_core_id = true;
+        ms->possible_cpus->cpus[n].props.core_id =
+            (n / ms->smp.threads) % ms->smp.cores;
         ms->possible_cpus->cpus[n].props.has_thread_id = true;
-        ms->possible_cpus->cpus[n].props.thread_id = n;
+        ms->possible_cpus->cpus[n].props.thread_id =
+            n % ms->smp.threads;
     }
     return ms->possible_cpus;
 }
-- 
2.23.0

[PATCH v8 4/5] hw/arm/virt: Fix CPU's default NUMA node ID

[Gavin Shan]

When CPU-to-NUMA association isn't explicitly provided by users,
the default one is given by mc->get_default_cpu_node_id(). However,
the CPU topology isn't fully considered in the default association
and this causes CPU topology broken warnings on booting Linux guest.

For example, the following warning messages are observed when the
Linux guest is booted with the following command lines.

  /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \
  -accel kvm -machine virt,gic-version=host               \
  -cpu host                                               \
  -smp 6,sockets=2,cores=3,threads=1                      \
  -m 1024M,slots=16,maxmem=64G                            \
  -object memory-backend-ram,id=mem0,size=128M            \
  -object memory-backend-ram,id=mem1,size=128M            \
  -object memory-backend-ram,id=mem2,size=128M            \
  -object memory-backend-ram,id=mem3,size=128M            \
  -object memory-backend-ram,id=mem4,size=128M            \
  -object memory-backend-ram,id=mem4,size=384M            \
  -numa node,nodeid=0,memdev=mem0                         \
  -numa node,nodeid=1,memdev=mem1                         \
  -numa node,nodeid=2,memdev=mem2                         \
  -numa node,nodeid=3,memdev=mem3                         \
  -numa node,nodeid=4,memdev=mem4                         \
  -numa node,nodeid=5,memdev=mem5
         :
  alternatives: patching kernel code
  BUG: arch topology borken
  the CLS domain not a subset of the MC domain
  <the above error log repeats>
  BUG: arch topology borken
  the DIE domain not a subset of the NODE domain

With current implementation of mc->get_default_cpu_node_id(),
CPU#0 to CPU#5 are associated with NODE#0 to NODE#5 separately.
That's incorrect because CPU#0/1/2 should be associated with same
NUMA node because they're seated in same socket.

This fixes the issue by considering the socket ID when the default
CPU-to-NUMA association is provided in virt_possible_cpu_arch_ids().
With this applied, no more CPU topology broken warnings are seen
from the Linux guest. The 6 CPUs are associated with NODE#0/1, but
there are no CPUs associated with NODE#2/3/4/5.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
---
 hw/arm/virt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0fd7f9a6a1..091054662c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2552,7 +2552,9 @@ virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 
 static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
 {
-    return idx % ms->numa_state->num_nodes;
+    int64_t socket_id = ms->possible_cpus->cpus[idx].props.socket_id;
+
+    return socket_id % ms->numa_state->num_nodes;
 }
 
 static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
-- 
2.23.0

[PATCH v8 5/5] hw/acpi/aml-build: Use existing CPU topology to build PPTT table

[Gavin Shan]

When the PPTT table is built, the CPU topology is re-calculated, but
it's unecessary because the CPU topology has been populated in
virt_possible_cpu_arch_ids() on arm/virt machine.

This reworks build_pptt() to avoid by reusing the existing IDs in
ms->possible_cpus. Currently, the only user of build_pptt() is
arm/virt machine.

Signed-off-by: Gavin Shan <gshan@redhat.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/aml-build.c | 111 +++++++++++++++++++-------------------------
 1 file changed, 48 insertions(+), 63 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 4086879ebf..e6bfac95c7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2002,86 +2002,71 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
                 const char *oem_id, const char *oem_table_id)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
-    GQueue *list = g_queue_new();
-    guint pptt_start = table_data->len;
-    guint parent_offset;
-    guint length, i;
-    int uid = 0;
-    int socket;
+    CPUArchIdList *cpus = ms->possible_cpus;
+    int64_t socket_id = -1, cluster_id = -1, core_id = -1;
+    uint32_t socket_offset = 0, cluster_offset = 0, core_offset = 0;
+    uint32_t pptt_start = table_data->len;
+    int n;
     AcpiTable table = { .sig = "PPTT", .rev = 2,
                         .oem_id = oem_id, .oem_table_id = oem_table_id };
 
     acpi_table_begin(&table, table_data);
 
-    for (socket = 0; socket < ms->smp.sockets; socket++) {
-        g_queue_push_tail(list,
-            GUINT_TO_POINTER(table_data->len - pptt_start));
-        build_processor_hierarchy_node(
-            table_data,
-            /*
-             * Physical package - represents the boundary
-             * of a physical package
-             */
-            (1 << 0),
-            0, socket, NULL, 0);
-    }
-
-    if (mc->smp_props.clusters_supported) {
-        length = g_queue_get_length(list);
-        for (i = 0; i < length; i++) {
-            int cluster;
-
-            parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list));
-            for (cluster = 0; cluster < ms->smp.clusters; cluster++) {
-                g_queue_push_tail(list,
-                    GUINT_TO_POINTER(table_data->len - pptt_start));
-                build_processor_hierarchy_node(
-                    table_data,
-                    (0 << 0), /* not a physical package */
-                    parent_offset, cluster, NULL, 0);
-            }
+    /*
+     * This works with the assumption that cpus[n].props.*_id has been
+     * sorted from top to down levels in mc->possible_cpu_arch_ids().
+     * Otherwise, the unexpected and duplicated containers will be
+     * created.
+     */
+    for (n = 0; n < cpus->len; n++) {
+        if (cpus->cpus[n].props.socket_id != socket_id) {
+            assert(cpus->cpus[n].props.socket_id > socket_id);
+            socket_id = cpus->cpus[n].props.socket_id;
+            cluster_id = -1;
+            core_id = -1;
+            socket_offset = table_data->len - pptt_start;
+            build_processor_hierarchy_node(table_data,
+                (1 << 0), /* Physical package */
+                0, socket_id, NULL, 0);
         }
-    }
 
-    length = g_queue_get_length(list);
-    for (i = 0; i < length; i++) {
-        int core;
-
-        parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list));
-        for (core = 0; core < ms->smp.cores; core++) {
-            if (ms->smp.threads > 1) {
-                g_queue_push_tail(list,
-                    GUINT_TO_POINTER(table_data->len - pptt_start));
-                build_processor_hierarchy_node(
-                    table_data,
-                    (0 << 0), /* not a physical package */
-                    parent_offset, core, NULL, 0);
-            } else {
-                build_processor_hierarchy_node(
-                    table_data,
-                    (1 << 1) | /* ACPI Processor ID valid */
-                    (1 << 3),  /* Node is a Leaf */
-                    parent_offset, uid++, NULL, 0);
+        if (mc->smp_props.clusters_supported) {
+            if (cpus->cpus[n].props.cluster_id != cluster_id) {
+                assert(cpus->cpus[n].props.cluster_id > cluster_id);
+                cluster_id = cpus->cpus[n].props.cluster_id;
+                core_id = -1;
+                cluster_offset = table_data->len - pptt_start;
+                build_processor_hierarchy_node(table_data,
+                    (0 << 0), /* Not a physical package */
+                    socket_offset, cluster_id, NULL, 0);
             }
+        } else {
+            cluster_offset = socket_offset;
         }
-    }
 
-    length = g_queue_get_length(list);
-    for (i = 0; i < length; i++) {
-        int thread;
+        if (ms->smp.threads == 1) {
+            build_processor_hierarchy_node(table_data,
+                (1 << 1) | /* ACPI Processor ID valid */
+                (1 << 3),  /* Node is a Leaf */
+                cluster_offset, n, NULL, 0);
+        } else {
+            if (cpus->cpus[n].props.core_id != core_id) {
+                assert(cpus->cpus[n].props.core_id > core_id);
+                core_id = cpus->cpus[n].props.core_id;
+                core_offset = table_data->len - pptt_start;
+                build_processor_hierarchy_node(table_data,
+                    (0 << 0), /* Not a physical package */
+                    cluster_offset, core_id, NULL, 0);
+            }
 
-        parent_offset = GPOINTER_TO_UINT(g_queue_pop_head(list));
-        for (thread = 0; thread < ms->smp.threads; thread++) {
-            build_processor_hierarchy_node(
-                table_data,
+            build_processor_hierarchy_node(table_data,
                 (1 << 1) | /* ACPI Processor ID valid */
                 (1 << 2) | /* Processor is a Thread */
                 (1 << 3),  /* Node is a Leaf */
-                parent_offset, uid++, NULL, 0);
+                core_offset, n, NULL, 0);
         }
     }
 
-    g_queue_free(list);
     acpi_table_end(linker, &table);
 }
 
-- 
2.23.0

Re: [PATCH v8 0/5] hw/arm/virt: Fix CPU's default NUMA node ID

[Gavin Shan]

Hi Peter and maintainers,

On 4/25/22 11:27 AM, Gavin Shan wrote:
> When the CPU-to-NUMA association isn't provided by user, the default NUMA
> node ID for the specific CPU is returned from virt_get_default_cpu_node_id().
> Unfortunately, the default NUMA node ID breaks socket boundary and leads to
> the broken CPU topology warning message in Linux guest. This series intends
> to fix the issue.
> 
>    PATCH[1/5] Add cluster-id to CPU instance property
>    PATCH[2/5] Fixes test failure in qtest/numa-test/aarch64_numa_cpu()
>    PATCH[3/5] Uses SMP configuration to populate CPU topology
>    PATCH[4/5] Fixes the broken CPU topology by considering the socket boundary
>               when the default NUMA node ID is given
>    PATCH[5/5] Uses the populated CPU topology to build PPTT table, instead of
>               calculate it again
> 

Could you help to check if it's eligible for 7.1? Thanks a lot
for your comments in advance.

[...]

> 
> Gavin Shan (5):
>    qapi/machine.json: Add cluster-id
>    qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()
>    hw/arm/virt: Consider SMP configuration in CPU topology
>    hw/arm/virt: Fix CPU's default NUMA node ID
>    hw/acpi/aml-build: Use existing CPU topology to build PPTT table
> 
>   hw/acpi/aml-build.c        | 111 ++++++++++++++++---------------------
>   hw/arm/virt.c              |  19 ++++++-
>   hw/core/machine-hmp-cmds.c |   4 ++
>   hw/core/machine.c          |  16 ++++++
>   qapi/machine.json          |   6 +-
>   tests/qtest/numa-test.c    |   3 +-
>   6 files changed, 91 insertions(+), 68 deletions(-)
> 

Thanks,
Gavin

Re: [PATCH v8 2/5] qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()

[Igor Mammedov]

On Mon, 25 Apr 2022 11:27:59 +0800
Gavin Shan <gshan@redhat.com> wrote:

> The CPU topology isn't enabled on arm/virt machine yet, but we're
> going to do it in next patch. After the CPU topology is enabled by
> next patch, "thrad-id=1" becomes invalid because the CPU core is
                 ^^^ typo

> preferred on arm/virt machine. It means these two CPUs have 0/1
> as their core IDs, but their thread IDs are all 0. It will trigger
> test failure as the following message indicates:
> 
>   [14/21 qemu:qtest+qtest-aarch64 / qtest-aarch64/numa-test  ERROR
>   1.48s   killed by signal 6 SIGABRT
>   >>> G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \  
>       QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon         \
>       QTEST_QEMU_BINARY=./qemu-system-aarch64                                       \
>       QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=83                                  \
>       /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
>   ――――――――――――――――――――――――――――――――――――――――――――――
>   stderr:
>   qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
> 
> This fixes the issue by providing comprehensive SMP configurations
> in aarch64_numa_cpu(). The SMP configurations aren't used before
> the CPU topology is enabled in next patch.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  tests/qtest/numa-test.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
> index 90bf68a5b3..aeda8c774c 100644
> --- a/tests/qtest/numa-test.c
> +++ b/tests/qtest/numa-test.c
> @@ -223,7 +223,8 @@ static void aarch64_numa_cpu(const void *data)
>      QTestState *qts;
>      g_autofree char *cli = NULL;
>  
> -    cli = make_cli(data, "-machine smp.cpus=2 "
> +    cli = make_cli(data, "-machine "
> +        "smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2 "
>          "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
>          "-numa cpu,node-id=1,thread-id=0 "
                                ^^^^
make it sensible as well, i.e. socket/cluster/cores-ids ...

>          "-numa cpu,node-id=0,thread-id=1");

Re: [PATCH v8 2/5] qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()

[Gavin Shan]

Hi Igor,

On 5/2/22 4:52 PM, Igor Mammedov wrote:
> On Mon, 25 Apr 2022 11:27:59 +0800
> Gavin Shan <gshan@redhat.com> wrote:
> 
>> The CPU topology isn't enabled on arm/virt machine yet, but we're
>> going to do it in next patch. After the CPU topology is enabled by
>> next patch, "thrad-id=1" becomes invalid because the CPU core is
>                   ^^^ typo
> 

hmm, my bad. Lets fix it in next revision.

>> preferred on arm/virt machine. It means these two CPUs have 0/1
>> as their core IDs, but their thread IDs are all 0. It will trigger
>> test failure as the following message indicates:
>>
>>    [14/21 qemu:qtest+qtest-aarch64 / qtest-aarch64/numa-test  ERROR
>>    1.48s   killed by signal 6 SIGABRT
>>    >>> G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \
>>        QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon         \
>>        QTEST_QEMU_BINARY=./qemu-system-aarch64                                       \
>>        QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=83                                  \
>>        /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
>>    ――――――――――――――――――――――――――――――――――――――――――――――
>>    stderr:
>>    qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
>>
>> This fixes the issue by providing comprehensive SMP configurations
>> in aarch64_numa_cpu(). The SMP configurations aren't used before
>> the CPU topology is enabled in next patch.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>>   tests/qtest/numa-test.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
>> index 90bf68a5b3..aeda8c774c 100644
>> --- a/tests/qtest/numa-test.c
>> +++ b/tests/qtest/numa-test.c
>> @@ -223,7 +223,8 @@ static void aarch64_numa_cpu(const void *data)
>>       QTestState *qts;
>>       g_autofree char *cli = NULL;
>>   
>> -    cli = make_cli(data, "-machine smp.cpus=2 "
>> +    cli = make_cli(data, "-machine "
>> +        "smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2 "
>>           "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
>>           "-numa cpu,node-id=1,thread-id=0 "
>                                  ^^^^
> make it sensible as well, i.e. socket/cluster/cores-ids ...
> 

Could you help if the following command lines are what you want? I don't
think we can do it. Without PATCH[v8 3/5] applied, {socket,cluster,core}-id
are invalid from arm/virt machine side and we will run into errors.

      -machine                                                           \
      smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2  \
      -numa node,nodeid=0,memdev=ram -numa node,nodeid=1                 \
      -numa cpu,node-id=1,socket-id=0,cluster-id=0,core-id=0,thread-id=0 \
      -numa cpu,node-id=0,socket-id=0,cluster-id=0,core-id=0,thread-id=1
     
      # make -j 10 check
        :
      >>> QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=237                        \
         QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon \
         QTEST_QEMU_BINARY=./qemu-system-aarch64                               \
         G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \
         /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
     ―――――――――――――――――――――――――――――――――――――――――――――― ✀  ――――――――――――――――――――――――――――――――――――――――――――――
     stderr:
     qemu-system-aarch64: -numa cpu,node-id=1,socket-id=0,cluster-id=0,core-id=0,thread-id=0: core-id is not supported
     Broken pipe

     (The error is reported from hw/core/machine.c::machine_set_cpu_numa_node())

By the way, could you also help to check if the following patches look
good to you? I hope to make next revision eligible for merge :)

     [PATCH v8 1/5] qapi/machine.json: Add cluster-id
     [PATCH v8 3/5] hw/arm/virt: Consider SMP configuration in CPU topology

>>           "-numa cpu,node-id=0,thread-id=1");
> 

Thanks,
Gavin

Re: [PATCH v8 2/5] qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()

[Igor Mammedov]

On Mon, 2 May 2022 18:07:00 +0800
Gavin Shan <gshan@redhat.com> wrote:

> Hi Igor,
> 
> On 5/2/22 4:52 PM, Igor Mammedov wrote:
> > On Mon, 25 Apr 2022 11:27:59 +0800
> > Gavin Shan <gshan@redhat.com> wrote:
> >   
> >> The CPU topology isn't enabled on arm/virt machine yet, but we're
> >> going to do it in next patch. After the CPU topology is enabled by
> >> next patch, "thrad-id=1" becomes invalid because the CPU core is  
> >                   ^^^ typo
> >   
> 
> hmm, my bad. Lets fix it in next revision.
> 
> >> preferred on arm/virt machine. It means these two CPUs have 0/1
> >> as their core IDs, but their thread IDs are all 0. It will trigger
> >> test failure as the following message indicates:
> >>
> >>    [14/21 qemu:qtest+qtest-aarch64 / qtest-aarch64/numa-test  ERROR
> >>    1.48s   killed by signal 6 SIGABRT  
> >>    >>> G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \  
> >>        QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon         \
> >>        QTEST_QEMU_BINARY=./qemu-system-aarch64                                       \
> >>        QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=83                                  \
> >>        /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
> >>    ――――――――――――――――――――――――――――――――――――――――――――――
> >>    stderr:
> >>    qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
> >>
> >> This fixes the issue by providing comprehensive SMP configurations
> >> in aarch64_numa_cpu(). The SMP configurations aren't used before
> >> the CPU topology is enabled in next patch.
> >>
> >> Signed-off-by: Gavin Shan <gshan@redhat.com>
> >> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
> >> ---
> >>   tests/qtest/numa-test.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
> >> index 90bf68a5b3..aeda8c774c 100644
> >> --- a/tests/qtest/numa-test.c
> >> +++ b/tests/qtest/numa-test.c
> >> @@ -223,7 +223,8 @@ static void aarch64_numa_cpu(const void *data)
> >>       QTestState *qts;
> >>       g_autofree char *cli = NULL;
> >>   
> >> -    cli = make_cli(data, "-machine smp.cpus=2 "
> >> +    cli = make_cli(data, "-machine "
> >> +        "smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2 "
> >>           "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
> >>           "-numa cpu,node-id=1,thread-id=0 "  
> >                                  ^^^^
> > make it sensible as well, i.e. socket/cluster/cores-ids ...
> >   
> 
> Could you help if the following command lines are what you want? I don't
> think we can do it. Without PATCH[v8 3/5] applied, {socket,cluster,core}-id
> are invalid from arm/virt machine side and we will run into errors.
you are right, you can only fix -numa after 3/5

btw: 
splitting threads between several numa nodes here probably is unreal
configuration. Should be fixed in follow up patches.

>       -machine                                                           \
>       smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2  \
>       -numa node,nodeid=0,memdev=ram -numa node,nodeid=1                 \
>       -numa cpu,node-id=1,socket-id=0,cluster-id=0,core-id=0,thread-id=0 \
>       -numa cpu,node-id=0,socket-id=0,cluster-id=0,core-id=0,thread-id=1
>      
>       # make -j 10 check
>         :
>       >>> QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=237                        \  
>          QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon \
>          QTEST_QEMU_BINARY=./qemu-system-aarch64                               \
>          G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \
>          /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
>      ―――――――――――――――――――――――――――――――――――――――――――――― ✀  ――――――――――――――――――――――――――――――――――――――――――――――
>      stderr:
>      qemu-system-aarch64: -numa cpu,node-id=1,socket-id=0,cluster-id=0,core-id=0,thread-id=0: core-id is not supported
>      Broken pipe
> 
>      (The error is reported from hw/core/machine.c::machine_set_cpu_numa_node())
> 
> By the way, could you also help to check if the following patches look
> good to you? I hope to make next revision eligible for merge :)
> 
>      [PATCH v8 1/5] qapi/machine.json: Add cluster-id
>      [PATCH v8 3/5] hw/arm/virt: Consider SMP configuration in CPU topology
> 
> >>           "-numa cpu,node-id=0,thread-id=1");  
> >   
> 
> Thanks,
> Gavin
> 

Re: [PATCH v8 1/5] qapi/machine.json: Add cluster-id

[Igor Mammedov]

On Mon, 25 Apr 2022 11:27:58 +0800
Gavin Shan <gshan@redhat.com> wrote:

> This adds cluster-id in CPU instance properties, which will be used
> by arm/virt machine. Besides, the cluster-id is also verified or
> dumped in various spots:
> 
>   * hw/core/machine.c::machine_set_cpu_numa_node() to associate
>     CPU with its NUMA node.
> 
>   * hw/core/machine.c::machine_numa_finish_cpu_init() to record
>     CPU slots with no NUMA mapping set.
> 
>   * hw/core/machine-hmp-cmds.c::hmp_hotpluggable_cpus() to dump
>     cluster-id.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/core/machine-hmp-cmds.c |  4 ++++
>  hw/core/machine.c          | 16 ++++++++++++++++
>  qapi/machine.json          |  6 ++++--
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
> index 4e2f319aeb..5cb5eecbfc 100644
> --- a/hw/core/machine-hmp-cmds.c
> +++ b/hw/core/machine-hmp-cmds.c
> @@ -77,6 +77,10 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
>          if (c->has_die_id) {
>              monitor_printf(mon, "    die-id: \"%" PRIu64 "\"\n", c->die_id);
>          }
> +        if (c->has_cluster_id) {
> +            monitor_printf(mon, "    cluster-id: \"%" PRIu64 "\"\n",
> +                           c->cluster_id);
> +        }
>          if (c->has_core_id) {
>              monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n", c->core_id);
>          }
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cb9bbc844d..700c1e76b8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -682,6 +682,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
>              return;
>          }
>  
> +        if (props->has_cluster_id && !slot->props.has_cluster_id) {
> +            error_setg(errp, "cluster-id is not supported");
> +            return;
> +        }
> +
>          if (props->has_socket_id && !slot->props.has_socket_id) {
>              error_setg(errp, "socket-id is not supported");
>              return;
> @@ -701,6 +706,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
>                  continue;
>          }
>  
> +        if (props->has_cluster_id &&
> +            props->cluster_id != slot->props.cluster_id) {
> +                continue;
> +        }
> +
>          if (props->has_die_id && props->die_id != slot->props.die_id) {
>                  continue;
>          }
> @@ -995,6 +1005,12 @@ static char *cpu_slot_to_string(const CPUArchId *cpu)
>          }
>          g_string_append_printf(s, "die-id: %"PRId64, cpu->props.die_id);
>      }
> +    if (cpu->props.has_cluster_id) {
> +        if (s->len) {
> +            g_string_append_printf(s, ", ");
> +        }
> +        g_string_append_printf(s, "cluster-id: %"PRId64, cpu->props.cluster_id);
> +    }
>      if (cpu->props.has_core_id) {
>          if (s->len) {
>              g_string_append_printf(s, ", ");
> diff --git a/qapi/machine.json b/qapi/machine.json
> index d25a481ce4..4c417e32a5 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -868,10 +868,11 @@
>  # @node-id: NUMA node ID the CPU belongs to
>  # @socket-id: socket number within node/board the CPU belongs to
>  # @die-id: die number within socket the CPU belongs to (since 4.1)
> -# @core-id: core number within die the CPU belongs to
> +# @cluster-id: cluster number within die the CPU belongs to (since 7.1)
> +# @core-id: core number within cluster the CPU belongs to
>  # @thread-id: thread number within core the CPU belongs to
>  #
> -# Note: currently there are 5 properties that could be present
> +# Note: currently there are 6 properties that could be present
>  #       but management should be prepared to pass through other
>  #       properties with device_add command to allow for future
>  #       interface extension. This also requires the filed names to be kept in
> @@ -883,6 +884,7 @@
>    'data': { '*node-id': 'int',
>              '*socket-id': 'int',
>              '*die-id': 'int',
> +            '*cluster-id': 'int',
>              '*core-id': 'int',
>              '*thread-id': 'int'
>    }

Re: [PATCH v8 3/5] hw/arm/virt: Consider SMP configuration in CPU topology

[Igor Mammedov]

On Mon, 25 Apr 2022 11:28:00 +0800
Gavin Shan <gshan@redhat.com> wrote:

> Currently, the SMP configuration isn't considered when the CPU
> topology is populated. In this case, it's impossible to provide
> the default CPU-to-NUMA mapping or association based on the socket
> ID of the given CPU.
> 
> This takes account of SMP configuration when the CPU topology
> is populated. The die ID for the given CPU isn't assigned since
> it's not supported on arm/virt machine. Besides, the used SMP
> configuration in qtest/numa-test/aarch64_numa_cpu() is corrcted
> to avoid testing failure
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>


Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/virt.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 5bdd98e4a1..0fd7f9a6a1 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2560,6 +2560,7 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>      int n;
>      unsigned int max_cpus = ms->smp.max_cpus;
>      VirtMachineState *vms = VIRT_MACHINE(ms);
> +    MachineClass *mc = MACHINE_GET_CLASS(vms);
>  
>      if (ms->possible_cpus) {
>          assert(ms->possible_cpus->len == max_cpus);
> @@ -2573,8 +2574,20 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>          ms->possible_cpus->cpus[n].type = ms->cpu_type;
>          ms->possible_cpus->cpus[n].arch_id =
>              virt_cpu_mp_affinity(vms, n);
> +
> +        assert(!mc->smp_props.dies_supported);
> +        ms->possible_cpus->cpus[n].props.has_socket_id = true;
> +        ms->possible_cpus->cpus[n].props.socket_id =
> +            n / (ms->smp.clusters * ms->smp.cores * ms->smp.threads);
> +        ms->possible_cpus->cpus[n].props.has_cluster_id = true;
> +        ms->possible_cpus->cpus[n].props.cluster_id =
> +            (n / (ms->smp.cores * ms->smp.threads)) % ms->smp.clusters;
> +        ms->possible_cpus->cpus[n].props.has_core_id = true;
> +        ms->possible_cpus->cpus[n].props.core_id =
> +            (n / ms->smp.threads) % ms->smp.cores;
>          ms->possible_cpus->cpus[n].props.has_thread_id = true;
> -        ms->possible_cpus->cpus[n].props.thread_id = n;
> +        ms->possible_cpus->cpus[n].props.thread_id =
> +            n % ms->smp.threads;
>      }
>      return ms->possible_cpus;
>  }

Re: [PATCH v8 2/5] qtest/numa-test: Specify CPU topology in aarch64_numa_cpu()

[Gavin Shan]

Hi Igor,

On 5/3/22 4:54 PM, Igor Mammedov wrote:
> On Mon, 2 May 2022 18:07:00 +0800
> Gavin Shan <gshan@redhat.com> wrote:
>> On 5/2/22 4:52 PM, Igor Mammedov wrote:
>>> On Mon, 25 Apr 2022 11:27:59 +0800
>>> Gavin Shan <gshan@redhat.com> wrote:
>>>    
>>>> The CPU topology isn't enabled on arm/virt machine yet, but we're
>>>> going to do it in next patch. After the CPU topology is enabled by
>>>> next patch, "thrad-id=1" becomes invalid because the CPU core is
>>>                    ^^^ typo
>>>    
>>
>> hmm, my bad. Lets fix it in next revision.
>>
>>>> preferred on arm/virt machine. It means these two CPUs have 0/1
>>>> as their core IDs, but their thread IDs are all 0. It will trigger
>>>> test failure as the following message indicates:
>>>>
>>>>     [14/21 qemu:qtest+qtest-aarch64 / qtest-aarch64/numa-test  ERROR
>>>>     1.48s   killed by signal 6 SIGABRT
>>>>     >>> G_TEST_DBUS_DAEMON=/home/gavin/sandbox/qemu.main/tests/dbus-vmstate-daemon.sh \
>>>>         QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon         \
>>>>         QTEST_QEMU_BINARY=./qemu-system-aarch64                                       \
>>>>         QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=83                                  \
>>>>         /home/gavin/sandbox/qemu.main/build/tests/qtest/numa-test --tap -k
>>>>     ――――――――――――――――――――――――――――――――――――――――――――――
>>>>     stderr:
>>>>     qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
>>>>
>>>> This fixes the issue by providing comprehensive SMP configurations
>>>> in aarch64_numa_cpu(). The SMP configurations aren't used before
>>>> the CPU topology is enabled in next patch.
>>>>
>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
>>>> ---
>>>>    tests/qtest/numa-test.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
>>>> index 90bf68a5b3..aeda8c774c 100644
>>>> --- a/tests/qtest/numa-test.c
>>>> +++ b/tests/qtest/numa-test.c
>>>> @@ -223,7 +223,8 @@ static void aarch64_numa_cpu(const void *data)
>>>>        QTestState *qts;
>>>>        g_autofree char *cli = NULL;
>>>>    
>>>> -    cli = make_cli(data, "-machine smp.cpus=2 "
>>>> +    cli = make_cli(data, "-machine "
>>>> +        "smp.cpus=2,smp.sockets=1,smp.clusters=1,smp.cores=1,smp.threads=2 "
>>>>            "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
>>>>            "-numa cpu,node-id=1,thread-id=0 "
>>>                                   ^^^^
>>> make it sensible as well, i.e. socket/cluster/cores-ids ...
>>>    
>>
>> Could you help if the following command lines are what you want? I don't
>> think we can do it. Without PATCH[v8 3/5] applied, {socket,cluster,core}-id
>> are invalid from arm/virt machine side and we will run into errors.
> you are right, you can only fix -numa after 3/5
> 
> btw:
> splitting threads between several numa nodes here probably is unreal
> configuration. Should be fixed in follow up patches.
> 

Lets fix it in PATCH[v9 4/6] with the following command lines. Besides,
socket/cluster/core/thread IDs should be checked when the NUMA IDs are
verified. It helps to check if the CPU topology is properly populated
at least.

v9 should be sent shortly after doing some tests. Please take a look.

     -machine                                                           \
     smp.cpus=2,smp.sockets=2,smp.clusters=1,smp.cores=1,smp.threads=1  \
     -numa node,nodeid=0,memdev=ram -numa node,nodeid=1                 \
     -numa cpu,node-id=0,socket-id=1,cluster-id=0,core-id=0,thread-id=0 \
     -numa cpu,node-id=1,socket-id=0,cluster-id=0,core-id=0,thread-id=0

[...]

Thanks,
Gavin