Re: [PATCH 1/1] arm: topology: parse the topology from the dt

[Ruifeng Zhang <ruifeng.zhang0110@gmail.com>]

Valentin Schneider <valentin.schneider@arm.com> 于2021年4月12日周一 下午11:33写道：
>
> On 12/04/21 20:20, Ruifeng Zhang wrote:
> > There is a armv8.3 cpu which should work normally both on aarch64 and aarch32.
> > The MPIDR has been written to the chip register in armv8.3 format.
> > For example,
> > core0: 0000000080000000
> > core1: 0000000080000100
> > core2: 0000000080000200
> > ...
> >
> > Its cpu topology can be parsed normally on aarch64 mode (both
> > userspace and kernel work on arm64).
> >
> > The problem is when it working on aarch32 mode (both userspace and
> > kernel work on arm 32-bit),
>
> I didn't know using aarch32 elsewhere than EL0 was something actually being
> used. Do you deploy this somewhere, or do you use it for testing purposes?

In Unisoc, the sc9863a SoC which using cortex-a55, it has two software
version, one
of them is the kernel running on EL1 using aarch32.
                          user(EL0)            kernel(EL1)
sc9863a_go      aarch32               aarch32
sc9863a            aarch64               aarch64
>
> > the cpu topology
> > will parse error because of the format is different between armv7 and armv8.3.
> > The arm 32-bit driver, arch/arm/kernel/topology will parse the MPIDR
> > and store to the topology with armv7,
> > and the result is all cpu core_id is 0, the bit[1:0] of armv7 MPIDR format.
> >
>
> I'm not fluent at all in armv7 (or most aarch32 compat mode stuff), but
> I couldn't find anything about MPIDR format differences:
>
>   DDI 0487G.a G8.2.113
>   """
>   AArch32 System register MPIDR bits [31:0] are architecturally mapped to
>   AArch64 System register MPIDR_EL1[31:0].
>   """
>
> Peeking at some armv7 doc and arm/kernel/topology.c the layout really looks
> just the same, i.e. for both of them, with your example of:

The cortex-a7 spec DDI0464F 4.3.5
https://developer.arm.com/documentation/ddi0464/f/?lang=en

The current arch/arm/kernel/topology code parse the MPIDR with a armv7 format.
the parse code is:
void store_cpu_topology(unsigned int cpuid)
{
    ...
    cpuid_topo->thread_id = -1;
    cpuid_topo->core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
    cpuid_topo->package_id = MPIDR_AFFINITY_LEVEL(mpidr, 1);
    ...
}
>
>   core0: 0000000080000000
>   core1: 0000000080000100
>   core2: 0000000080000200
>   ...
>
> we'll get:
>
>   |       | aff2 | aff1 | aff0 |
>   |-------+------+------+------|
>   | Core0 |    0 |    0 |    0 |
>   | Core1 |    0 |    1 |    0 |
>   | Core2 |    0 |    2 |    0 |
>       ...
>
> Now, arm64 doesn't fallback to MPIDR for topology information anymore since
>
>   3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information")
>
> so without DT we would get:
>   |       | package_id | core_id |
>   |-------+------------+---------|
>   | Core0 |          0 |       0 |
>   | Core1 |          0 |       1 |
>   | Core2 |          0 |       2 |
>
> Whereas with an arm kernel we'll end up parsing MPIDR as:
>   |       | package_id | core_id |
>   |-------+------------+---------|
>   | Core0 |          0 |       0 |
>   | Core1 |          1 |       0 |
>   | Core2 |          2 |       0 |
>
> Did I get this right? Is this what you're observing?

Yes, this is a problem if an armv8.2 or above cpu is running a 32-bit
kernel on EL1.
>
> > In addition, I think arm should also allow customers to configure cpu
> > topologies via DT.