Re: [RFC PATCH 4/4] x86/TSC: Use RDTSCP

[Andy Lutomirski <luto@kernel.org>]

> On Dec 11, 2018, at 3:39 PM, Borislav Petkov <bp@alien8.de> wrote:
>
>> On Tue, Dec 11, 2018 at 11:12:41PM +0000, Lendacky, Thomas wrote:
>> It does seem overloaded in that sense, but the feature means that LFENCE
>> is serializing and so can be used in rdtsc_ordered. In the same sense,
>> barrier_nospec is looking for whether LFENCE is serializing and preferring
>> that over MFENCE since it is lighter weight.
>>
>> In light of how they're being used now, they could probably stand to be
>> renamed in some way.
>
> Actually, come to think of it, what really matters here is whether
> LFENCE is serializing or not. Because if so, you wanna replace with LFENCE
> as it is lighter. And in that case a single alternative() - not _2() -
> should suffice.
>
> BUT(!), that still is not good enough if you do some qemu CPU models
> like pentium or so which don't even have MFENCE and cause stuff like
> this:
>
> https://lkml.kernel.org/r/20181123200307.GA6223@roeck-us.net
>
> Which means, that you *do* have to alternate between
>
> * no insn at all
> * MFENCE
> * LFENCE, if it is serializing
>
> so barrier_nospec() does the right thing, AFAICS. And this is why we
> need an ALTERNATIVE_3() to add RDTSCP into the mix too.
>
> WRT renaming, I guess we can do something like:
>
> * X86_FEATURE_MFENCE_RDTSC -> X86_FEATURE_MFENCE - to mean that CPU has
> MFENCE support.
>
> and
>
> * X86_FEATURE_LFENCE_RDTSC -> X86_FEATURE_LFENCE_SERIALIZING
>
> Or something to that effect.

This makes me nervous, since no one knows what “serializing” means.
IIRC AMD specifically documents that MFENCE is required before RDTSC
to get sensible ordering.  So it’s entirely plausible to me that
LFENCE is okay for Spectre mitigation but MFENCE is needed for RDTSC
on some CPU.