[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
From: |
Dorjoy Chowdhury |
Subject: |
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT |
Date: |
Sun, 21 Apr 2024 14:40:43 +0600 |
On Sun, Apr 21, 2024 at 11:40 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/19/24 11:31, Dorjoy Chowdhury wrote:
> > +
> > + /*
> > + * Instantiate a temporary CPU object to build mp_affinity
> > + * of the possible CPUs.
> > + */
> > + cpuobj = object_new(ms->cpu_type);
> > + armcpu = ARM_CPU(cpuobj);
> > +
> > for (n = 0; n < ms->possible_cpus->len; n++) {
> > ms->possible_cpus->cpus[n].type = ms->cpu_type;
> > ms->possible_cpus->cpus[n].arch_id =
> > - sbsa_ref_cpu_mp_affinity(sms, n);
> > + sbsa_ref_cpu_mp_affinity(armcpu, n);
> > ms->possible_cpus->cpus[n].props.has_thread_id = true;
> > ms->possible_cpus->cpus[n].props.thread_id = n;
> > }
> > +
> > + object_unref(cpuobj);
>
> Why is this instantiation necessary?
>
The instantiation is necessary (like the one in hw/arm/virt.c in
virt_possible_cpu_arch_ids) because the
"sbsa_ref_possible_cpu_arch_ids" function is called (via
possible_cpu_arch_ids) before the CPUs are created in the
"sbsa_ref_init" function. There was some related discussion here
(qemu-devel archive URL):
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg02074.html . I
picked this up from the same thing done in hw/arm/virt.c in the
"machvirt_init" function in the "if (!vms->memap) {" block.
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1314,8 +1314,18 @@ static void arm_cpu_dump_state(CPUState *cs, FILE
> > *f, int flags)
> > }
> > }
> >
> > -uint64_t arm_build_mp_affinity(int idx, uint8_t clustersz)
> > +uint64_t arm_build_mp_affinity(ARMCPU *cpu, int idx, uint8_t clustersz)
> > {
> > + if (cpu->has_smt) {
> > + /*
> > + * Right now, the ARM CPUs with SMT supported by QEMU only have
> > + * one thread per core. So Aff0 is always 0.
> > + */
>
> Well, this isn't true.
>
> -smp
> [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets]
>
> [,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]
>
> I would expect all of Aff[0-3] to be settable with the proper topology
> parameters.
>
Sorry, maybe I misunderstood. From the gitlab bug ticket (URL:
https://gitlab.com/qemu-project/qemu/-/issues/1608) and the discussion
in qemu-devel (URL:
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg01964.html) I
looked at the technical reference manuals of the MPIDR register of the
a-55, a-76, a-710, neoverse-v1, neoverse-n1, neoverse-n2 CPUs and all
of them had MT=1 and one thread per core so Aff0 is always 0.
I don't know what Aff3 should be set to. I see this comment in the
target/arm/cpu.c "arm_cpu_realizefn" function
/* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will
override it.│
* We don't support setting cluster ID ([16..23]) (known as Aff2
│
* in later ARM ARM versions), or any of the higher affinity level
fields, │
* so these bits always RAZ.
│
*/
Right now we don't seem to set Aff3[39:32] at all in the mp_affinity
property. Let me know what should be the expected behavior here as I
don't have enough knowledge here. I will try to fix it. Thanks!
Regards,
Dorjoy