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

[Jan Kiszka]

On 2015-03-02 11:19, Marc Zyngier wrote:
> 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.

We'll have to in order to share code, so this service has to help out.

> 
>> 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.

I'll make this another weak function. Unfolding won't work well because
it should be used in common psci.S.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux