On 04/23/2012 06:37 PM, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa <yoshikawa.takuya@xxxxxxxxxxxxx>
>
> Although ModRM byte is read for group decoding, it is soon pushed back
> to make decode_modrm() fetch it later.
>
> We should consistently read it, only once, in decode_opcode() instead.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@xxxxxxxxxxxxx>
> Cc: Takuya Yoshikawa <takuya.yoshikawa@xxxxxxxxx>
> ---
> arch/x86/kvm/emulate.c | 19 ++++++++++++-------
> 1 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index e87570e..8729773 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -972,7 +972,6 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
> ctxt->modrm_rm = base_reg = (ctxt->rex_prefix & 1) << 3; /* REG.B */
> }
>
> - ctxt->modrm = insn_fetch(u8, ctxt);
> ctxt->modrm_mod |= (ctxt->modrm & 0xc0) >> 6;
> ctxt->modrm_reg |= (ctxt->modrm & 0x38) >> 3;
> ctxt->modrm_rm |= (ctxt->modrm & 0x07);
> @@ -3969,6 +3968,7 @@ done:
> * @ctxt: emulation context
> *
> * Decodes opcode bytes and reads opcode from table.
> + * The ModRM byte, if exists, is also fetched into ctxt->modrm.
> *
> * Returns X86EMUL_CONTINUE on success.
> */
> @@ -3977,6 +3977,8 @@ static int decode_opcode(struct x86_emulate_ctxt *ctxt)
> int rc = X86EMUL_CONTINUE;
> int goffset, simd_prefix;
> struct opcode opcode;
> + bool modrm_fetched = false;
> + u64 gtype;
>
> /* Two-byte opcode? */
> if (ctxt->b == 0x0f) {
> @@ -3988,17 +3990,18 @@ static int decode_opcode(struct x86_emulate_ctxt *ctxt)
>
> ctxt->d = opcode.flags;
>
> - while (ctxt->d & GroupMask) {
> - switch (ctxt->d & GroupMask) {
> - case Group:
> + while ((gtype = ctxt->d & GroupMask)) {
> + if (!modrm_fetched && gtype != Prefix) {
> ctxt->modrm = insn_fetch(u8, ctxt);
> - --ctxt->_eip;
> + modrm_fetched = true;
> + }
> +
> + switch (gtype) {
> + case Group:
> goffset = (ctxt->modrm >> 3) & 7;
> opcode = opcode.u.group[goffset];
> break;
> case GroupDual:
> - ctxt->modrm = insn_fetch(u8, ctxt);
> - --ctxt->_eip;
> goffset = (ctxt->modrm >> 3) & 7;
> if ((ctxt->modrm >> 6) == 3)
> opcode = opcode.u.gdual->mod3[goffset];
> @@ -4039,6 +4042,8 @@ static int decode_opcode(struct x86_emulate_ctxt *ctxt)
> ctxt->check_perm = opcode.check_perm;
> ctxt->intercept = opcode.intercept;
>
> + if (!modrm_fetched && (ctxt->d & ModRM))
> + ctxt->modrm = insn_fetch(u8, ctxt);
Instead of adding yet another conditional, how about doing something like
if ((c->d & ModRM) || (gtype == Group) || (gtype == GroupDual))
ctxt->modrm = insn_fetch(u8, ctxt);
somewhere early?
In fact even that is too much. All groups have ModRM somewhere in their
encoding; all we have to do is move it to the main tables (opcode_table
or twobyte_table) and just move the existing modrm fetch before group
parsing.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
