* [PATCH 1/6] x86emul: CMPXCHG{8,16}B ignore prefixes
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
@ 2016-12-13 14:01 ` Jan Beulich
2016-12-13 14:13 ` [PATCH 1/6] x86emul: CMPXCHG{8, 16}B " Andrew Cooper
2016-12-13 14:02 ` [PATCH 2/6] x86emul: reduce CMPXCHG{8, 16}B footprint and casting Jan Beulich
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:01 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
This removes 0F C7 from the list of two-byte opcodes treating prefixes
66, F3, and F2 as opcode extensions. We better manually handle this in
the opcode specific code:
- CMPXCHG8B ignores all these prefixes (its handling is being adjusted
accordingly, with a respective test case added as well, to avoid
re-introducing the subject of XSA-200),
- RDRAND/RDSEED (support to be added subsequently) honor 66, but treat
F3 and F2 as opcode extensions (resolving to RDPID in the RDSEED
case, which in turn ignores 66).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -364,6 +364,24 @@ int main(int argc, char **argv)
goto fail;
printf("okay\n");
+ printf("%-40s", "Testing cmpxchg8b (%edi) [opsize]...");
+ instr[0] = 0x66; instr[1] = 0x0f; instr[2] = 0xc7; instr[3] = 0x0f;
+ res[0] = 0x12345678;
+ res[1] = 0x87654321;
+ regs.eflags = 0x200;
+ regs.eip = (unsigned long)&instr[0];
+ regs.edi = (unsigned long)res;
+ rc = x86_emulate(&ctxt, &emulops);
+ if ( (rc != X86EMUL_OKAY) ||
+ (res[0] != 0x12345678) ||
+ (res[1] != 0x87654321) ||
+ (regs.eax != 0x12345678) ||
+ (regs.edx != 0x87654321) ||
+ ((regs.eflags&0x240) != 0x200) ||
+ (regs.eip != (unsigned long)&instr[4]) )
+ goto fail;
+ printf("okay\n");
+
printf("%-40s", "Testing movsxbd (%eax),%ecx...");
instr[0] = 0x0f; instr[1] = 0xbe; instr[2] = 0x08;
regs.eflags = 0x200;
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1955,7 +1955,7 @@ x86_decode_twobyte(
case 0x79 ... 0x7f:
case 0xae:
case 0xc2:
- case 0xc4 ... 0xc7:
+ case 0xc4 ... 0xc6:
case 0xd0 ... 0xfe:
ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
break;
@@ -5271,8 +5271,12 @@ x86_emulate(
generate_exception_if((modrm_reg & 7) != 1, EXC_UD);
generate_exception_if(ea.type != OP_MEM, EXC_UD);
if ( op_bytes == 8 )
+ {
host_and_vcpu_must_have(cx16);
- op_bytes *= 2;
+ op_bytes = 16;
+ }
+ else
+ op_bytes = 8;
/* Get actual old value. */
if ( (rc = ops->read(ea.mem.seg, ea.mem.off, old, op_bytes,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/6] x86emul: CMPXCHG{8, 16}B ignore prefixes
2016-12-13 14:01 ` [PATCH 1/6] x86emul: CMPXCHG{8,16}B ignore prefixes Jan Beulich
@ 2016-12-13 14:13 ` Andrew Cooper
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Cooper @ 2016-12-13 14:13 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 13/12/16 14:01, Jan Beulich wrote:
> This removes 0F C7 from the list of two-byte opcodes treating prefixes
> 66, F3, and F2 as opcode extensions. We better manually handle this in
> the opcode specific code:
> - CMPXCHG8B ignores all these prefixes (its handling is being adjusted
> accordingly, with a respective test case added as well, to avoid
> re-introducing the subject of XSA-200),
> - RDRAND/RDSEED (support to be added subsequently) honor 66, but treat
> F3 and F2 as opcode extensions (resolving to RDPID in the RDSEED
> case, which in turn ignores 66).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/6] x86emul: reduce CMPXCHG{8, 16}B footprint and casting
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
2016-12-13 14:01 ` [PATCH 1/6] x86emul: CMPXCHG{8,16}B ignore prefixes Jan Beulich
@ 2016-12-13 14:02 ` Jan Beulich
2016-12-13 14:26 ` Andrew Cooper
2016-12-13 14:03 ` [PATCH 3/6] x86emul: check for CMPXCHG8B availability Jan Beulich
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:02 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 4056 bytes --]
Re-use an existing stack variable (reducing stack footprint, which also
results in smaller code due to some stack accesses no longer needing a
32-bit displacement), at once using a union instead of casts. Also
switch to rex_prefix based conditionals instead of op_bytes ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -31,6 +31,11 @@
#define likely(x) __builtin_expect(!!(x), true)
#define unlikely(x) __builtin_expect(!!(x), false)
+#define container_of(ptr, type, member) ({ \
+ typeof(((type *)0)->member) *mptr__ = (ptr); \
+ (type *)((char *)mptr__ - offsetof(type, member)); \
+})
+
#define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
#define MMAP_SZ 16384
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -5282,11 +5282,14 @@ x86_emulate(
break;
case X86EMUL_OPC(0x0f, 0xc7): /* Grp9 (cmpxchg8b/cmpxchg16b) */ {
- unsigned long old[2], exp[2], new[2];
+ union {
+ uint32_t u32[2];
+ uint64_t u64[2];
+ } *old, *aux;
generate_exception_if((modrm_reg & 7) != 1, EXC_UD);
generate_exception_if(ea.type != OP_MEM, EXC_UD);
- if ( op_bytes == 8 )
+ if ( rex_prefix & REX_W )
{
host_and_vcpu_must_have(cx16);
op_bytes = 16;
@@ -5295,35 +5298,52 @@ x86_emulate(
else
op_bytes = 8;
+ old = container_of(&mmvalp->ymm[0], typeof(*old), u64[0]);
+ aux = container_of(&mmvalp->ymm[2], typeof(*old), u64[0]);
+
/* Get actual old value. */
if ( (rc = ops->read(ea.mem.seg, ea.mem.off, old, op_bytes,
- ctxt)) != 0 )
+ ctxt)) != X86EMUL_OKAY )
goto done;
- /* Get expected and proposed values. */
- if ( op_bytes == 8 )
+ /* Get expected value. */
+ if ( !(rex_prefix & REX_W) )
{
- ((uint32_t *)exp)[0] = _regs.eax; ((uint32_t *)exp)[1] = _regs.edx;
- ((uint32_t *)new)[0] = _regs.ebx; ((uint32_t *)new)[1] = _regs.ecx;
+ aux->u32[0] = _regs.eax;
+ aux->u32[1] = _regs.edx;
}
else
{
- exp[0] = _regs.eax; exp[1] = _regs.edx;
- new[0] = _regs.ebx; new[1] = _regs.ecx;
+ aux->u64[0] = _regs.eax;
+ aux->u64[1] = _regs.edx;
}
- if ( memcmp(old, exp, op_bytes) )
+ if ( memcmp(old, aux, op_bytes) )
{
/* Expected != actual: store actual to rDX:rAX and clear ZF. */
- _regs.eax = (op_bytes == 8) ? ((uint32_t *)old)[0] : old[0];
- _regs.edx = (op_bytes == 8) ? ((uint32_t *)old)[1] : old[1];
+ _regs.eax = !(rex_prefix & REX_W) ? old->u32[0] : old->u64[0];
+ _regs.edx = !(rex_prefix & REX_W) ? old->u32[1] : old->u64[1];
_regs.eflags &= ~EFLG_ZF;
}
else
{
- /* Expected == actual: attempt atomic cmpxchg and set ZF. */
- if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old,
- new, op_bytes, ctxt)) != 0 )
+ /*
+ * Expected == actual: Get proposed value, attempt atomic cmpxchg
+ * and set ZF.
+ */
+ if ( !(rex_prefix & REX_W) )
+ {
+ aux->u32[0] = _regs.ebx;
+ aux->u32[1] = _regs.ecx;
+ }
+ else
+ {
+ aux->u64[0] = _regs.ebx;
+ aux->u64[1] = _regs.ecx;
+ }
+
+ if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old, aux,
+ op_bytes, ctxt)) != X86EMUL_OKAY )
goto done;
_regs.eflags |= EFLG_ZF;
}
[-- Attachment #2: x86emul-CMPXCHGnB-casts.patch --]
[-- Type: text/plain, Size: 4106 bytes --]
x86emul: reduce CMPXCHG{8,16}B footprint and casting
Re-use an existing stack variable (reducing stack footprint, which also
results in smaller code due to some stack accesses no longer needing a
32-bit displacement), at once using a union instead of casts. Also
switch to rex_prefix based conditionals instead of op_bytes ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -31,6 +31,11 @@
#define likely(x) __builtin_expect(!!(x), true)
#define unlikely(x) __builtin_expect(!!(x), false)
+#define container_of(ptr, type, member) ({ \
+ typeof(((type *)0)->member) *mptr__ = (ptr); \
+ (type *)((char *)mptr__ - offsetof(type, member)); \
+})
+
#define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
#define MMAP_SZ 16384
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -5282,11 +5282,14 @@ x86_emulate(
break;
case X86EMUL_OPC(0x0f, 0xc7): /* Grp9 (cmpxchg8b/cmpxchg16b) */ {
- unsigned long old[2], exp[2], new[2];
+ union {
+ uint32_t u32[2];
+ uint64_t u64[2];
+ } *old, *aux;
generate_exception_if((modrm_reg & 7) != 1, EXC_UD);
generate_exception_if(ea.type != OP_MEM, EXC_UD);
- if ( op_bytes == 8 )
+ if ( rex_prefix & REX_W )
{
host_and_vcpu_must_have(cx16);
op_bytes = 16;
@@ -5295,35 +5298,52 @@ x86_emulate(
else
op_bytes = 8;
+ old = container_of(&mmvalp->ymm[0], typeof(*old), u64[0]);
+ aux = container_of(&mmvalp->ymm[2], typeof(*old), u64[0]);
+
/* Get actual old value. */
if ( (rc = ops->read(ea.mem.seg, ea.mem.off, old, op_bytes,
- ctxt)) != 0 )
+ ctxt)) != X86EMUL_OKAY )
goto done;
- /* Get expected and proposed values. */
- if ( op_bytes == 8 )
+ /* Get expected value. */
+ if ( !(rex_prefix & REX_W) )
{
- ((uint32_t *)exp)[0] = _regs.eax; ((uint32_t *)exp)[1] = _regs.edx;
- ((uint32_t *)new)[0] = _regs.ebx; ((uint32_t *)new)[1] = _regs.ecx;
+ aux->u32[0] = _regs.eax;
+ aux->u32[1] = _regs.edx;
}
else
{
- exp[0] = _regs.eax; exp[1] = _regs.edx;
- new[0] = _regs.ebx; new[1] = _regs.ecx;
+ aux->u64[0] = _regs.eax;
+ aux->u64[1] = _regs.edx;
}
- if ( memcmp(old, exp, op_bytes) )
+ if ( memcmp(old, aux, op_bytes) )
{
/* Expected != actual: store actual to rDX:rAX and clear ZF. */
- _regs.eax = (op_bytes == 8) ? ((uint32_t *)old)[0] : old[0];
- _regs.edx = (op_bytes == 8) ? ((uint32_t *)old)[1] : old[1];
+ _regs.eax = !(rex_prefix & REX_W) ? old->u32[0] : old->u64[0];
+ _regs.edx = !(rex_prefix & REX_W) ? old->u32[1] : old->u64[1];
_regs.eflags &= ~EFLG_ZF;
}
else
{
- /* Expected == actual: attempt atomic cmpxchg and set ZF. */
- if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old,
- new, op_bytes, ctxt)) != 0 )
+ /*
+ * Expected == actual: Get proposed value, attempt atomic cmpxchg
+ * and set ZF.
+ */
+ if ( !(rex_prefix & REX_W) )
+ {
+ aux->u32[0] = _regs.ebx;
+ aux->u32[1] = _regs.ecx;
+ }
+ else
+ {
+ aux->u64[0] = _regs.ebx;
+ aux->u64[1] = _regs.ecx;
+ }
+
+ if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old, aux,
+ op_bytes, ctxt)) != X86EMUL_OKAY )
goto done;
_regs.eflags |= EFLG_ZF;
}
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/6] x86emul: reduce CMPXCHG{8, 16}B footprint and casting
2016-12-13 14:02 ` [PATCH 2/6] x86emul: reduce CMPXCHG{8, 16}B footprint and casting Jan Beulich
@ 2016-12-13 14:26 ` Andrew Cooper
2016-12-13 15:08 ` Jan Beulich
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Cooper @ 2016-12-13 14:26 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 13/12/16 14:02, Jan Beulich wrote:
> Re-use an existing stack variable (reducing stack footprint, which also
> results in smaller code due to some stack accesses no longer needing a
> 32-bit displacement), at once using a union instead of casts. Also
> switch to rex_prefix based conditionals instead of op_bytes ones.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/tools/tests/x86_emulator/x86_emulate.h
> +++ b/tools/tests/x86_emulator/x86_emulate.h
> @@ -31,6 +31,11 @@
> #define likely(x) __builtin_expect(!!(x), true)
> #define unlikely(x) __builtin_expect(!!(x), false)
>
> +#define container_of(ptr, type, member) ({ \
> + typeof(((type *)0)->member) *mptr__ = (ptr); \
> + (type *)((char *)mptr__ - offsetof(type, member)); \
> +})
> +
Please could we use __builtin_containerof()? I believe It is available
on all of the compilers we support.
This particular construct causes UBSAN to have a fit.
> #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
>
> #define MMAP_SZ 16384
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -5282,11 +5282,14 @@ x86_emulate(
> break;
>
> case X86EMUL_OPC(0x0f, 0xc7): /* Grp9 (cmpxchg8b/cmpxchg16b) */ {
> - unsigned long old[2], exp[2], new[2];
> + union {
> + uint32_t u32[2];
> + uint64_t u64[2];
> + } *old, *aux;
>
> generate_exception_if((modrm_reg & 7) != 1, EXC_UD);
> generate_exception_if(ea.type != OP_MEM, EXC_UD);
> - if ( op_bytes == 8 )
> + if ( rex_prefix & REX_W )
> {
> host_and_vcpu_must_have(cx16);
> op_bytes = 16;
> @@ -5295,35 +5298,52 @@ x86_emulate(
> else
> op_bytes = 8;
>
> + old = container_of(&mmvalp->ymm[0], typeof(*old), u64[0]);
> + aux = container_of(&mmvalp->ymm[2], typeof(*old), u64[0]);
This should be typeof(*aux), although it makes no difference at the moment.
> +
> /* Get actual old value. */
> if ( (rc = ops->read(ea.mem.seg, ea.mem.off, old, op_bytes,
> - ctxt)) != 0 )
> + ctxt)) != X86EMUL_OKAY )
> goto done;
>
> - /* Get expected and proposed values. */
> - if ( op_bytes == 8 )
> + /* Get expected value. */
> + if ( !(rex_prefix & REX_W) )
> {
> - ((uint32_t *)exp)[0] = _regs.eax; ((uint32_t *)exp)[1] = _regs.edx;
> - ((uint32_t *)new)[0] = _regs.ebx; ((uint32_t *)new)[1] = _regs.ecx;
> + aux->u32[0] = _regs.eax;
> + aux->u32[1] = _regs.edx;
> }
> else
> {
> - exp[0] = _regs.eax; exp[1] = _regs.edx;
> - new[0] = _regs.ebx; new[1] = _regs.ecx;
> + aux->u64[0] = _regs.eax;
> + aux->u64[1] = _regs.edx;
> }
>
> - if ( memcmp(old, exp, op_bytes) )
> + if ( memcmp(old, aux, op_bytes) )
> {
> /* Expected != actual: store actual to rDX:rAX and clear ZF. */
> - _regs.eax = (op_bytes == 8) ? ((uint32_t *)old)[0] : old[0];
> - _regs.edx = (op_bytes == 8) ? ((uint32_t *)old)[1] : old[1];
> + _regs.eax = !(rex_prefix & REX_W) ? old->u32[0] : old->u64[0];
> + _regs.edx = !(rex_prefix & REX_W) ? old->u32[1] : old->u64[1];
> _regs.eflags &= ~EFLG_ZF;
This clearing of ZF should be unconditional. I think this is a latent
bug, but if the cmpxchg() fails, we would exit having failed the xchg,
but leaving ZF stale.
~Andrew
> }
> else
> {
> - /* Expected == actual: attempt atomic cmpxchg and set ZF. */
> - if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old,
> - new, op_bytes, ctxt)) != 0 )
> + /*
> + * Expected == actual: Get proposed value, attempt atomic cmpxchg
> + * and set ZF.
> + */
> + if ( !(rex_prefix & REX_W) )
> + {
> + aux->u32[0] = _regs.ebx;
> + aux->u32[1] = _regs.ecx;
> + }
> + else
> + {
> + aux->u64[0] = _regs.ebx;
> + aux->u64[1] = _regs.ecx;
> + }
> +
> + if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old, aux,
> + op_bytes, ctxt)) != X86EMUL_OKAY )
> goto done;
> _regs.eflags |= EFLG_ZF;
> }
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/6] x86emul: reduce CMPXCHG{8, 16}B footprint and casting
2016-12-13 14:26 ` Andrew Cooper
@ 2016-12-13 15:08 ` Jan Beulich
0 siblings, 0 replies; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 15:08 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
>>> On 13.12.16 at 15:26, <andrew.cooper3@citrix.com> wrote:
> On 13/12/16 14:02, Jan Beulich wrote:
>> --- a/tools/tests/x86_emulator/x86_emulate.h
>> +++ b/tools/tests/x86_emulator/x86_emulate.h
>> @@ -31,6 +31,11 @@
>> #define likely(x) __builtin_expect(!!(x), true)
>> #define unlikely(x) __builtin_expect(!!(x), false)
>>
>> +#define container_of(ptr, type, member) ({ \
>> + typeof(((type *)0)->member) *mptr__ = (ptr); \
>> + (type *)((char *)mptr__ - offsetof(type, member)); \
>> +})
>> +
>
> Please could we use __builtin_containerof()? I believe It is available
> on all of the compilers we support.
Where have you found reference to this? I can't find it in the doc
(nor anything similar, consulting the index), and grep-ing the gcc
sources doesn't reveal anything either.
>> @@ -5295,35 +5298,52 @@ x86_emulate(
>> else
>> op_bytes = 8;
>>
>> + old = container_of(&mmvalp->ymm[0], typeof(*old), u64[0]);
>> + aux = container_of(&mmvalp->ymm[2], typeof(*old), u64[0]);
>
> This should be typeof(*aux), although it makes no difference at the moment.
Oh, right - copy-and-paste mistake. Fixed.
>> - if ( memcmp(old, exp, op_bytes) )
>> + if ( memcmp(old, aux, op_bytes) )
>> {
>> /* Expected != actual: store actual to rDX:rAX and clear ZF. */
>> - _regs.eax = (op_bytes == 8) ? ((uint32_t *)old)[0] : old[0];
>> - _regs.edx = (op_bytes == 8) ? ((uint32_t *)old)[1] : old[1];
>> + _regs.eax = !(rex_prefix & REX_W) ? old->u32[0] : old->u64[0];
>> + _regs.edx = !(rex_prefix & REX_W) ? old->u32[1] : old->u64[1];
>> _regs.eflags &= ~EFLG_ZF;
>
> This clearing of ZF should be unconditional. I think this is a latent
> bug, but if the cmpxchg() fails, we would exit having failed the xchg,
> but leaving ZF stale.
First of all - this request of yours is unrelated to this patch: The
conditions under which ZF gets altered don't get changed here
at all. And then this comment or yours is similar to one you gave
elsewhere not all that long ago; my answer is the same here -
we don't commit _regs.eflags to ctxt->regs if anything fails. And
namely when ops->cmpxchg() returns RETRY, then we indeed
want to retry with the flag unaltered.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/6] x86emul: check for CMPXCHG8B availability
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
2016-12-13 14:01 ` [PATCH 1/6] x86emul: CMPXCHG{8,16}B ignore prefixes Jan Beulich
2016-12-13 14:02 ` [PATCH 2/6] x86emul: reduce CMPXCHG{8, 16}B footprint and casting Jan Beulich
@ 2016-12-13 14:03 ` Jan Beulich
2016-12-13 14:27 ` Andrew Cooper
2016-12-13 14:03 ` [PATCH 4/6] x86emul: check for CLFLUSH{, OPT} availability Jan Beulich
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:03 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 964 bytes --]
We can't exclude someone wanting to hide the instruction from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1285,6 +1285,7 @@ static bool vcpu_has(
}
#define vcpu_has_fpu() vcpu_has( 1, EDX, 0, ctxt, ops)
+#define vcpu_has_cx8() vcpu_has( 1, EDX, 8, ctxt, ops)
#define vcpu_has_cmov() vcpu_has( 1, EDX, 15, ctxt, ops)
#define vcpu_has_clflush() vcpu_has( 1, EDX, 19, ctxt, ops)
#define vcpu_has_mmx() vcpu_has( 1, EDX, 23, ctxt, ops)
@@ -5296,7 +5297,10 @@ x86_emulate(
op_bytes = 16;
}
else
+ {
+ vcpu_must_have(cx8);
op_bytes = 8;
+ }
old = container_of(&mmvalp->ymm[0], typeof(*old), u64[0]);
aux = container_of(&mmvalp->ymm[2], typeof(*old), u64[0]);
[-- Attachment #2: x86emul-check-CPUID-cx8.patch --]
[-- Type: text/plain, Size: 1003 bytes --]
x86emul: check for CMPXCHG8B availability
We can't exclude someone wanting to hide the instruction from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1285,6 +1285,7 @@ static bool vcpu_has(
}
#define vcpu_has_fpu() vcpu_has( 1, EDX, 0, ctxt, ops)
+#define vcpu_has_cx8() vcpu_has( 1, EDX, 8, ctxt, ops)
#define vcpu_has_cmov() vcpu_has( 1, EDX, 15, ctxt, ops)
#define vcpu_has_clflush() vcpu_has( 1, EDX, 19, ctxt, ops)
#define vcpu_has_mmx() vcpu_has( 1, EDX, 23, ctxt, ops)
@@ -5296,7 +5297,10 @@ x86_emulate(
op_bytes = 16;
}
else
+ {
+ vcpu_must_have(cx8);
op_bytes = 8;
+ }
old = container_of(&mmvalp->ymm[0], typeof(*old), u64[0]);
aux = container_of(&mmvalp->ymm[2], typeof(*old), u64[0]);
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 4/6] x86emul: check for CLFLUSH{, OPT} availability
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
` (2 preceding siblings ...)
2016-12-13 14:03 ` [PATCH 3/6] x86emul: check for CMPXCHG8B availability Jan Beulich
@ 2016-12-13 14:03 ` Jan Beulich
2016-12-13 14:27 ` Andrew Cooper
2016-12-13 14:06 ` [PATCH 5/6] x86emul: check for SYSENTER/SYSEXIT availability Jan Beulich
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:03 UTC (permalink / raw)
To: xen-devel, Jan Beulich; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]
We can't exclude someone wanting to hide either of the instructions
from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1296,6 +1296,7 @@ static bool vcpu_has(
#define vcpu_has_hle() vcpu_has( 7, EBX, 4, ctxt, ops)
#define vcpu_has_rtm() vcpu_has( 7, EBX, 11, ctxt, ops)
#define vcpu_has_smap() vcpu_has( 7, EBX, 20, ctxt, ops)
+#define vcpu_has_clflushopt() vcpu_has( 7, EBX, 23, ctxt, ops)
#define vcpu_must_have(feat) \
generate_exception_if(!vcpu_has_##feat(), EXC_UD)
@@ -5104,6 +5105,10 @@ x86_emulate(
{
case 7: /* clflush{,opt} */
fail_if(modrm_mod == 3);
+ if ( !vex.pfx )
+ vcpu_must_have(clflush);
+ else
+ vcpu_must_have(clflushopt);
fail_if(ops->wbinvd == NULL);
if ( (rc = ops->wbinvd(ctxt)) != 0 )
goto done;
[-- Attachment #2: x86emul-check-CPUID-clflush.patch --]
[-- Type: text/plain, Size: 1097 bytes --]
x86emul: check for CLFLUSH{,OPT} availability
We can't exclude someone wanting to hide either of the instructions
from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1296,6 +1296,7 @@ static bool vcpu_has(
#define vcpu_has_hle() vcpu_has( 7, EBX, 4, ctxt, ops)
#define vcpu_has_rtm() vcpu_has( 7, EBX, 11, ctxt, ops)
#define vcpu_has_smap() vcpu_has( 7, EBX, 20, ctxt, ops)
+#define vcpu_has_clflushopt() vcpu_has( 7, EBX, 23, ctxt, ops)
#define vcpu_must_have(feat) \
generate_exception_if(!vcpu_has_##feat(), EXC_UD)
@@ -5104,6 +5105,10 @@ x86_emulate(
{
case 7: /* clflush{,opt} */
fail_if(modrm_mod == 3);
+ if ( !vex.pfx )
+ vcpu_must_have(clflush);
+ else
+ vcpu_must_have(clflushopt);
fail_if(ops->wbinvd == NULL);
if ( (rc = ops->wbinvd(ctxt)) != 0 )
goto done;
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 5/6] x86emul: check for SYSENTER/SYSEXIT availability
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
` (3 preceding siblings ...)
2016-12-13 14:03 ` [PATCH 4/6] x86emul: check for CLFLUSH{, OPT} availability Jan Beulich
@ 2016-12-13 14:06 ` Jan Beulich
2016-12-13 14:28 ` Andrew Cooper
2016-12-13 14:07 ` [PATCH 6/6] x86emul: check for LAHF_LM availability Jan Beulich
2016-12-13 14:11 ` [PATCH 7/6] x86emul: MOVNTI does not allow REP prefixes Jan Beulich
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:06 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]
We can't exclude someone wanting to hide the instructions from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Looks like I can't count - I've mistakenly omitted this patch from
the overview mail, so there'll be a total of 7.
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1306,6 +1306,7 @@ static bool vcpu_has(
#define vcpu_has_fpu() vcpu_has( 1, EDX, 0, ctxt, ops)
#define vcpu_has_cx8() vcpu_has( 1, EDX, 8, ctxt, ops)
+#define vcpu_has_sep() vcpu_has( 1, EDX, 11, ctxt, ops)
#define vcpu_has_cmov() vcpu_has( 1, EDX, 15, ctxt, ops)
#define vcpu_has_clflush() vcpu_has( 1, EDX, 19, ctxt, ops)
#define vcpu_has_mmx() vcpu_has( 1, EDX, 23, ctxt, ops)
@@ -5132,6 +5133,7 @@ x86_emulate(
uint64_t msr_content;
int lm;
+ vcpu_must_have(sep);
generate_exception_if(mode_ring0(), EXC_GP, 0);
generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
@@ -5179,6 +5181,7 @@ x86_emulate(
{
uint64_t msr_content;
+ vcpu_must_have(sep);
generate_exception_if(!mode_ring0(), EXC_GP, 0);
generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
[-- Attachment #2: x86emul-check-CPUID-sep.patch --]
[-- Type: text/plain, Size: 1251 bytes --]
x86emul: check for SYSENTER/SYSEXIT availability
We can't exclude someone wanting to hide the instructions from guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1306,6 +1306,7 @@ static bool vcpu_has(
#define vcpu_has_fpu() vcpu_has( 1, EDX, 0, ctxt, ops)
#define vcpu_has_cx8() vcpu_has( 1, EDX, 8, ctxt, ops)
+#define vcpu_has_sep() vcpu_has( 1, EDX, 11, ctxt, ops)
#define vcpu_has_cmov() vcpu_has( 1, EDX, 15, ctxt, ops)
#define vcpu_has_clflush() vcpu_has( 1, EDX, 19, ctxt, ops)
#define vcpu_has_mmx() vcpu_has( 1, EDX, 23, ctxt, ops)
@@ -5132,6 +5133,7 @@ x86_emulate(
uint64_t msr_content;
int lm;
+ vcpu_must_have(sep);
generate_exception_if(mode_ring0(), EXC_GP, 0);
generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
@@ -5179,6 +5181,7 @@ x86_emulate(
{
uint64_t msr_content;
+ vcpu_must_have(sep);
generate_exception_if(!mode_ring0(), EXC_GP, 0);
generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 6/6] x86emul: check for LAHF_LM availability
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
` (4 preceding siblings ...)
2016-12-13 14:06 ` [PATCH 5/6] x86emul: check for SYSENTER/SYSEXIT availability Jan Beulich
@ 2016-12-13 14:07 ` Jan Beulich
2016-12-13 14:28 ` Andrew Cooper
2016-12-13 14:11 ` [PATCH 7/6] x86emul: MOVNTI does not allow REP prefixes Jan Beulich
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:07 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]
We can't exclude someone wanting to hide LAHF/SAHF from 64-bit guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1291,6 +1291,7 @@ static bool vcpu_has(
#define vcpu_has_sse4_2() vcpu_has( 1, ECX, 20, ctxt, ops)
#define vcpu_has_movbe() vcpu_has( 1, ECX, 22, ctxt, ops)
#define vcpu_has_avx() vcpu_has( 1, ECX, 28, ctxt, ops)
+#define vcpu_has_lahf_lm() vcpu_has(0x80000001, ECX, 0, ctxt, ops)
#define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX, 5, ctxt, ops)
#define vcpu_has_misalignsse() vcpu_has(0x80000001, ECX, 7, ctxt, ops)
#define vcpu_has_bmi1() vcpu_has( 7, EBX, 3, ctxt, ops)
@@ -3201,11 +3202,15 @@ x86_emulate(
}
case 0x9e: /* sahf */
+ if ( mode_64bit() )
+ vcpu_must_have(lahf_lm);
*(uint8_t *)&_regs.eflags = (((uint8_t *)&_regs.eax)[1] &
EFLAGS_MASK) | EFLG_MBS;
break;
case 0x9f: /* lahf */
+ if ( mode_64bit() )
+ vcpu_must_have(lahf_lm);
((uint8_t *)&_regs.eax)[1] = (_regs.eflags & EFLAGS_MASK) | EFLG_MBS;
break;
[-- Attachment #2: x86emul-check-CPUID-lahf_lm.patch --]
[-- Type: text/plain, Size: 1317 bytes --]
x86emul: check for LAHF_LM availability
We can't exclude someone wanting to hide LAHF/SAHF from 64-bit guests.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1291,6 +1291,7 @@ static bool vcpu_has(
#define vcpu_has_sse4_2() vcpu_has( 1, ECX, 20, ctxt, ops)
#define vcpu_has_movbe() vcpu_has( 1, ECX, 22, ctxt, ops)
#define vcpu_has_avx() vcpu_has( 1, ECX, 28, ctxt, ops)
+#define vcpu_has_lahf_lm() vcpu_has(0x80000001, ECX, 0, ctxt, ops)
#define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX, 5, ctxt, ops)
#define vcpu_has_misalignsse() vcpu_has(0x80000001, ECX, 7, ctxt, ops)
#define vcpu_has_bmi1() vcpu_has( 7, EBX, 3, ctxt, ops)
@@ -3201,11 +3202,15 @@ x86_emulate(
}
case 0x9e: /* sahf */
+ if ( mode_64bit() )
+ vcpu_must_have(lahf_lm);
*(uint8_t *)&_regs.eflags = (((uint8_t *)&_regs.eax)[1] &
EFLAGS_MASK) | EFLG_MBS;
break;
case 0x9f: /* lahf */
+ if ( mode_64bit() )
+ vcpu_must_have(lahf_lm);
((uint8_t *)&_regs.eax)[1] = (_regs.eflags & EFLAGS_MASK) | EFLG_MBS;
break;
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 7/6] x86emul: MOVNTI does not allow REP prefixes
2016-12-13 13:50 [PATCH 0/6] x86emul: XSA-200 follow-up Jan Beulich
` (5 preceding siblings ...)
2016-12-13 14:07 ` [PATCH 6/6] x86emul: check for LAHF_LM availability Jan Beulich
@ 2016-12-13 14:11 ` Jan Beulich
2016-12-13 14:32 ` Andrew Cooper
6 siblings, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2016-12-13 14:11 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]
Just like 66, prefixes F3 and F2 cause #UD.
Also adjust a related comment, which in its previous wording was
misleading (as in 16-bit mode there would nothing be undone when
adjusting operand size from 2 to 4).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1954,8 +1954,7 @@ x86_decode_twobyte(
case 0x50 ... 0x77:
case 0x79 ... 0x7f:
case 0xae:
- case 0xc2:
- case 0xc4 ... 0xc6:
+ case 0xc2 ... 0xc6:
case 0xd0 ... 0xfe:
ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
break;
@@ -2461,8 +2460,8 @@ x86_decode(
}
/*
- * Undo the operand-size override effect of prefix 66 when it was
- * determined to have another meaning.
+ * When prefix 66 has a meaning different from operand-size override,
+ * operand size defaults to 4 and can't be overridden to 2.
*/
if ( op_bytes == 2 &&
(ctxt->opcode & X86EMUL_OPC_PFX_MASK) == X86EMUL_OPC_66(0, 0) )
@@ -5291,7 +5290,6 @@ x86_emulate(
case X86EMUL_OPC(0x0f, 0xc3): /* movnti */
/* Ignore the non-temporal hint for now. */
vcpu_must_have(sse2);
- generate_exception_if(dst.bytes <= 2, EXC_UD);
dst.val = src.val;
break;
[-- Attachment #2: x86emul-MOVNTI-no-REP.patch --]
[-- Type: text/plain, Size: 1404 bytes --]
x86emul: MOVNTI does not allow REP prefixes
Just like 66, prefixes F3 and F2 cause #UD.
Also adjust a related comment, which in its previous wording was
misleading (as in 16-bit mode there would nothing be undone when
adjusting operand size from 2 to 4).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1954,8 +1954,7 @@ x86_decode_twobyte(
case 0x50 ... 0x77:
case 0x79 ... 0x7f:
case 0xae:
- case 0xc2:
- case 0xc4 ... 0xc6:
+ case 0xc2 ... 0xc6:
case 0xd0 ... 0xfe:
ctxt->opcode |= MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
break;
@@ -2461,8 +2460,8 @@ x86_decode(
}
/*
- * Undo the operand-size override effect of prefix 66 when it was
- * determined to have another meaning.
+ * When prefix 66 has a meaning different from operand-size override,
+ * operand size defaults to 4 and can't be overridden to 2.
*/
if ( op_bytes == 2 &&
(ctxt->opcode & X86EMUL_OPC_PFX_MASK) == X86EMUL_OPC_66(0, 0) )
@@ -5291,7 +5290,6 @@ x86_emulate(
case X86EMUL_OPC(0x0f, 0xc3): /* movnti */
/* Ignore the non-temporal hint for now. */
vcpu_must_have(sse2);
- generate_exception_if(dst.bytes <= 2, EXC_UD);
dst.val = src.val;
break;
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread