Re: [PATCH] x86: prefer RDTSCP in rdtsc_ordered()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.10.2024 12:02, Jan Beulich wrote:
> On 01.10.2024 11:45, Andrew Cooper wrote:
>> On 01/10/2024 9:12 am, Jan Beulich wrote:
>>> On 30.09.2024 18:40, Andrew Cooper wrote:
>>>> On 30/09/2024 4:08 pm, Jan Beulich wrote:
>>>>> --- a/xen/arch/x86/include/asm/msr.h
>>>>> +++ b/xen/arch/x86/include/asm/msr.h
>>>>> @@ -108,18 +108,30 @@ static inline uint64_t rdtsc(void)
>>>>>  
>>>>>  static inline uint64_t rdtsc_ordered(void)
>>>>>  {
>>>>> - /*
>>>>> -  * The RDTSC instruction is not ordered relative to memory access.
>>>>> -  * The Intel SDM and the AMD APM are both vague on this point, but
>>>>> -  * empirically an RDTSC instruction can be speculatively executed
>>>>> -  * before prior loads.  An RDTSC immediately after an appropriate
>>>>> -  * barrier appears to be ordered as a normal load, that is, it
>>>>> -  * provides the same ordering guarantees as reading from a global
>>>>> -  * memory location that some other imaginary CPU is updating
>>>>> -  * continuously with a time stamp.
>>>>> -  */
>>>>> - alternative("lfence", "mfence", X86_FEATURE_MFENCE_RDTSC);
>>>>> - return rdtsc();
>>>>> +    uint64_t low, high, aux;
>>>>> +
>>>>> +    /*
>>>>> +     * The RDTSC instruction is not ordered relative to memory access.
>>>>> +     * The Intel SDM and the AMD APM are both vague on this point, but
>>>>> +     * empirically an RDTSC instruction can be speculatively executed
>>>>> +     * before prior loads.
>>>> This part of the comment is stale now.  For RDTSC, AMD state:
>>>>
>>>> "This instruction is not serializing. Therefore, there is no guarantee
>>>> that all instructions have completed at the time the time-stamp counter
>>>> is read."
>>>>
>>>> and for RDTSCP:
>>>>
>>>> "Unlike the RDTSC instruction, RDTSCP forces all older instructions to
>>>> retire before reading the time-stamp counter."
>>>>
>>>> i.e. it's dispatch serialising, given our new post-Spectre terminology.
>>> I don't read that as truly "dispatch serializing";
>>
>> That is precisely what dispatch serialising is and means.
>>
>> Both LFENCE and RDTSCP wait at dispatch until they're the only
>> instruction in the pipeline.  That is how they get the property of
>> waiting for all older instructions to retire before executing.
>>
>>> both Intel and AMD
>>> leave open whether subsequent insns would also be affected, or whether
>>> those could pass the RDTSCP.
>>
>> Superscalar pipelines which can dispatch more than one uop per cycle can
>> issue LFENCE/RDTSCP concurrently with younger instructions.
>>
>> This is why LFENCE; JMP * was retracted as safe alternative to
>> retpoline, and why the Intel docs call out explicitly that you need
>> LFENCE following the RDTSC(P) if you want it to complete before
>> subsequent instructions start.
> 
> Yet what you describe still only puts in place a relationship between
> RDTSCP and what follows. What I was saying is that there's no guarantee
> that insns following RDTSCP can't actually execute not only in parallel
> with RDTSCP, but also in parallel with / ahead of earlier insns. Aiui
> LFENCE makes this guarantee. IOW in
> 
>       ADD ...; LFENCE; SUB ...
> 
> the SUB is guaranteed to dispatch only after the ADD, whereas in
> 
>       ADD ...; RDTSCP; SUB ...
> 
> there doesn't appear to be such a guarantee; the only guarantee here is
> for RDTSCP to dispatch after the ADD.

And btw, if there wasn't this difference, why would RDTSCP be any better
than LFENCE; RDTSC?

Jan