Re: [PATCH 05/12] MIPS: Switch to arch_topology

[Jiaxun Yang <jiaxun.yang@flygoat.com>]

On Thu, 9 Apr 2020 11:58:32 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

> On Thu, Apr 09, 2020 at 06:35:21PM +0800, Jiaxun Yang wrote:
> >
> >
> > 于 2020年4月9日 GMT+08:00 下午6:31:21, Sudeep Holla
> > <sudeep.holla@arm.com> 写到:  
> > >On Wed, Apr 08, 2020 at 07:34:15PM +0800, Jiaxun Yang wrote:  
> > >> Previously, MIPS is using self-defined "globalnumber" in struct
> > >> mips_cpuinfo to store topology information. However, it's not  
> > >friendly  
> > >> to DeviceTree based systems and lack of cpu_capacity related
> > >> feature which can take advantage of multi-cluster system.
> > >>
> > >> Here, we enabled arch_topology for MIPS and adapted some
> > >> functions to fit arch_topology structure.
> > >> Also, we implmented smp_store_cpu_info to probe CPU's topology  
> > >information  
> > >> by "globalnumber" registers in VP ASE or Ebase.CPUNum for legacy
> > >>  
> > >systems.  
> > >>
> > >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > >> ---
> > >>  arch/mips/Kconfig                |  1 +
> > >>  arch/mips/include/asm/cpu-info.h | 49
> > >> ---------------------------- arch/mips/include/asm/smp.h      |
> > >> 2 -- arch/mips/include/asm/topology.h | 48
> > >> +++++++++++++++++++++++++--- arch/mips/kernel/cpu-probe.c     |
> > >> 43 ------------------------- arch/mips/kernel/setup.c         |
> > >> 1 + arch/mips/kernel/smp.c           | 55  
> > >++++----------------------------  
> > >>  arch/mips/kernel/topology.c      | 42 ++++++++++++++++++++++++
> > >>  8 files changed, 93 insertions(+), 148 deletions(-)
> > >>  
> > >
> > >[...]
> > >  
> > >> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > >> index 8a418783a6bb..b9fefc5dc702 100644
> > >> --- a/arch/mips/kernel/setup.c
> > >> +++ b/arch/mips/kernel/setup.c
> > >> @@ -784,6 +784,7 @@ void __init setup_arch(char **cmdline_p)
> > >>  	dmi_setup();
> > >>
> > >>  	resource_init();
> > >> +	init_cpu_topology();
> > >>  	plat_smp_setup();
> > >>  
> > >
> > >Continuing my reply on previous patch, I see possible_cpu_mask
> > >being set up in plat_smp_setup. Why not reverse the order above.
> > >Further I see
> > >that the logical->physical CPU mapping is done in plat_smp_setup
> > >which is required to store/save any topology information.  
> >
> > Some plat_smp_setup is touching topology so we must reset before
> > that. 
> 
> Can you elaborate on this ? What gets overwritten if
> init_cpu_topology is called after plat_smp_setup. I don't see any
> plat_smp_setup setting up sibling masks.

See arch/mips/kernel/smp-cps.c,
in cps_smp_setup function, it is setting "cpu_data[nvpes + v].core",
"cpu_data[nvpes + v].vpe_id", which is actually sibling thread_id and
core_id after my patch.

Thanks.

> 
> --
> Regards,
> Sudeep