Thanks, I have some comments on the decoding logic.
On 26/06/2018 14:14, Mason Lee Back wrote:
> + if (ctxt->b == 0xC4 || ctxt->b == 0xC5) {
This should be exactly the condition that you are removing below:
if ((ctxt->b == 0xc5 || ctxt->b == 0xc4) &&
(mode == X86EMUL_MODE_PROT64 || (ctxt->modrm & 0xc0) == 0xc0))
so that you don't need the "reinterpret as LES or LDS" todo. It should also
be moved earlier, right after done_prefixes. The existing code starting
at
/* Opcode byts(s). */
and ending just before
ctxt->d = opcode.flags;
will become the "else" branch.
Otherwise looks at least... sane. :)
Paolo
> + ctxt->vex.prefix = ctxt->b;
> + if (ctxt->b == 0xC4) {
> + ctxt->vex.value = insn_fetch(u16, ctxt);
> + } else {
> + ctxt->vex.value = insn_fetch(u8, ctxt) << 8;
> + ctxt->vex.r = ctxt->vex.w;
> + ctxt->vex.w = 1;
> + ctxt->vex.x = 1;
> + ctxt->vex.b = 1;
> + ctxt->vex.m = 1;
> + }
>
> - /* vex-prefix instructions are not implemented */
> - if (ctxt->opcode_len == 1 && (ctxt->b == 0xc5 || ctxt->b == 0xc4) &&
> - (mode == X86EMUL_MODE_PROT64 || (ctxt->modrm & 0xc0) == 0xc0)) {
> - ctxt->d = NotImpl;
> + if (mode != X86EMUL_MODE_PROT64 && (!ctxt->vex.r ||
> !ctxt->vex.x)) {
> + /* todo: reinterpret as LES (0xC4) or LDS (0xC5)
> instruction */
> + return EMULATION_FAILED;
> + }
> +
> + ctxt->rex_prefix |= ctxt->vex.r ? 0 : (1 << 2); /* rex.r */
> + ctxt->rex_prefix |= ctxt->vex.x ? 0 : (1 << 1); /* rex.x */
> + ctxt->rex_prefix |= ctxt->vex.b ? 0 : (1 << 0); /* rex.b */
> + if (mode == X86EMUL_MODE_PROT64 && ctxt->vex.w) {
> + ctxt->op_bytes = 8;
> + }
Please set ctxt->rex_prefix too for consistency.
> +
> + ctxt->b = insn_fetch(u8, ctxt);
> + switch (ctxt->vex.m) {
> + case 1:
> + opcode = twobyte_table[ctxt->b];
"ctxt->opcode_len = 2;" missing here.
> + break;
> + case 2:
> + opcode = opcode_map_0f_38[ctxt->b];
"ctxt->opcode_len = 3;" missing here.
> + break;
> + case 3:
> + /* KVM doesn't support this */
> + return EMULATION_FAILED;
> + default:
> + return EMULATION_FAILED;
> + }
> }
