| Message ID | 57D17F9D020000780010D160@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/09/16 14:11, Jan Beulich wrote: > This way we can at least size (and e.g. skip) them if needed, and we > also won't raise the wrong fault due to not having read all relevant > bytes. > > 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 > @@ -279,6 +279,12 @@ static const opcode_desc_t twobyte_table > ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM > }; > > +static const opcode_desc_t xop_table[] = { > + DstReg|SrcImmByte|ModRM, > + DstReg|SrcMem|ModRM, > + DstReg|SrcImm|ModRM, > +}; > + > #define REX_PREFIX 0x40 > #define REX_B 0x01 > #define REX_X 0x02 > @@ -1580,6 +1586,9 @@ struct x86_emulate_state { > ext_0f = vex_0f, > ext_0f38 = vex_0f38, > ext_0f3a = vex_0f3a, > + ext_8f08 = 8, > + ext_8f09, > + ext_8f0a, What is this = 8 for? I presume you didn't slip it in accidentally, but I still can't figure out why. > } ext; > uint8_t opcode; > uint8_t modrm, modrm_mod, modrm_reg, modrm_rm; > @@ -1802,7 +1811,7 @@ x86_decode( > modrm = insn_fetch_type(uint8_t); > modrm_mod = (modrm & 0xc0) >> 6; > > - if ( !ext && ((b & ~1) == 0xc4) ) > + if ( !ext && ((b & ~1) == 0xc4 || (b == 0x8f && (modrm & 0x18))) ) > switch ( def_ad_bytes ) > { > default: > @@ -1816,11 +1825,11 @@ x86_decode( > break; > /* fall through */ > case 8: > - /* VEX */ > + /* VEX / XOP */ > generate_exception_if(rex_prefix || vex.pfx, EXC_UD, -1); > > vex.raw[0] = modrm; > - if ( b & 1 ) > + if ( b == 0xc5 ) > { > vex.raw[1] = modrm; > vex.opcx = vex_0f; > @@ -1848,18 +1857,30 @@ x86_decode( > rex_prefix |= REX_R; > > b = insn_fetch_type(uint8_t); > - switch ( ext = vex.opcx ) > + ext = vex.opcx; > + if ( b != 0x8f ) > + { > + switch ( ext ) > + { > + case vex_0f: > + d = twobyte_table[b]; > + break; > + case vex_0f38: > + d = twobyte_table[0x38]; > + break; > + case vex_0f3a: > + d = twobyte_table[0x3a]; > + break; > + default: > + rc = X86EMUL_UNHANDLEABLE; > + goto done; > + } > + } > + else if ( ext < ext_8f08 + > + sizeof(xop_table) / sizeof(*xop_table) ) ARRAY_SIZE() ? ~Andrew
>>> On 14.09.16 at 18:11, <andrew.cooper3@citrix.com> wrote: > On 08/09/16 14:11, Jan Beulich wrote: >> @@ -1580,6 +1586,9 @@ struct x86_emulate_state { >> ext_0f = vex_0f, >> ext_0f38 = vex_0f38, >> ext_0f3a = vex_0f3a, >> + ext_8f08 = 8, >> + ext_8f09, >> + ext_8f0a, > > What is this = 8 for? I presume you didn't slip it in accidentally, but > I still can't figure out why. So I can use the value directly from vex.opcx, without further adjustment. >> @@ -1848,18 +1857,30 @@ x86_decode( >> rex_prefix |= REX_R; >> >> b = insn_fetch_type(uint8_t); >> - switch ( ext = vex.opcx ) >> + ext = vex.opcx; >> + if ( b != 0x8f ) >> + { >> + switch ( ext ) >> + { >> + case vex_0f: >> + d = twobyte_table[b]; >> + break; >> + case vex_0f38: >> + d = twobyte_table[0x38]; >> + break; >> + case vex_0f3a: >> + d = twobyte_table[0x3a]; >> + break; >> + default: >> + rc = X86EMUL_UNHANDLEABLE; >> + goto done; >> + } >> + } >> + else if ( ext < ext_8f08 + >> + sizeof(xop_table) / sizeof(*xop_table) ) > > ARRAY_SIZE() ? If we want to add another helper #define to the test code, yes. It being a single instance, that addition didn't seem worth it to me. Jan
On 14/09/16 17:21, Jan Beulich wrote: >>>> On 14.09.16 at 18:11, <andrew.cooper3@citrix.com> wrote: >> On 08/09/16 14:11, Jan Beulich wrote: >>> @@ -1580,6 +1586,9 @@ struct x86_emulate_state { >>> ext_0f = vex_0f, >>> ext_0f38 = vex_0f38, >>> ext_0f3a = vex_0f3a, >>> + ext_8f08 = 8, >>> + ext_8f09, >>> + ext_8f0a, >> What is this = 8 for? I presume you didn't slip it in accidentally, but >> I still can't figure out why. > So I can use the value directly from vex.opcx, without further > adjustment. Ok, but please leave a comment to that effect. ~Andrew
--- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -279,6 +279,12 @@ static const opcode_desc_t twobyte_table ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM }; +static const opcode_desc_t xop_table[] = { + DstReg|SrcImmByte|ModRM, + DstReg|SrcMem|ModRM, + DstReg|SrcImm|ModRM, +}; + #define REX_PREFIX 0x40 #define REX_B 0x01 #define REX_X 0x02 @@ -1580,6 +1586,9 @@ struct x86_emulate_state { ext_0f = vex_0f, ext_0f38 = vex_0f38, ext_0f3a = vex_0f3a, + ext_8f08 = 8, + ext_8f09, + ext_8f0a, } ext; uint8_t opcode; uint8_t modrm, modrm_mod, modrm_reg, modrm_rm; @@ -1802,7 +1811,7 @@ x86_decode( modrm = insn_fetch_type(uint8_t); modrm_mod = (modrm & 0xc0) >> 6; - if ( !ext && ((b & ~1) == 0xc4) ) + if ( !ext && ((b & ~1) == 0xc4 || (b == 0x8f && (modrm & 0x18))) ) switch ( def_ad_bytes ) { default: @@ -1816,11 +1825,11 @@ x86_decode( break; /* fall through */ case 8: - /* VEX */ + /* VEX / XOP */ generate_exception_if(rex_prefix || vex.pfx, EXC_UD, -1); vex.raw[0] = modrm; - if ( b & 1 ) + if ( b == 0xc5 ) { vex.raw[1] = modrm; vex.opcx = vex_0f; @@ -1848,18 +1857,30 @@ x86_decode( rex_prefix |= REX_R; b = insn_fetch_type(uint8_t); - switch ( ext = vex.opcx ) + ext = vex.opcx; + if ( b != 0x8f ) + { + switch ( ext ) + { + case vex_0f: + d = twobyte_table[b]; + break; + case vex_0f38: + d = twobyte_table[0x38]; + break; + case vex_0f3a: + d = twobyte_table[0x3a]; + break; + default: + rc = X86EMUL_UNHANDLEABLE; + goto done; + } + } + else if ( ext < ext_8f08 + + sizeof(xop_table) / sizeof(*xop_table) ) + d = xop_table[ext - ext_8f08]; + else { - case vex_0f: - d = twobyte_table[b]; - break; - case vex_0f38: - d = twobyte_table[0x38]; - break; - case vex_0f3a: - d = twobyte_table[0x3a]; - break; - default: rc = X86EMUL_UNHANDLEABLE; goto done; } @@ -1921,6 +1942,9 @@ x86_decode( case ext_0f: case ext_0f3a: + case ext_8f08: + case ext_8f09: + case ext_8f0a: break; case ext_0f38: @@ -2112,6 +2136,9 @@ x86_decode( case ext_0f38: case ext_0f3a: + case ext_8f08: + case ext_8f09: + case ext_8f0a: break; default: @@ -2332,6 +2359,9 @@ x86_emulate( default: ASSERT_UNREACHABLE(); case ext_0f3a: + case ext_8f08: + case ext_8f09: + case ext_8f0a: goto cannot_emulate; }