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

From: Jiaxun Yang <jiaxun.yang@flygoat.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: linux-mips@vger.kernel.org,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Huacai Chen <chenhc@lemote.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Robert Richter <rric@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Marc Zyngier <maz@kernel.org>,
	Paul Burton <paulburton@kernel.org>,
	Allison Randal <allison@lohutok.net>,
	Enrico Weigelt <info@metux.net>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	Vladimir Kondratiev <vladimir.kondratiev@intel.com>,
	Richard Fontana <rfontana@redhat.com>,
	Paul Cercueil <paul@crapouillou.net>,
	Zhou Yanjie <zhouyanjie@zoho.com>
Subject: Re: [PATCH 05/12] MIPS: Switch to arch_topology
Date: Thu, 9 Apr 2020 19:07:13 +0800	[thread overview]
Message-ID: <20200409190713.3a695115@flygoat-x1e> (raw)
In-Reply-To: <20200409105832.GF25948@bogus>

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