linux-kernel - Re: [PATCH v6 8/8] KVM: VMX: Preserve host's DEBUGCTLMSR_FREEZE_IN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF1yni8U6XNkyfRf@google.com>
Date: Thu, 26 Jun 2025 09:17:34 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: mlevitsk@...hat.com
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH v6 8/8] KVM: VMX: Preserve host's DEBUGCTLMSR_FREEZE_IN_SMM
 while running the guest

On Tue, Jun 24, 2025, mlevitsk@...hat.com wrote:
> On Tue, 2025-06-10 at 16:20 -0700, Sean Christopherson wrote:
> > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> > index c20a4185d10a..076af78af151 100644
> > --- a/arch/x86/kvm/vmx/vmx.h
> > +++ b/arch/x86/kvm/vmx/vmx.h
> > @@ -419,12 +419,25 @@ bool vmx_is_valid_debugctl(struct kvm_vcpu *vcpu, u64 data, bool host_initiated)
> >  
> >  static inline void vmx_guest_debugctl_write(struct kvm_vcpu *vcpu, u64 val)
> >  {
> > +	WARN_ON_ONCE(val & DEBUGCTLMSR_FREEZE_IN_SMM);
> > +
> > +	val |= vcpu->arch.host_debugctl & DEBUGCTLMSR_FREEZE_IN_SMM;
> >  	vmcs_write64(GUEST_IA32_DEBUGCTL, val);
> >  }
> >  
> >  static inline u64 vmx_guest_debugctl_read(void)
> >  {
> > -	return vmcs_read64(GUEST_IA32_DEBUGCTL);
> > +	return vmcs_read64(GUEST_IA32_DEBUGCTL) & ~DEBUGCTLMSR_FREEZE_IN_SMM;
> > +}
> > +
> > +static inline void vmx_reload_guest_debugctl(struct kvm_vcpu *vcpu)
> > +{
> > +	u64 val = vmcs_read64(GUEST_IA32_DEBUGCTL);
> > +
> > +	if (!((val ^ vcpu->arch.host_debugctl) & DEBUGCTLMSR_FREEZE_IN_SMM))
> > +		return;
> > +
> > +	vmx_guest_debugctl_write(vcpu, val & ~DEBUGCTLMSR_FREEZE_IN_SMM);
> >  }
> 
> 
> Wouldn't it be better to use kvm_x86_ops.HOST_OWNED_DEBUGCTL here as well
> to avoid logic duplication?

Hmm, yeah.  I used DEBUGCTLMSR_FREEZE_IN_SMM directly to avoid a memory load
just to get at a constant literal.

What about this?  It doesn't completely dedup the logic, but I think it gets us
close enough to a single source of truth.

--
From: Sean Christopherson <seanjc@...gle.com>
Date: Thu, 26 Jun 2025 09:14:20 -0700
Subject: [PATCH] KVM: VMX: Add a macro to track which DEBUGCTL bits are
 host-owned

Add VMX_HOST_OWNED_DEBUGCTL_BITS to track which bits are host-owned, i.e.
need to be preserved when running the guest, to dedup the logic without
having to incur a memory load to get at kvm_x86_ops.HOST_OWNED_DEBUGCTL.

No functional change intended.

Suggested-by: Maxim Levitsky <mlevitsk@...hat.com>
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
 arch/x86/kvm/vmx/main.c |  2 +-
 arch/x86/kvm/vmx/vmx.h  | 12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 8c6435fdda18..dbab1c15b0cd 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -883,7 +883,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
 	.vcpu_load = vt_op(vcpu_load),
 	.vcpu_put = vt_op(vcpu_put),
 
-	.HOST_OWNED_DEBUGCTL = DEBUGCTLMSR_FREEZE_IN_SMM,
+	.HOST_OWNED_DEBUGCTL = VMX_HOST_OWNED_DEBUGCTL_BITS,
 
 	.update_exception_bitmap = vt_op(update_exception_bitmap),
 	.get_feature_msr = vmx_get_feature_msr,
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 87174d961c85..d3389baf3ab3 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -410,27 +410,29 @@ void vmx_update_cpu_dirty_logging(struct kvm_vcpu *vcpu);
 u64 vmx_get_supported_debugctl(struct kvm_vcpu *vcpu, bool host_initiated);
 bool vmx_is_valid_debugctl(struct kvm_vcpu *vcpu, u64 data, bool host_initiated);
 
+#define VMX_HOST_OWNED_DEBUGCTL_BITS	(DEBUGCTLMSR_FREEZE_IN_SMM)
+
 static inline void vmx_guest_debugctl_write(struct kvm_vcpu *vcpu, u64 val)
 {
-	WARN_ON_ONCE(val & DEBUGCTLMSR_FREEZE_IN_SMM);
+	WARN_ON_ONCE(val & VMX_HOST_OWNED_DEBUGCTL_BITS);
 
-	val |= vcpu->arch.host_debugctl & DEBUGCTLMSR_FREEZE_IN_SMM;
+	val |= vcpu->arch.host_debugctl & VMX_HOST_OWNED_DEBUGCTL_BITS;
 	vmcs_write64(GUEST_IA32_DEBUGCTL, val);
 }
 
 static inline u64 vmx_guest_debugctl_read(void)
 {
-	return vmcs_read64(GUEST_IA32_DEBUGCTL) & ~DEBUGCTLMSR_FREEZE_IN_SMM;
+	return vmcs_read64(GUEST_IA32_DEBUGCTL) & ~VMX_HOST_OWNED_DEBUGCTL_BITS;
 }
 
 static inline void vmx_reload_guest_debugctl(struct kvm_vcpu *vcpu)
 {
 	u64 val = vmcs_read64(GUEST_IA32_DEBUGCTL);
 
-	if (!((val ^ vcpu->arch.host_debugctl) & DEBUGCTLMSR_FREEZE_IN_SMM))
+	if (!((val ^ vcpu->arch.host_debugctl) & VMX_HOST_OWNED_DEBUGCTL_BITS))
 		return;
 
-	vmx_guest_debugctl_write(vcpu, val & ~DEBUGCTLMSR_FREEZE_IN_SMM);
+	vmx_guest_debugctl_write(vcpu, val & ~VMX_HOST_OWNED_DEBUGCTL_BITS);
 }
 
 /*

base-commit: 6c7ecd725e503bf2ca69ff52c6cc48bb650b1f11
--