[PATCH v10 4/6] ARM: add vdso user-space code

[Catalin Marinas]

On Fri, Feb 13, 2015 at 02:56:41PM +0000, Venkappa Mala wrote:
> Catalin Marinas <catalin.marinas <at> arm.com> writes:
> > On Fri, Feb 13, 2015 at 12:01:40PM +0000, Venkappa Mala wrote:
> > > Thanks Catalin for your promote reply. Yes, DTS contained
> > > arm,cpu-registers-not-fw-configured so that it uses generic physical
> > > timer instead of virtual timer. Now, the vDSO gettimeofday
> > > optimization is also possible since the generic timer is presented and
> > > enabled. However, vDSO gettimeofday implementation has been using
> > > virtual timer (VCT  user access) but the current implementation will
> > > not be sufficient to handle when the firmware is not taken care about
> > > reset CNTVOFF to zero. Let’s assume,neither firmware taken care nor
> > > booted with HYP mode, then Can we use physical timer (PCT user access
> > > needs to be enabled) for vDSO in the safe manner?
> > 
> > The short answer: no.
> > 
> > The problem is that user space is very "innovative" in making use of
> > features in a different way than what the kernel people intended and
> > it will be claimed "user ABI" afterwards. Take this glibc revert for
> > example, luckily we noticed it early:
> > 
> > https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=33ef2f0c763b51e1df7896d7d39d585824558c75
> > 
> > CNTVCT is not intended to be used outside of the VDSO but, apparently,
> > the VDSO->kernel interface is seen as user ABI by some.
> > 
> > CNTPCT cannot always be used in user space (VDSO and not even the
> > kernel), especially when the kernel boots at EL1. Choosing CNTPCT vs
> > CNTVCT in VDSO dynamically (or at boot time) is technically doable but
> > there is a high risk that user space ends up using them directly when it
> > detects a gettimeofday VDSO and such code will break when CNTPCT is no
> > longer accessible.
> 
> Ok Catalin, I have understood about your concern.
> From ARMv7 TRM,
> An implementation of the Generic Timer always includes a virtual 
> counter, that indicates virtual time:
> [1]  In a processor implementation that does not include the 
> Virtualization Extensions, virtual time is identical to physical time, 
> and the virtual counter contains the same value as the physical counter.
> 
> [2]  If In a processor implementation that includes the Virtualization 
> Extensions, then it is already taken care in the kernel with the 
> following if boot loader boots with HYP mode,
>        commit 0af0b189abf73d232af782df2f999235cd2fed7f 
> 	ARM: hyp: initialize CNTVOFF to zero
> 	
> Currently, we have been noticing an issue with [1] with virtual offset 
> when we enable VCT since it is not an initialize to zero.

I highly doubt you are having a problem with [1] since you mentioned
Cortex-A7 which falls under [2]. If you have a _processor_
implementation with generic timers but no Hyp, as per the ARMv7 ARM the
CNTVCT == CNTPCT, otherwise it is a CPU bug.

What you actually have is [2] but without code to initialise the Hyp
mode. If your kernel is running non-secure, sane Hyp registers
initialisation is mandatory even when the kernel does not boot in Hyp.

> Perhaps, the best way to handle this one in firmware itself rather than 
> bringing too much complexity.
> Are you recommending this one?

Yes, definitely. But note that it's not only (secondary) boot but coming
out of a lower power mode which normally involves firmware anyway.

> If that is true, then we are going to force everyone to take care 
> CNTVOFF initialization in the firmware if they want to utilize vDSO on 
> ARMv7 where HYP mode is disabled.

Even for the kernel, this should be the case unless
"arm,cpu-registers-not-fw-configured" is passed in the DT (this was
really only meant as workaround for broken firmware; I think
Documentation/arm/Booting needs to be updated with the state of the
generic timers on entry).

IIRC there were some discussions about Linux booting in secure mode on
ARMv7 with no firmware to initialise the Hyp registers. If that's the
case for you, we could restart that discussion and try to initialise
CNTVOFF in the kernel running in _secure_ mode (possibly with some CPU
notifiers in the generic timer code).

-- 
Catalin