[PATCH v2 0/8] x86, KVM: Optimize SEV cache flushing

[PATCH v2 0/8] x86, KVM: Optimize SEV cache flushing

[Sean Christopherson]

This is the combination of Kevin's WBNOINVD series[1] with Zheyun's targeted
flushing series[2].  The combined goal is to use WBNOINVD instead of WBINVD
when doing cached maintenance to prevent data corruption due to C-bit aliasing,
and to reduce the number of cache invalidations by only performing flushes on
CPUs that have entered the relevant VM since the last cache flush.

Assuming I get the appropate acks (and that I didn't manage to break anything
when rebasing), my plan is to take this through the kvm-x86 tree in the 6.17
timeframe.

v2:
 - Add a missing -ENOMEM in __sev_guest_init(). [Tom]
 - Collect reviews. [Kai, Tom]
 - Fix stub prototypes. [Zheyun]
 - Kill off dead pr_err() code on DRM's wbinvd_on_all_cpus() usage.

v1: https://lore.kernel.org/all/20250227014858.3244505-1-seanjc@google.com

[1] https://lore.kernel.org/all/20250201000259.3289143-1-kevinloughlin@google.com
[2] https://lore.kernel.org/all/20250128015345.7929-1-szy0127@sjtu.edu.cn

Kevin Loughlin (2):
  x86, lib: Add WBNOINVD helper functions
  KVM: SEV: Prefer WBNOINVD over WBINVD for cache maintenance efficiency

Sean Christopherson (3):
  drm/gpu: Remove dead checks on wbinvd_on_all_cpus()'s return value
  x86, lib: Drop the unused return value from wbinvd_on_all_cpus()
  KVM: x86: Use wbinvd_on_cpu() instead of an open-coded equivalent

Zheyun Shen (3):
  KVM: SVM: Remove wbinvd in sev_vm_destroy()
  x86, lib: Add wbinvd and wbnoinvd helpers to target multiple CPUs
  KVM: SVM: Flush cache only on CPUs running SEV guest

 arch/x86/include/asm/smp.h           | 23 ++++++--
 arch/x86/include/asm/special_insns.h | 19 ++++++-
 arch/x86/kvm/svm/sev.c               | 81 +++++++++++++++++++---------
 arch/x86/kvm/svm/svm.h               |  1 +
 arch/x86/kvm/x86.c                   | 11 +---
 arch/x86/lib/cache-smp.c             | 26 ++++++++-
 drivers/gpu/drm/drm_cache.c          |  9 ++--
 7 files changed, 125 insertions(+), 45 deletions(-)


base-commit: 7ef51a41466bc846ad794d505e2e34ff97157f7f
-- 
2.49.0.1112.g889b7c5bd8-goog

[PATCH v2 1/8] KVM: SVM: Remove wbinvd in sev_vm_destroy()

[Sean Christopherson]

From: Zheyun Shen <szy0127@sjtu.edu.cn>

Before sev_vm_destroy() is called, kvm_arch_guest_memory_reclaimed()
has been called for SEV and SEV-ES and kvm_arch_gmem_invalidate()
has been called for SEV-SNP. These functions have already handled
flushing the memory. Therefore, this wbinvd_on_all_cpus() can
simply be dropped.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 978a0088a3f1..e9fd3bcec37a 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2883,12 +2883,6 @@ void sev_vm_destroy(struct kvm *kvm)
 		return;
 	}
 
-	/*
-	 * Ensure that all guest tagged cache entries are flushed before
-	 * releasing the pages back to the system for use. CLFLUSH will
-	 * not do this, so issue a WBINVD.
-	 */
-	wbinvd_on_all_cpus();
 
 	/*
 	 * if userspace was terminated before unregistering the memory regions
-- 
2.49.0.1112.g889b7c5bd8-goog

[PATCH v2 2/8] drm/gpu: Remove dead checks on wbinvd_on_all_cpus()'s return value

[Sean Christopherson]

Remove the checks and associated pr_err() on wbinvd_on_all_cpus() failure,
as the helper has unconditionally returned 0/success since commit
caa759323c73 ("smp: Remove smp_call_function() and on_each_cpu() return
values").

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 drivers/gpu/drm/drm_cache.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 7051c9c909c2..ea1d2d5d2c66 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -93,8 +93,7 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages)
 		return;
 	}
 
-	if (wbinvd_on_all_cpus())
-		pr_err("Timed out waiting for cache flush\n");
+	wbinvd_on_all_cpus();
 
 #elif defined(__powerpc__)
 	unsigned long i;
@@ -139,8 +138,7 @@ drm_clflush_sg(struct sg_table *st)
 		return;
 	}
 
-	if (wbinvd_on_all_cpus())
-		pr_err("Timed out waiting for cache flush\n");
+	wbinvd_on_all_cpus();
 #else
 	WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
 #endif
@@ -172,8 +170,7 @@ drm_clflush_virt_range(void *addr, unsigned long length)
 		return;
 	}
 
-	if (wbinvd_on_all_cpus())
-		pr_err("Timed out waiting for cache flush\n");
+	wbinvd_on_all_cpus();
 #else
 	WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
 #endif
-- 
2.49.0.1112.g889b7c5bd8-goog

[PATCH v2 3/8] x86, lib: Drop the unused return value from wbinvd_on_all_cpus()

[Sean Christopherson]

Drop wbinvd_on_all_cpus()'s return value; both the "real" version and the
stub always return '0', and none of the callers check the return.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/smp.h | 5 ++---
 arch/x86/lib/cache-smp.c   | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 0c1c68039d6f..028f126018c9 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -112,7 +112,7 @@ void __noreturn hlt_play_dead(void);
 void native_play_dead(void);
 void play_dead_common(void);
 void wbinvd_on_cpu(int cpu);
-int wbinvd_on_all_cpus(void);
+void wbinvd_on_all_cpus(void);
 
 void smp_kick_mwait_play_dead(void);
 void __noreturn mwait_play_dead(unsigned int eax_hint);
@@ -148,10 +148,9 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
 
 #else /* !CONFIG_SMP */
 #define wbinvd_on_cpu(cpu)     wbinvd()
-static inline int wbinvd_on_all_cpus(void)
+static inline void wbinvd_on_all_cpus(void)
 {
 	wbinvd();
-	return 0;
 }
 
 static inline struct cpumask *cpu_llc_shared_mask(int cpu)
diff --git a/arch/x86/lib/cache-smp.c b/arch/x86/lib/cache-smp.c
index 7af743bd3b13..079c3f3cd32c 100644
--- a/arch/x86/lib/cache-smp.c
+++ b/arch/x86/lib/cache-smp.c
@@ -14,9 +14,8 @@ void wbinvd_on_cpu(int cpu)
 }
 EXPORT_SYMBOL(wbinvd_on_cpu);
 
-int wbinvd_on_all_cpus(void)
+void wbinvd_on_all_cpus(void)
 {
 	on_each_cpu(__wbinvd, NULL, 1);
-	return 0;
 }
 EXPORT_SYMBOL(wbinvd_on_all_cpus);
-- 
2.49.0.1112.g889b7c5bd8-goog

Re: [PATCH v2 3/8] x86, lib: Drop the unused return value from wbinvd_on_all_cpus()

[Ingo Molnar]

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

> Drop wbinvd_on_all_cpus()'s return value; both the "real" version and the
> stub always return '0', and none of the callers check the return.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/smp.h | 5 ++---
>  arch/x86/lib/cache-smp.c   | 3 +--
>  2 files changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

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

[Sean Christopherson]

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);
 }
 
 static inline unsigned long __read_cr4(void)
diff --git a/arch/x86/lib/cache-smp.c b/arch/x86/lib/cache-smp.c
index 079c3f3cd32c..1789db5d8825 100644
--- a/arch/x86/lib/cache-smp.c
+++ b/arch/x86/lib/cache-smp.c
@@ -19,3 +19,14 @@ void wbinvd_on_all_cpus(void)
 	on_each_cpu(__wbinvd, NULL, 1);
 }
 EXPORT_SYMBOL(wbinvd_on_all_cpus);
+
+static void __wbnoinvd(void *dummy)
+{
+	wbnoinvd();
+}
+
+void wbnoinvd_on_all_cpus(void)
+{
+	on_each_cpu(__wbnoinvd, NULL, 1);
+}
+EXPORT_SYMBOL(wbnoinvd_on_all_cpus);
-- 
2.49.0.1112.g889b7c5bd8-goog

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

[Ingo Molnar]

* 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

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

[Sean Christopherson]

On Sat, May 17, 2025, Ingo Molnar wrote:
> 
> * 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).

While poking around the SDM and APM to figure out a decent comment, I realized
this comment is somewhat misleading since WBNOINVD is itself backwards compatible.
I still think it's a good idea to use alternative(), e.g. so that explicitly
disabling WBNOINVD in the event of a hardware issue works as expected.

> > +	 */
> > +	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),

Due to memory encryption technologies, I don't think there is a "typical" behavior
these days.  E.g. I'm pretty sure CPUs that support MKTME or SEV+ invalidate all
caches on a package, but I wouldn't classify that as the typical behavior since
there are likely still a huge number of CPUs in the wild that don't poke into the
lower level caches of other CPUs.

> 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.

I wouldn't call it a misnomer, the NO part makes it semantically accurate.  I
actually think the mnemonic was well chosen, as it helps capture the relationships
and behaviors of INVD, WBINVD, and WBNOINVD.

How about this?

diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 6266d6b9e0b8..f2240c4ac0ea 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -115,9 +115,39 @@ static inline void wrpkru(u32 pkru)
 }
 #endif
 
+/*
+ * Write back all modified lines in all levels of cache associated with this
+ * logical processor to main memory, and then invalidate all caches.  Depending
+ * on the micro-architecture, WBINVD (and WBNOINVD below) may or may not affect
+ * lower level caches associated with another logical processor that shares any
+ * level of this processor’s cache hierarchy.
+ *
+ * Note, AMD CPUs enumerate the behavior or WB{NO}{INVD} with respect to other
+ * logical, non-originating processors in CPUID 0x8000001D.EAX[N:0].
+ */
 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)
+
+/*
+ * Write back all modified lines in all levels of cache associated with this
+ * logical processor to main memory, but do NOT explicitly invalidate caches,
+ * i.e. leave all/most cache lines in the hierarchy in non-modified state.
+ */
+static __always_inline void wbnoinvd(void)
+{
+       /*
+        * Explicitly encode WBINVD if X86_FEATURE_WBNOINVD is unavailable even
+        * though WBNOINVD is backwards compatible (it's simply WBINVD with an
+        * ignored REP prefix), to guarantee that WBNOINVD isn't used if it
+        * needs to be avoided for any reason.  For all supported usage in the
+        * kernel, WBINVD is functionally a superset of WBNOINVD.
+        */
+       alternative("wbinvd", ASM_WBNOINVD, X86_FEATURE_WBNOINVD);
 }
 
 static inline unsigned long __read_cr4(void)

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

[Ingo Molnar]

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

> > 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.
> 
> I wouldn't call it a misnomer, the NO part makes it semantically 
> accurate.

If only 'NO' in that context was unambiguous: initially I fully read it 
as some sort of acronym or abbreviation :) Why wasn't it named WBCACHE 
or so? But I digress.

> [...]  I actually think the mnemonic was well chosen, as it helps 
> capture the relationships and behaviors of INVD, WBINVD, and 
> WBNOINVD.
> 
> How about this?

Much better!

> +/*
> + * Write back all modified lines in all levels of cache associated with this
> + * logical processor to main memory, and then invalidate all caches.  Depending
> + * on the micro-architecture, WBINVD (and WBNOINVD below) may or may not affect
> + * lower level caches associated with another logical processor that shares any
> + * level of this processor’s cache hierarchy.
> + *
> + * Note, AMD CPUs enumerate the behavior or WB{NO}{INVD} with respect to other
> + * logical, non-originating processors in CPUID 0x8000001D.EAX[N:0].
> + */
>  static __always_inline void wbinvd(void)
>  {
> +       asm volatile("wbinvd" : : : "memory");
> +}
> +
> +/* Instruction encoding provided for binutils backwards compatibility. */
> +#define ASM_WBNOINVD _ASM_BYTES(0xf3,0x0f,0x09)
> +
> +/*
> + * Write back all modified lines in all levels of cache associated with this
> + * logical processor to main memory, but do NOT explicitly invalidate caches,
> + * i.e. leave all/most cache lines in the hierarchy in non-modified state.
> + */
> +static __always_inline void wbnoinvd(void)
> +{
> +       /*
> +        * Explicitly encode WBINVD if X86_FEATURE_WBNOINVD is unavailable even
> +        * though WBNOINVD is backwards compatible (it's simply WBINVD with an
> +        * ignored REP prefix), to guarantee that WBNOINVD isn't used if it
> +        * needs to be avoided for any reason.  For all supported usage in the
> +        * kernel, WBINVD is functionally a superset of WBNOINVD.
> +        */
> +       alternative("wbinvd", ASM_WBNOINVD, X86_FEATURE_WBNOINVD);
>  }

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

[PATCH v2 5/8] KVM: SEV: Prefer WBNOINVD over WBINVD for cache maintenance efficiency

[Sean Christopherson]

From: Kevin Loughlin <kevinloughlin@google.com>

AMD CPUs currently execute WBINVD in the host when unregistering SEV
guest memory or when deactivating SEV guests. Such cache maintenance is
performed to prevent data corruption, wherein the encrypted (C=1)
version of a dirty cache line might otherwise only be written back
after the memory is written in a different context (ex: C=0), yielding
corruption. However, WBINVD is performance-costly, especially because
it invalidates processor caches.

Strictly-speaking, unless the SEV ASID is being recycled (meaning the
SNP firmware requires the use of WBINVD prior to DF_FLUSH), the cache
invalidation triggered by WBINVD is unnecessary; only the writeback is
needed to prevent data corruption in remaining scenarios.

To improve performance in these scenarios, use WBNOINVD when available
instead of WBINVD. WBNOINVD still writes back all dirty lines
(preventing host data corruption by SEV guests) but does *not*
invalidate processor caches. Note that the implementation of wbnoinvd()
ensures fall back to WBINVD if WBNOINVD is unavailable.

In anticipation of forthcoming optimizations to limit the WBNOINVD only
to physical CPUs that have executed SEV guests, place the call to
wbnoinvd_on_all_cpus() in a wrapper function sev_writeback_caches().

Signed-off-by: Kevin Loughlin <kevinloughlin@google.com>
Reviewed-by: Mingwei Zhang <mizhang@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Link: https://lore.kernel.org/r/20250201000259.3289143-3-kevinloughlin@google.com
[sean: tweak comment regarding CLFUSH]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index e9fd3bcec37a..9dcdeea954d3 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -116,6 +116,7 @@ static int sev_flush_asids(unsigned int min_asid, unsigned int max_asid)
 	 */
 	down_write(&sev_deactivate_lock);
 
+	/* SNP firmware requires use of WBINVD for ASID recycling. */
 	wbinvd_on_all_cpus();
 
 	if (sev_snp_enabled)
@@ -707,6 +708,18 @@ static void sev_clflush_pages(struct page *pages[], unsigned long npages)
 	}
 }
 
+static void sev_writeback_caches(void)
+{
+	/*
+	 * Ensure that all dirty guest tagged cache entries are written back
+	 * before releasing the pages back to the system for use.  CLFLUSH will
+	 * not do this without SME_COHERENT, and flushing many cache lines
+	 * individually is slower than blasting WBINVD for large VMs, so issue
+	 * WBNOINVD (or WBINVD if the "no invalidate" variant is unsupported).
+	 */
+	wbnoinvd_on_all_cpus();
+}
+
 static unsigned long get_num_contig_pages(unsigned long idx,
 				struct page **inpages, unsigned long npages)
 {
@@ -2757,12 +2770,7 @@ int sev_mem_enc_unregister_region(struct kvm *kvm,
 		goto failed;
 	}
 
-	/*
-	 * Ensure that all guest tagged cache entries are flushed before
-	 * releasing the pages back to the system for use. CLFLUSH will
-	 * not do this, so issue a WBINVD.
-	 */
-	wbinvd_on_all_cpus();
+	sev_writeback_caches();
 
 	__unregister_enc_region_locked(kvm, region);
 
@@ -3114,30 +3122,29 @@ static void sev_flush_encrypted_page(struct kvm_vcpu *vcpu, void *va)
 
 	/*
 	 * VM Page Flush takes a host virtual address and a guest ASID.  Fall
-	 * back to WBINVD if this faults so as not to make any problems worse
-	 * by leaving stale encrypted data in the cache.
+	 * back to full writeback of caches if this faults so as not to make
+	 * any problems worse by leaving stale encrypted data in the cache.
 	 */
 	if (WARN_ON_ONCE(wrmsrl_safe(MSR_AMD64_VM_PAGE_FLUSH, addr | asid)))
-		goto do_wbinvd;
+		goto do_sev_writeback_caches;
 
 	return;
 
-do_wbinvd:
-	wbinvd_on_all_cpus();
+do_sev_writeback_caches:
+	sev_writeback_caches();
 }
 
 void sev_guest_memory_reclaimed(struct kvm *kvm)
 {
 	/*
 	 * With SNP+gmem, private/encrypted memory is unreachable via the
-	 * hva-based mmu notifiers, so these events are only actually
-	 * pertaining to shared pages where there is no need to perform
-	 * the WBINVD to flush associated caches.
+	 * hva-based mmu notifiers, i.e. these events are explicitly scoped to
+	 * shared pages, where there's no need to flush caches.
 	 */
 	if (!sev_guest(kvm) || sev_snp_guest(kvm))
 		return;
 
-	wbinvd_on_all_cpus();
+	sev_writeback_caches();
 }
 
 void sev_free_vcpu(struct kvm_vcpu *vcpu)
@@ -3901,7 +3908,7 @@ void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
 	 * From this point forward, the VMSA will always be a guest-mapped page
 	 * rather than the initial one allocated by KVM in svm->sev_es.vmsa. In
 	 * theory, svm->sev_es.vmsa could be free'd and cleaned up here, but
-	 * that involves cleanups like wbinvd_on_all_cpus() which would ideally
+	 * that involves cleanups like flushing caches, which would ideally be
 	 * be handled during teardown rather than guest boot.  Deferring that
 	 * also allows the existing logic for SEV-ES VMSAs to be re-used with
 	 * minimal SNP-specific changes.
@@ -4899,7 +4906,7 @@ void sev_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end)
 
 		/*
 		 * SEV-ES avoids host/guest cache coherency issues through
-		 * WBINVD hooks issued via MMU notifiers during run-time, and
+		 * WBNOINVD hooks issued via MMU notifiers during run-time, and
 		 * KVM's VM destroy path at shutdown. Those MMU notifier events
 		 * don't cover gmem since there is no requirement to map pages
 		 * to a HVA in order to use them for a running guest. While the
-- 
2.49.0.1112.g889b7c5bd8-goog

Re: [PATCH v2 5/8] KVM: SEV: Prefer WBNOINVD over WBINVD for cache maintenance efficiency

[Francesco Lavra]

On 2025-05-16 at 21:28, Sean Christopherson wrote:
> @@ -3901,7 +3908,7 @@ void sev_snp_init_protected_guest_state(struct
> kvm_vcpu *vcpu)
>  	 * From this point forward, the VMSA will always be a guest-
> mapped page
>  	 * rather than the initial one allocated by KVM in svm-
> >sev_es.vmsa. In
>  	 * theory, svm->sev_es.vmsa could be free'd and cleaned up here,
> but
> -	 * that involves cleanups like wbinvd_on_all_cpus() which would
> ideally
> +	 * that involves cleanups like flushing caches, which would
> ideally be
>  	 * be handled during teardown rather than guest boot.  Deferring
> that

Duplicate "be"

[PATCH v2 6/8] KVM: x86: Use wbinvd_on_cpu() instead of an open-coded equivalent

[Sean Christopherson]

Use wbinvd_on_cpu() to target a single CPU instead of open-coding an
equivalent.  In addition to deduplicating code, this will allow removing
KVM's wbinvd_ipi() once the other usage is gone.

No functional change intended.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f9f798f286ce..b8b72e8dac6e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5009,8 +5009,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		if (kvm_x86_call(has_wbinvd_exit)())
 			cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask);
 		else if (vcpu->cpu != -1 && vcpu->cpu != cpu)
-			smp_call_function_single(vcpu->cpu,
-					wbinvd_ipi, NULL, 1);
+			wbinvd_on_cpu(vcpu->cpu);
 	}
 
 	kvm_x86_call(vcpu_load)(vcpu, cpu);
-- 
2.49.0.1112.g889b7c5bd8-goog

[PATCH v2 7/8] x86, lib: Add wbinvd and wbnoinvd helpers to target multiple CPUs

[Sean Christopherson]

From: Zheyun Shen <szy0127@sjtu.edu.cn>

Extract KVM's open-coded calls to do writeback caches on multiple CPUs to
common library helpers for both WBINVD and WBNOINVD (KVM will use both).
Put the onus on the caller to check for a non-empty mask to simplify the
SMP=n implementation, e.g. so that it doesn't need to check that the one
and only CPU in the system is present in the mask.

Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Link: https://lore.kernel.org/r/20250128015345.7929-2-szy0127@sjtu.edu.cn
[sean: move to lib, add SMP=n helpers, clarify usage]
Acked-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/smp.h | 12 ++++++++++++
 arch/x86/kvm/x86.c         |  8 +-------
 arch/x86/lib/cache-smp.c   | 12 ++++++++++++
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index e08f1ae25401..fe98e021f7f8 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -113,7 +113,9 @@ void native_play_dead(void);
 void play_dead_common(void);
 void wbinvd_on_cpu(int cpu);
 void wbinvd_on_all_cpus(void);
+void wbinvd_on_many_cpus(struct cpumask *cpus);
 void wbnoinvd_on_all_cpus(void);
+void wbnoinvd_on_many_cpus(struct cpumask *cpus);
 
 void smp_kick_mwait_play_dead(void);
 void __noreturn mwait_play_dead(unsigned int eax_hint);
@@ -154,11 +156,21 @@ static inline void wbinvd_on_all_cpus(void)
 	wbinvd();
 }
 
+static inline void wbinvd_on_many_cpus(struct cpumask *cpus)
+{
+	wbinvd();
+}
+
 static inline void wbnoinvd_on_all_cpus(void)
 {
 	wbnoinvd();
 }
 
+static inline void wbnoinvd_on_many_cpus(struct cpumask *cpus)
+{
+	wbnoinvd();
+}
+
 static inline struct cpumask *cpu_llc_shared_mask(int cpu)
 {
 	return (struct cpumask *)cpumask_of(0);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b8b72e8dac6e..e00a4b3a0e8c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4981,11 +4981,6 @@ long kvm_arch_dev_ioctl(struct file *filp,
 	return r;
 }
 
-static void wbinvd_ipi(void *garbage)
-{
-	wbinvd();
-}
-
 static bool need_emulate_wbinvd(struct kvm_vcpu *vcpu)
 {
 	return kvm_arch_has_noncoherent_dma(vcpu->kvm);
@@ -8286,8 +8281,7 @@ static int kvm_emulate_wbinvd_noskip(struct kvm_vcpu *vcpu)
 		int cpu = get_cpu();
 
 		cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask);
-		on_each_cpu_mask(vcpu->arch.wbinvd_dirty_mask,
-				wbinvd_ipi, NULL, 1);
+		wbinvd_on_many_cpus(vcpu->arch.wbinvd_dirty_mask);
 		put_cpu();
 		cpumask_clear(vcpu->arch.wbinvd_dirty_mask);
 	} else
diff --git a/arch/x86/lib/cache-smp.c b/arch/x86/lib/cache-smp.c
index 1789db5d8825..ebbc91b3ac67 100644
--- a/arch/x86/lib/cache-smp.c
+++ b/arch/x86/lib/cache-smp.c
@@ -20,6 +20,12 @@ void wbinvd_on_all_cpus(void)
 }
 EXPORT_SYMBOL(wbinvd_on_all_cpus);
 
+void wbinvd_on_many_cpus(struct cpumask *cpus)
+{
+	on_each_cpu_mask(cpus, __wbinvd, NULL, 1);
+}
+EXPORT_SYMBOL_GPL(wbinvd_on_many_cpus);
+
 static void __wbnoinvd(void *dummy)
 {
 	wbnoinvd();
@@ -30,3 +36,9 @@ void wbnoinvd_on_all_cpus(void)
 	on_each_cpu(__wbnoinvd, NULL, 1);
 }
 EXPORT_SYMBOL(wbnoinvd_on_all_cpus);
+
+void wbnoinvd_on_many_cpus(struct cpumask *cpus)
+{
+	on_each_cpu_mask(cpus, __wbnoinvd, NULL, 1);
+}
+EXPORT_SYMBOL_GPL(wbnoinvd_on_many_cpus);
-- 
2.49.0.1112.g889b7c5bd8-goog

Re: [PATCH v2 7/8] x86, lib: Add wbinvd and wbnoinvd helpers to target multiple CPUs

[Ingo Molnar]

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

> From: Zheyun Shen <szy0127@sjtu.edu.cn>
> 
> Extract KVM's open-coded calls to do writeback caches on multiple CPUs to
> common library helpers for both WBINVD and WBNOINVD (KVM will use both).
> Put the onus on the caller to check for a non-empty mask to simplify the
> SMP=n implementation, e.g. so that it doesn't need to check that the one
> and only CPU in the system is present in the mask.
> 
> Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Link: https://lore.kernel.org/r/20250128015345.7929-2-szy0127@sjtu.edu.cn
> [sean: move to lib, add SMP=n helpers, clarify usage]
> Acked-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/smp.h | 12 ++++++++++++
>  arch/x86/kvm/x86.c         |  8 +-------
>  arch/x86/lib/cache-smp.c   | 12 ++++++++++++
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index e08f1ae25401..fe98e021f7f8 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -113,7 +113,9 @@ void native_play_dead(void);
>  void play_dead_common(void);
>  void wbinvd_on_cpu(int cpu);
>  void wbinvd_on_all_cpus(void);
> +void wbinvd_on_many_cpus(struct cpumask *cpus);
>  void wbnoinvd_on_all_cpus(void);
> +void wbnoinvd_on_many_cpus(struct cpumask *cpus);

Let's go with the _on_cpumask() suffix:

    void wbinvd_on_cpu(int cpu);
   +void wbinvd_on_cpumask(struct cpumask *cpus);
    void wbinvd_on_all_cpus(void);

And the wb*invd_all_cpus() methods should probably be inlined wrappers 
with -1 as the cpumask, or so - not two separate functions?

In fact it would be nice to have the DRM preparatory patch and all the 
x86 patches at the beginning of the next version of the series, so 
those 4 patches can be applied to the x86 tree. Can make it a separate 
permanent branch based on v6.15-rc6/rc7.

Thanks,

	Ingo

Re: [PATCH v2 7/8] x86, lib: Add wbinvd and wbnoinvd helpers to target multiple CPUs

[Sean Christopherson]

On Sat, May 17, 2025, Ingo Molnar wrote:
> 
> * Sean Christopherson <seanjc@google.com> wrote:
> 
> > From: Zheyun Shen <szy0127@sjtu.edu.cn>
> > 
> > Extract KVM's open-coded calls to do writeback caches on multiple CPUs to
> > common library helpers for both WBINVD and WBNOINVD (KVM will use both).
> > Put the onus on the caller to check for a non-empty mask to simplify the
> > SMP=n implementation, e.g. so that it doesn't need to check that the one
> > and only CPU in the system is present in the mask.
> > 
> > Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > Link: https://lore.kernel.org/r/20250128015345.7929-2-szy0127@sjtu.edu.cn
> > [sean: move to lib, add SMP=n helpers, clarify usage]
> > Acked-by: Kai Huang <kai.huang@intel.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/include/asm/smp.h | 12 ++++++++++++
> >  arch/x86/kvm/x86.c         |  8 +-------
> >  arch/x86/lib/cache-smp.c   | 12 ++++++++++++
> >  3 files changed, 25 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> > index e08f1ae25401..fe98e021f7f8 100644
> > --- a/arch/x86/include/asm/smp.h
> > +++ b/arch/x86/include/asm/smp.h
> > @@ -113,7 +113,9 @@ void native_play_dead(void);
> >  void play_dead_common(void);
> >  void wbinvd_on_cpu(int cpu);
> >  void wbinvd_on_all_cpus(void);
> > +void wbinvd_on_many_cpus(struct cpumask *cpus);
> >  void wbnoinvd_on_all_cpus(void);
> > +void wbnoinvd_on_many_cpus(struct cpumask *cpus);
> 
> Let's go with the _on_cpumask() suffix:
> 
>     void wbinvd_on_cpu(int cpu);
>    +void wbinvd_on_cpumask(struct cpumask *cpus);
>     void wbinvd_on_all_cpus(void);
> 
> And the wb*invd_all_cpus() methods should probably be inlined wrappers 
> with -1 as the cpumask, or so - not two separate functions?

Using two separate functions allows _on_all_cpus() to defer the mask generation
to on_each_cpu(), i.e. avoids having to duplicate the passing of cpu_online_mask.
IMO, duplicating passing __wbinvd is preferable to duplicating the use of
cpu_online_mask.
 
> In fact it would be nice to have the DRM preparatory patch and all the 
> x86 patches at the beginning of the next version of the series, so 
> those 4 patches can be applied to the x86 tree. Can make it a separate 
> permanent branch based on v6.15-rc6/rc7.

Can do, assuming there's no lurking dependency I'm missing.

Re: [PATCH v2 7/8] x86, lib: Add wbinvd and wbnoinvd helpers to target multiple CPUs

[Sean Christopherson]

On Sat, May 17, 2025, Ingo Molnar wrote:
> 
> * Sean Christopherson <seanjc@google.com> wrote:
> 
> > From: Zheyun Shen <szy0127@sjtu.edu.cn>
> > 
> > Extract KVM's open-coded calls to do writeback caches on multiple CPUs to
> > common library helpers for both WBINVD and WBNOINVD (KVM will use both).
> > Put the onus on the caller to check for a non-empty mask to simplify the
> > SMP=n implementation, e.g. so that it doesn't need to check that the one
> > and only CPU in the system is present in the mask.
> > 
> > Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > Link: https://lore.kernel.org/r/20250128015345.7929-2-szy0127@sjtu.edu.cn
> > [sean: move to lib, add SMP=n helpers, clarify usage]
> > Acked-by: Kai Huang <kai.huang@intel.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/include/asm/smp.h | 12 ++++++++++++
> >  arch/x86/kvm/x86.c         |  8 +-------
> >  arch/x86/lib/cache-smp.c   | 12 ++++++++++++
> >  3 files changed, 25 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> > index e08f1ae25401..fe98e021f7f8 100644
> > --- a/arch/x86/include/asm/smp.h
> > +++ b/arch/x86/include/asm/smp.h
> > @@ -113,7 +113,9 @@ void native_play_dead(void);
> >  void play_dead_common(void);
> >  void wbinvd_on_cpu(int cpu);
> >  void wbinvd_on_all_cpus(void);
> > +void wbinvd_on_many_cpus(struct cpumask *cpus);
> >  void wbnoinvd_on_all_cpus(void);
> > +void wbnoinvd_on_many_cpus(struct cpumask *cpus);
> 
> Let's go with the _on_cpumask() suffix:
> 
>     void wbinvd_on_cpu(int cpu);
>    +void wbinvd_on_cpumask(struct cpumask *cpus);
>     void wbinvd_on_all_cpus(void);

How about wbinvd_on_cpus_mask(), to make it more obvious that it operates on
multiple CPUs?  At a glance, wbinvd_on_cpumask() could be mistaken for a masked
version of wbinvd_on_cpu().

Re: [PATCH v2 7/8] x86, lib: Add wbinvd and wbnoinvd helpers to target multiple CPUs

[Ingo Molnar]

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

> On Sat, May 17, 2025, Ingo Molnar wrote:
> > 
> > * Sean Christopherson <seanjc@google.com> wrote:
> > 
> > > From: Zheyun Shen <szy0127@sjtu.edu.cn>
> > > 
> > > Extract KVM's open-coded calls to do writeback caches on multiple CPUs to
> > > common library helpers for both WBINVD and WBNOINVD (KVM will use both).
> > > Put the onus on the caller to check for a non-empty mask to simplify the
> > > SMP=n implementation, e.g. so that it doesn't need to check that the one
> > > and only CPU in the system is present in the mask.
> > > 
> > > Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
> > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > > Link: https://lore.kernel.org/r/20250128015345.7929-2-szy0127@sjtu.edu.cn
> > > [sean: move to lib, add SMP=n helpers, clarify usage]
> > > Acked-by: Kai Huang <kai.huang@intel.com>
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > >  arch/x86/include/asm/smp.h | 12 ++++++++++++
> > >  arch/x86/kvm/x86.c         |  8 +-------
> > >  arch/x86/lib/cache-smp.c   | 12 ++++++++++++
> > >  3 files changed, 25 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> > > index e08f1ae25401..fe98e021f7f8 100644
> > > --- a/arch/x86/include/asm/smp.h
> > > +++ b/arch/x86/include/asm/smp.h
> > > @@ -113,7 +113,9 @@ void native_play_dead(void);
> > >  void play_dead_common(void);
> > >  void wbinvd_on_cpu(int cpu);
> > >  void wbinvd_on_all_cpus(void);
> > > +void wbinvd_on_many_cpus(struct cpumask *cpus);
> > >  void wbnoinvd_on_all_cpus(void);
> > > +void wbnoinvd_on_many_cpus(struct cpumask *cpus);
> > 
> > Let's go with the _on_cpumask() suffix:
> > 
> >     void wbinvd_on_cpu(int cpu);
> >    +void wbinvd_on_cpumask(struct cpumask *cpus);
> >     void wbinvd_on_all_cpus(void);
> 
> How about wbinvd_on_cpus_mask(), to make it more obvious that it operates on
> multiple CPUs?  At a glance, wbinvd_on_cpumask() could be mistaken for a masked
> version of wbinvd_on_cpu().

Works for me!

Thanks,

	Ingo

[PATCH v2 8/8] KVM: SVM: Flush cache only on CPUs running SEV guest

[Sean Christopherson]

From: Zheyun Shen <szy0127@sjtu.edu.cn>

On AMD CPUs without ensuring cache consistency, each memory page
reclamation in an SEV guest triggers a call to do WBNOINVD/WBINVD on all
CPUs, thereby affecting the performance of other programs on the host.

Typically, an AMD server may have 128 cores or more, while the SEV guest
might only utilize 8 of these cores. Meanwhile, host can use qemu-affinity
to bind these 8 vCPUs to specific physical CPUs.

Therefore, keeping a record of the physical core numbers each time a vCPU
runs can help avoid flushing the cache for all CPUs every time.

Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 46 +++++++++++++++++++++++++++++++++++-------
 arch/x86/kvm/svm/svm.h |  1 +
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 9dcdeea954d3..e40fb2105dbc 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -446,7 +446,12 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
 	init_args.probe = false;
 	ret = sev_platform_init(&init_args);
 	if (ret)
-		goto e_free;
+		goto e_free_asid;
+
+	if (!zalloc_cpumask_var(&sev->have_run_cpus, GFP_KERNEL_ACCOUNT)) {
+		ret = -ENOMEM;
+		goto e_free_asid;
+	}
 
 	/* This needs to happen after SEV/SNP firmware initialization. */
 	if (vm_type == KVM_X86_SNP_VM) {
@@ -464,6 +469,8 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
 	return 0;
 
 e_free:
+	free_cpumask_var(sev->have_run_cpus);
+e_free_asid:
 	argp->error = init_args.error;
 	sev_asid_free(sev);
 	sev->asid = 0;
@@ -708,16 +715,31 @@ static void sev_clflush_pages(struct page *pages[], unsigned long npages)
 	}
 }
 
-static void sev_writeback_caches(void)
+static void sev_writeback_caches(struct kvm *kvm)
 {
+	/*
+	 * Note, the caller is responsible for ensuring correctness if the mask
+	 * can be modified, e.g. if a CPU could be doing VMRUN.
+	 */
+	if (cpumask_empty(to_kvm_sev_info(kvm)->have_run_cpus))
+		return;
+
 	/*
 	 * Ensure that all dirty guest tagged cache entries are written back
 	 * before releasing the pages back to the system for use.  CLFLUSH will
 	 * not do this without SME_COHERENT, and flushing many cache lines
 	 * individually is slower than blasting WBINVD for large VMs, so issue
-	 * WBNOINVD (or WBINVD if the "no invalidate" variant is unsupported).
+	 * WBNOINVD (or WBINVD if the "no invalidate" variant is unsupported)
+	 * on CPUs that have done VMRUN, i.e. may have dirtied data using the
+	 * VM's ASID.
+	 *
+	 * For simplicity, never remove CPUs from the bitmap.  Ideally, KVM
+	 * would clear the mask when flushing caches, but doing so requires
+	 * serializing multiple calls and having responding CPUs (to the IPI)
+	 * mark themselves as still running if they are running (or about to
+	 * run) a vCPU for the VM.
 	 */
-	wbnoinvd_on_all_cpus();
+	wbnoinvd_on_many_cpus(to_kvm_sev_info(kvm)->have_run_cpus);
 }
 
 static unsigned long get_num_contig_pages(unsigned long idx,
@@ -2770,7 +2792,7 @@ int sev_mem_enc_unregister_region(struct kvm *kvm,
 		goto failed;
 	}
 
-	sev_writeback_caches();
+	sev_writeback_caches(kvm);
 
 	__unregister_enc_region_locked(kvm, region);
 
@@ -2918,6 +2940,7 @@ void sev_vm_destroy(struct kvm *kvm)
 	}
 
 	sev_asid_free(sev);
+	free_cpumask_var(sev->have_run_cpus);
 }
 
 void __init sev_set_cpu_caps(void)
@@ -3131,7 +3154,7 @@ static void sev_flush_encrypted_page(struct kvm_vcpu *vcpu, void *va)
 	return;
 
 do_sev_writeback_caches:
-	sev_writeback_caches();
+	sev_writeback_caches(vcpu->kvm);
 }
 
 void sev_guest_memory_reclaimed(struct kvm *kvm)
@@ -3144,7 +3167,7 @@ void sev_guest_memory_reclaimed(struct kvm *kvm)
 	if (!sev_guest(kvm) || sev_snp_guest(kvm))
 		return;
 
-	sev_writeback_caches();
+	sev_writeback_caches(kvm);
 }
 
 void sev_free_vcpu(struct kvm_vcpu *vcpu)
@@ -3476,6 +3499,15 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu)
 	if (sev_es_guest(kvm) && !VALID_PAGE(svm->vmcb->control.vmsa_pa))
 		return -EINVAL;
 
+	/*
+	 * To optimize cache flushes when memory is reclaimed from an SEV VM,
+	 * track physical CPUs that enter the guest for SEV VMs and thus can
+	 * have encrypted, dirty data in the cache, and flush caches only for
+	 * CPUs that have entered the guest.
+	 */
+	if (!cpumask_test_cpu(cpu, to_kvm_sev_info(kvm)->have_run_cpus))
+		cpumask_set_cpu(cpu, to_kvm_sev_info(kvm)->have_run_cpus);
+
 	/* Assign the asid allocated with this SEV guest */
 	svm->asid = asid;
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index e6f3c6a153a0..a7c6f07260cf 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -113,6 +113,7 @@ struct kvm_sev_info {
 	void *guest_req_buf;    /* Bounce buffer for SNP Guest Request input */
 	void *guest_resp_buf;   /* Bounce buffer for SNP Guest Request output */
 	struct mutex guest_req_mutex; /* Must acquire before using bounce buffers */
+	cpumask_var_t have_run_cpus; /* CPUs that have done VMRUN for this VM. */
 };
 
 #define SEV_POLICY_NODBG	BIT_ULL(0)
-- 
2.49.0.1112.g889b7c5bd8-goog

Re: [PATCH v2 8/8] KVM: SVM: Flush cache only on CPUs running SEV guest

[Tom Lendacky]

On 5/16/25 16:28, Sean Christopherson wrote:
> From: Zheyun Shen <szy0127@sjtu.edu.cn>
> 
> On AMD CPUs without ensuring cache consistency, each memory page
> reclamation in an SEV guest triggers a call to do WBNOINVD/WBINVD on all
> CPUs, thereby affecting the performance of other programs on the host.
> 
> Typically, an AMD server may have 128 cores or more, while the SEV guest
> might only utilize 8 of these cores. Meanwhile, host can use qemu-affinity
> to bind these 8 vCPUs to specific physical CPUs.
> 
> Therefore, keeping a record of the physical core numbers each time a vCPU
> runs can help avoid flushing the cache for all CPUs every time.
> 
> Signed-off-by: Zheyun Shen <szy0127@sjtu.edu.cn>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  arch/x86/kvm/svm/sev.c | 46 +++++++++++++++++++++++++++++++++++-------
>  arch/x86/kvm/svm/svm.h |  1 +
>  2 files changed, 40 insertions(+), 7 deletions(-)
>