diff mbox series

x86/bugs/mmio: Rename mmio_stale_data_clear to cpu_buf_vm_clear

Message ID 20250410-mmio-rename-v1-1-fd4b2e7fc04e@linux.intel.com (mailing list archive)
State New
Headers show
Series x86/bugs/mmio: Rename mmio_stale_data_clear to cpu_buf_vm_clear | expand

Commit Message

Pawan Gupta April 10, 2025, 5:49 p.m. UTC 
The static key mmio_stale_data_clear controls the KVM-only mitigation for
MMIO Stale Data vulnerability. Rename it to reflect its purpose.

No functional change.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/include/asm/nospec-branch.h |  2 +-
 arch/x86/kernel/cpu/bugs.c           | 16 ++++++++++------
 arch/x86/kvm/vmx/vmx.c               |  2 +-
 3 files changed, 12 insertions(+), 8 deletions(-)


---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250410-mmio-rename-cf8acecb563e

Comments

Borislav Petkov April 10, 2025, 6:04 p.m. UTC | #1 
On Thu, Apr 10, 2025 at 10:49:51AM -0700, Pawan Gupta wrote:
> The static key mmio_stale_data_clear controls the KVM-only mitigation for
> MMIO Stale Data vulnerability. Rename it to reflect its purpose.
> 
> No functional change.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/x86/include/asm/nospec-branch.h |  2 +-
>  arch/x86/kernel/cpu/bugs.c           | 16 ++++++++++------
>  arch/x86/kvm/vmx/vmx.c               |  2 +-
>  3 files changed, 12 insertions(+), 8 deletions(-)

Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
Sean Christopherson April 10, 2025, 8:39 p.m. UTC | #2 
On Thu, Apr 10, 2025, Pawan Gupta wrote:
> The static key mmio_stale_data_clear controls the KVM-only mitigation for
> MMIO Stale Data vulnerability. Rename it to reflect its purpose.
> 
> No functional change.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/x86/include/asm/nospec-branch.h |  2 +-
>  arch/x86/kernel/cpu/bugs.c           | 16 ++++++++++------
>  arch/x86/kvm/vmx/vmx.c               |  2 +-
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index 8a5cc8e70439e10aab4eeb5b0f5e116cf635b43d..c0474e2b741737dad129159adf3b5fc056b6097c 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -561,7 +561,7 @@ DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
>  
>  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
>  
> -DECLARE_STATIC_KEY_FALSE(mmio_stale_data_clear);
> +DECLARE_STATIC_KEY_FALSE(cpu_buf_vm_clear);

Could we tack on "if_mmio" or something?  E.g. cpu_buf_vm_clear_if_mmio.  FWIW,
I don't love that name, so if anyone can come up with something better...

I like the idea of tying the static key back to X86_FEATURE_CLEAR_CPU_BUF, but
when looking at just the usage in KVM, "cpu_buf_vm_clear" doesn't provide any
hints as to when/why KVM needs to clear buffers.
Pawan Gupta April 10, 2025, 10:13 p.m. UTC | #3 
On Thu, Apr 10, 2025 at 01:39:28PM -0700, Sean Christopherson wrote:
> On Thu, Apr 10, 2025, Pawan Gupta wrote:
> > The static key mmio_stale_data_clear controls the KVM-only mitigation for
> > MMIO Stale Data vulnerability. Rename it to reflect its purpose.
> > 
> > No functional change.
> > 
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > ---
> >  arch/x86/include/asm/nospec-branch.h |  2 +-
> >  arch/x86/kernel/cpu/bugs.c           | 16 ++++++++++------
> >  arch/x86/kvm/vmx/vmx.c               |  2 +-
> >  3 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > index 8a5cc8e70439e10aab4eeb5b0f5e116cf635b43d..c0474e2b741737dad129159adf3b5fc056b6097c 100644
> > --- a/arch/x86/include/asm/nospec-branch.h
> > +++ b/arch/x86/include/asm/nospec-branch.h
> > @@ -561,7 +561,7 @@ DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
> >  
> >  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
> >  
> > -DECLARE_STATIC_KEY_FALSE(mmio_stale_data_clear);
> > +DECLARE_STATIC_KEY_FALSE(cpu_buf_vm_clear);
> 
> Could we tack on "if_mmio" or something?  E.g. cpu_buf_vm_clear_if_mmio.  FWIW,
> I don't love that name, so if anyone can come up with something better...

Keeping it generic has an advantage that it plays nicely with "Attack vector
controls" series:

  https://lore.kernel.org/lkml/20250310164023.779191-1-david.kaplan@amd.com/

The idea being to allow mitigations to be enabled/disabled based on
user-defined threat model. MDS/TAA mitigations may be able to take
advantage this KVM-only control.

> I like the idea of tying the static key back to X86_FEATURE_CLEAR_CPU_BUF, but
> when looking at just the usage in KVM, "cpu_buf_vm_clear" doesn't provide any
> hints as to when/why KVM needs to clear buffers.

Thats fair, can we cover that with a comment like below:

---
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c79720aad3df..cddad4a6eb46 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7358,6 +7358,10 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
 	 * mitigation for MDS is done late in VMentry and is still
 	 * executed in spite of L1D Flush. This is because an extra VERW
 	 * should not matter much after the big hammer L1D Flush.
+	 *
+	 * cpu_buf_vm_clear is used when system is not vulnerable to MDS/TAA,
+	 * but is affected by MMIO Stale Data that only needs mitigation
+	 * against a rogue guest.
 	 */
 	if (static_branch_unlikely(&vmx_l1d_should_flush))
 		vmx_l1d_flush(vcpu);
Sean Christopherson April 10, 2025, 11:35 p.m. UTC | #4 
On Thu, Apr 10, 2025, Pawan Gupta wrote:
> On Thu, Apr 10, 2025 at 01:39:28PM -0700, Sean Christopherson wrote:
> > On Thu, Apr 10, 2025, Pawan Gupta wrote:
> > > The static key mmio_stale_data_clear controls the KVM-only mitigation for
> > > MMIO Stale Data vulnerability. Rename it to reflect its purpose.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > > ---
> > >  arch/x86/include/asm/nospec-branch.h |  2 +-
> > >  arch/x86/kernel/cpu/bugs.c           | 16 ++++++++++------
> > >  arch/x86/kvm/vmx/vmx.c               |  2 +-
> > >  3 files changed, 12 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > > index 8a5cc8e70439e10aab4eeb5b0f5e116cf635b43d..c0474e2b741737dad129159adf3b5fc056b6097c 100644
> > > --- a/arch/x86/include/asm/nospec-branch.h
> > > +++ b/arch/x86/include/asm/nospec-branch.h
> > > @@ -561,7 +561,7 @@ DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
> > >  
> > >  DECLARE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
> > >  
> > > -DECLARE_STATIC_KEY_FALSE(mmio_stale_data_clear);
> > > +DECLARE_STATIC_KEY_FALSE(cpu_buf_vm_clear);
> > 
> > Could we tack on "if_mmio" or something?  E.g. cpu_buf_vm_clear_if_mmio.  FWIW,
> > I don't love that name, so if anyone can come up with something better...
> 
> Keeping it generic has an advantage that it plays nicely with "Attack vector
> controls" series:
> 
>   https://lore.kernel.org/lkml/20250310164023.779191-1-david.kaplan@amd.com/
> 
> The idea being to allow mitigations to be enabled/disabled based on
> user-defined threat model. MDS/TAA mitigations may be able to take
> advantage this KVM-only control.
> 
> > I like the idea of tying the static key back to X86_FEATURE_CLEAR_CPU_BUF, but
> > when looking at just the usage in KVM, "cpu_buf_vm_clear" doesn't provide any
> > hints as to when/why KVM needs to clear buffers.
> 
> Thats fair, can we cover that with a comment like below:
> 
> ---
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index c79720aad3df..cddad4a6eb46 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7358,6 +7358,10 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>  	 * mitigation for MDS is done late in VMentry and is still
>  	 * executed in spite of L1D Flush. This is because an extra VERW
>  	 * should not matter much after the big hammer L1D Flush.
> +	 *
> +	 * cpu_buf_vm_clear is used when system is not vulnerable to MDS/TAA,
> +	 * but is affected by MMIO Stale Data that only needs mitigation
> +	 * against a rogue guest.

Would this be accurate?

	 * Clear CPU buffers if the vCPU has access to host MMIO, and the CPU
	 * is NOT vulnerable to MSD/TAA but is affected by MMIO Stale Data, as
	 * indicated by cpu_buf_vm_clear.  If the CPU is vulnerable to MDS/TAA,
	 * CPU buffers will be cleared just before every VM-Entry.

>  	 */
>  	if (static_branch_unlikely(&vmx_l1d_should_flush))
>  		vmx_l1d_flush(vcpu);
Pawan Gupta April 11, 2025, 12:17 a.m. UTC | #5 
On Thu, Apr 10, 2025 at 04:35:58PM -0700, Sean Christopherson wrote:
> > > I like the idea of tying the static key back to X86_FEATURE_CLEAR_CPU_BUF, but
> > > when looking at just the usage in KVM, "cpu_buf_vm_clear" doesn't provide any
> > > hints as to when/why KVM needs to clear buffers.
> > 
> > Thats fair, can we cover that with a comment like below:
> > 
> > ---
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index c79720aad3df..cddad4a6eb46 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7358,6 +7358,10 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
> >  	 * mitigation for MDS is done late in VMentry and is still
> >  	 * executed in spite of L1D Flush. This is because an extra VERW
> >  	 * should not matter much after the big hammer L1D Flush.
> > +	 *
> > +	 * cpu_buf_vm_clear is used when system is not vulnerable to MDS/TAA,
> > +	 * but is affected by MMIO Stale Data that only needs mitigation
> > +	 * against a rogue guest.
> 
> Would this be accurate?

I believe this is accurate as things stand today. But, when Attack Vector
Control series leverages cpu_buf_vm_clear, the comment also needs to be
updated.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 8a5cc8e70439e10aab4eeb5b0f5e116cf635b43d..c0474e2b741737dad129159adf3b5fc056b6097c 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -561,7 +561,7 @@  DECLARE_STATIC_KEY_FALSE(mds_idle_clear);
 
 DECLARE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
 
-DECLARE_STATIC_KEY_FALSE(mmio_stale_data_clear);
+DECLARE_STATIC_KEY_FALSE(cpu_buf_vm_clear);
 
 extern u16 mds_verw_sel;
 
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4386aa6c69e12c9a8d66758e9f7cfff816ccbbe3..dcf029fed3beec38a2e8b6292ec7d0660f3ec678 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -128,9 +128,13 @@  EXPORT_SYMBOL_GPL(mds_idle_clear);
  */
 DEFINE_STATIC_KEY_FALSE(switch_mm_cond_l1d_flush);
 
-/* Controls CPU Fill buffer clear before KVM guest MMIO accesses */
-DEFINE_STATIC_KEY_FALSE(mmio_stale_data_clear);
-EXPORT_SYMBOL_GPL(mmio_stale_data_clear);
+/*
+ * Controls CPU Fill buffer clear before VMenter. This is a subset of
+ * X86_FEATURE_CLEAR_CPU_BUF, and should only be enabled when KVM-only
+ * mitigation is required
+ */
+DEFINE_STATIC_KEY_FALSE(cpu_buf_vm_clear);
+EXPORT_SYMBOL_GPL(cpu_buf_vm_clear);
 
 void __init cpu_select_mitigations(void)
 {
@@ -450,9 +454,9 @@  static void __init mmio_select_mitigation(void)
 	 * mitigations, disable KVM-only mitigation in that case.
 	 */
 	if (boot_cpu_has(X86_FEATURE_CLEAR_CPU_BUF))
-		static_branch_disable(&mmio_stale_data_clear);
+		static_branch_disable(&cpu_buf_vm_clear);
 	else
-		static_branch_enable(&mmio_stale_data_clear);
+		static_branch_enable(&cpu_buf_vm_clear);
 
 	/*
 	 * If Processor-MMIO-Stale-Data bug is present and Fill Buffer data can
@@ -572,7 +576,7 @@  static void __init md_clear_update_mitigation(void)
 		taa_select_mitigation();
 	}
 	/*
-	 * MMIO_MITIGATION_OFF is not checked here so that mmio_stale_data_clear
+	 * MMIO_MITIGATION_OFF is not checked here so that cpu_buf_vm_clear
 	 * gets updated correctly as per X86_FEATURE_CLEAR_CPU_BUF state.
 	 */
 	if (boot_cpu_has_bug(X86_BUG_MMIO_STALE_DATA)) {
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5c5766467a61d434ba2baa79a5faba99bcbd9997..c79720aad3df265ec8060dfe754bc816104f8c7b 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7361,7 +7361,7 @@  static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
 	 */
 	if (static_branch_unlikely(&vmx_l1d_should_flush))
 		vmx_l1d_flush(vcpu);
-	else if (static_branch_unlikely(&mmio_stale_data_clear) &&
+	else if (static_branch_unlikely(&cpu_buf_vm_clear) &&
 		 kvm_arch_has_assigned_device(vcpu->kvm))
 		mds_clear_cpu_buffers();