[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 1 Oct 2024 10:12:50 +0200
  • Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 01 Oct 2024 08:13:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 30.09.2024 18:40, Andrew Cooper wrote:
> On 30/09/2024 4:08 pm, Jan Beulich wrote:
>> If available, its use is supposed to be cheaper than LFENCE+RDTSC, and
>> is virtually guaranteed to be cheaper than MFENCE+RDTSC.
>>
>> Unlike in rdtsc() use 64-bit local variables, eliminating the need for
> 
> I'd drop this reference to rdtsc() seeing as you adjust it in a parallel
> patch.

Already done, with that other commit now having gone in. When I wrote this,
I wasn't finally decided yet whether to also make that other adjustment.

>> --- 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"; both Intel and AMD
leave open whether subsequent insns would also be affected, or whether
those could pass the RDTSCP. Either form is fine for our purposes here
aiui.

> Intel OTOH have much more extensive information.  For RDTSC:
> 
> The RDTSC instruction is not a serializing instruction. It does not
> necessarily wait until all previous instructions have been executed
> before reading the counter. Similarly, subsequent instructions may begin
> execution before the read operation is performed. The following items
> may guide software seeking to order executions of RDTSC:
> 
> •If software requires RDTSC to be executed only after all previous
> instructions have executed and all previous loads are globally visible,1
> it can execute LFENCE immediately before RDTSC.
> 
> •If software requires RDTSC to be executed only after all previous
> instructions have executed and all previous loads and stores are
> globally visible, it can execute the sequence MFENCE;LFENCE immediately
> before RDTSC.
> 
> •If software requires RDTSC to be executed prior to execution of any
> subsequent instruction (including any memory accesses), it can execute
> the sequence LFENCE immediately after RDTSC.
> 
> Similarly, for RDTSCP:
> 
> The RDTSCP instruction is not a serializing instruction, but it does
> wait until all previous instructions have executed and all previous
> loads are globally visible.  But it does not wait for previous stores to
> be globally visible, and subsequent instructions may begin execution
> before the read operation is performed. The following items may guide
> software seeking to order executions of RDTSCP:
> 
> •If software requires RDTSCP to be executed only after all previous
> stores are globally visible, it can execute MFENCE immediately before
> RDTSCP.
> 
> •If software requires RDTSCP to be executed prior to execution of any
> subsequent instruction (including any memory accesses), it can execute
> LFENCE immediately after RDTSCP.
> 
> 
> 
> I'd delete most of the paragraph, and just state the recommendation to
> use LFENCE.

I was in fact wondering whether to. I'll send a v2 with updated (and
shortened) commentary. I think I will want to keep mentioning MFENCE
there though, ...

> In truth, X86_FEATURE_MFENCE_RDTSC is useless now that we unilaterally
> activate LFENCE_DISPATCH on CPUs where it's optional.  Linux went as far
> as removing the case entirely, because if you're running under a
> hypervisor which hasn't set LFENCE_DISPATCH, then the misbehaviour of
> lfence;rdtsc is the least of your problems.

... despite this (orthogonal) observation. We can independently decide
whether to drop MFENCE_RDTSC.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.