From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mats Petersson Subject: Re: [PATCH V3] vmx/nmi: Do not use self_nmi() in VMEXIT handler Date: Thu, 22 Nov 2012 15:22:03 +0000 Message-ID: <50AE431B.5090605@citrix.com> References: <50AE4F8402000078000AAA02@nat28.tlf.novell.com> <50AE41DD.2030703@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50AE41DD.2030703@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 22/11/12 15:16, Andrew Cooper wrote: > On 22/11/12 15:15, Jan Beulich wrote: >>>>> On 22.11.12 at 16:00, Andrew Cooper wrote: >>> The self_nmi() code cause's an NMI to be triggered by sending an APIC >>> message to the local processor. However, NMIs are blocked by the >>> VMEXIT, until the next iret or VMENTER. >>> >>> Volume 3 Chapter 27 Section 1 of the Intel SDM states: >>> >>> An NMI causes subsequent NMIs to be blocked, but only after the VM exit >>> completes. >>> >>> As a result, as soon as the VMENTER happens, an immediate VMEXIT >>> happens as a result of the queued NMI. We have seen hundreds of >>> iterations of this VMEXIT/VMENTER loop before the HVM guest resumes >>> normal operation. >>> >>> Signed-off-by: Malcolm Crossley >>> Signed-off-by: Andrew Cooper >>> >>> -- >>> Changes since v2 >>> * Switch from 'int $2' to do_nmi() >>> * Reworked commit message to more clearly explain the problem >>> >>> 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(); >> But that's only half of it, at least as far as I recall the outcome of >> the discussion: You want an IRET (or VMRESUME) before possibly >> going into the scheduler (and hence not back to the current VM). >> And the same also on the PV exit path from an NMI. >> >> Jan > When I read this codepath, there seemed to be no consideration for > re-scheduling. I will double check, but I think the VMRESUME is > unconditional if the VMEXIT reason was a real NMI. Technically, we could probably get a timer interrupt which causes a scheduling event, between the NMI and the VMEXIT. However, that timer interrupt will have an IRET at the end of it, so it fixes itself. > > ~Andrew > >>> break; >>> case EXIT_REASON_MCE_DURING_VMENTRY: >>> do_machine_check(regs); >>> @@ -2442,7 +2450,6 @@ 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. */ >>> break; >>> case TRAP_machine_check: >>> HVMTRACE_0D(MCE); >>