Re: [Xen-devel] [PATCH V3] vmx/nmi: Do not use self_nmi() in VMEXIT handler

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.11.12 at 16:16, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

> On 22/11/12 15:15, Jan Beulich wrote:
>>>>> On 22.11.12 at 16:00, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>> The self_nmi() code cause's an NMI to be triggered by sending an APIC
>>> message to the local processor.  However, NMIs are blocked by the
>>> VMEXIT, until the next iret or VMENTER.
>>>
>>> Volume 3 Chapter 27 Section 1 of the Intel SDM states:
>>>
>>> An NMI causes subsequent NMIs to be blocked, but only after the VM exit
>>> completes.
>>>
>>> As a result, as soon as the VMENTER happens, an immediate VMEXIT
>>> happens as a result of the queued NMI.  We have seen hundreds of
>>> iterations of this VMEXIT/VMENTER loop before the HVM guest resumes
>>> normal operation.
>>>
>>> Signed-off-by: Malcolm Crossley <malcolm.crossley@xxxxxxxxxx>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>
>>> --
>>> Changes since v2
>>>  * Switch from 'int $2' to do_nmi()
>>>  * Reworked commit message to more clearly explain the problem
>>>
>>> diff -r 2489c2926698 -r d7ea938044ac xen/arch/x86/hvm/vmx/vmx.c
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -2269,6 +2269,14 @@ void vmx_vmexit_handler(struct cpu_user_
>>>          vector = intr_info & INTR_INFO_VECTOR_MASK;
>>>          if ( vector == TRAP_machine_check )
>>>              do_machine_check(regs);
>>> +        else if ( vector == TRAP_nmi &&
>>> +                ( (intr_info & INTR_INFO_INTR_TYPE_MASK) ==
>>> +                  (X86_EVENTTYPE_NMI << 8) ) )
>>> +            /* Must be called before interrupts are enabled to ensure
>>> +             * the NMI handler code is run before the first IRET. The
>>> +             * IRET unblocks subsequent NMI's (Intel SDM Vol 3, 6.7.1)
>>> +             */
>>> +            do_nmi();
>> But that's only half of it, at least as far as I recall the outcome of
>> the discussion: You want an IRET (or VMRESUME) before possibly
>> going into the scheduler (and hence not back to the current VM).
>> And the same also on the PV exit path from an NMI.
> 
> When I read this codepath, there seemed to be no consideration for
> re-scheduling.  I will double check, but I think the VMRESUME is
> unconditional if the VMEXIT reason was a real NMI.

Interesting - I see nothing NMI-related in vmx/entry.S, i.e. it is
my understanding that just like in any other case you may end
up calling do_softirq on the way out from handling the NMI.

Jan


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