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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch identifies that DR6_ACTIVE_LOW unconditionally sets DR6_BUS_LOCK (bit 11) to 1, and replaces it with kvm_dr6_fixed() in nested VMCB02 preparation. However, the patch fails to replace DR6_ACTIVE_LOW in L1 vCPU initialization and #DB payload processing paths, completely breaking Bus Lock Detect for L1 guests.
--

--- Patch [3]: [PATCH v3 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed() ---
commit 0ee3ae36939ed695ccbd636580058ad35113add9
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 84248e6665cfe..0a9e28a02692e 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -811,7 +811,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 is a pre-existing issue, but since this patch correctly identifies that
DR6_ACTIVE_LOW unconditionally forces bit 11 (DR6_BUS_LOCK) to 1, should
DR6_ACTIVE_LOW also be replaced in the L1 vCPU initialization and #DB payload
processing paths?

Currently, DR6_ACTIVE_LOW is still used to initialize L1's dr6:

arch/x86/kvm/x86.c:kvm_vcpu_reset() {
    ...
    vcpu->arch.dr6 = DR6_ACTIVE_LOW;
    ...
}

It is also used as a flip mask when KVM translates the exception payload to
guest DR6. Because Bus Lock Detect is actually active-high, XORing it with
DR6_ACTIVE_LOW incorrectly inverts the bus lock bit the guest sees:

arch/x86/kvm/x86.c:kvm_deliver_exception_payload() {
    ...
    vcpu->arch.dr6 ^= ex->payload & DR6_ACTIVE_LOW;
    ...
}

Similarly, SVM constructs the #DB payload by XORing the hardware exit DR6
with DR6_ACTIVE_LOW:

arch/x86/kvm/svm/svm.c:db_interception() {
    ...
    u32 payload = svm->vmcb->save.dr6 ^ DR6_ACTIVE_LOW;
    ...
}

Could this leave Bus Lock Detect functionally broken for L1 guests by causing
them to observe corrupted hardware state (false positives or false negatives
for bus locks)?

>  		vmcb_mark_dirty(vmcb02, VMCB_DR);
>  	}

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