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

[Lendacky, Thomas]

On 12/12/2018 08:15 AM, Tom Lendacky wrote:
> 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.

I should add that I'll see about getting documentation updated with this
information.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>