Re: [PATCH] x86/vdso: Remove direct HPET access through the vDSO

[Andy Lutomirski]

On Apr 8, 2016 3:36 AM, "Borislav Petkov" <bp@xxxxxxxxx> wrote:
>
> On Thu, Apr 07, 2016 at 05:16:59PM -0700, Andy Lutomirski wrote:
> > Allowing user code to map the HPET is problematic.  HPET
> > implementations are notoriously buggy, and there are probably many
> > machines on which even MMIO reads from bogus HPET addresses are
> > problematic.
> >
> > We have a report that the Dell Precision M2800 with:
> >
> > ACPI: HPET 0x00000000C8FE6238 000038 (v01 DELL   CBX3  01072009 AMI. 00000005)
> >
> > is either so slow when accessing the HPET or actually hangs in some
> > regard, causing soft lockups to be reported if users do unexpected
> > things to the HPET.
> >
> > The vclock HPET code has also always been a questionable speedup.
> > Accessing an HPET is exceedingly slow (on the order of several
> > microseconds), so the added overhead in requiring a syscall to read
> > the HPET is a small fraction of the total code of accessing it.
> >
> > To avoid future problems, let's just delete the code entirely.
> >
> > In the long run, this could actually be a speedup.  Waiman Long as a
> > patch to optimize the case where multiple CPUs contend for the HPET,
> > but that won't help unless all the accesses are mediated by the
> > kernel.
> >
> > Cc: Waiman Long <waiman.long@xxxxxxx>
> > Reported-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> > ---
> >  arch/x86/entry/vdso/vclock_gettime.c  | 15 ---------------
> >  arch/x86/entry/vdso/vdso-layout.lds.S |  5 ++---
> >  arch/x86/entry/vdso/vma.c             | 11 -----------
> >  arch/x86/include/asm/clocksource.h    |  9 ++++-----
> >  arch/x86/kernel/hpet.c                |  1 -
> >  arch/x86/kvm/trace.h                  |  3 +--
> >  6 files changed, 7 insertions(+), 37 deletions(-)
>
> I like diffstats which remove more lines than add :)
>
> In any case, I'm not well versed in the vdso game but from my experience
> so far, I'm starting to believe that the 'H' in HPET stands for
> Horrific. So the less people get exposed to it, the better.
>
> A question though: userspace does not rely on the hpet page being always
> present and can do fallback. IOW, we're not breaking any existing
> usages, right?

It shouldn't break anything.  It'll just cause the vdso to issue a
real syscall instead of reading the HPET directly.  I strongly doubt
there's user code that pokes the HPET mapping outside the vdso, since
it would have broken when we moved the HPET mapping out of the fixmap
to a randomized address.

>
> --
> Regards/Gruss,
>     Boris.
>
> ECO tip #101: Trim your mails when you reply.