[U-Boot] [PATCH v2 12/12] tegra: Set CNTFRQ for secondary CPUs

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2015-02-17 11:21, Mark Rutland wrote:
> On Tue, Feb 17, 2015 at 07:01:57AM +0000, Jan Kiszka wrote:
>> On 2015-02-16 15:02, Jan Kiszka wrote:
>>> On 2015-02-16 14:51, Mark Rutland wrote:
>>>> On Mon, Feb 16, 2015 at 01:44:36PM +0000, Jan Kiszka wrote:
>>>>> On 2015-02-16 14:37, Mark Rutland wrote:
>>>>>> On Mon, Feb 16, 2015 at 12:54:49PM +0000, Jan Kiszka wrote:
>>>>>>> We only set CNTFRQ in arch_timer_init for the boot CPU. But this has to
>>>>>>> happen for all cores.
>>>>>>>
>>>>>>> Fixing this resolves problems of KVM with emulating the generic
>>>>>>> timer/counter.
>>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>> ---
>>>>>>>  arch/arm/cpu/armv7/tegra-common/psci.S | 13 +++++++++++++
>>>>>>>  1 file changed, 13 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/cpu/armv7/tegra-common/psci.S b/arch/arm/cpu/armv7/tegra-common/psci.S
>>>>>>> index b7501fb..119c246 100644
>>>>>>> --- a/arch/arm/cpu/armv7/tegra-common/psci.S
>>>>>>> +++ b/arch/arm/cpu/armv7/tegra-common/psci.S
>>>>>>> @@ -51,12 +51,25 @@ ENTRY(psci_arch_init)
>>>>>>>  
>>>>>>>  	mrc	p15, 0, r4, c0, c0, 5	@ MPIDR
>>>>>>>  	and	r4, r4, #7		@ number of CPUs in cluster
>>>>>>> +
>>>>>>> +	adr	r5, _sys_clock_freq
>>>>>>> +	cmp	r4, #0
>>>>>>> +
>>>>>>> +	mrceq   p15, 0, r7, c14, c0, 0	@ read CNTFRQ from CPU0
>>>>>>> +	streq	r7, [r5]
>>>>>>> +
>>>>>>> +	ldrne	r7, [r5]
>>>>>>> +	mcrne   p15, 0, r7, c14, c0, 0	@ write CNTFRQ to CPU1..3
>>>>>>
>>>>>> Is it not possible to have a hook that uses the same variable as
>>>>>> arch_timer_init rather than doing a here copy? It seems a shame to
>>>>>> duplicate the effort.
>>>>>
>>>>> The problem is related to the different address spaces. Here we run in
>>>>> the secure monitor, arch_timer_init - to my understanding - in
>>>>> non-secure mode. Didn't find a pattern so far how to transfer data (and
>>>>> that shouldn't be more complex than the above code).
>>>>
>>>> Surely arch_timer_init must be run in a secure mode in order to be
>>>> allowed to write to CNTFRQ?
>>>
>>> Ah, right.
>>>
>>>>
>>>> If this is simply the easiest way of moving the data around then there's
>>>> no real problem with it; it's just a shame that it only happens in the
>>>> PSCI case.
>>>
>>> OK, I'll check again. Maybe it's easier than I thought.
>>
>> It isn't: the variable we would have to write - conditionally, i.e. not
>> for the SPL build - is not yet where it will finally be. The monitor is
>> copied later on, but the symbol points to the destination already. One
>> can account for that, but the result won't get simpler and cleaner IMHO.
> 
> Fair enough. As I mentioned earlier there's no real problem with the
> above, it just seemed a shame that this was only done in the PSCI case.
> 
> I take it for the quoted sequence above that the primary CPU runs
> through this path before sending the secondaries through (and that
> either there's a DSB somewhere between the write and waking up
> secondaries or memory accesses are strongly ordered at this point).

Yes to both: The primary performs this init before starting the target
OS, and the dsb is at latest in psci_cpu_on before kicking off a
secondary CPU.

Jan

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