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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

>>>> What about a little stub-asm return_from_nmi / reenable_nmis with
>>>> something like:
>>>>    pushf
>>>>         pushq $__HYPERVISOR_CS
>>>>    pushq 1f
>>>>    iret
>>>> 1: ...
>>>>
>>>> That should re-enable NMIs at whichever point we think is appropriate?
>>> If an iret is what's needed then replacing self_nmi() with 'int $2'
>>> (i.e. the proposed patch) seems like a neater fix.  And section 6.7.1 of
>>> the manual suggests that iret is indeed what we want, so:
>> An IRET just cannot be the only thing that ends the NMI masked
>> window.  At least some forms of #VMENTER should have that
>> effect too, as otherwise NMIs would remain masked for arbitrary
>> periods of time.
>>
>> Further, under the assumption that the self_nmi() worked at least
>> in some cases (since you likely tested it), there would also be room
>> for the NMI not being masked when getting here. Which would get
>> us into the stack trouble that Andrew mentioned in his reply.
> I got the impression that Andrew and Malcolm were seeing a long loop of
> exit/self_nmi()/enter/NMI/exit/...,

Yes - thousands of instantaneous exits from the VM (the eip of the guest
is always the same while this is going on), and this is trivial
behaviour to demonstrate with a CPU-bound VCPU task.

>  eventually broken by a real
> interrupt or an IPI causing (by its iret) the NMI to get delivered in
> root mode.  So the NMI does get handled, just not immediately.

We are still not certain exactly what breaks the PCPU out of this cycle,
but timer interrupts were seen as the first deviation from the above cycle.

My interpretation of the Intel SDM manual along with the observable
evidence, is that NMIs are disabled from the vmexit until the vmresume
(as there appear to be no other irets on that path).  The action of
receiving a vmexit with reason NMI means that an external NMI has
occurred and we should run the handler.

With the current code, we raise a self_nmi(), which queues up a brand
new NMI to interrupt us as soon as we vmresume and NMIs are re-enabled.

>
> The fact that there was a loop and not just a delay in the NMI handling
> suggests that VMENTER does indeed re-enable NMIs (or at least
> NMI-exiting) but I couldn't find that in the manual.  In any case, I
> think the int $2 version is correcter than the direct call as it also
> disables normal interrupts.
>
> Tim.

With the asm("int $2") solution, we jump into the NMI handler (on the
NMI stack) with NMIs disabled (due to the vmexit), run the handler for
the real NMI which caused the vmexit in the first place, then iret from
the NMI handler which re-enables NMIs.  We are then free to take NMIs
for the remainder of the codepath back to the vmresume.

The question then becomes whether we wish to enable NMIs or not, which
affects whether we issue "int $2" or "do_nmi()".  Can someone from Intel
weigh in and confirm the exact behaviour of NMIs around this code?

(Irrespective of this question at hand, having read that LWN article
about nested NMIs, I am not certain that we are immune to the issue, so
should think about implementing a similar fix)

~Andrew

>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel