* [PATCH] xen svm: fix PAUSE instruction intercept
@ 2010-08-12 9:22 Christoph Egger
2010-08-12 10:07 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Egger @ 2010-08-12 9:22 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 627 bytes --]
Hi!
Attached patch moves PAUSE instruction intercept handling
into svm_vmexit_do_pause() analogous to the other instruction
intercepts.
In svm_vmexit_do_pause() update the instruction pointer.
This fixes strange PAUSE intercepts on UP guests.
Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
P.S.: This bug has been found with nested virtualization.
--
---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
[-- Attachment #2: xen_pausefilter.diff --]
[-- Type: text/x-diff, Size: 3381 bytes --]
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 Wed Aug 11 14:36:28 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, 2, 0xf3, 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 Wed Aug 11 14:36:28 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 Wed Aug 11 14:36:28 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 Wed Aug 11 14:36:28 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 Wed Aug 11 14:36:28 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__ */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xen svm: fix PAUSE instruction intercept
2010-08-12 9:22 [PATCH] xen svm: fix PAUSE instruction intercept Christoph Egger
@ 2010-08-12 10:07 ` Jan Beulich
2010-08-12 12:50 ` Christoph Egger
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2010-08-12 10:07 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel
>>> On 12.08.10 at 11:22, Christoph Egger <Christoph.Egger@amd.com> 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.
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().
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xen svm: fix PAUSE instruction intercept
2010-08-12 10:07 ` Jan Beulich
@ 2010-08-12 12:50 ` Christoph Egger
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Egger @ 2010-08-12 12:50 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
On Thursday 12 August 2010 12:07:54 Jan Beulich wrote:
> >>> On 12.08.10 at 11:22, Christoph Egger <Christoph.Egger@amd.com> 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 <Christoph.Egger@amd.com>
> 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
[-- Attachment #2: xen_pausefilter.diff --]
[-- Type: text/x-diff, Size: 3375 bytes --]
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__ */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-08-12 12:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-12 9:22 [PATCH] xen svm: fix PAUSE instruction intercept Christoph Egger
2010-08-12 10:07 ` Jan Beulich
2010-08-12 12:50 ` Christoph Egger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.