[PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Shivansh Dhiman]

Bus Lock Detect (BLD) is a CPU feature that raises a #DB trap when an
instruction acquires a bus lock, allowing software to detect and act on bus
locks. It is enabled via MSR_IA32_DEBUGCTLMSR bit 2.

This series adds BLD support to AMD SVM and exposes the capability to the
guest. On AMD, MSR_IA32_DEBUGCTLMSR is virtualized only when LBR
Virtualization (LBRV) is enabled, so BLD depends on LBRV. Enabling that
dependency cleanly required first refactoring the LBRV handling, so the bulk
of the series is preparatory:

  1: Refactor svm_update_lbrv() so new LBRV dependencies are trivial to add.
  2: Sanitize V_LBR in the nested control cache and drop the now-redundant
     X86_FEATURE_LBRV checks.
  3: Use kvm_dr6_fixed() for nested DR6 so DR6_BUS_LOCK (bit 11) is forced
     per guest CPUID instead of unconditionally.
  4: Turn the DEBUGCTL_RESERVED_BITS macro into a per-vCPU helper (prep).
  5: Add Bus Lock Detect support, wiring BLD to LBRV, gating
     DEBUGCTLMSR_BUS_LOCK_DETECT on guest CPUID, and exposing the cap.

Regards,
Shivansh

---
Changelog:
v3 -> v4:
 * Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled() (Yosry).
 * Drop the redundant !lbrv check and the extra comment (Yosry).
 * Spell out in the changelog what breaks when DR6_BUS_LOCK is forced to 1
   (Nikunj).
 * Make it a pure NFC macro-to-helper conversion, and move the
   BUS_LOCK_DETECT gating to patch 5 (Nikunj).
 * Fold in the DEBUGCTLMSR_BUS_LOCK_DETECT gating moved from patch 4.
 * Collect Reviewed-by tags.

v2 -> v3:
 * Reworked the single v2 patch into a prep series (patches 1-4) plus the
   feature.
 * Rewrite svm_update_lbrv() as 'if' statements so the BLD LBRV
   dependency can be added cleanly (Yosry Ahmed).
 * Sanitize V_LBR in the nested control cache like NP/GMET and drop the
   redundant X86_FEATURE_LBRV checks (Yosry Ahmed).
 * Use kvm_dr6_fixed() for nested DR6 instead of DR6_FIXED_1 | DR6_RTM.
 * Compute DEBUGCTL reserved bits per-vCPU, gating BUS_LOCK_DETECT on
   guest CPUID rather than a static macro.

v2 resend:
 * Rebased on kvm-x86-next-2026.06.24.

v1 -> v2:
 * Used guest_cpu_cap_has() instead of guest_cpuid_has().

v3: https://lore.kernel.org/kvm/20260709082953.69434-1-shivansh.dhiman@amd.com/
v2 Resend: https://lore.kernel.org/kvm/20260629081018.60618-1-shivansh.dhiman@amd.com/
v2: https://lore.kernel.org/kvm/20251121081228.426974-1-shivansh.dhiman@amd.com/
v1: https://lore.kernel.org/all/20240808062937.1149-5-ravi.bangoria@amd.com

---
Shivansh Dhiman (5):
  KVM: SVM: Refactor svm_update_lbrv()
  KVM: nSVM: Disable LBRV in nested control cache when unsupported
  KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()
  KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper
  KVM: SVM: Add Bus Lock Detect support

 arch/x86/kvm/regs.c       |  3 ++-
 arch/x86/kvm/regs.h       |  1 +
 arch/x86/kvm/svm/nested.c | 26 ++++++++++++++------------
 arch/x86/kvm/svm/svm.c    | 28 ++++++++++++++++++++--------
 arch/x86/kvm/svm/svm.h    | 13 +++++++++++--
 5 files changed, 48 insertions(+), 23 deletions(-)


base-commit: 50406d35f5635e1cc523e61409d57e851b5f5df8
-- 
2.43.0

[PATCH v4 1/5] KVM: SVM: Refactor svm_update_lbrv()

[Shivansh Dhiman]

Rewrite the enable_lbrv computation in svm_update_lbrv() as a series of
'if' statements. Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled(),
expose it, and use it instead of open-coding the nested LBRV check.

No functional change intended.

Suggested-by: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
Reviewed-by: Yosry Ahmed <yosry@kernel.org>
---
Changelog:
v3 -> v4:
 * Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled() (Yosry).
 * Collected Reviewed-by from Yosry.

v3:
 * New patch.
 * Refactor the enable_lbrv computation into 'if' statements (Yosry Ahmed).

---
 arch/x86/kvm/svm/nested.c |  8 ++++----
 arch/x86/kvm/svm/svm.c    | 10 +++++++---
 arch/x86/kvm/svm/svm.h    |  1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index c1485c3e691c..52b8d0f9b139 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -736,7 +736,7 @@ static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
 	return 0;
 }
 
-static bool nested_vmcb12_has_lbrv(struct kvm_vcpu *vcpu)
+bool nested_lbrv_enabled(struct kvm_vcpu *vcpu)
 {
 	return guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
 		(to_svm(vcpu)->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
@@ -812,7 +812,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
 		vmcb_mark_dirty(vmcb02, VMCB_DR);
 	}
 
-	if (nested_vmcb12_has_lbrv(vcpu)) {
+	if (nested_lbrv_enabled(vcpu)) {
 		/*
 		 * Reserved bits of DEBUGCTL are ignored.  Be consistent with
 		 * svm_set_msr's definition of reserved bits.
@@ -1301,7 +1301,7 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
 	if (guest_cpu_cap_has(vcpu, X86_FEATURE_NRIPS))
 		vmcb12->control.next_rip  = vmcb02->control.next_rip;
 
-	if (nested_vmcb12_has_lbrv(vcpu))
+	if (nested_lbrv_enabled(vcpu))
 		svm_copy_lbrs(&vmcb12->save, &vmcb02->save);
 
 	vmcb12->control.event_inj	  = 0;
@@ -1379,7 +1379,7 @@ void nested_svm_vmexit(struct vcpu_svm *svm)
 	if (!nested_exit_on_intr(svm))
 		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
 
-	if (!nested_vmcb12_has_lbrv(vcpu)) {
+	if (!nested_lbrv_enabled(vcpu)) {
 		svm_copy_lbrs(&vmcb01->save, &vmcb02->save);
 		vmcb_mark_dirty(vmcb01, VMCB_LBR);
 	}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index ef69a51ab27f..e9f2456982d4 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -880,9 +880,13 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	bool current_enable_lbrv = svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR;
-	bool enable_lbrv = (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR) ||
-			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
-			    (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR));
+	bool enable_lbrv = false;
+
+	if (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR)
+		enable_lbrv = true;
+
+	if (is_guest_mode(vcpu) && nested_lbrv_enabled(vcpu))
+		enable_lbrv = true;
 
 	if (enable_lbrv && !current_enable_lbrv)
 		__svm_enable_lbrv(vcpu);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 716be21fba33..d52010e4de97 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -899,6 +899,7 @@ void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
 void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
 				    struct vmcb_save_area *save);
 void nested_sync_control_from_vmcb02(struct vcpu_svm *svm);
+bool nested_lbrv_enabled(struct kvm_vcpu *vcpu);
 void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb);
 
 
-- 
2.43.0

[PATCH v4 2/5] KVM: nSVM: Disable LBRV in nested control cache when unsupported

[Shivansh Dhiman]

Clear SVM_MISC2_ENABLE_V_LBR in __nested_copy_vmcb_control_to_cache() when
the vCPU does not support LBR Virtualization. This lets the cached value be
consumed directly instead of re-checking X86_FEATURE_LBRV on every access.

Suggested-by: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
---
Changelog:
v3 -> v4:
 * Drop the redundant !lbrv check and the extra comment (Yosry).

v3:
 * New patch.
 * Sanitize V_LBR in the cache and drop the redundant X86_FEATURE_LBRV
   checks (Yosry Ahmed).

---
 arch/x86/kvm/svm/nested.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 52b8d0f9b139..8d1967f5c099 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -528,12 +528,16 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
 
 	/* Always clear misc_ctl bits that the guest cannot use */
 	to->misc_ctl = from->misc_ctl;
+	to->misc_ctl2 = from->misc_ctl2;
 	if (!guest_cpu_cap_has(vcpu, X86_FEATURE_NPT))
 		to->misc_ctl &= ~SVM_MISC_ENABLE_NP;
 
 	if (!gmet_enabled || !guest_cpu_cap_has(vcpu, X86_FEATURE_GMET))
 		to->misc_ctl &= ~SVM_MISC_ENABLE_GMET;
 
+	if (!guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV))
+		to->misc_ctl2 &= ~SVM_MISC2_ENABLE_V_LBR;
+
 	to->iopm_base_pa        = from->iopm_base_pa & PAGE_MASK;
 	to->msrpm_base_pa       = from->msrpm_base_pa & PAGE_MASK;
 	to->tsc_offset          = from->tsc_offset;
@@ -551,7 +555,6 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
 	to->event_inj_err       = from->event_inj_err;
 	to->next_rip            = from->next_rip;
 	to->nested_cr3          = from->nested_cr3;
-	to->misc_ctl2		= from->misc_ctl2;
 	to->pause_filter_count  = from->pause_filter_count;
 	to->pause_filter_thresh = from->pause_filter_thresh;
 
@@ -738,8 +741,7 @@ static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
 
 bool nested_lbrv_enabled(struct kvm_vcpu *vcpu)
 {
-	return guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
-		(to_svm(vcpu)->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
+	return to_svm(vcpu)->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR;
 }
 
 static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
-- 
2.43.0

[PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[Shivansh Dhiman]

When preparing vmcb02 for nested VMRUN, KVM ORs DR6_ACTIVE_LOW into the
guest DR6 to force the fixed bits to 1. DR6_ACTIVE_LOW forces bit 11
(DR6_BUS_LOCK) to 1 unconditionally.

DR6_BUS_LOCK is active-low (the CPU clears it to 0 to report a bus lock), so
forcing it to 1 unconditionally would prevent an L2 from ever observing a
bus lock (DR6.BLD == 0) across a nested VMRUN.

Use kvm_dr6_fixed() instead, which forces DR6_RTM and DR6_BUS_LOCK based on
the guest's CPUID. DR6_RTM is a reserved bit on AMD and is thus always set
to 1. DR6_BUS_LOCK is left writable once the guest supports Bus Lock
Detect.

Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
---
Changelog:
v3 -> v4:
 * Spell out what breaks when DR6_BUS_LOCK is forced to 1 (Nikunj).

v3:
 * New patch.
 * Use kvm_dr6_fixed() instead of open-coding DR6_FIXED_1 | DR6_RTM.

---
 arch/x86/kvm/regs.c       | 3 ++-
 arch/x86/kvm/regs.h       | 1 +
 arch/x86/kvm/svm/nested.c | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
index d2caf5a67dba..1b5dfa765140 100644
--- a/arch/x86/kvm/regs.c
+++ b/arch/x86/kvm/regs.c
@@ -764,7 +764,7 @@ void kvm_update_dr7(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_update_dr7);
 
-static u64 kvm_dr6_fixed(struct kvm_vcpu *vcpu)
+u64 kvm_dr6_fixed(struct kvm_vcpu *vcpu)
 {
 	u64 fixed = DR6_FIXED_1;
 
@@ -775,6 +775,7 @@ static u64 kvm_dr6_fixed(struct kvm_vcpu *vcpu)
 		fixed |= DR6_BUS_LOCK;
 	return fixed;
 }
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_dr6_fixed);
 
 int kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val)
 {
diff --git a/arch/x86/kvm/regs.h b/arch/x86/kvm/regs.h
index 94fd86728fed..e85c3ff3e120 100644
--- a/arch/x86/kvm/regs.h
+++ b/arch/x86/kvm/regs.h
@@ -23,6 +23,7 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3);
 int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4);
 int kvm_set_cr8(struct kvm_vcpu *vcpu, unsigned long cr8);
 int kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val);
+u64 kvm_dr6_fixed(struct kvm_vcpu *vcpu);
 unsigned long kvm_get_dr(struct kvm_vcpu *vcpu, int dr);
 unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu);
 void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw);
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 8d1967f5c099..9b1afa47c54c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -810,7 +810,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
 
 	if (unlikely(new_vmcb12 || vmcb12_is_dirty(control, VMCB_DR))) {
 		vmcb02->save.dr7 = svm->nested.save.dr7 | DR7_FIXED_1;
-		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | DR6_ACTIVE_LOW;
+		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | kvm_dr6_fixed(vcpu);
 		vmcb_mark_dirty(vmcb02, VMCB_DR);
 	}
 
-- 
2.43.0

[PATCH v4 4/5] KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper

[Shivansh Dhiman]

Replace the static DEBUGCTL_RESERVED_BITS macro with a helper,
svm_get_debugctl_reserved_bits(), and plumb the vCPU into
svm_copy_vmrun_state() so it can be passed to the helper.

The vCPU argument is currently unused (marked __maybe_unused).
It'll be used by features like Bus Lock Detect.

No functional change intended.

Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
---
Changelog:
v3 -> v4:
 * Make it a pure NFC macro-to-helper conversion, and move the
   BUS_LOCK_DETECT gating to patch 5 (Nikunj).
 * Dropped Ravi's Co-developed-by/Signed-off-by, because the
   entire BLD gating is moved to patch 5. Collected Reviewed-by
   from Nikunj.

v3:
 * New patch.
 * Replaced the static DEBUGCTL_RESERVED_BITS macro with a per-vCPU helper
   that gates the bit on guest CPUID.

---
 arch/x86/kvm/svm/nested.c | 8 ++++----
 arch/x86/kvm/svm/svm.c    | 7 +++----
 arch/x86/kvm/svm/svm.h    | 7 +++++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 9b1afa47c54c..dbde186ccb81 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -820,7 +820,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
 		 * svm_set_msr's definition of reserved bits.
 		 */
 		svm_copy_lbrs(&vmcb02->save, save);
-		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
+		vmcb02->save.dbgctl &= ~svm_get_debugctl_reserved_bits(vcpu);
 	} else {
 		svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
 	}
@@ -1203,7 +1203,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 }
 
 /* Copy state save area fields which are handled by VMRUN */
-void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
+void svm_copy_vmrun_state(struct kvm_vcpu *vcpu, struct vmcb_save_area *to_save,
 			  struct vmcb_save_area *from_save)
 {
 	to_save->es = from_save->es;
@@ -1230,7 +1230,7 @@ void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
 
 	if (kvm_cpu_cap_has(X86_FEATURE_LBRV)) {
 		svm_copy_lbrs(to_save, from_save);
-		to_save->dbgctl &= ~DEBUGCTL_RESERVED_BITS;
+		to_save->dbgctl &= ~svm_get_debugctl_reserved_bits(vcpu);
 	}
 }
 
@@ -2071,7 +2071,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 
 	svm->nested.vmcb12_gpa = kvm_state->hdr.svm.vmcb_pa;
 
-	svm_copy_vmrun_state(&svm->vmcb01.ptr->save, save);
+	svm_copy_vmrun_state(vcpu, &svm->vmcb01.ptr->save, save);
 	nested_copy_vmcb_control_to_cache(svm, ctl);
 
 	svm_switch_vmcb(svm, &svm->nested.vmcb02);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e9f2456982d4..ccf85ea79d37 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3164,7 +3164,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 			data &= ~DEBUGCTLMSR_BTF;
 		}
 
-		if (data & DEBUGCTL_RESERVED_BITS)
+		if (data & svm_get_debugctl_reserved_bits(vcpu))
 			return 1;
 
 		if (svm->vmcb->save.dbgctl == data)
@@ -5037,8 +5037,7 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, union kvm_smram *smram)
 
 	BUILD_BUG_ON(offsetof(struct vmcb, save) != 0x400);
 
-	svm_copy_vmrun_state(map_save.hva + 0x400,
-			     &svm->vmcb01.ptr->save);
+	svm_copy_vmrun_state(vcpu, map_save.hva + 0x400, &svm->vmcb01.ptr->save);
 
 	kvm_vcpu_unmap(vcpu, &map_save);
 	return 0;
@@ -5081,7 +5080,7 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const union kvm_smram *smram)
 	 * used during SMM (see svm_enter_smm())
 	 */
 
-	svm_copy_vmrun_state(&svm->vmcb01.ptr->save, map_save.hva + 0x400);
+	svm_copy_vmrun_state(vcpu, &svm->vmcb01.ptr->save, map_save.hva + 0x400);
 
 	/*
 	 * Enter the nested guest now
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index d52010e4de97..696f1b4b8f8f 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -783,7 +783,10 @@ BUILD_SVM_MSR_BITMAP_HELPERS(bool, test, test)
 BUILD_SVM_MSR_BITMAP_HELPERS(void, clear, __clear)
 BUILD_SVM_MSR_BITMAP_HELPERS(void, set, __set)
 
-#define DEBUGCTL_RESERVED_BITS (~DEBUGCTLMSR_LBR)
+static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu __maybe_unused)
+{
+	return ~DEBUGCTLMSR_LBR;
+}
 
 /* svm.c */
 extern bool dump_invalid_vmcb;
@@ -873,7 +876,7 @@ void svm_leave_nested(struct kvm_vcpu *vcpu);
 void svm_free_nested(struct vcpu_svm *svm);
 int svm_allocate_nested(struct vcpu_svm *svm);
 int nested_svm_vmrun(struct kvm_vcpu *vcpu);
-void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
+void svm_copy_vmrun_state(struct kvm_vcpu *vcpu, struct vmcb_save_area *to_save,
 			  struct vmcb_save_area *from_save);
 void svm_copy_vmloadsave_state(struct vmcb *to_vmcb, struct vmcb *from_vmcb);
 void nested_svm_vmexit(struct vcpu_svm *svm);
-- 
2.43.0

[PATCH v4 5/5] KVM: SVM: Add Bus Lock Detect support

[Shivansh Dhiman]

Add Bus Lock Detect support in AMD SVM. Bus Lock Detect is enabled through
MSR_IA32_DEBUGCTLMSR and MSR_IA32_DEBUGCTLMSR is virtualized only if LBR
Virtualization is enabled. Add this dependency in the SVM.

Also, allow DEBUGCTLMSR_BUS_LOCK_DETECT only when the guest supports Bus
Lock Detect.

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Co-developed-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
---
Changelog:
v3 -> v4:
 * Fold in the DEBUGCTLMSR_BUS_LOCK_DETECT gating moved from patch 4.

v2 -> v3:
 * Split refactor and prep changes out into patches 1-4; this patch now
   only wires up the LBRV dependency and exposes the capability.

v2 Resend:
 * Rebased on top of tag: kvm-x86-next-2026.06.24.

v1 -> v2:
 * Rebased and used guest_cpu_cap_has() instead of guest_cpuid_has().

 v2 Resend: https://lore.kernel.org/kvm/20260629081018.60618-1-shivansh.dhiman@amd.com/
 v2: https://lore.kernel.org/kvm/20251121081228.426974-1-shivansh.dhiman@amd.com/
 v1: https://lore.kernel.org/all/20240808062937.1149-5-ravi.bangoria@amd.com
---
 arch/x86/kvm/svm/svm.c | 13 +++++++++++--
 arch/x86/kvm/svm/svm.h |  9 +++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index ccf85ea79d37..b0f217721873 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -882,7 +882,8 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
 	bool current_enable_lbrv = svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR;
 	bool enable_lbrv = false;
 
-	if (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR)
+	/* Bus Lock Detect in guest depends on LBR Virtualization */
+	if (svm->vmcb->save.dbgctl & (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
 		enable_lbrv = true;
 
 	if (is_guest_mode(vcpu) && nested_lbrv_enabled(vcpu))
@@ -5594,9 +5595,17 @@ static __init void svm_set_cpu_caps(void)
 	 * Clear capabilities that are automatically configured by common code,
 	 * but that require explicit SVM support (that isn't yet implemented).
 	 */
-	kvm_cpu_cap_clear(X86_FEATURE_BUS_LOCK_DETECT);
 	kvm_cpu_cap_clear(X86_FEATURE_MSR_IMM);
 
+	/*
+	 * LBR Virtualization must be enabled to support BusLockTrap inside the
+	 * guest, since BusLockTrap is enabled through MSR_IA32_DEBUGCTLMSR and
+	 * MSR_IA32_DEBUGCTLMSR is virtualized only if LBR Virtualization is
+	 * enabled.
+	 */
+	if (!lbrv)
+		kvm_cpu_cap_clear(X86_FEATURE_BUS_LOCK_DETECT);
+
 	kvm_setup_xss_caps();
 	kvm_finalize_cpu_caps();
 }
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 696f1b4b8f8f..1d1ff6ed1049 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -783,9 +783,14 @@ BUILD_SVM_MSR_BITMAP_HELPERS(bool, test, test)
 BUILD_SVM_MSR_BITMAP_HELPERS(void, clear, __clear)
 BUILD_SVM_MSR_BITMAP_HELPERS(void, set, __set)
 
-static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu __maybe_unused)
+static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu)
 {
-	return ~DEBUGCTLMSR_LBR;
+	u64 debugctl = DEBUGCTLMSR_LBR;
+
+	if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
+		debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;
+
+	return ~debugctl;
 }
 
 /* svm.c */
-- 
2.43.0

Re: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] kvm_deliver_exception_payload() incorrectly clears previously accumulated active-low DR6 events (such as DR6_BUS_LOCK and DR6_RTM) upon subsequent #DB exceptions.
--

--- Patch [3]: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed() ---
commit eaefa84a8068404a2e75f11446b9fa345fef2bae
Author: Shivansh Dhiman <shivansh.dhiman@amd.com>

KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

When preparing vmcb02 for nested VMRUN, KVM ORs DR6_ACTIVE_LOW into the
guest DR6 to force the fixed bits to 1. DR6_ACTIVE_LOW forces bit 11
(DR6_BUS_LOCK) to 1 unconditionally.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 8d1967f5c099f..9b1afa47c54c9 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -810,7 +810,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  
>  	if (unlikely(new_vmcb12 || vmcb12_is_dirty(control, VMCB_DR))) {
>  		vmcb02->save.dr7 = svm->nested.save.dr7 | DR7_FIXED_1;
> -		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | DR6_ACTIVE_LOW;
> +		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | kvm_dr6_fixed(vcpu);

[Severity: High]
This isn't a bug introduced by this patch, but kvm_deliver_exception_payload()
in arch/x86/kvm/x86.c has the exact same flaw with DR6_ACTIVE_LOW.

Could it be fixed using the newly exported kvm_dr6_fixed() helper?

Currently, kvm_deliver_exception_payload() executes:

    case DB_VECTOR:
        ...
        vcpu->arch.dr6 |= DR6_ACTIVE_LOW;
        vcpu->arch.dr6 |= ex->payload;
        vcpu->arch.dr6 ^= ex->payload & DR6_ACTIVE_LOW;

This unconditionally forces all active-low bits to 1 before applying the
current exception payload, destroying any previously accumulated 0 bits.

If a guest OS relies on the Bus Lock Detect #DB to mitigate DoS attacks,
and another #DB event (like a Single Step) occurs before the guest clears
DR6, kvm_deliver_exception_payload() will force DR6 bit 11 back to 1.

Because the new #DB payload does not have the bus lock bit set, the
subsequent XOR leaves bit 11 as 1.

Does this destroy the accumulated event and allow a malicious task to evade
detection?

Could this be updated to use kvm_dr6_fixed(vcpu) instead of DR6_ACTIVE_LOW
to avoid clearing previously accumulated active-low DR6 events?

>  		vmcb_mark_dirty(vmcb02, VMCB_DR);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721050600.87268-1-shivansh.dhiman@amd.com?part=3

Re: [PATCH v4 1/5] KVM: SVM: Refactor svm_update_lbrv()

[Nikunj A. Dadhania]

On 7/21/2026 10:35 AM, Shivansh Dhiman wrote:
> Rewrite the enable_lbrv computation in svm_update_lbrv() as a series of
> 'if' statements. Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled(),
> expose it, and use it instead of open-coding the nested LBRV check.
> 
> No functional change intended.
> 
> Suggested-by: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>

Re: [PATCH v4 1/5] KVM: SVM: Refactor svm_update_lbrv()

[Shivansh Dhiman]

On 21-07-26 12:10, Nikunj A. Dadhania wrote:
> 
> 
> On 7/21/2026 10:35 AM, Shivansh Dhiman wrote:
>> Rewrite the enable_lbrv computation in svm_update_lbrv() as a series of
>> 'if' statements. Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled(),
>> expose it, and use it instead of open-coding the nested LBRV check.
>>
>> No functional change intended.
>>
>> Suggested-by: Yosry Ahmed <yosry@kernel.org>
>> Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
>> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
> 
> Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> 

Thanks for reviewing my patch.

Regards,
Shivansh

Re: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[Sean Christopherson]

On Tue, Jul 21, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 8d1967f5c099f..9b1afa47c54c9 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -810,7 +810,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> >  
> >  	if (unlikely(new_vmcb12 || vmcb12_is_dirty(control, VMCB_DR))) {
> >  		vmcb02->save.dr7 = svm->nested.save.dr7 | DR7_FIXED_1;
> > -		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | DR6_ACTIVE_LOW;
> > +		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | kvm_dr6_fixed(vcpu);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but kvm_deliver_exception_payload()
> in arch/x86/kvm/x86.c has the exact same flaw with DR6_ACTIVE_LOW.
> 
> Could it be fixed using the newly exported kvm_dr6_fixed() helper?
> 
> Currently, kvm_deliver_exception_payload() executes:
> 
>     case DB_VECTOR:
>         ...
>         vcpu->arch.dr6 |= DR6_ACTIVE_LOW;
>         vcpu->arch.dr6 |= ex->payload;
>         vcpu->arch.dr6 ^= ex->payload & DR6_ACTIVE_LOW;
> 
> This unconditionally forces all active-low bits to 1 before applying the
> current exception payload, destroying any previously accumulated 0 bits.
> 
> If a guest OS relies on the Bus Lock Detect #DB to mitigate DoS attacks,
> and another #DB event (like a Single Step) occurs before the guest clears
> DR6, kvm_deliver_exception_payload() will force DR6 bit 11 back to 1.
> 
> Because the new #DB payload does not have the bus lock bit set, the
> subsequent XOR leaves bit 11 as 1.
> 
> Does this destroy the accumulated event and allow a malicious task to evade
> detection?
> 
> Could this be updated to use kvm_dr6_fixed(vcpu) instead of DR6_ACTIVE_LOW
> to avoid clearing previously accumulated active-low DR6 events?

No, the whole thing needs to be rewritten.  The issue isn't active-low vs. fixed-1,
it's that the semantics of each bit are feature specific, i.e. don't correlate with
active-high vs. active-low.

Re: [PATCH v4 1/5] KVM: SVM: Refactor svm_update_lbrv()

[Sean Christopherson]

The shortlog is effectively "do work".  Be more precise.

On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> Rewrite the enable_lbrv computation in svm_update_lbrv() as a series of
> 'if' statements. Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled(),
> expose it, and use it instead of open-coding the nested LBRV check.

This is super duper obviously two separate and *barely* related patches.

> No functional change intended.
> 
> Suggested-by: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
> ---
> Changelog:
> v3 -> v4:
>  * Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled() (Yosry).

I don't like this change.  Unlike nested_vgif_enabled(), this is *only* checking
the effective vmcb12.  Once the guest_cpu_cap_has() code is moved elsewhere, I
don't see any point in keeping the helper.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ef69a51ab27f..e9f2456982d4 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -880,9 +880,13 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	bool current_enable_lbrv = svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR;
> -	bool enable_lbrv = (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR) ||
> -			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
> -			    (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR));
> +	bool enable_lbrv = false;
> +
> +	if (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR)
> +		enable_lbrv = true;
> +
> +	if (is_guest_mode(vcpu) && nested_lbrv_enabled(vcpu))
> +		enable_lbrv = true;

This is begging for short-circuit logic (which was kinda the point of the existing
code).  An if-elif is silly, and I agree that squeezing everything into variable
initialization is hard to read, so I think we should do:

static bool svm_need_lbr_virtualization(struct kvm_vcpu *vcpu)
{
	struct vcpu_svm *svm = to_svm(vcpu);

	if (svm->vmcb->save.dbgctl & (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
		return true;

	return is_guest_mode(vcpu) &&
	       (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
}


		bool enable_lbrv = svm_need_lbr_virtualization(vcpu);

Re: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[Sean Christopherson]

The shortlog is again not precise enough.  With this:

  KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed

the reader doesn't actually know what behavior is being modified.  It's also way
too literal; the shortlog+changelog should strive to describe the change in human-
friendly words, e.g. in conversational language, not be a play-by-play of the code
change.  And that matters in this case, because the poorly named kvm_dr6_fixed()
makes it even hard to understand what is actually happening.

  KVM: nSVM: Don't assume all active-low bits DR6 are fixed-1

On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> When preparing vmcb02 for nested VMRUN, KVM ORs DR6_ACTIVE_LOW into the
> guest DR6 to force the fixed bits to 1. DR6_ACTIVE_LOW forces bit 11
> (DR6_BUS_LOCK) to 1 unconditionally.
> 
> DR6_BUS_LOCK is active-low (the CPU clears it to 0 to report a bus lock), so
> forcing it to 1 unconditionally would prevent an L2 from ever observing a
> bus lock (DR6.BLD == 0) across a nested VMRUN.
> 
> Use kvm_dr6_fixed() instead, which forces DR6_RTM and DR6_BUS_LOCK based on

We should kill off DR6_FIXED_1 and rename kvm_dr6_fixed() to kvm_get_dr6_fixed_1()
as prep patches.

As above, the changelog is too much of a play-by-play.  The names of the macros
don't matter, and knowing the exact bit position isn't necessary to describe and
understand the change.

  When preparing vmcb02 for nested VMRUN, force only the actual fixed-1 bits
  instead of setting all active-low bits.  The flaw is currently benign, as
  the only active-low bits supported by KVM are RTM (Restricted Transactional
  Memory) and BLD (Bus Lock Detect), neither of which is currently supported
  on SVM, but that's about to change.  I.e. this will break upcoming Bus Lock
  Detect support as the guest will never see DR6.BLD=0.

> the guest's CPUID. DR6_RTM is a reserved bit on AMD and is thus always set
> to 1. DR6_BUS_LOCK is left writable once the guest supports Bus Lock
> Detect.

Re: [PATCH v4 4/5] KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper

[Sean Christopherson]

On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> Replace the static DEBUGCTL_RESERVED_BITS macro with a helper,
> svm_get_debugctl_reserved_bits(), and plumb the vCPU into
> svm_copy_vmrun_state() so it can be passed to the helper.
> 
> The vCPU argument is currently unused (marked __maybe_unused).

There's no need to tag parameters __maybe_unused, AFAIK no compiler ever complains
about parameters, only local/global variables.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 9b1afa47c54c..dbde186ccb81 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -820,7 +820,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  		 * svm_set_msr's definition of reserved bits.
>  		 */
>  		svm_copy_lbrs(&vmcb02->save, save);
> -		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
> +		vmcb02->save.dbgctl &= ~svm_get_debugctl_reserved_bits(vcpu);
>  	} else {
>  		svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
>  	}
> @@ -1203,7 +1203,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
>  }
>  
>  /* Copy state save area fields which are handled by VMRUN */
> -void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
> +void svm_copy_vmrun_state(struct kvm_vcpu *vcpu, struct vmcb_save_area *to_save,
>  			  struct vmcb_save_area *from_save)
>  {
>  	to_save->es = from_save->es;
> @@ -1230,7 +1230,7 @@ void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
>  
>  	if (kvm_cpu_cap_has(X86_FEATURE_LBRV)) {
>  		svm_copy_lbrs(to_save, from_save);
> -		to_save->dbgctl &= ~DEBUGCTL_RESERVED_BITS;
> +		to_save->dbgctl &= ~svm_get_debugctl_reserved_bits(vcpu);
>  	}
>  }

...

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e9f2456982d4..ccf85ea79d37 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3164,7 +3164,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  			data &= ~DEBUGCTLMSR_BTF;
>  		}
>  
> -		if (data & DEBUGCTL_RESERVED_BITS)
> +		if (data & svm_get_debugctl_reserved_bits(vcpu))
>  			return 1;
>  
>  		if (svm->vmcb->save.dbgctl == data)
> @@ -5037,8 +5037,7 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, union kvm_smram *smram)
>  
>  	BUILD_BUG_ON(offsetof(struct vmcb, save) != 0x400);
>  
> -	svm_copy_vmrun_state(map_save.hva + 0x400,
> -			     &svm->vmcb01.ptr->save);
> +	svm_copy_vmrun_state(vcpu, map_save.hva + 0x400, &svm->vmcb01.ptr->save);
>  
>  	kvm_vcpu_unmap(vcpu, &map_save);
>  	return 0;
> @@ -5081,7 +5080,7 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const union kvm_smram *smram)
>  	 * used during SMM (see svm_enter_smm())
>  	 */
>  
> -	svm_copy_vmrun_state(&svm->vmcb01.ptr->save, map_save.hva + 0x400);
> +	svm_copy_vmrun_state(vcpu, &svm->vmcb01.ptr->save, map_save.hva + 0x400);
>  
>  	/*
>  	 * Enter the nested guest now
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index d52010e4de97..696f1b4b8f8f 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -783,7 +783,10 @@ BUILD_SVM_MSR_BITMAP_HELPERS(bool, test, test)
>  BUILD_SVM_MSR_BITMAP_HELPERS(void, clear, __clear)
>  BUILD_SVM_MSR_BITMAP_HELPERS(void, set, __set)
>  
> -#define DEBUGCTL_RESERVED_BITS (~DEBUGCTLMSR_LBR)
> +static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu __maybe_unused)

The polarity is cumbersome and annoyingly different than VMX, quite silly since
most paths care about supported bits, and the resulting code once BLD comes along
is downright ridiculous.  Invert the polarity so that we end up with

  static inline u64 svm_get_supported_debugctl(struct kvm_vcpu *vcpu)
  {
	u64 debugctl = DEBUGCTLMSR_LBR;

	if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
		debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;

	return debugctl;
  }


instead of:

  static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu)
  {
       u64 debugctl = DEBUGCTLMSR_LBR;

       if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
               debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;

       return ~debugctl;
  }

Re: [PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Sean Christopherson]

On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> Shivansh Dhiman (5):
>   KVM: SVM: Refactor svm_update_lbrv()
>   KVM: nSVM: Disable LBRV in nested control cache when unsupported
>   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()
>   KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper
>   KVM: SVM: Add Bus Lock Detect support

I'll send a v5 as I have variety of changes (I already responded to each patch),
and I'm not in the mood to deal with another version as I'm quite grumpy that no
one bothered to follow up on Sashiko's bug report, or to write tests.

I'll also post a KUT testcase, which I created to verify the bug Sashiko pointed
out as well as the fix. 

>  arch/x86/kvm/regs.c       |  3 ++-
>  arch/x86/kvm/regs.h       |  1 +
>  arch/x86/kvm/svm/nested.c | 26 ++++++++++++++------------
>  arch/x86/kvm/svm/svm.c    | 28 ++++++++++++++++++++--------
>  arch/x86/kvm/svm/svm.h    | 13 +++++++++++--
>  5 files changed, 48 insertions(+), 23 deletions(-)

Re: [PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Sean Christopherson]

On Fri, Sep 25, 2026, Sean Christopherson wrote:
> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> > Shivansh Dhiman (5):
> >   KVM: SVM: Refactor svm_update_lbrv()
> >   KVM: nSVM: Disable LBRV in nested control cache when unsupported
> >   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()
> >   KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper
> >   KVM: SVM: Add Bus Lock Detect support
> 
> I'll send a v5 as I have variety of changes (I already responded to each patch),
> and I'm not in the mood to deal with another version as I'm quite grumpy that no
> one bothered to follow up on Sashiko's bug report, or to write tests.
> 
> I'll also post a KUT testcase, which I created to verify the bug Sashiko pointed
> out as well as the fix. 

You know what I *love* doing on a Friday afternoon?  Debugging ucode bugs because
apparently hardware engineers are also allergic to testing.  Or maybe the APM just
sucks more than usual.   But the behavior fundamentally breaks virtualization, so
IMO it's a hardware/ucode bug.

On Turin (the only AMD hardware with Bus Lock Detect I've tested), hardware/ucode
forces DR6.BLD=1 if DEBUGCTL.BLCKDB=0, even on software writes and even on loads
of DR6 via VMRUN (this last bit is what really throws a wrench in virtualization).

Software can still read DR6.BLD=0 if DEBUGCTL.BLCKDB=0, but if anything touches
DR6, DR6.BLD gets clobbered back to '1'.  E.g. if the guest gets into a state
where DR6.BLD=0 and DEBUGCTL.BLCKDB=0, then AFAICT *any* #VMEXIT will end up
setting guest.DR6.BLD=1 on the subsequent VMRUN, which is just a wee bit problematic
because it means asynchronous #VMEXITs, e.g. for host IRQs, clobber guest state.

I'm still going to post v5 because I'm fairly confident the KVM implementation
is correct, and the bug is easy enough to workaround in the testcases, but needless
to say, I'm not happy at the moment.

Re: [PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Shivansh Dhiman]

On 26-09-26 04:31, Sean Christopherson wrote:
> On Fri, Sep 25, 2026, Sean Christopherson wrote:
>> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>>> Shivansh Dhiman (5):
>>>   KVM: SVM: Refactor svm_update_lbrv()
>>>   KVM: nSVM: Disable LBRV in nested control cache when unsupported
>>>   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()
>>>   KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper
>>>   KVM: SVM: Add Bus Lock Detect support
>>
>> I'll send a v5 as I have variety of changes (I already responded to each patch),
>> and I'm not in the mood to deal with another version as I'm quite grumpy that no
>> one bothered to follow up on Sashiko's bug report, or to write tests.
>>
>> I'll also post a KUT testcase, which I created to verify the bug Sashiko pointed
>> out as well as the fix. 
> 
> You know what I *love* doing on a Friday afternoon?  Debugging ucode bugs because
> apparently hardware engineers are also allergic to testing.  Or maybe the APM just
> sucks more than usual.   But the behavior fundamentally breaks virtualization, so
> IMO it's a hardware/ucode bug.
> 
> On Turin (the only AMD hardware with Bus Lock Detect I've tested), hardware/ucode
> forces DR6.BLD=1 if DEBUGCTL.BLCKDB=0, even on software writes and even on loads
> of DR6 via VMRUN (this last bit is what really throws a wrench in virtualization).
> 
> Software can still read DR6.BLD=0 if DEBUGCTL.BLCKDB=0, but if anything touches
> DR6, DR6.BLD gets clobbered back to '1'.  E.g. if the guest gets into a state
> where DR6.BLD=0 and DEBUGCTL.BLCKDB=0, then AFAICT *any* #VMEXIT will end up
> setting guest.DR6.BLD=1 on the subsequent VMRUN, which is just a wee bit problematic
> because it means asynchronous #VMEXITs, e.g. for host IRQs, clobber guest state.
> 
> I'm still going to post v5 because I'm fairly confident the KVM implementation
> is correct, and the bug is easy enough to workaround in the testcases, but needless
> to say, I'm not happy at the moment.

Thanks, Sean, for taking up this series and digging into this issue. We're looking
into the DR6.BLD behavior and I'll follow up with more information.

Shivansh

Re: [PATCH v4 1/5] KVM: SVM: Refactor svm_update_lbrv()

[Shivansh Dhiman]

Hi Sean,

Thanks for reviewing my series. Replying to v4.

On 25-09-26 23:01, Sean Christopherson wrote:
> The shortlog is effectively "do work".  Be more precise.

Ack.

> 
> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>> Rewrite the enable_lbrv computation in svm_update_lbrv() as a series of
>> 'if' statements. Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled(),
>> expose it, and use it instead of open-coding the nested LBRV check.
> 
> This is super duper obviously two separate and *barely* related patches.

Makes sense.

> 
>> No functional change intended.
>>
>> Suggested-by: Yosry Ahmed <yosry@kernel.org>
>> Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
>> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
>> ---
>> Changelog:
>> v3 -> v4:
>>  * Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled() (Yosry).
> 
> I don't like this change.  Unlike nested_vgif_enabled(), this is *only* checking
> the effective vmcb12.  Once the guest_cpu_cap_has() code is moved elsewhere, I
> don't see any point in keeping the helper.

Ack.

> 
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index ef69a51ab27f..e9f2456982d4 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -880,9 +880,13 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
>>  {
>>  	struct vcpu_svm *svm = to_svm(vcpu);
>>  	bool current_enable_lbrv = svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR;
>> -	bool enable_lbrv = (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR) ||
>> -			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
>> -			    (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR));
>> +	bool enable_lbrv = false;
>> +
>> +	if (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR)
>> +		enable_lbrv = true;
>> +
>> +	if (is_guest_mode(vcpu) && nested_lbrv_enabled(vcpu))
>> +		enable_lbrv = true;
> 
> This is begging for short-circuit logic (which was kinda the point of the existing
> code).  An if-elif is silly, and I agree that squeezing everything into variable
> initialization is hard to read, so I think we should do:
> 
> static bool svm_need_lbr_virtualization(struct kvm_vcpu *vcpu)
> {
> 	struct vcpu_svm *svm = to_svm(vcpu);
> 
> 	if (svm->vmcb->save.dbgctl & (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
> 		return true;
> 
> 	return is_guest_mode(vcpu) &&
> 	       (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
> }
> 
> 
> 		bool enable_lbrv = svm_need_lbr_virtualization(vcpu);

That looks cleaner. There's another series I've posted on SVM vLBRv2 which will
benefit from this refactor. Thanks.

-Shivansh

Re: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[Shivansh Dhiman]

On 25-09-26 23:09, Sean Christopherson wrote:
> The shortlog is again not precise enough.  With this:>
>   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed
> 
> the reader doesn't actually know what behavior is being modified.  It's also way
> too literal; the shortlog+changelog should strive to describe the change in human-
> friendly words, e.g. in conversational language, not be a play-by-play of the code
> change.  And that matters in this case, because the poorly named kvm_dr6_fixed()
> makes it even hard to understand what is actually happening.
> 
>   KVM: nSVM: Don't assume all active-low bits DR6 are fixed-1

That reads better. I'll keep this mind when posting any new series.

> 
> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>> When preparing vmcb02 for nested VMRUN, KVM ORs DR6_ACTIVE_LOW into the
>> guest DR6 to force the fixed bits to 1. DR6_ACTIVE_LOW forces bit 11
>> (DR6_BUS_LOCK) to 1 unconditionally.
>>
>> DR6_BUS_LOCK is active-low (the CPU clears it to 0 to report a bus lock), so
>> forcing it to 1 unconditionally would prevent an L2 from ever observing a
>> bus lock (DR6.BLD == 0) across a nested VMRUN.
>>
>> Use kvm_dr6_fixed() instead, which forces DR6_RTM and DR6_BUS_LOCK based on
> 
> We should kill off DR6_FIXED_1 and rename kvm_dr6_fixed() to kvm_get_dr6_fixed_1()
> as prep patches.
> 
> As above, the changelog is too much of a play-by-play.  The names of the macros
> don't matter, and knowing the exact bit position isn't necessary to describe and
> understand the change.
> 
>   When preparing vmcb02 for nested VMRUN, force only the actual fixed-1 bits
>   instead of setting all active-low bits.  The flaw is currently benign, as
>   the only active-low bits supported by KVM are RTM (Restricted Transactional
>   Memory) and BLD (Bus Lock Detect), neither of which is currently supported
>   on SVM, but that's about to change.  I.e. this will break upcoming Bus Lock
>   Detect support as the guest will never see DR6.BLD=0.

Thanks for the reword.

-Shivansh

> 
>> the guest's CPUID. DR6_RTM is a reserved bit on AMD and is thus always set
>> to 1. DR6_BUS_LOCK is left writable once the guest supports Bus Lock
>> Detect.

Re: [PATCH v4 4/5] KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper

[Shivansh Dhiman]

On 25-09-26 23:13, Sean Christopherson wrote:
> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>> Replace the static DEBUGCTL_RESERVED_BITS macro with a helper,
>> svm_get_debugctl_reserved_bits(), and plumb the vCPU into
>> svm_copy_vmrun_state() so it can be passed to the helper.
>>
>> The vCPU argument is currently unused (marked __maybe_unused).
> 
> There's no need to tag parameters __maybe_unused, AFAIK no compiler ever complains
> about parameters, only local/global variables.

I remember my compiler giving some warning about the unused parameters IIRC.
So, I added it to preserve clean builds between patches. Anyway, I'll check
my build environment once.

>> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
>> index d52010e4de97..696f1b4b8f8f 100644
>> --- a/arch/x86/kvm/svm/svm.h
>> +++ b/arch/x86/kvm/svm/svm.h
>> @@ -783,7 +783,10 @@ BUILD_SVM_MSR_BITMAP_HELPERS(bool, test, test)
>>  BUILD_SVM_MSR_BITMAP_HELPERS(void, clear, __clear)
>>  BUILD_SVM_MSR_BITMAP_HELPERS(void, set, __set)
>>  
>> -#define DEBUGCTL_RESERVED_BITS (~DEBUGCTLMSR_LBR)
>> +static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu __maybe_unused)
> 
> The polarity is cumbersome and annoyingly different than VMX, quite silly since
> most paths care about supported bits, and the resulting code once BLD comes along
> is downright ridiculous.  Invert the polarity so that we end up with
> 
>   static inline u64 svm_get_supported_debugctl(struct kvm_vcpu *vcpu)
>   {
> 	u64 debugctl = DEBUGCTLMSR_LBR;
> 
> 	if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
> 		debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;
> 
> 	return debugctl;
>   }

Checked the VMX side of it, and this looks better. Thanks.

-Shivansh

> 
> 
> instead of:
> 
>   static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu)
>   {
>        u64 debugctl = DEBUGCTLMSR_LBR;
> 
>        if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
>                debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;
> 
>        return ~debugctl;
>   }

Re: [PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Shivansh Dhiman]

On 25-09-26 23:15, Sean Christopherson wrote:
> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>> Shivansh Dhiman (5):
>>   KVM: SVM: Refactor svm_update_lbrv()
>>   KVM: nSVM: Disable LBRV in nested control cache when unsupported
>>   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()
>>   KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper
>>   KVM: SVM: Add Bus Lock Detect support
> 
> I'll send a v5 as I have variety of changes (I already responded to each patch),
> and I'm not in the mood to deal with another version as I'm quite grumpy that no
> one bothered to follow up on Sashiko's bug report, or to write tests.

Thanks for taking this over, and sorry for missing Sashiko's report. Ravi had
posted a KUT fix for the default DR6 value [1]. I just hadn't rebased and
resent it yet and your KUT tests cover that as well.

I ran them with v5 on AMD hardware and they all pass. I also reproduced
Sashiko's issue and confirmed that v5 fixes it.

[1] https://lore.kernel.org/all/20250224112601.6504-1-ravi.bangoria@amd.com

Regards,
Shivansh

> 
> I'll also post a KUT testcase, which I created to verify the bug Sashiko pointed
> out as well as the fix. 
> 
>>  arch/x86/kvm/regs.c       |  3 ++-
>>  arch/x86/kvm/regs.h       |  1 +
>>  arch/x86/kvm/svm/nested.c | 26 ++++++++++++++------------
>>  arch/x86/kvm/svm/svm.c    | 28 ++++++++++++++++++++--------
>>  arch/x86/kvm/svm/svm.h    | 13 +++++++++++--
>>  5 files changed, 48 insertions(+), 23 deletions(-)

Re: [PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Shivansh Dhiman]

On 26-09-26 04:31, Sean Christopherson wrote:
> On Fri, Sep 25, 2026, Sean Christopherson wrote:
>> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>>> Shivansh Dhiman (5):
>>>   KVM: SVM: Refactor svm_update_lbrv()
>>>   KVM: nSVM: Disable LBRV in nested control cache when unsupported
>>>   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()
>>>   KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper
>>>   KVM: SVM: Add Bus Lock Detect support
>>
>> I'll send a v5 as I have variety of changes (I already responded to each patch),
>> and I'm not in the mood to deal with another version as I'm quite grumpy that no
>> one bothered to follow up on Sashiko's bug report, or to write tests.
>>
>> I'll also post a KUT testcase, which I created to verify the bug Sashiko pointed
>> out as well as the fix. 
> 
> You know what I *love* doing on a Friday afternoon?  Debugging ucode bugs because
> apparently hardware engineers are also allergic to testing.  Or maybe the APM just
> sucks more than usual.   But the behavior fundamentally breaks virtualization, so
> IMO it's a hardware/ucode bug.
> 
> On Turin (the only AMD hardware with Bus Lock Detect I've tested), hardware/ucode
> forces DR6.BLD=1 if DEBUGCTL.BLCKDB=0, even on software writes and even on loads
> of DR6 via VMRUN (this last bit is what really throws a wrench in virtualization).
> 
> Software can still read DR6.BLD=0 if DEBUGCTL.BLCKDB=0, but if anything touches
> DR6, DR6.BLD gets clobbered back to '1'.  E.g. if the guest gets into a state
> where DR6.BLD=0 and DEBUGCTL.BLCKDB=0, then AFAICT *any* #VMEXIT will end up
> setting guest.DR6.BLD=1 on the subsequent VMRUN, which is just a wee bit problematic
> because it means asynchronous #VMEXITs, e.g. for host IRQs, clobber guest state.

I reproduced this on Turin with a small KUT test and a trace_printk() of
VMCB.DR6/DEBUGCTL around VMRUN. In each log, the first #VMEXIT (0x26) is
the read_dr6(), which re-executes after the VMRUN.


(1) BLCKDB=1: DR6.BLD=0 is preserved

	wrmsr(DEBUGCTL, BLCKDB);
	write_dr6(0xffff07f0);		/* DR6.BLD=0 */
	cpuid(0);			/* unrelated #VMEXIT */
	read_dr6();			/* -> ffff07f0 */

  #VMEXIT: exit_code=26, vmcb dr6=ffff0ff0
  VMRUN:   vmcb dr6=ffff07f0, dbgctl=4, v_lbr=1
  #VMEXIT: exit_code=7b, vmcb dr6=ffff07f0

As expected.


(2) BLCKDB=0: DR6.BLD=0 is lost

	/* KUT test continued from above */
	write_dr6(0xffff07f0);		/* DR6.BLD=0 */
	wrmsr(DEBUGCTL, 0);
	cpuid(0);			/* unrelated #VMEXIT */
	read_dr6();			/* -> ffff0ff0 */

  #VMEXIT: exit_code=26, vmcb dr6=ffff0ff0
  VMRUN:   vmcb dr6=ffff07f0, dbgctl=0, v_lbr=0
  #VMEXIT: exit_code=7b, vmcb dr6=ffff07f0

KVM loads BLD=0, yet the guest reads BLD=1, so KVM isn't the one setting it.
The VMCB still shows BLD=0 at the next #VMEXIT, though.


(3) BLCKDB=1 again: DR6.BLD=0 comes back

	/* KUT test continued from above */
	wrmsr(DEBUGCTL, BLCKDB);
	read_dr6();			/* -> ffff07f0 */

  #VMEXIT: exit_code=26, vmcb dr6=ffff0ff0
  VMRUN:   vmcb dr6=ffff07f0, dbgctl=4, v_lbr=1
  #VMEXIT: exit_code=7b, vmcb dr6=ffff07f0

At the last #VMEXIT in (2), KVM saved VMCB.DR6 (BLD=0) into its copy of the
guest's DR6. Here it reloads that copy, now with BLCKDB=1, and the guest
sees BLD=0 again. So in this sequence BLD=0 was "hidden" from the guest while
BLCKDB=0, but survived in KVM's saved state.

---

I wanted to understand the practical side better, since I may well be missing
something. My reading of APM 13.1.3.6 is that the processor clears DR6.BLD
only for a bus lock #DB (so only while BLCKDB=1), and that system software
then sets it back to 1 before returning to the interrupted task. I'm not
sure if any software is intended to work by clearing DR6.BLD themselves.

Are there any flows where a guest can end up with DR6.BLD=0 and BLCKDB=0? That
would help me understand where this bites.

-Shivansh

> 
> I'm still going to post v5 because I'm fairly confident the KVM implementation
> is correct, and the bug is easy enough to workaround in the testcases, but needless
> to say, I'm not happy at the moment.

Re: [PATCH v4 0/5] KVM: SVM: Add Bus Lock Detect support and refactor LBRV

[Sean Christopherson]

On Fri, Oct 02, 2026, Shivansh Dhiman wrote:
> On 26-09-26 04:31, Sean Christopherson wrote:
> (2) BLCKDB=0: DR6.BLD=0 is lost
> 
> 	/* KUT test continued from above */
> 	write_dr6(0xffff07f0);		/* DR6.BLD=0 */
> 	wrmsr(DEBUGCTL, 0);
> 	cpuid(0);			/* unrelated #VMEXIT */
> 	read_dr6();			/* -> ffff0ff0 */
> 
>   #VMEXIT: exit_code=26, vmcb dr6=ffff0ff0
>   VMRUN:   vmcb dr6=ffff07f0, dbgctl=0, v_lbr=0
>   #VMEXIT: exit_code=7b, vmcb dr6=ffff07f0
> 
> KVM loads BLD=0, yet the guest reads BLD=1, so KVM isn't the one setting it.
> The VMCB still shows BLD=0 at the next #VMEXIT, though.

Yep.  This is why I view this as a hardware/ucode flaw.

> I wanted to understand the practical side better, since I may well be missing
> something. My reading of APM 13.1.3.6 is that the processor clears DR6.BLD
> only for a bus lock #DB (so only while BLCKDB=1), and that system software
> then sets it back to 1 before returning to the interrupted task. I'm not
> sure if any software is intended to work by clearing DR6.BLD themselves.

It has to, because that's effectively what VMRUN does by loading DR6.  It's not
as straightforward as MOV DR6 (obviously), but for all intents and purposes it's
still a software write.

> Are there any flows where a guest can end up with DR6.BLD=0 and BLCKDB=0? That
> would help me understand where this bites.

In practice, I doubt it will be problematic.  But it's not completely absurd that
software might do something like temporarily disable DEBUGCTL.BUS_LOCK at the
start of its #DB handler, before reading DR6.  That would work on bare metal and
*usually* work in a VM, but would cause the guest to "miss" a Bus Lock #DB if a
#VMEXIT hit between disabling DEBUGCTL.BUS_LOCK and reading DR6.