| Message ID | 20210414122326.5255-2-ruifeng.zhang0110@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm: topology: parse the topology from the dt | expand |
Dear Dietmar, In the last, update_cpu_capacity(cpuid) must be called in store_cpu_topology because the cpuid_topo->package_id is always be the initialization value. Because of added the DT parsing logic, the cpuid_topo->package_id will be parse in driver/base/arch_topology and the update_cpu_capacity can't be called if DT has cpu-map. Update cpu capacity isn't related with cpu topology, so move it to the beginning of this function. Please help to review and test the new patch, thanks. Ruifeng Zhang <ruifeng.zhang0110@gmail.com> 于2021年4月14日周三 下午8:24写道： > > From: Ruifeng Zhang <ruifeng.zhang1@unisoc.com> > > The arm topology still parse from the MPIDR, but it is incomplete. When > the armv8.2 or above cpu runs kernel in EL1 with aarch32 mode, it will > parse out the wrong topology. > > Changed: > 1) Arm using the same parse_dt_topology function as arm64. > 2) For compatibility to keep the function of get capacity from default > cputype, renamed arm parse_dt_topology to get_efficiency_capacity and > delete related logic of parse from dt. > 3) The update_cpu_capacity function should not depend on the topology, so > it is moved to the beginning of store_cpu_topology. > > The arm device boot step is to look for the efficiency_capacity firstly. > Then parse the topology and capacity from dt to replace efficiency value. > > Signed-off-by: Ruifeng Zhang <ruifeng.zhang1@unisoc.com> > Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com> > --- > arch/arm/kernel/topology.c | 22 ++++++---------------- > drivers/base/arch_topology.c | 4 ++-- > include/linux/arch_topology.h | 1 + > 3 files changed, 9 insertions(+), 18 deletions(-) > > diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c > index ef0058de432b..93d875320cc4 100644 > --- a/arch/arm/kernel/topology.c > +++ b/arch/arm/kernel/topology.c > @@ -72,7 +72,6 @@ static unsigned long *__cpu_capacity; > #define cpu_capacity(cpu) __cpu_capacity[cpu] > > static unsigned long middle_capacity = 1; > -static bool cap_from_dt = true; > > /* > * Iterate all CPUs' descriptor in DT and compute the efficiency > @@ -82,7 +81,7 @@ static bool cap_from_dt = true; > * 'average' CPU is of middle capacity. Also see the comments near > * table_efficiency[] and update_cpu_capacity(). > */ > -static void __init parse_dt_topology(void) > +static void __init get_coretype_capacity(void) > { > const struct cpu_efficiency *cpu_eff; > struct device_node *cn = NULL; > @@ -105,13 +104,6 @@ static void __init parse_dt_topology(void) > continue; > } > > - if (topology_parse_cpu_capacity(cn, cpu)) { > - of_node_put(cn); > - continue; > - } > - > - cap_from_dt = false; > - > for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++) > if (of_device_is_compatible(cn, cpu_eff->compatible)) > break; > @@ -151,9 +143,6 @@ static void __init parse_dt_topology(void) > else > middle_capacity = ((max_capacity / 3) > >> (SCHED_CAPACITY_SHIFT-1)) + 1; > - > - if (cap_from_dt) > - topology_normalize_cpu_scale(); > } > > /* > @@ -163,7 +152,7 @@ static void __init parse_dt_topology(void) > */ > static void update_cpu_capacity(unsigned int cpu) > { > - if (!cpu_capacity(cpu) || cap_from_dt) > + if (!cpu_capacity(cpu)) > return; > > topology_set_cpu_scale(cpu, cpu_capacity(cpu) / middle_capacity); > @@ -173,7 +162,7 @@ static void update_cpu_capacity(unsigned int cpu) > } > > #else > -static inline void parse_dt_topology(void) {} > +static inline void get_cputype_capacity(void) {} > static inline void update_cpu_capacity(unsigned int cpuid) {} > #endif > > @@ -187,6 +176,8 @@ void store_cpu_topology(unsigned int cpuid) > struct cpu_topology *cpuid_topo = &cpu_topology[cpuid]; > unsigned int mpidr; > > + update_cpu_capacity(cpuid); > + > if (cpuid_topo->package_id != -1) > goto topology_populated; > > @@ -221,8 +212,6 @@ void store_cpu_topology(unsigned int cpuid) > cpuid_topo->package_id = -1; > } > > - update_cpu_capacity(cpuid); > - > pr_info("CPU%u: thread %d, cpu %d, socket %d, mpidr %x\n", > cpuid, cpu_topology[cpuid].thread_id, > cpu_topology[cpuid].core_id, > @@ -241,5 +230,6 @@ void __init init_cpu_topology(void) > reset_cpu_topology(); > smp_wmb(); > > + get_coretype_capacity(); > parse_dt_topology(); > } > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index de8587cc119e..a45aec356ec4 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -295,7 +295,7 @@ static void parsing_done_workfn(struct work_struct *work) > core_initcall(free_raw_capacity); > #endif > > -#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_RISCV) > /* > * This function returns the logic cpu number of the node. > * There are basically three kinds of return values: > @@ -441,7 +441,7 @@ static int __init parse_cluster(struct device_node *cluster, int depth) > return 0; > } > > -static int __init parse_dt_topology(void) > +int __init parse_dt_topology(void) > { > struct device_node *cn, *map; > int ret = 0; > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > index 0f6cd6b73a61..cfa5a5072aa0 100644 > --- a/include/linux/arch_topology.h > +++ b/include/linux/arch_topology.h > @@ -66,6 +66,7 @@ extern struct cpu_topology cpu_topology[NR_CPUS]; > #define topology_llc_cpumask(cpu) (&cpu_topology[cpu].llc_sibling) > void init_cpu_topology(void); > void store_cpu_topology(unsigned int cpuid); > +int __init parse_dt_topology(void); > const struct cpumask *cpu_coregroup_mask(int cpu); > void update_siblings_masks(unsigned int cpu); > void remove_cpu_topology(unsigned int cpuid); > -- > 2.17.1 >
On 14/04/21 20:23, 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.2 or above cpu runs kernel in EL1 with aarch32 mode, it will > parse out the wrong topology. > Per my other email, isn't the problem that MPIDR can't be trusted to properly represent the topology, and thus a new method of describing the topology (cpu-map) has to be used?
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c index ef0058de432b..93d875320cc4 100644 --- a/arch/arm/kernel/topology.c +++ b/arch/arm/kernel/topology.c @@ -72,7 +72,6 @@ static unsigned long *__cpu_capacity; #define cpu_capacity(cpu) __cpu_capacity[cpu] static unsigned long middle_capacity = 1; -static bool cap_from_dt = true; /* * Iterate all CPUs' descriptor in DT and compute the efficiency @@ -82,7 +81,7 @@ static bool cap_from_dt = true; * 'average' CPU is of middle capacity. Also see the comments near * table_efficiency[] and update_cpu_capacity(). */ -static void __init parse_dt_topology(void) +static void __init get_coretype_capacity(void) { const struct cpu_efficiency *cpu_eff; struct device_node *cn = NULL; @@ -105,13 +104,6 @@ static void __init parse_dt_topology(void) continue; } - if (topology_parse_cpu_capacity(cn, cpu)) { - of_node_put(cn); - continue; - } - - cap_from_dt = false; - for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++) if (of_device_is_compatible(cn, cpu_eff->compatible)) break; @@ -151,9 +143,6 @@ static void __init parse_dt_topology(void) else middle_capacity = ((max_capacity / 3) >> (SCHED_CAPACITY_SHIFT-1)) + 1; - - if (cap_from_dt) - topology_normalize_cpu_scale(); } /* @@ -163,7 +152,7 @@ static void __init parse_dt_topology(void) */ static void update_cpu_capacity(unsigned int cpu) { - if (!cpu_capacity(cpu) || cap_from_dt) + if (!cpu_capacity(cpu)) return; topology_set_cpu_scale(cpu, cpu_capacity(cpu) / middle_capacity); @@ -173,7 +162,7 @@ static void update_cpu_capacity(unsigned int cpu) } #else -static inline void parse_dt_topology(void) {} +static inline void get_cputype_capacity(void) {} static inline void update_cpu_capacity(unsigned int cpuid) {} #endif @@ -187,6 +176,8 @@ void store_cpu_topology(unsigned int cpuid) struct cpu_topology *cpuid_topo = &cpu_topology[cpuid]; unsigned int mpidr; + update_cpu_capacity(cpuid); + if (cpuid_topo->package_id != -1) goto topology_populated; @@ -221,8 +212,6 @@ void store_cpu_topology(unsigned int cpuid) cpuid_topo->package_id = -1; } - update_cpu_capacity(cpuid); - pr_info("CPU%u: thread %d, cpu %d, socket %d, mpidr %x\n", cpuid, cpu_topology[cpuid].thread_id, cpu_topology[cpuid].core_id, @@ -241,5 +230,6 @@ void __init init_cpu_topology(void) reset_cpu_topology(); smp_wmb(); + get_coretype_capacity(); parse_dt_topology(); } diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index de8587cc119e..a45aec356ec4 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -295,7 +295,7 @@ static void parsing_done_workfn(struct work_struct *work) core_initcall(free_raw_capacity); #endif -#if defined(CONFIG_ARM64) || defined(CONFIG_RISCV) +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_RISCV) /* * This function returns the logic cpu number of the node. * There are basically three kinds of return values: @@ -441,7 +441,7 @@ static int __init parse_cluster(struct device_node *cluster, int depth) return 0; } -static int __init parse_dt_topology(void) +int __init parse_dt_topology(void) { struct device_node *cn, *map; int ret = 0; diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h index 0f6cd6b73a61..cfa5a5072aa0 100644 --- a/include/linux/arch_topology.h +++ b/include/linux/arch_topology.h @@ -66,6 +66,7 @@ extern struct cpu_topology cpu_topology[NR_CPUS]; #define topology_llc_cpumask(cpu) (&cpu_topology[cpu].llc_sibling) void init_cpu_topology(void); void store_cpu_topology(unsigned int cpuid); +int __init parse_dt_topology(void); const struct cpumask *cpu_coregroup_mask(int cpu); void update_siblings_masks(unsigned int cpu); void remove_cpu_topology(unsigned int cpuid);