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

From: Lendacky, Thomas
Date: Wed Dec 12 2018 - 09:16:07 EST


On 12/11/2018 08:24 PM, Andy Lutomirski wrote:
>> On Dec 11, 2018, at 3:39 PM, Borislav Petkov <bp@xxxxxxxxx> 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@xxxxxxxxxxxx
>>
>> 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.

As long as MSR 0xc0011029[1] is set (MSR_F10H_DECFG_LFENCE_SERIALIZE_BIT),
then LFENCE is a proper, lighter weight solution for ordering RDTSC. So
we're good there.

Thanks,
Tom

>