Hi,
On Tue, Oct 16, 2018 at 02:25:42PM +0800, Wanpeng Li wrote:
> On Tue, 16 Oct 2018 at 04:45, Christian Ehrhardt <lk@xxxxxxx> wrote:
> >
> >
> > Hi,
> >
> > while debugging a nested VM setup where Linux runs as an L1
> > guest, I found lots of VM exits due to %dr6 writes. However,
> > neither L0 nor L1 or L2 where using debug registers.
> >
> > The behaviour was introdued with change efdab992 and the
> > original author suggested the patch below.
> >
> > BTW: I'm not related to the other guy with the same name.
> >
> > regards Christian
> >
> >
> > From e18794eaa15a6bd8829a86a806bc8acb964eefb9 Mon Sep 17 00:00:00 2001
> > From: Christian Ehrhardt <lk@xxxxxx>
> > Date: Mon, 15 Oct 2018 20:05:48 +0200
> > Subject: [PATCH 1/2] KVM: x86: Only clear %dr6 if really neccessary
> >
> > Change efdab992: "KVM: x86: fix escape of guest dr6 to the host"
> > has negative effects if Linux runs as the L1 guest in a nested
> > VM setup:
> >
> > Each call to kvm_arch_vcpu_put causes an additional L1->L0 exit due
> > to the %dr6 write even if neither L1 nor L2 uses debug registers.
> > As the current hardware value of %dr6 is already in vcpu->arch.dr6
> > we can avoid the debug register write if the value is already zero.
> >
> > Update the comment and reformat it while there.
> >
> > Signed-off-by: Christian Ehrhardt <lk@xxxxxxx>
> > ---
> > arch/x86/kvm/x86.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index ca717737347e..37d046c5c9c1 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -3185,11 +3185,13 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> > kvm_x86_ops->vcpu_put(vcpu);
> > vcpu->arch.last_host_tsc = rdtsc();
> > /*
> > - * If userspace has set any breakpoints or watchpoints, dr6 is restored
> > - * on every vmexit, but if not, we might have a stale dr6 from the
> > - * guest. do_debug expects dr6 to be cleared after it runs, do the same.
> > + * If userspace has set any breakpoints or watchpoints, dr6 is
> > + * restored on every vmexit, but if not, we might have a stale
> > + * dr6 from the guest. do_debug expects dr6 to be cleared after
> > + * it runs, do the same if dr6 has a non-zero value.
> > */
> > - set_debugreg(0, 6);
> > + if (vcpu->arch.dr6)
> > + set_debugreg(0, 6);
>
> I tried this immediately after the offline discussion between Paolo
> and you several months ago, however, I can observe the issue was still
> present.
True. After re-reading the intel manual that's because man bits
in %dr6 are reserved and set to 1. The check above must be
if (vcpu->arch.dr6 & DR6_RESERVED)
set_debugreg(0, 6);
instead of
if (vcpu->arch.dr6)
set_debugreg(0, 6);
I'll prepare an updated patch tonight. However, I still cannot test
this easily as this would require recompiling the distribution
kernel used for L1...
regards Christian
