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, 28 Feb 2013 15:52:30 +0000 Message-ID: <512F7D3E.9070300@citrix.com> References: <50AE4F8402000078000AAA02@nat28.tlf.novell.com> <50AE41DD.2030703@citrix.com> <50AE511302000078000AAA14@nat28.tlf.novell.com> <50AE46B0.6010807@citrix.com> <50AE590502000078000AAA48@nat28.tlf.novell.com> <50AE4D2C.1040104@citrix.com> <512F384602000078000C1D5B@nat28.tlf.novell.com> <20130228130008.GE27704@ocelot.phlegethon.org> <512F6C2502000078000C2042@nat28.tlf.novell.com> <20130228142503.GH27704@ocelot.phlegethon.org> <512F7ADE02000078000C211A@nat28.tlf.novell.com> <512F88B002000078000C21AA@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <512F88B002000078000C21AA@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 28/02/13 15:41, Jan Beulich wrote: >>>> On 28.02.13 at 15:42, "Jan Beulich" wrote: >> ... this must not be done when on the NMI stack (i.e. when the >> NMI was raised while in hypervisor context). Checking for this >> here would be strait forward, but I was really considering to do >> all of this in the assembly exit path, and I was still undecided >> whether we shouldn't follow Linux in skipping softirq processing >> (and hence scheduling) on the way out from an NMI (I don't >> think we'd need to do the same for MCE). > Like this: > > x86: skip processing events on the NMI exit path > > Otherwise, we may end up in the scheduler, keeping NMIs masked for a > possibly unbounded time (until whenever the next IRET gets executed). > > Of course it's open for discussion whether to always use the strait > exit path from handle_ist_exception. > > Signed-off-by: Jan Beulich The logic looks good to me. As for the other IST exceptions, we will never return from a double fault, and will rarely return from an MCE, so I would say it doesn't really matter at the moment. Acked-by: Andrew Cooper ~Andrew > > --- a/xen/arch/x86/x86_64/compat/entry.S > +++ b/xen/arch/x86/x86_64/compat/entry.S > @@ -171,7 +171,7 @@ compat_bad_hypercall: > jmp compat_test_all_events > > /* %rbx: struct vcpu, interrupts disabled */ > -compat_restore_all_guest: > +ENTRY(compat_restore_all_guest) > ASSERT_INTERRUPTS_DISABLED > RESTORE_ALL adj=8 compat=1 > .Lft0: iretq > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -635,6 +635,9 @@ ENTRY(early_page_fault) > jmp restore_all_xen > .popsection > > +ENTRY(nmi) > + pushq $0 > + movl $TRAP_nmi,4(%rsp) > handle_ist_exception: > SAVE_ALL > testb $3,UREGS_cs(%rsp) > @@ -649,12 +652,17 @@ handle_ist_exception: > movzbl UREGS_entry_vector(%rsp),%eax > leaq exception_table(%rip),%rdx > callq *(%rdx,%rax,8) > - jmp ret_from_intr > + cmpb $TRAP_nmi,UREGS_entry_vector(%rsp) > + jne ret_from_intr > > -ENTRY(nmi) > - pushq $0 > - movl $TRAP_nmi,4(%rsp) > - jmp handle_ist_exception > + /* We want to get strait to the IRET in the NMI exit path. */ > + testb $3,UREGS_cs(%rsp) > + GET_CURRENT(%rbx) > + jz restore_all_xen > + movq VCPU_domain(%rbx),%rax > + testb $1,DOMAIN_is_32bit_pv(%rax) > + jz restore_all_guest > + jmp compat_restore_all_guest > > ENTRY(nmi_crash) > pushq $0 > > >