On 15/06/2022 11:28, Jan Beulich wrote: > The Map6 encoding space is a very sparse clone of the "0f38" one. Once > again re-use that table, as the entries corresponding to invalid opcodes > in Map6 are simply benign with simd_size forced to other than simd_none > (preventing undue memory reads in SrcMem handling early in > x86_emulate()).
Again, this needs communicating in the code. > > Signed-off-by: Jan Beulich <[email protected]> > > --- a/xen/arch/x86/x86_emulate/decode.c > +++ b/xen/arch/x86/x86_emulate/decode.c > @@ -1231,6 +1231,16 @@ int x86emul_decode(struct x86_emulate_st > d = twobyte_table[b].desc; > s->simd_size = twobyte_table[b].size ?: simd_other; > break; > + > + case evex_map6: > + if ( !evex_encoded() ) > + { > + rc = X86EMUL_UNRECOGNIZED; > + goto done; > + } > + opcode |= MASK_INSR(6, X86EMUL_OPC_EXT_MASK); > + d = twobyte_table[0x38].desc; So the manual says that map spaces 2, 3, 5 and 6 are regular maps (insn length doesn't depend on the opcode byte), with map 3 being the only one which takes an imm byte. I think this means that SrcImm and SrcImmByte will cause x86_decode() to get the wrong instruction length. > + break; > } > } > else if ( s->ext < ext_8f08 + ARRAY_SIZE(xop_table) ) > @@ -1479,6 +1489,24 @@ int x86emul_decode(struct x86_emulate_st > disp8scale = decode_disp8scale(twobyte_table[b].d8s, s); > break; > > + case ext_map6: > + d = ext0f38_table[b].to_mem ? DstMem | SrcReg > + : DstReg | SrcMem; > + if ( ext0f38_table[b].two_op ) > + d |= TwoOp; ... but here we discard the table desc and construct it from first principles. Why are we processing it twice? ~Andrew
