Thanks for your review. Patch-v2 that solve the capacity issue will be uploaded as soon as possible. : )
Valentin Schneider <valentin.schnei...@arm.com> 于2021年4月13日周二 下午7:40写道: > > On 13/04/21 14:13, Ruifeng Zhang wrote: > > Valentin Schneider <valentin.schnei...@arm.com> 于2021年4月12日周一 下午11:33写道: > >> 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 > > > > Ah, so that's where the core_id=bit[1:0] comes from. That does still > conform to the MPIDR format, and as you point out below that's being parsed > the same (aff2, aff1, aff0) == mpidr([23:16][15:8][7:0]) > > > 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. > > > With the above MPIDR(_EL1) values, you would have the same problem in > aarch64 mode on any kernel predating > > 3102bc0e6ac7 ("arm64: topology: Stop using MPIDR for topology information") > > since all Aff0 values are 0. Arguably those MPIDR(_EL1) values don't > make much sense (cores in the same cluster should have different Aff0 > values, unless SMT), but in arm64 that's usually "corrected" by DT. > > As you pointed out, arm doesn't currently leverage the cpu-map DT entry. I > don't see any obvious problem with adding support for it, so if you can fix > the capacity issue Dietmar reported, I think we could consider it.