From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2] KVM: SVM: Make stepping out of NMI handlers more robust Date: Tue, 16 Feb 2010 11:05:55 +0100 Message-ID: <4B7A6E03.2020606@siemens.com> References: <8cb458e40c9d594f27975b10bb24ecb9a90d8102.1266257833.git.jan.kiszka@siemens.com> <20100216080436.GX2995@redhat.com> <4B7A6210.5060606@siemens.com> <20100216093416.GA2995@redhat.com> <4B7A692B.8050207@siemens.com> <20100216094954.GB2995@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , kvm To: Gleb Natapov Return-path: Received: from thoth.sbs.de ([192.35.17.2]:15007 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932136Ab0BPKGO (ORCPT ); Tue, 16 Feb 2010 05:06:14 -0500 In-Reply-To: <20100216094954.GB2995@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. > >>> 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. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux