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

[Ruifeng Zhang]

Dietmar Eggemann <dietmar.eggemann@xxxxxxx> 于2021年4月12日周一 下午8:40写道：
>
> On 12/04/2021 14:20, Ruifeng Zhang wrote:
> > Valentin Schneider <valentin.schneider@xxxxxxx> 于2021年4月12日周一 下午7:32写道：
> >>
> >>
> >> Hi,
> >>
> >> On 12/04/21 15:08, Ruifeng Zhang wrote:
> >>> From: Ruifeng Zhang <ruifeng.zhang1@xxxxxxxxxx>
> >>>
> >>> 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.

Thanks for your test, I will update patch-V2 to solve this problem.
>
>