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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
