From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH V3] vmx/nmi: Do not use self_nmi() in VMEXIT handler Date: Thu, 22 Nov 2012 15:37:20 +0000 Message-ID: <50AE46B0.6010807@citrix.com> References: <50AE4F8402000078000AAA02@nat28.tlf.novell.com> <50AE41DD.2030703@citrix.com> <50AE511302000078000AAA14@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50AE511302000078000AAA14@nat28.tlf.novell.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: Jan Beulich Cc: Malcolm Crossley , "Tim (Xen.org)" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 22/11/12 15:21, Jan Beulich wrote: >>>> On 22.11.12 at 16: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. >> 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. > Interesting - I see nothing NMI-related in vmx/entry.S, i.e. it is > my understanding that just like in any other case you may end > up calling do_softirq on the way out from handling the NMI. > > Jan > Ah yes - vmx_asm_do_vmentry does have the potential to process softirqs, which we will want to do with NMIs enabled. I missed that last time I looked. So we need to deliberately execute an iretd. int $2 is unusable because it adds an extra dimension to the fake NMI corruption issue if you get the combination int $2->MCE->NMI before the int $2 has correctly saved its exception frame. A quick solution would be to execute a noop function with run_in_exception_handler(). Alternatively, I can code enable_nmi() or so which does an inline iret to itself. Which of these would you prefer? -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com