Re: [PATCH 5/5] KVM: VMX: Handle RFLAGS.TF in skip_emulated_instruction

[Kyle Huey <me@kylehuey.com>]

On Mon, Nov 28, 2016 at 9:19 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 28/11/2016 17:13, Kyle Huey wrote:
>> On Mon, Nov 28, 2016 at 3:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> This code is pretty much the same as kvm_vcpu_check_singlestep.  Let's
>>> not duplicate the code and implement skip_emulated_instruction can be
>>> implemented in x86.c, like
>>>
>>>         unsigned long rflags = kvm_x86_ops->get_rflags(vcpu);
>>>         int r = EMULATE_DONE;
>>>
>>>         /* This would be the no_trap variant */
>>>         kvm_x86_ops->skip_emulated_instruction(vcpu);
>>>         kvm_vcpu_check_singlestep(vcpu, rflags, &r);
>>>         return r == EMULATE_DONE;
>>>
>>> (because x86.c/vmx.c/svm.c are separate modules, when moving the function
>>> to x86.c you should rename it to kvm_skip_emulated_instruction).
>>>
>>> Paolo
>>
>> They're not exactly the same.  For some reason I don't understand
>> kvm_vcpu_check_singlestep clears the trap flag.  Perhaps that is also
>> a bug?
>
> The Intel manual says "The processor clears the TF flag before calling
> the exception handler" (17.3.1.4), so I think you should do it too.

The processor does this automatically. "When accessing an exception or
interrupt handler through either an interrupt gate or a trap gate, the
processor clears the TF flag in the EFLAGS register after it saves the
contents of the EFLAGS register on the stack." (Vol 3, 6.12.1.2)
Empirically, this holds when injecting an exception on VM entry. If
you take the x86/debug.c test from kvm-unit-tests and inspect RFLAGS
in handle_db (not regs->rflags, but the actual RFLAGS register while
running the exception handler) the TF is clear. And, if you modify my
patch to clear TF before returning, the single stepping ceases after
the CPUID instruction because the TF was in fact cleared for good.

- Kyle