Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT

[Dorjoy Chowdhury <dorjoychy111@gmail.com>]

On Fri, May 3, 2024 at 10:28 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 19 Apr 2024 at 19:31, Dorjoy Chowdhury <dorjoychy111@gmail.com> wrote:
> >
> > Some ARM CPUs advertise themselves as SMT by having the MT[24] bit set
> > to 1 in the MPIDR register. These CPUs have the thread id in Aff0[7:0]
> > bits, CPU id in Aff1[15:8] bits and cluster id in Aff2[23:16] bits in
> > MPIDR.
> >
> > On the other hand, ARM CPUs without SMT have the MT[24] bit set to 0,
> > CPU id in Aff0[7:0] bits and cluster id in Aff1[15:8] bits in MPIDR.
> >
> > The mpidr_read_val() function always reported non-SMT i.e., MT=0 style
> > MPIDR value which means it was wrong for the following CPUs with SMT
> > supported by QEMU:
> >     - cortex-a55
> >     - cortex-a76
> >     - cortex-a710
> >     - neoverse-v1
> >     - neoverse-n1
> >     - neoverse-n2
>
> This has definitely turned out to be rather more complicated
> than I thought it would be when I wrote up the original issue
> in gitlab, so sorry about that.
>
> I still need to think through how we should deal with the
> interaction between what the CPU type implies about the MPIDR
> format and the topology information provided by the user.
> I probably won't get to that next week, because I'm on holiday
> for most of it, but I will see if I can at least make a start.
>
> In the meantime, there is one tiny bit of this that we can
> do now:
>
> > diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> > index cc68b5d8f1..9d5dcf1a3f 100644
> > --- a/hw/arm/npcm7xx.c
> > +++ b/hw/arm/npcm7xx.c
> > @@ -487,7 +487,7 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
> >      /* CPUs */
> >      for (i = 0; i < nc->num_cpus; i++) {
> >          object_property_set_int(OBJECT(&s->cpu[i]), "mp-affinity",
> > -                                arm_build_mp_affinity(i, NPCM7XX_MAX_NUM_CPUS),
> > +                                arm_build_mp_affinity(ARM_CPU(&s->cpu[i]), i, NPCM7XX_MAX_NUM_CPUS),
> >                                  &error_abort);
> >          object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
> >                                  NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
>
> In this file, the value of the mp-affinity property that the
> board is setting is always the same as the default value it
> would have anyway. So we can delete the call to
> object_property_set_int() entirely, which gives us one fewer
> place we need to deal with when we do eventually figure out
> how the MPIDR values should work.
>

Before I send the patch removing the "object_property_set_int" line
for "mp-affinity", just so that I understand, where else is it that
for npcm7xx the mp_affinity is being set? I can't follow the code
easily and I am not seeing where else it is being set to the same
value. It's a bit hard to follow the initialization codes in QEMU.

Regards,
Dorjoy