|
From: | Gavin Shan |
Subject: | Re: [PATCH v7 2/4] hw/arm/virt: Consider SMP configuration in CPU topology |
Date: | Fri, 22 Apr 2022 19:24:10 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0 |
Hi Yanan, On 4/21/22 7:50 PM, wangyanan (Y) wrote:
Hi Gavin, Sorry I missed the v6.
No problem at all. thanks for your review again :)
On 2022/4/20 18:49, 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. 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 ++++++++++++++- tests/qtest/numa-test.c | 3 ++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index d2e5ecd234..5443ecae92 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,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));nit: so the outermost "()" is unnecessary too.
It was kept by intention so that it has same style as to other fields like cluster_id. I will remove it in v8 and it doesn't matter actually.
+ 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; } 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");
As discussed with Igor, the changes to test/qtest/numa-test.c will be split into a separate patch in v8, which goes before this one. I assume your reviewed-by tag is still valid, even for the separate patch. Thanks, Gavin
[Prev in Thread] | Current Thread | [Next in Thread] |