Re: [PATCH v2 1/1] arm: topology: parse the topology from the dt
From: Ruifeng Zhang
Date: Wed Apr 14 2021 - 08:38:58 EST
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@xxxxxxxxx> 于2021年4月14日周三 下午8:24写道：
>
> From: Ruifeng Zhang <ruifeng.zhang1@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Nianfu Bai <nianfu.bai@xxxxxxxxxx>
> ---
> 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
>