Re: Questions about "QEMU gives wrong MPIDR value for Arm CPU types with MT=1" (gitlab issue #1608)

[Peter Maydell]

On Mon, 15 Apr 2024 at 19:18, Dorjoy Chowdhury <dorjoychy111@gmail.com> wrote:
>
> On Mon, Apr 15, 2024 at 5:35 PM Peter Maydell <peter.maydell@linaro.org> 
> wrote:
> > This bit of the codebase has got a bit more complicated since
> > I wrote up the bug report. I will look into this and get back
> > to you, but my suspicion is that these calls must return the
> > same value that the actual CPU MPIDR affinity values have,
> > because these values are going to end up in the DTB and ACPI
> > tables, and the OS will want them to match up with MPIDRs.

> Sounds great. Let me know. It sounds like then it could make sense to
> change the "arm_build_mp_affinity" function signature in file
> "target/arm/cpu.c" file to be like this:
>
> uint64_t arm_build_mp_affinity(bool has_smt, int idx, uint8_t clusterz)
>
> I think in all the files where it is invoked it should be possible to
> know the SMT status of the cpu using ARMCPU(qemu_get_cpu(cpu)) or
> similar. Let me know what you think.

I think in some places it is going to be annoyingly difficult,
because the virt_possible_cpu_arch_ids() function (and the equivalent
one in sbsa-ref) are called before the machine init function and
before any CPUs have been created. We know the name of the type
of the CPU we're going to use, but we can't call qemu_get_cpu() there.

On the other hand, the use in npcm7xx.c is easy: there we are
setting the mp-affinity property on the CPU, but we set it to
the exact same value it would have by default. So we can
simply delete the property-setting code line entirely. (We should
do that as a separate patch in the patchseries.)

> > The other thing we need to do is check the TRM (technical reference
> > manual) for the CPUs that were added since I filed that bug in
> > April 2023, to see if they need to have the flag set or not. The
> > ones we need to check are:
> >  * cortex-a710
> >  * neoverse-n2
> >  * neoverse-v1
> >
>
> Good point. I have now looked at the TRMs of the a710, neoverse-n2,
> neoverse-v2 and they are similar like the ones mentioned in the gitlab
> bug ticket i.e., MT bit should be 1, Aff0 should be 0, Aff1 should be
> core index, Aff2 should be cluster id.

Cool, thanks for checking that.

> Let me know what you think. If everything sounds alright, I will try
> and post a patch.

I'm still not sure the right way to handle the callsites in the
virt_possible_cpu_arch_ids() callbacks, but you can post a
patch anyway and we can iterate from there.

thanks
-- PMM