On Tue, Feb 16, 2010 at 11:05:55AM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Tue, Feb 16, 2010 at 10:45:15AM +0100, Jan Kiszka wrote:
> >> Gleb Natapov wrote:
> >>> On Tue, Feb 16, 2010 at 10:14:56AM +0100, Jan Kiszka wrote:
> >>>> Gleb Natapov wrote:
> >>>>> On Mon, Feb 15, 2010 at 07:17:18PM +0100, Jan Kiszka wrote:
> >>>>>> As there is no interception on AMD on the end of an NMI handler but
> >>>>>> only
> >>>>>> on its iret, we are forced to step out by setting TF in rflags. This
> >>>>>> can
> >>>>>> collide with host or guest using single-step mode, and it may leak the
> >>>>>> flags onto the guest stack if IRET triggers some exception.
> >>>>> The code is trying to handle the case where debugger used TF flags and
> >>>>> we
> >>>>> want to single step from NMI handler simultaneously. Do you see problem
> >>>>> with
> >>>>> that code? Uf yes may be it sill be much simpler to fix it? TF leakage
> >>>>> is real,
> >>>>> but what problem it may cause? Note that your patch does not solve this
> >>>>> problem
> >>>>> too. See the comment that you've deleted:
> >>>>> /* Something prevents NMI from been injected. Single step over
> >>>>> possible problem (IRET or exception injection or interrupt
> >>>>> shadow) */
> >>>>> So the reason for single step is not necessary IRET, _any_ exception
> >>>>> is possible at this point.
> >>>> That is exactly what my code tries to avoid: Exceptions are all (famous
> >>>> last word) caught, and single-stepping is disabled until that is
> >>>> resolved. So no more leakage, and only IRET remains as reason here (thus
> >>>> my deletion).
> >>>>
> >>> I don't understand why only IRET remains as a reason here? Code will get
> >>> there if interrupt shadow is in effect too and then next instruction may
> >>> generate any exception not only those that IRET generates.
> >> OK, so the faults raised by the instruction under the interrupt shadow
> >> can still cause troubles. Guess we have to live with it unless we what
> >> to trap all exceptions that instructions can raise. Will adjust the
> >> comment.
> >>
> > I don't see the point to complicate code significantly to fix it only
> > partially.
>
> Maybe we can even fix it completely, just need to move some code around
> and add checks to those few existing exception handlers. Will think
> about it.
>
By catching all exceptions may be, but is it worth it?
> >
> >>> Also you haven't answered what is the problem with current code (except
> >>> TF leakage) and why TF leakage is so important. BTW are you sure that TF
> >>> leakage actually happens? I see in Intel SDM:
> >>>
> >>> The processor clears the TF flag before calling the exception handler.
> >> Does it clear it _for_ the exception handler or also in rflags pushed on
> >> the stack?
> > Have no idea. Looking for relevant info in SDM.
> >
> >> Besides this, proper #DB forwarding to the guest was missing.
> > During NMI injection? How to reproduce?
>
> Inject, e.g., an NMI over code with TF set. A bit harder is placing a
> guest HW breakpoint at the spot the NMI handler returns to.
>
Will try to reproduce.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html