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

[Dietmar Eggemann <dietmar.eggemann@arm.com>]

On 12/04/2021 14:20, Ruifeng Zhang wrote:
> Valentin Schneider <valentin.schneider@arm.com> 于2021年4月12日周一 下午7:32写道：
>>
>>
>> Hi,
>>
>> On 12/04/21 15:08, Ruifeng Zhang wrote:
>>> From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com>
>>>
>>> The arm topology still parse from the MPIDR, but it is incomplete.  When
>>> the armv8.3 cpu runs in aarch32 mode, it will parse out the wrong topology.
>>>
>>> armv7 (A7) mpidr is:
>>> [11:8]      [7:2]       [1:0]
>>> cluster     reserved    cpu
>>>
>>> armv8.3 (A55) mpidr is:
>>> [23:16]     [15:8]      [7:0]
>>> cluster     cpu         thread
>>>
>>> For compatibility to keep the function of get capacity from default
>>> cputype, renamed arm parse_dt_topology to get_cputype_capacity and delete
>>> related logic of parse from dt.
>>> Arm using the same parse_dt_topology function as arm64.
>>>
>>> The arm device boot step is to look for the default cputype and get cpu
>>> capacity firstly. Then parse the topology and capacity from dt to replace
>>> default values.
>>>
>>
>> I'm afraid I don't get it.
>>
>> CONFIG_COMPAT lets you run 32-bit stuff at EL0, but the kernel is still
>> arm64. So if you take your armv8.3 system, the topology parsed by the
>> kernel will be the same regardless of CONFIG_COMPAT.
>>
>> Could you elaborate on what problem you are trying to fix here?
> 
> 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), 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.
> 
> In addition, I think arm should also allow customers to configure cpu
> topologies via DT.

This patch ruins the CPU capacity detection based on capacity-dmips-mhz
(Documentation/devicetree/bindings/arm/cpu-capacity.txt) on my TC2 [L B
B L L] (armv7).

tip/sched/core with *mainline* multi_v7_defconfig:

root@linaro-nano:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity
516
1024
1024
516
516

your patch with mainline multi_v7_defconfig:

root@linaro-nano:~#  cat /sys/devices/system/cpu/cpu*/cpu_capacity
1024
1024
1024
1024
1024


There are 2 capacity detection mechanism in arch/arm/kernel/topology.c:

(1) cpu_efficiency (only for armv7 a15 and a7) based, relies on
clock-frequency dt property

(2) capacity-dmips-mhz dt property based

I currently don't see how this different MPIDR layout leads to you code
changes.