Re: [PATCH v3 4/5] KVM: SVM: Compute DEBUGCTL reserved bits dynamically

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The dynamic reserved bit check for DEBUGCTLMSR_BUS_LOCK_DETECT is dead code because KVM explicitly clears the feature from SVM's supported CPUID capabilities.
- [High] Hardware silently ignores the guest's Bus Lock Detect configuration on VMRUN unless the guest also enables LBRs, due to AMD SVM requirements for loading DEBUGCTLMSR.
- [High] Bus Lock Detect support is fundamentally broken because DEBUGCTLMSR state management is strictly gated by unrelated LBR Virtualization parameters and host capabilities.
--

--- Patch [4]: [PATCH v3 4/5] KVM: SVM: Compute DEBUGCTL reserved bits dynamically ---
commit 8b16c8234c21a39b440d6a7688ffcc624702cdf8
Author: Shivansh Dhiman <shivansh.dhiman@amd.com>

KVM: SVM: Compute DEBUGCTL reserved bits dynamically

This patch replaces the static DEBUGCTL_RESERVED_BITS macro with a
per-vCPU helper that derives reserved bits from the guest's CPUID. This
allows DEBUGCTLMSR_BUS_LOCK_DETECT when the guest supports it.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c

[ ... ]

> @@ -1231,7 +1231,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);

[Severity: High]
Does Bus Lock Detect incorrectly depend on LBR Virtualization being supported
and enabled?

Here, the guest's dbgctl is only copied if the host supports X86_FEATURE_LBRV.

Similarly, in svm_set_msr(), writes to MSR_IA32_DEBUGCTLMSR are unconditionally
rejected if the lbrv module parameter is disabled:

svm_set_msr() {
        ...
        case MSR_IA32_DEBUGCTLMSR:
                if (!lbrv) {
                        kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
                        break;
                }
        ...
}

And earlier in nested_vmcb02_prepare_save(), L2's dbgctl is replaced with
L1's dbgctl if L1 does not explicitly enable LBRV for L2:

nested_vmcb02_prepare_save() {
        ...
        if (nested_vmcb12_has_lbrv(vcpu)) {
                ...
        } else {
                svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
        }
        ...
}

Will this break Bus Lock Detect for guests when LBR features are unavailable
or disabled?

>  	}
>  }

> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h

[ ... ]

> @@ -783,7 +783,15 @@ 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)
> +{
> +	u64 debugctl = DEBUGCTLMSR_LBR;
> +
> +	if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
> +		debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;

[Severity: High]
Is it possible for a guest to actually have this feature under SVM?

In svm_set_cpu_caps(), KVM unconditionally clears the feature:

svm_set_cpu_caps() {
        ...
        kvm_cpu_cap_clear(X86_FEATURE_BUS_LOCK_DETECT);
        ...
}

Because of this, guest_cpu_cap_has() will always evaluate to false,
rendering this new logic dead code. Guest writes to enable the feature
will inject a #GP.

[Severity: High]
If the dead-code issue above is fixed and KVM accepts the write to
DEBUGCTLMSR_BUS_LOCK_DETECT, will the hardware silently ignore the guest's
configuration on VMRUN unless the guest also enables LBRs?

AMD SVM only loads DEBUGCTLMSR from the VMCB save area during VMRUN if LBR
Virtualization is enabled. In svm_update_lbrv(), KVM disables V_LBR if the
LBR bit is not set:

svm_update_lbrv() {
        ...
        if (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR)
                enable_lbrv = true;
        ...
        else if (!enable_lbrv && current_enable_lbrv)
                __svm_disable_lbrv(vcpu);
        ...
}

Could this create a state desynchronization where KVM accepts the feature
but the CPU ignores it?

> +
> +	return ~debugctl;
> +}

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