Re: [PATCH 2/2] KVM: SVM: Make stepping out of NMI handlers more robust

[Jan Kiszka <jan.kiszka@siemens.com>]

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).

> 
>> This patch addresses the TF leakage by trapping all exceptions that may
>> show up on IRET execution and cleaning up the state again before
>> reinjecting the exception. Collisions with concurrent users are avoided
>> by carefully checking for their presence and forwarding #DB exceptions
>> whenever required.
> This patch is scary to apply without test cases.

Do you have one for the exception case? The others should have been
covered manually here, but I agree we should write a unit test, also for
the sake of VMX.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux