Messages in this thread | | | Subject | Re: [PATCH] kvm: implement VEX prefix decoder, bextr/andn | From | Paolo Bonzini <> | Date | Fri, 29 Jun 2018 13:47:00 +0200 |
| |
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; > + } > }
| |