Re: [PATCH v2 4/8] x86, lib: Add WBNOINVD helper functions

[Ingo Molnar <mingo@kernel.org>]

* Sean Christopherson <seanjc@google.com> wrote:

> From: Kevin Loughlin <kevinloughlin@google.com>
> 
> In line with WBINVD usage, add WBONINVD helper functions.  Fall back to
> WBINVD (via alternative()) if WBNOINVD isn't supported, as WBINVD provides
> a superset of functionality, just more slowly.
> 
> Note, alternative() ensures compatibility with early boot code as needed.
> 
> Signed-off-by: Kevin Loughlin <kevinloughlin@google.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> [sean: massage changelog and comments, use ASM_WBNOINVD and _ASM_BYTES]
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/smp.h           |  6 ++++++
>  arch/x86/include/asm/special_insns.h | 19 ++++++++++++++++++-
>  arch/x86/lib/cache-smp.c             | 11 +++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 028f126018c9..e08f1ae25401 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -113,6 +113,7 @@ void native_play_dead(void);
>  void play_dead_common(void);
>  void wbinvd_on_cpu(int cpu);
>  void wbinvd_on_all_cpus(void);
> +void wbnoinvd_on_all_cpus(void);
>  
>  void smp_kick_mwait_play_dead(void);
>  void __noreturn mwait_play_dead(unsigned int eax_hint);
> @@ -153,6 +154,11 @@ static inline void wbinvd_on_all_cpus(void)
>  	wbinvd();
>  }
>  
> +static inline void wbnoinvd_on_all_cpus(void)
> +{
> +	wbnoinvd();
> +}
> +
>  static inline struct cpumask *cpu_llc_shared_mask(int cpu)
>  {
>  	return (struct cpumask *)cpumask_of(0);
> diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
> index 6266d6b9e0b8..46b3961e3e4b 100644
> --- a/arch/x86/include/asm/special_insns.h
> +++ b/arch/x86/include/asm/special_insns.h
> @@ -117,7 +117,24 @@ static inline void wrpkru(u32 pkru)
>  
>  static __always_inline void wbinvd(void)
>  {
> -	asm volatile("wbinvd": : :"memory");
> +	asm volatile("wbinvd" : : : "memory");
> +}
> +
> +/* Instruction encoding provided for binutils backwards compatibility. */
> +#define ASM_WBNOINVD _ASM_BYTES(0xf3,0x0f,0x09)
> +
> +/*
> + * Cheaper version of wbinvd(). Call when caches need to be written back but
> + * not invalidated.
> + */
> +static __always_inline void wbnoinvd(void)
> +{
> +	/*
> +	 * If WBNOINVD is unavailable, fall back to the compatible but
> +	 * more destructive WBINVD (which still writes the caches back
> +	 * but also invalidates them).
> +	 */
> +	alternative("wbinvd", ASM_WBNOINVD, X86_FEATURE_WBNOINVD);
>  }

Would be nice here to use the opportunity and document both WBINVD and 
WBNOINVD a bit more comprehensively, to point out that WBINVD writes 
back and flushes the caches (and point out which level of caches this 
affects typically), and to point out that the 'invalidate' part of the 
WBNOINVD name is a misnomer, as it doesn't invalidate anything, it only 
writes back dirty cachelines.

Thanks,

	Ingo