Multiplexing RFLAGS.TF

Multiplexing RFLAGS.TF

[Avi Kivity]

  static int db_interception(struct vcpu_svm *svm)
{
     struct kvm_run *kvm_run = svm->vcpu.run;

     if (!(svm->vcpu.guest_debug &
           (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) &&
         !svm->nmi_singlestep) {
         kvm_queue_exception(&svm->vcpu, DB_VECTOR);
         return 1;
     }

     if (svm->nmi_singlestep) {
         svm->nmi_singlestep = false;
         if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP))
             svm->vmcb->save.rflags &=
                 ~(X86_EFLAGS_TF | X86_EFLAGS_RF);
         update_db_intercept(&svm->vcpu);
     }

This code assumes that either the guest is debugging itself, or 
(nmi_singlestep | guest debugging).  However if the guest is debugging 
itself and takes an NMI, or if both host and guest are debugging the 
guest, things will go wrong.

So we need an rflags_guest_owned_bits, usually set to -1ULL, but 
sometimes (NMI, host debugging) clearing EFLAGS_TF.  When we do that, we 
need to intercept instructions that influence RFLAGS.TF (POPF, IRET, 
INTn) and emulate them.  Otherwise, the guest can disable tracing which 
was enabled on behalf of the host.

We also need to drop the 'return 1' on the top of the function to allow 
both guest and host tracing.

On Intel, the situation is harder.  We can't trap POPF or IRET.  What we 
can do, is use the Monitor Trap Flag on hosts that have it.

Comments?  Perhaps I missed something.  Maybe I'll try writing a test 
case to prove the brokenness, it's fashionable these days.

Jan, as this is your code, are you interested in doing this?

Sheng, the Intel bits?

-- 
error compiling committee.c: too many arguments to function

Re: Multiplexing RFLAGS.TF

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]

Am 29.07.2010 10:37, Avi Kivity wrote:
>  static int db_interception(struct vcpu_svm *svm)
> {
>     struct kvm_run *kvm_run = svm->vcpu.run;
> 
>     if (!(svm->vcpu.guest_debug &
>           (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) &&
>         !svm->nmi_singlestep) {
>         kvm_queue_exception(&svm->vcpu, DB_VECTOR);
>         return 1;
>     }
> 
>     if (svm->nmi_singlestep) {
>         svm->nmi_singlestep = false;
>         if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP))
>             svm->vmcb->save.rflags &=
>                 ~(X86_EFLAGS_TF | X86_EFLAGS_RF);
>         update_db_intercept(&svm->vcpu);
>     }
> 
> This code assumes that either the guest is debugging itself, or
> (nmi_singlestep | guest debugging).  However if the guest is debugging
> itself and takes an NMI, or if both host and guest are debugging the
> guest, things will go wrong.

I know.

> 
> So we need an rflags_guest_owned_bits, usually set to -1ULL, but
> sometimes (NMI, host debugging) clearing EFLAGS_TF.  When we do that, we
> need to intercept instructions that influence RFLAGS.TF (POPF, IRET,
> INTn) and emulate them.  Otherwise, the guest can disable tracing which
> was enabled on behalf of the host.

I was still waiting on some smart idea from AMD how to properly
implement NMIs without having to fully emulate IRET. Probably there is
no alternative...

> 
> We also need to drop the 'return 1' on the top of the function to allow
> both guest and host tracing.

Support for host and guest-initiated tracing at the same time would be
nice, but I would not spend to much effort on this corner case of the
corner cases. If it happens to fall off from the NMI fix, OK. But
otherwise let the host rule TF if it wants to.

> 
> On Intel, the situation is harder.  We can't trap POPF or IRET.  What we
> can do, is use the Monitor Trap Flag on hosts that have it.

Setting TF before POPF and IRET should give us at least the chance to
provide host-overrules-guest tracing support. Adding monitor trap
support would be nice. It would allow more things actually, but it may
then require some additional knob in the user/kernel interface to
control the mode (MTF steps into exceptions/interrupts, TF not).

> 
> Comments?  Perhaps I missed something.  Maybe I'll try writing a test
> case to prove the brokenness, it's fashionable these days.
> 
> Jan, as this is your code, are you interested in doing this?

I'm not very keen on writing complex and error-prone opcode emulations,
but in principle resolving the AMD issue is on my long to-do list - with
moderate prio though.

Cheers from Lancaster County PA,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: Multiplexing RFLAGS.TF

[Avi Kivity]

  On 08/02/2010 04:17 AM, Jan Kiszka wrote:
>
>
>> So we need an rflags_guest_owned_bits, usually set to -1ULL, but
>> sometimes (NMI, host debugging) clearing EFLAGS_TF.  When we do that, we
>> need to intercept instructions that influence RFLAGS.TF (POPF, IRET,
>> INTn) and emulate them.  Otherwise, the guest can disable tracing which
>> was enabled on behalf of the host.
> I was still waiting on some smart idea from AMD how to properly
> implement NMIs without having to fully emulate IRET. Probably there is
> no alternative...

Well, there's the existing singlestep implementation, it just needs to 
be fixed not to assume the host has exclusive ownership of TF.  It's 
probably faster than emulation, and certainly more accurate.

>> We also need to drop the 'return 1' on the top of the function to allow
>> both guest and host tracing.
> Support for host and guest-initiated tracing at the same time would be
> nice, but I would not spend to much effort on this corner case of the
> corner cases. If it happens to fall off from the NMI fix, OK. But
> otherwise let the host rule TF if it wants to.

Taking an NMI while the guest is tracing itself is not a corner case.  I 
agree about simulataneous debugging.

>> On Intel, the situation is harder.  We can't trap POPF or IRET.  What we
>> can do, is use the Monitor Trap Flag on hosts that have it.

Actually, I think a POPF or IRET that disables TF still takes a last 
trap?  If so it's workable.

> Setting TF before POPF and IRET should give us at least the chance to
> provide host-overrules-guest tracing support. Adding monitor trap
> support would be nice. It would allow more things actually, but it may
> then require some additional knob in the user/kernel interface to
> control the mode (MTF steps into exceptions/interrupts, TF not).

There's also branch trace in debugctlmsr, that allows you to quickly 
step out of a function.

>> Comments?  Perhaps I missed something.  Maybe I'll try writing a test
>> case to prove the brokenness, it's fashionable these days.
>>
>> Jan, as this is your code, are you interested in doing this?
> I'm not very keen on writing complex and error-prone opcode emulations,
> but in principle resolving the AMD issue is on my long to-do list - with
> moderate prio though.
>

Definitely all this code has to be accompanied by test cases.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.