Re: [RESEND PATCH] arm64: arch_timer: Workaround for Cortex-A73 erratum 858921

[Leo Yan <leo.yan@linaro.org>]

Hi Marc,

On Wed, Aug 30, 2017 at 09:46:26AM +0100, Marc Zyngier wrote:

[...]

> >> This is completely busted:
> >>
> >> - You are only addressing the kernel side, and ignore userspace (which
> >> is just as broken as the kernel).
> > 
> > Thanks for quick reviewing.
> > 
> > Yeah, I remembered there have some code directly reads arch timer
> > virtual counter from userspace. But can you remind for userspace
> > broken issue, is the code in libc or vdso?
> 
> You're missing the point. The virtual counter is freely available to
> userspace to play with (this is a de-facto ABI). Since it can return bad
> values, it needs to be trapped to be correctly emulated (together with
> cntfrq_el0), and the VDSO disabled.
> 
> > Here I have another question is: after applied the whole workaround
> > infrastructure, can it also fix userspace broken issue?
> 
> Yup. That's why I ended-up with a 18 patches series, and not just this
> single one. Trust me, I'm lazy. There is nothing I hate more than doing
> useless work.

Thanks for detailed explaination. Now it's much clear for me.

> >> - You lie in the description of the option (this is in no way dynamic,
> >> since you didn't backport the whole workaround infrastructure).
> > 
> > I should fix it.
> > 
> >> So I'm afraid I'm NAKing this. Please refrain from blindly backporting
> >> random patches. fa8d815fac96e7c9 only makes sense in the context of the
> >> whole series, and on its own gives you a very false sense of having
> >> properly addressed it.
> > 
> > IIUC, at least fa8d815fac96e7c9 can fix issue in kernel side, such like
> > for sched_clock() roll back issue [1].
> 
> What's the point of fixing the kernel if userspace is just as likely to
> fail?

Understand now.

> > So for this issue, are you suggesting we need backport whole workaround
> > infrastructure onto kernel 4.4 and 4.9? Many ARM devices are working
> > with these two kernels.
> 
> Then these systems are completely broken if they use a Cortex-A73.
> Either they run mainline (which will be just fine), or they get a fully
> backported workaround infrastructure.

Yeah, we should do right thing.

Have ARM kernel team ported this patch series (or is in planning)? I
also will check with Linaro kernel team as well, I just want to avoid
duplicate efforts if these patches have been back ported. Otherwise, I
will backport the patch series.

Thanks,
Leo Yan