[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



At 13:39 +0000 on 28 Feb (1362058773), Jan Beulich wrote:
> >>> On 28.02.13 at 14:00, Tim Deegan <tim@xxxxxxx> wrote:
> > commit d278beed1df2d226911dce92295411018c9bba2f
> > Author: Tim Deegan <tim@xxxxxxx>
> > Date:   Thu Feb 28 12:42:15 2013 +0000
> > 
> >     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.
> 
> Isn't that a gross understatement, considering that you or Malcolm
> had found that the use of self_nmi() here was actively broken?
> 
> >     Signed-off-by: Tim Deegan <tim@xxxxxxx>
> 
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> (realizing that dealing with the PV side of the issue will be left to
> me in the end)

For PV, would you be happy with something like this, or do you want to
avoid the extra IRET in cases where we would be returning with IRET
anyway?

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 04dbefb..598a5b5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2316,10 +2316,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *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);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d36eddd..2a66080 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3211,7 +3211,7 @@ void do_nmi(struct cpu_user_regs *regs)
     ++nmi_count(cpu);
 
     if ( nmi_callback(regs, cpu) )
-        return;
+        goto out;
 
     if ( nmi_watchdog )
         nmi_watchdog_tick(regs);
@@ -3227,6 +3227,9 @@ void do_nmi(struct cpu_user_regs *regs)
         if ( !(reason & 0xc0) && !nmi_watchdog )
             unknown_nmi_error(regs, reason);
     }
+
+out:
+    enable_nmis();
 }
 
 void set_nmi_callback(nmi_callback_t callback)



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.