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

[Sudeep Holla <sudeep.holla@arm.com>]

On Thu, Apr 09, 2020 at 07:07:13PM +0800, Jiaxun Yang wrote:
> 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.
>

Sorry, I still don't understand how these relate to cpu_topology in
arch_topology.c

--
Regards,
Sudeep