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

[Tim Deegan <tim@xxxxxxx>]

At 12:17 +0000 on 13 Nov (1352809049), Ian Campbell wrote:
> On Tue, 2012-11-13 at 12:05 +0000, Tim Deegan wrote:
> > At 11:47 +0000 on 13 Nov (1352807234), Tim Deegan wrote:
> > > At 11:38 +0000 on 13 Nov (1352806689), Jan Beulich wrote:
> > > > > diff -r 62885b3c34c8 -r ea756059a8da xen/arch/x86/hvm/vmx/vmx.c
> > > > > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > > > > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > > > > @@ -2442,7 +2442,7 @@ void vmx_vmexit_handler(struct cpu_user_
> > > > >                   (X86_EVENTTYPE_NMI << 8) )
> > > > >                  goto exit_and_crash;
> > > > >              HVMTRACE_0D(NMI);
> > > > > -            self_nmi(); /* Real NMI, vector 2: normal processing. */
> > > > > +            asm("int $2"); /* Real NMI, vector 2: normal processing. 
> > > > > */
> > > > 
> > > > In any case - why can't you call do_nmi() directly from here?
> > > 
> > > ... this is my doing.  There used to be a call to do_nmi() here, but
> > > do_nmi() doesn't block NMIs, so you can't just call it here in case you
> > > get _another_ NMI while you're in the NMI handler.
> > 
> > Oh wait, I see -- you're saying that this (20059:76a65bf2aa4d) is wrong
> > because NMIs are indeed blocked, and have been since the VMEXIT.
> > 
> > In that case, I agree that we should just run the NMI handler, but first
> > I would really like to know what _unblocks_ NMIs in this case.  Any of
> > the things I can think of (the next vmenter, the next iret, ??) will
> > need some handling to make sure they actually happen before, say, we
> > take this CPU into the idle loop...
> 
> 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:

Acked-by: Tim Deegan <tim@xxxxxxx>

Cheers,

Tim

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