Yes, a kvm-unit-test shows that bit 16 of the exit qualification for
#DB should be *set* if a debug exception (#DB) or a breakpoint
exception (#BP) occurred inside an RTM region while advanced debugging
of RTM transactional regions was enabled. This is the opposite of
DR6.RTM. I'll post the test shortly.
Sean: Can you get Table 27-1 of the SDM (volume 3) corrected?
On Fri, Sep 21, 2018 at 9:00 AM, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
> 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
>>>
