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

[Mark Rutland <mark.rutland@arm.com>]

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

Thanks,
Mark.