From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH V3] vmx/nmi: Do not use self_nmi() in VMEXIT handler Date: Mon, 26 Nov 2012 11:50:58 +0000 Message-ID: <50B357A2.2050205@eu.citrix.com> References: <20121122173404.GC83155@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20121122173404.GC83155@ocelot.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: Andrew Cooper , Malcolm Crossley , Jan Beulich , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 22/11/12 17:34, Tim Deegan wrote: > At 15:00 +0000 on 22 Nov (1353596446), Andrew Cooper wrote: >> 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(); >> break; >> case EXIT_REASON_MCE_DURING_VMENTRY: >> do_machine_check(regs); > I think it might also make sense to move the HVMTARCE invocations that > are just above here down to after this block. There's quite a lot of > code behind there and though I don't see any potential faults they might > well get added later (including in places like printk() and > tasklet_schedule()). > > I had a look at the rest of the code that runs between the vmexit and > here, and it looks OK to me. > > George, would that make the tracing more confusing? Well, it would mildly, because you'd get the trace in vmx_do_extint() before the trace for the VMEXIT that caused it. :-) If it's important for correctness, then xenalyze will just have to deal. But it would be a lot nicer not to have to deal. -George