From: Christoph Egger Subject: Re: [PATCH] xen svm: fix PAUSE =?iso-8859-1?q?instruction=09?= intercept Date: Thu, 12 Aug 2010 14:50:52 +0200 Message-ID: <201008121450.52671.Christoph.Egger@amd.com> References: <201008121122.36706.Christoph.Egger@amd.com> <4C63E41A020000780000F773@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Boundary-00=_s4+YMyGU+qDUW0/" Return-path: In-Reply-To: <4C63E41A020000780000F773@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: Jan Beulich List-Id: xen-devel@lists.xenproject.org --Boundary-00=_s4+YMyGU+qDUW0/ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Thursday 12 August 2010 12:07:54 Jan Beulich wrote: > >>> On 12.08.10 at 11:22, Christoph Egger wrote: > > > > In svm_vmexit_do_pause() update the instruction pointer. > > This fixes strange PAUSE intercepts on UP guests. > > This won't work when the instruction needs to actually be decoded: > __get_instruction_length_from_list() strips all prefix bytes from > the byte stream being decoded, and since 'pause' uses a prefix the > actual opcode comparison will never produce a match. Thank you for pointing this out. New attached version fixes this. Signed-off-by: Christoph Egger > Even in the case that CPUs supporting the pause filter would always > support nextrip I think that the special case minimally deserves a > code comment to that effect, but perhaps even warrants an > ASSERT() or BUG_ON(). This is true for real hardware but not necessarily when it comes to nested virtualization, because you emulate SVM features in software. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 --Boundary-00=_s4+YMyGU+qDUW0/ Content-Type: text/x-diff; charset="iso 8859-15"; name="xen_pausefilter.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xen_pausefilter.diff" diff -r f45026ec8db5 xen/arch/x86/hvm/svm/emulate.c --- a/xen/arch/x86/hvm/svm/emulate.c Mon Aug 09 18:29:50 2010 +0100 +++ b/xen/arch/x86/hvm/svm/emulate.c Thu Aug 12 14:45:24 2010 +0200 @@ -100,6 +100,7 @@ MAKE_INSTR(VMCALL, 3, 0x0f, 0x01, 0xd9); MAKE_INSTR(HLT, 1, 0xf4); MAKE_INSTR(INT3, 1, 0xcc); MAKE_INSTR(RDTSC, 2, 0x0f, 0x31); +MAKE_INSTR(PAUSE, 1, 0x90); static const u8 *opc_bytes[INSTR_MAX_COUNT] = { @@ -111,7 +112,8 @@ static const u8 *opc_bytes[INSTR_MAX_COU [INSTR_VMCALL] = OPCODE_VMCALL, [INSTR_HLT] = OPCODE_HLT, [INSTR_INT3] = OPCODE_INT3, - [INSTR_RDTSC] = OPCODE_RDTSC + [INSTR_RDTSC] = OPCODE_RDTSC, + [INSTR_PAUSE] = OPCODE_PAUSE, }; static int fetch(struct vcpu *v, u8 *buf, unsigned long addr, int len) diff -r f45026ec8db5 xen/arch/x86/hvm/svm/svm.c --- a/xen/arch/x86/hvm/svm/svm.c Mon Aug 09 18:29:50 2010 +0100 +++ b/xen/arch/x86/hvm/svm/svm.c Thu Aug 12 14:45:24 2010 +0200 @@ -1261,6 +1261,22 @@ static void svm_vmexit_do_rdtsc(struct c hvm_rdtsc_intercept(regs); } +static void svm_vmexit_do_pause(struct cpu_user_regs *regs) +{ + unsigned int inst_len; + + if ( (inst_len = __get_instruction_length(current, INSTR_PAUSE)) == 0 ) + return; + __update_guest_eip(regs, inst_len); + + /* + * The guest is running a contended spinlock and we've detected it. + * Do something useful, like reschedule the guest + */ + perfc_incr(pauseloop_exits); + do_sched_op_compat(SCHEDOP_yield, 0); +} + static void svm_vmexit_ud_intercept(struct cpu_user_regs *regs) { struct hvm_emulate_ctxt ctxt; @@ -1655,12 +1671,7 @@ asmlinkage void svm_vmexit_handler(struc break; case VMEXIT_PAUSE: - /* - * The guest is running a contended spinlock and we've detected it. - * Do something useful, like reschedule the guest - */ - perfc_incr(pauseloop_exits); - do_sched_op_compat(SCHEDOP_yield, 0); + svm_vmexit_do_pause(regs); break; default: diff -r f45026ec8db5 xen/arch/x86/hvm/svm/vmcb.c --- a/xen/arch/x86/hvm/svm/vmcb.c Mon Aug 09 18:29:50 2010 +0100 +++ b/xen/arch/x86/hvm/svm/vmcb.c Thu Aug 12 14:45:24 2010 +0200 @@ -253,7 +253,7 @@ static int construct_vmcb(struct vcpu *v if ( cpu_has_pause_filter ) { - vmcb->pause_filter_count = 3000; + vmcb->pause_filter_count = SVM_PAUSEFILTER_INIT; vmcb->general1_intercepts |= GENERAL1_INTERCEPT_PAUSE; } diff -r f45026ec8db5 xen/include/asm-x86/hvm/svm/emulate.h --- a/xen/include/asm-x86/hvm/svm/emulate.h Mon Aug 09 18:29:50 2010 +0100 +++ b/xen/include/asm-x86/hvm/svm/emulate.h Thu Aug 12 14:45:24 2010 +0200 @@ -31,6 +31,7 @@ enum instruction_index { INSTR_HLT, INSTR_INT3, INSTR_RDTSC, + INSTR_PAUSE, INSTR_MAX_COUNT /* Must be last - Number of instructions supported */ }; diff -r f45026ec8db5 xen/include/asm-x86/hvm/svm/svm.h --- a/xen/include/asm-x86/hvm/svm/svm.h Mon Aug 09 18:29:50 2010 +0100 +++ b/xen/include/asm-x86/hvm/svm/svm.h Thu Aug 12 14:45:24 2010 +0200 @@ -76,4 +76,6 @@ extern u32 svm_feature_flags; #define cpu_has_svm_nrips test_bit(SVM_FEATURE_NRIPS, &svm_feature_flags) #define cpu_has_pause_filter test_bit(SVM_FEATURE_PAUSEF, &svm_feature_flags) +#define SVM_PAUSEFILTER_INIT 3000 + #endif /* __ASM_X86_HVM_SVM_H__ */ --Boundary-00=_s4+YMyGU+qDUW0/ Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --Boundary-00=_s4+YMyGU+qDUW0/--