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

[Tim Deegan <tim@xxxxxxx>]

At 13:42 +0000 on 28 Feb (1362058925), Jan Beulich wrote:
> >>> On 28.02.13 at 14:00, Tim Deegan <tim@xxxxxxx> wrote:
> >     vmx: handle NMIs before re-enabling interrupts.
> >     
> >     Also, switch to calling do_nmi() and explicitly re-enabling NMIs
> >     rather than raising a fake NMI and relying on the NMI processing to
> >     IRET, since that handling code is likely to change a fair amount in
> >     future.
> >     
> >     Signed-off-by: Tim Deegan <tim@xxxxxxx>
> 
> Sorry, my Acked-by just sent requires one change to be done first:
> 
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -2313,6 +2313,13 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> >          vector = intr_info & INTR_INFO_VECTOR_MASK;
> >          if ( vector == TRAP_machine_check )
> >              do_machine_check(regs);
> > +        if ( vector == TRAP_machine_check
> 
> This almost certainly needs to be TRAP_nmi.

!

I was foolishly using the watchdog for testing, and of course losing
NMIs doesn't cause the watchdog to trigger. :|

Here's v2, with that fixed and updating the description:


commit 7dd3b06ff031c9a8c727df16c5def2afb382101c
Author: Tim Deegan <tim@xxxxxxx>
Date:   Thu Feb 28 12:42:15 2013 +0000

    vmx: fix handling of NMI VMEXIT.
    
    Call do_nmi() directly and explicitly re-enable NMIs rather than
    raising an NMI through the APIC. Since NMIs are disabled after the
    VMEXIT, the raised NMI would be blocked until the next IRET
    instruction (i.e. the next real interrupt, or after scheduling a PV
    guest) and in the meantime the guest will spin taking NMI VMEXITS.
    
    Also, handle NMIs before re-enabling interrupts, since if we handle an
    interrupt (and therefore IRET) before calling do_nmi(), we may end up
    running the NMI handler with NMIs enabled.
    
    Signed-off-by: Tim Deegan <tim@xxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 5378928..04dbefb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2313,6 +2313,13 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
         vector = intr_info & INTR_INFO_VECTOR_MASK;
         if ( vector == TRAP_machine_check )
             do_machine_check(regs);
+        if ( vector == TRAP_nmi
+             && ((intr_info & INTR_INFO_INTR_TYPE_MASK) ==
+                 (X86_EVENTTYPE_NMI << 8)) )
+        {
+            do_nmi(regs);
+            enable_nmis();
+        }
         break;
     case EXIT_REASON_MCE_DURING_VMENTRY:
         do_machine_check(regs);
@@ -2486,7 +2493,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
                  (X86_EVENTTYPE_NMI << 8) )
                 goto exit_and_crash;
             HVMTRACE_0D(NMI);
-            self_nmi(); /* Real NMI, vector 2: normal processing. */
+            /* Already handled above. */
             break;
         case TRAP_machine_check:
             HVMTRACE_0D(MCE);

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