Re: [PATCH 2/2] arm64: topology: add MPIDR-based detection

[Zi Shen Lim <zlim@broadcom.com>]

On Wed, Apr 23, 2014 at 11:36:48AM +0100, Mark Brown wrote:
> On Tue, Apr 22, 2014 at 06:40:14PM -0700, Zi Shen Lim wrote:
> 
> > +		/* Multiprocessor system */
> > +		if (mpidr & MPIDR_MT_BITMASK) {
> > +			/* Multi-threads per core */
> > +			cpuid_topo->thread_id  = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > +			cpuid_topo->core_id    = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> > +			cpuid_topo->cluster_id = MPIDR_AFFINITY_LEVEL(mpidr, 2);
> > +		} else {
> > +			/* Single-thread per core */
> > +			cpuid_topo->thread_id  = -1;
> > +			cpuid_topo->core_id    = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > +			cpuid_topo->cluster_id = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> > +		}
> > +	}
> 
> This means that we ignore affinity level 3 and on non-MT cores we ignore
> affinity level 2.  That means that if it runs on some system where we do
> have multiple levels of clustering (for example some future multi socket
> server) or if for some reason the hardware engineers have decided to use
> one of the higher affinity levels then we will incorrectly report cores
> from several clusters as being part of a single cluster.
> 

Good points.

"Cluster" in my case is actually "Socket".

I guess single-socket in b.L configuration could have multiple clusters,
in which case, a new 'socket_id' would map to aff3 in MT case.

	thread	= aff0
	core	= aff1
	cluster	= aff2
	socket	= aff3

For non-MT, it could very well be aff2?

	thread	= -1
	core	= aff0
	cluster	= aff1
	socket	= aff2

Or is it likely that some folks may opt to skip aff2, and simply use aff3?
Mark, is there precedence for such usage of affinity levels?

Maybe ARM folks can point us to documentation or conventions w.r.t.
usage of those affinity levels.  Otherwise, this has potential to become
a creative playground :)

> I had been intending to just combine all the bits from affinitly levels
> above the CPU number into a single number until we know what to do with
> them individually.  We shouldn't just ignore them.
> 

I agree we shouldn't ignore aff3 if someone is already using it.

I'm not sure how combining them into a single number helps with topology.
We already started out with a cpuid, no?

Perhaps we should just add a new 'socket_id' and that will accommodate
all cases (up to aff3).

> > +	pr_info("CPU%u: cluster %d core %d thread %d mpidr %llx\n",
> > +		cpuid, cpuid_topo->cluster_id, cpuid_topo->core_id,
> > +		cpuid_topo->thread_id, mpidr);
> > +
> 
> Catalin or Lorenzo asked for stuff like that to be taken out.

Is it ok to change it pr_debug instead? Or drop it completely?