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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Nested DR6 fixed bits are manually calculated incorrectly, breaking architectural invariants for Bus Lock Detect.
- [Medium] L1 guests can bypass CPUID limits and enable Bus Lock Detect for L2 nested guests via the nested VMCB, even if X86_FEATURE_BUS_LOCK_DETECT is disabled for L1.
--

commit 859bd7557c537342acbc2ffe091acc01a0a180b9
Author: Ravi Bangoria <ravi.bangoria@amd.com>

KVM: SVM: Add Bus Lock Detect support

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.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index c1485c3e691c4..4fdc58d38afe0 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;

[Severity: Medium]
Will this assignment cause an L2 guest to incorrectly observe DR6 bit 11 as 0
if the guest lacks the BUS_LOCK_DETECT feature?

By bypassing kvm_dr6_fixed(vcpu) and hardcoding DR6_FIXED_1 | DR6_RTM, it
appears KVM fails to force bit 11 (DR6_BUS_LOCK) to 1. If an untrusted
L1 guest without X86_FEATURE_BUS_LOCK_DETECT runs an L2 guest and writes 0
to bit 11 of vmcb12->save.dr6, won't the L2 guest read DR6 and incorrectly
see bit 11 as 0 instead of 1?

>  		vmcb_mark_dirty(vmcb02, VMCB_DR);
>  	}

[ ... ]

> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 716be21fba335..c65dc3acb5d1d 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))

[Severity: Medium]
Could this redefinition allow an L1 guest to bypass CPUID limits and enable
Bus Lock Detect for L2 nested guests, even if X86_FEATURE_BUS_LOCK_DETECT
is disabled for L1?

Looking at nested_vmcb02_prepare_save() in arch/x86/kvm/svm/nested.c, it
copies the nested guest's dbgctl MSR value directly into the hardware VMCB
using this mask:

	svm_copy_lbrs(&vmcb02->save, save);
	vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;

Since this mask now allows the bus lock bit unconditionally without validating
against the L1 guest's CPUID limits, could an untrusted L1 guest with
X86_FEATURE_LBRV write DEBUGCTLMSR_BUS_LOCK_DETECT to vmcb12->save.dbgctl
and successfully enable the feature in L2?

>  
>  /* svm.c */
>  extern bool dump_invalid_vmcb;

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