Even if the documentation is stale, backwards compatibility requires
that bit 16 of the exit qualification (corresponding to DR6.RTM, per
Figure 17-1 of the SDM, volume 3) must have the opposite polarity of
DR6.RTM (just as bit 16 of the pending debug exceptions does). If
DR6.RTM is set, then bit 16 of the exit qualification must be clear.
Certainly, on hardware without TSX, it works this way. Now, perhaps if
DR6.RTM is clear, then bit 16 of the exit qualification should be set.
I can write a kvm-unit-test to find out, or you can ask around.
DR6.BT actually is an issue. As you point out, it will never be set in
the exit qualification for a #DB exception. However, it may be set in
DR6. Hence, when we try to infer the exit qualification from a
prematurely modified DR6, bit 15 should be masked off.
On Thu, Sep 20, 2018 at 5:07 PM, Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
> On Thu, Sep 20, 2018 at 04:02:43PM -0700, Jim Mattson wrote:
>> Per volume 2 of the SDM, bits 63:15 and 12:4 of the exit qualification
>> field for debug exceptions are reserved (cleared to 0).
>
> Ugh, I suspect we have a documentation bug regarding bit 16 (DR6.BTM),
> and maybe bit 15 (DR6.BT) as well. I'm pretty sure DR6.BTM should be
> preserved if either HLE or RTM is supported. DR6.BT will never be set,
> but that's because it's impossible to take a task-switch #DB in guest
> due to task switches unconditionally taking VMExit and not because the
> bit is explicitly cleared. DR6.BT doesn't really matter but I'll ask
> since I need to bug someone about DR6.BTM anyways.
>
> Maybe just explicitly clear DR6_FIXED_1 for now? DR6_BT should be a
> non-issue. DR6.BTM is weird because of its inverted behavior, not
> sure what the safest approach for that bit would be.
>
> P.S. I assume you meant Volume 3?
>
>> There is still an issue with stale DR6 bits potentially being
>> misreported for the current debug exception. DR6 should not have been
>> modified before vectoring the #DB exception, and the "new DR6 bits"
>> should be available somewhere, but it was and they aren't.
>>
>> Fixes: b96fb439774e1 ("KVM: nVMX: fixes to nested virt interrupt injection")
>> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
>> ---
>> arch/x86/include/asm/kvm_host.h | 1 +
>> arch/x86/kvm/vmx.c | 3 ++-
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 8e90488c3d56..234f7320ba59 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -177,6 +177,7 @@ enum {
>>
>> #define DR6_BD (1 << 13)
>> #define DR6_BS (1 << 14)
>> +#define DR6_BT (1 << 15)
>> #define DR6_RTM (1 << 16)
>> #define DR6_FIXED_1 0xfffe0ff0
>> #define DR6_INIT 0xffff0ff0
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 533a327372c8..01f729830fd9 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3293,7 +3293,8 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned long *exit
>> } else {
>> if (vmcs12->exception_bitmap & (1u << nr)) {
>> if (nr == DB_VECTOR)
>> - *exit_qual = vcpu->arch.dr6;
>> + *exit_qual = vcpu->arch.dr6 &
>> + ~(DR6_FIXED_1 | DR6_BT | DR6_RTM);
>>
>> else
>> *exit_qual = 0;
>> return 1;
>> --
>> 2.19.0.444.g18242da7ef-goog
>>
