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

[Peter Maydell]

On Sat, 13 Apr 2024 at 20:59, Dorjoy Chowdhury <dorjoychy111@gmail.com> wrote:
>
> Hi,
> Hope everyone is doing well. I was looking at "Bite Sized" tagged QEMU
> issues in gitlab to see if there is anything that makes sense for me
> as a first time contributor. I see this issue "QEMU gives wrong MPIDR
> value for Arm CPU types with MT=1" (gitlab URL:
> https://gitlab.com/qemu-project/qemu/-/issues/1608 ).
>
> From the bug ticket description, it is very clear that I will need to
> add a bool member variable in the "AarchCPU" struct which is in
> "target/arm/cpu.h" file. I believe the next logical change is to set
> this member variable to "true" in the corresponding arm cpu "initfn"
> functions (a55, a76, noeverse_n1) which are in "target/arm/cpu64.c"
> file. I have a few questions about the following steps as I am looking
> through the code.
>
> 1. I believe I will need to update the "arm_build_mp_affinity"
> function in "target/arm/cpu.c" file to now also take in a bool
> parameter that will indicate if the function should instead put the
> "core index" in the "aff1" bits instead of the existing logic of
> "aff0" bits and the cluster id in the "aff2" bits instead of the
> existing logic of "aff1" bits. But I see this function being invoked
> from 3 other files: "hw/arm/sbsa-ref.c", "hw/arm/virt.c",
> "hw/arm/npcm7xx.c". Should the function calls in these files always
> have that corresponding argument set to "false"?

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.

> 2. As per the bug ticket description, I will also need to update the
> "mpidr_read_val" function in the "target/arm/helper.c" file to only
> set the MT bit (24th) to 1 if the member variable is true. I think
> there is nothing else to be done in this function apart from checking
> and then setting the MT bit. Is my assumption correct?

Yes, that's right.

> I think doing the above steps should fix the bug and probably we don't
> need anything else. It would be great if someone can help me answer
> the questions or any suggestion would be great if my assumptions are
> wrong. Thanks.

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

thanks
-- PMM