Re: [PATCH V2] xen: vmx: Use an INT 2 call to process real NMI's instead of self_nmi() in VMEXIT handler

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/11/12 13:53, Tim Deegan wrote:
> At 11:52 +0000 on 16 Nov (1353066779), Andrew Cooper wrote:
>> On 16/11/12 11:23, Jan Beulich wrote:
>>>>>> On 16.11.12 at 11:56, Tim Deegan <tim@xen.org> wrote:
>>>> At 08:07 +0000 on 16 Nov (1353053247), Jan Beulich wrote:
>>>>>> We could potentially solve the problem by having the MCE handler check
>>>>>> whether it's returning to the NMI stack, and do a normal return in that
>>>>>> case.  It's a bit of extra code but only in the MCE handler, which is
>>>>>> not performance-critical. 
>>>>> Yes, that could solve that nesting case (again not very difficult
>>>>> to implement).
>>>> How about we just have the MCE handler return without IRET in _all_
>>>> cases where it's returning to ring 0?  I think that entirely solves the
>>>> MCE-in-NMI problem, without all the extra mechanism meeded for the
>>>> linux-style solution.
>>> Good suggestion.
>>>
>>>>  (Unless we want to allow other traps in either
>>>> the NMI or MCE handlers).
>>> We should absolutely avoid that.
>>>
>>>> [And it occurs to me that the linux-style solution is tricky because
>>>> detecting the case where you've taken an NMI and not yet set the
>>>> nmi-in-progress flag is hard in both SVM (in the NMI handler but on the
>>>> normal stack) and VMX (in the _vmexit_ handler and on the normal
>>>> stack).]
>>> Agreed.
>> But we never need to detect this case.  If we take an NMI and ensure
>> there is no possibility for a trap before setting the nmi-in-progress
>> flag
> The problem is that there is no way to do that -- the trap we're worried
> about is MCE, which can happen at any time.  That's why linux has the
> backstop check for the case where the flag's not set but the return
> address is on the NMI stack.

D'oh - your quite correct.  I overlooked that possibility.

>
>> (which is not very hard, with it being a handful of instructions in
>> the handler),
> It's quite a bit more than that in the VMX case.  I guess we need to
> audit that code for possible faults.

But if we fix the underlying NMI/MCE reentrant problem, then faults on
the vmexit patch cease to be an issue, do they not?  If and when
MCEs/NMIs/interrupts occur, they will be dealt with in the same manor as
any other interruption to hypervisor code.

~Andrew

>> then we guarantee that NMIs are still blocked, and thus
>> cant be reentrant.
>>
>> Also, for what it is worth, we do have traps on the NMI path in the form
>> of BUG()s, WARN()s and panic gubbins, although the host is in a fairly
>> dire state if we actually ever hit any of these.
> Ergh.  If there are any WARN()s we should get rid of them.  BUG()s are
> fine. :)
>
> Tim.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com