Re: [PATCH 9/9] KVM: VMX: pass correct DR6 for GD userspace exit

[Peter Xu]

On Thu, May 07, 2020 at 06:21:18PM +0200, Paolo Bonzini wrote:
> On 07/05/20 18:18, Peter Xu wrote:
> >>  		if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> >> -			vcpu->run->debug.arch.dr6 = vcpu->arch.dr6;
> >> +			vcpu->run->debug.arch.dr6 = DR6_BD | DR6_RTM | DR6_FIXED_1;
> > After a second thought I'm thinking whether it would be okay to have BS set in
> > that test case.  I just remembered there's a test case in the kvm-unit-test
> > that checks explicitly against BS leftover as long as dr6 is not cleared
> > explicitly by the guest code, while the spec seems to have no explicit
> > description on this case.
> 
> Yes, I noticed that test as well.  But I don't like having different
> behavior for Intel and AMD, and the Intel behavior is more sensible.
> Also...

Do you mean the AMD behavior is more sensible instead? :)

> 
> > Intead of above, I'm thinking whether we should allow the userspace to also
> > change dr6 with the KVM_SET_GUEST_DEBUG ioctl when they wanted to (right now
> > iiuc dr6 from userspace is completely ignored), instead of offering a fake dr6.
> > Or to make it simple, maybe we can just check BD bit only?
> 
> ... I'm afraid that this would be a backwards-incompatible change, and
> it would require changes in userspace.  If you look at v2, emulating the
> Intel behavior in AMD turns out to be self-contained and relatively
> elegant (will be better when we finish cleaning up nested SVM).

I'm still trying to read the other patches (I need some more digest because I'm
even less familiar with nested...).  I agree that it would be good to keep the
same behavior across Intel/AMD.  Actually that also does not violate Intel spec
because the AMD one is stricter.  However I guess then we might also want to
fixup the kvm-unit-test too to aligh with the behaviors on leftover set bits.

Thanks,

-- 
Peter Xu