Re: [RESEND PATCH v2] KVM: SVM: Add Bus Lock Detect support

[Shivansh Dhiman <shivansh.dhiman@amd.com>]

On 30-06-26 04:38, Yosry Ahmed wrote:
> On Mon, Jun 29, 2026 at 08:10:18AM +0000, Shivansh Dhiman wrote:
>> From: Ravi Bangoria <ravi.bangoria@amd.com>
>>
>> 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.
>>
>> While adding Bus Lock Detect support, also fix DR6 handling in nested
>> virtualization. Using DR6_FIXED_1 to prevent reset of BLD bit (bit 11)
>> between VMRUNs. However, it preserves DR6_RTM, which is a reserved bit
>> on AMD processors. So, DR6_RTM bit must always be set to 1.
>>
>> 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:
>> v2 --> v2 Resend
>>  * No functional changes.
>>  * 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: 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/nested.c |  3 ++-
>>  arch/x86/kvm/svm/svm.c    | 17 ++++++++++++++++-
>>  arch/x86/kvm/svm/svm.h    |  2 +-
>>  3 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
>> index c1485c3e691c..4fdc58d38afe 100644
>> --- a/arch/x86/kvm/svm/nested.c
>> +++ b/arch/x86/kvm/svm/nested.c
>> @@ -808,7 +808,8 @@ 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;
>> +		/* DR6_RTM is a reserved bit on AMD and as such must be set to 1 */
>> +		svm->vcpu.arch.dr6  = svm->nested.save.dr6 | DR6_FIXED_1 | DR6_RTM;
>>  		vmcb_mark_dirty(vmcb02, VMCB_DR);
>>  	}
>>  
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index ef69a51ab27f..b4b0fa730916 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -884,6 +884,9 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
>>  			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
>>  			    (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR));
>>  
>> +	/* Bus Lock Detect depends on LBR Virtualization */
>> +	enable_lbrv |= (svm->vmcb->save.dbgctl & DEBUGCTLMSR_BUS_LOCK_DETECT);
>> +
> 
> A few lines above we have:
> 
>         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));
> 
> We probably want to combine "svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR"
> with the new added check, and use nested_vmcb12_has_lbrv(). Maybe
> end up with something like this (you'll probably want to refactor in a
> separate patch):
> 
> 	bool enable_lbrv = false;
> 
> 	if (svm->vmcb->save.dbgctl & (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
> 		enable_lbrv = true;
> 
> 	if (is_guest_mode(vcpu) && nested_vmcb12_has_lbrv(vcpu))
> 		enable_lbrv = true;
> 
> 

Agreed. I'll split this into a separate refactoring patch before the
Bus Lock Detect change in v3.

> ---
> 
> Completely unrelated to this patch, but we should probably just clear
> SVM_MISC2_ENABLE_V_LBR in __nested_copy_vmcb_control_to_cache() if the
> guest vCPU doesn't have X86_FEATURE_LBRV instead of checking
> X86_FEATURE_LBRV every time, similar to SVM_MISC_ENABLE_NP
> and SVM_MISC_ENABLE_GMET.

Makes sense, I'll add that as a separate patch too.

Thanks for the review!

> 
>>  	if (enable_lbrv && !current_enable_lbrv)
>>  		__svm_enable_lbrv(vcpu);
>>  	else if (!enable_lbrv && current_enable_lbrv)
>> @@ -3160,6 +3163,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>>  			data &= ~DEBUGCTLMSR_BTF;
>>  		}
>>  
>> +		if ((data & DEBUGCTLMSR_BUS_LOCK_DETECT) &&
>> +		    !guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
>> +			return 1;
>> +
>>  		if (data & DEBUGCTL_RESERVED_BITS)
>>  			return 1;
>>  
>> @@ -5591,9 +5598,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 716be21fba33..c65dc3acb5d1 100644
>> --- a/arch/x86/kvm/svm/svm.h
>> +++ b/arch/x86/kvm/svm/svm.h
>> @@ -783,7 +783,7 @@ 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)
>> +#define DEBUGCTL_RESERVED_BITS (~(DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
>>  
>>  /* svm.c */
>>  extern bool dump_invalid_vmcb;
>>
>> base-commit: 50406d35f5635e1cc523e61409d57e851b5f5df8
>> -- 
>> 2.43.0
>>
>>