Hi Yanan,

On 4/14/22 10:27 AM, wangyanan (Y) wrote:
On 2022/4/14 8:08, Gavin Shan wrote:
On 4/13/22 8:39 PM, wangyanan (Y) wrote:
On 2022/4/3 22:59, Gavin Shan 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 yet.

Signed-off-by: Gavin Shan <gs...@redhat.com>
---
  hw/arm/virt.c | 16 +++++++++++++++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d2e5ecd234..3174526730 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2505,6 +2505,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);
@@ -2518,8 +2519,21 @@ 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->smp.sockets;
No need for "% ms->smp.sockets".

Yeah, lets remove it in v6.

+ 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;
  }
Otherwise, looks good to me:
Reviewed-by: Yanan Wang <wangyana...@huawei.com>


Thanks for your time to review :)


Oh, after further testing this patch breaks numa-test for aarch64,
which should be checked and fixed. I guess it's because we have
more IDs supported for ARM. We have to fully running the QEMU
tests before sending some patches to ensure that they are not
breaking anything. :)


Thanks for catching the failure and reporting back. I'm not
too much familar with QEMU's test workframe. Could you please
share the detailed commands to reproduce the failure? I will
fix in v6, which will be done in a separate patch :)

Thanks,
Gavin



Reply via email to