[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] 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
At 14:21 +0000 on 13 Nov (1352816476), Jan Beulich wrote: > >>> On 13.11.12 at 13:39, Tim Deegan <tim@xxxxxxx> wrote: > > 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: > > 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/..., 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. 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. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |