Re: [PATCH 6.6.y] KVM: x86: Load DR6 with guest value only before entering .vcpu_run() loop

[Sean Christopherson <seanjc@google.com>]

On Thu, May 01, 2025, James Houghton wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> [ Upstream commit c2fee09fc167c74a64adb08656cb993ea475197e ]
> 
> Move the conditional loading of hardware DR6 with the guest's DR6 value
> out of the core .vcpu_run() loop to fix a bug where KVM can load hardware
> with a stale vcpu->arch.dr6.
> 
> When the guest accesses a DR and host userspace isn't debugging the guest,
> KVM disables DR interception and loads the guest's values into hardware on
> VM-Enter and saves them on VM-Exit.  This allows the guest to access DRs
> at will, e.g. so that a sequence of DR accesses to configure a breakpoint
> only generates one VM-Exit.
> 
> For DR0-DR3, the logic/behavior is identical between VMX and SVM, and also
> identical between KVM_DEBUGREG_BP_ENABLED (userspace debugging the guest)
> and KVM_DEBUGREG_WONT_EXIT (guest using DRs), and so KVM handles loading
> DR0-DR3 in common code, _outside_ of the core kvm_x86_ops.vcpu_run() loop.
> 
> But for DR6, the guest's value doesn't need to be loaded into hardware for
> KVM_DEBUGREG_BP_ENABLED, and SVM provides a dedicated VMCB field whereas
> VMX requires software to manually load the guest value, and so loading the
> guest's value into DR6 is handled by {svm,vmx}_vcpu_run(), i.e. is done
> _inside_ the core run loop.
> 
> Unfortunately, saving the guest values on VM-Exit is initiated by common
> x86, again outside of the core run loop.  If the guest modifies DR6 (in
> hardware, when DR interception is disabled), and then the next VM-Exit is
> a fastpath VM-Exit, KVM will reload hardware DR6 with vcpu->arch.dr6 and
> clobber the guest's actual value.
> 
> The bug shows up primarily with nested VMX because KVM handles the VMX
> preemption timer in the fastpath, and the window between hardware DR6
> being modified (in guest context) and DR6 being read by guest software is
> orders of magnitude larger in a nested setup.  E.g. in non-nested, the
> VMX preemption timer would need to fire precisely between #DB injection
> and the #DB handler's read of DR6, whereas with a KVM-on-KVM setup, the
> window where hardware DR6 is "dirty" extends all the way from L1 writing
> DR6 to VMRESUME (in L1).

...

> Reported-by: John Stultz <jstultz@google.com>
> Closes: https://lkml.kernel.org/r/CANDhNCq5_F3HfFYABqFGCA1bPd_%2BxgNj-iDQhH4tDk%2Bwi8iZZg%40mail.gmail.com
> Fixes: 375e28ffc0cf ("KVM: X86: Set host DR6 only on VMX and for KVM_DEBUGREG_WONT_EXIT")
> Fixes: d67668e9dd76 ("KVM: x86, SVM: isolate vcpu->arch.dr6 from vmcb->save.dr6")
> Cc: stable@vger.kernel.org
> Cc: Jim Mattson <jmattson@google.com>
> Tested-by: John Stultz <jstultz@google.com>
> Link: https://lore.kernel.org/r/20250125011833.3644371-1-seanjc@google.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> [jth: Handled conflicts with kvm_x86_ops reshuffle]
> Signed-off-by: James Houghton <jthoughton@google.com>
> ---

Acked-by: Sean Christopherson <seanjc@google.com>