[U-Boot] [PATCH v4 02/14] ARM: Factor out armv7_get_cpu_id macro

[Marc Zyngier]

On 02/03/15 09:40, Jan Kiszka wrote:
> On 2015-02-28 14:56, Marc Zyngier wrote:
>> On Fri, 27 Feb 2015 13:28:01 +0000
>> Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>
>>> Handy for obtaining the ID of the current CPU. We will have more use
>>> cases.
>>>
>>> CC: Marc Zyngier <marc.zyngier at arm.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
>>> ---
>>>  arch/arm/cpu/armv7/sunxi/psci.S | 4 ++--
>>>  arch/arm/include/asm/macro.h    | 7 +++++++
>>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/sunxi/psci.S
>>> b/arch/arm/cpu/armv7/sunxi/psci.S index 9e898f2..0523217 100644
>>> --- a/arch/arm/cpu/armv7/sunxi/psci.S
>>> +++ b/arch/arm/cpu/armv7/sunxi/psci.S
>>> @@ -19,6 +19,7 @@
>>>  
>>>  #include <config.h>
>>>  #include <asm/gic.h>
>>> +#include <asm/macro.h>
>>>  #include <asm/psci.h>
>>>  #include <asm/arch/cpu.h>
>>>  
>>> @@ -315,8 +316,7 @@ psci_arch_init:
>>>  	mcr	p15, 0, r5, c1, c1, 0	@ Write SCR
>>>  	isb
>>>  
>>> -	mrc	p15, 0, r4, c0, c0, 5	@ MPIDR
>>> -	and	r4, r4, #3		@ cpu number in cluster
>>> +	armv7_get_cpu_id r4
>>>  	mov	r5, #0x400		@ 1kB of stack per CPU
>>>  	mul	r4, r4, r5
>>>  
>>> diff --git a/arch/arm/include/asm/macro.h
>>> b/arch/arm/include/asm/macro.h index 1c8c425..0bc925a 100644
>>> --- a/arch/arm/include/asm/macro.h
>>> +++ b/arch/arm/include/asm/macro.h
>>> @@ -198,6 +198,13 @@ lr	.req	x30
>>>  .endm
>>>  #endif
>>>  
>>> +#else /* !CONFIG_ARM64 */
>>> +
>>> +.macro armv7_get_cpu_id rn
>>> +	mrc	p15, 0, \rn, c0, c0, 5	/* read MPIDR */
>>> +	and	\rn, \rn, #0xff		/* return CPU ID
>>> in cluster */
>>> +.endm
>>> +
>>
>> How does this work in a multi-cluster situation? Or when you have
>> sparse MPIDRs?
> 
> I have no idea. That masking was stolen from your code.

Well, it is perfectly correct in the context of the original code
(single cluster, dense ID space), but is utterly wrong as a general
implementation.

> The model we assume for PSCI is that there is no cluster and that we
> have enough per-cpu space for up to the maximum cpu ID obtained that way.

I don't think you can rely on this assumption.

> If you are concerned about signaling a false general applicability of
> that macro, I can fold it back into the callers or add some comments
> about restrictions. My plan was just to make the caller site a bit more
> readable.

I understand the concern, but making this a general macro is very
misleading. Most systems with more than 4 cores will have clusters and a
very sparse ID space, and these systems are quite common these days.

Computing a CPU number is not a simple task, specially in the absence of
a discovery protocol (the Linux kernel relies on DT for that), so I'm
afraid you have to either keep this on a per platform basis, or provide
ways to override this macro.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...