Re: [patch 2 of 2] Emulate CMPS instruction

Avi Kivity
2007-11-23T17:54:46Z

Guillaume Thouvenin wrote:
> This patch emulates the CMPS instruction.
>
> Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin-Z51IpKcfGtLk1uMJSBkQmQ@public.gmane.org>
> ---
>
> drivers/kvm/x86_emulate.c | 54 +++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
> index cee60eb..db744cf 100644
> --- a/drivers/kvm/x86_emulate.c
> +++ b/drivers/kvm/x86_emulate.c
> @@ -445,6 +445,29 @@ static u16 twobyte_table[256] = {
> register_address_increment(c->eip, rel); \
> } while (0)
>
> +/* Test if the repeat string operation prefix is REPE/REPZ or
> + * REPNE/REPNZ and if it's the case it tests the corresponding
> + * termination condition according to:
> + * - if REPE/REPZ and ZF = 0 then done
> + * - if REPNE/REPNZ and ZF = 1 then done
> + */
> +#define handle_rep_prefix(c) \
> + do { \
> + if ((c->b == 0xa6) || (c->b == 0xa7) || \
> + (c->b == 0xae) || (c->b == 0xaf)) { \
> + if ((c->rep_prefix == REPE_PREFIX) && \
> + ((ctxt->eflags & EFLG_ZF) == 0)) { \
> + ctxt->vcpu->rip = c->eip; \
> + goto done; \
> + } \
> + if ((c->rep_prefix == REPNE_PREFIX) && \
> + ((ctxt->eflags & EFLG_ZF) == EFLG_ZF)) {\
> + ctxt->vcpu->rip = c->eip; \
> + goto done; \
> + } \
> + } \
> + } while (0)
> +
>

No new macros in the emulator please. Just inline it at the callsite.

> static int do_fetch_insn_byte(struct x86_emulate_ctxt *ctxt,
> struct x86_emulate_ops *ops,
> unsigned long linear, u8 *dest)
> @@ -1540,10 +1563,15 @@ special_insn:
> break;
> }
> if (c->rep_prefix) {
> + /* All REP prefixes have the same first termination condition */
> if (c->regs[VCPU_REGS_RCX] == 0) {
> ctxt->vcpu->rip = c->eip;
> goto done;
> }
> + /* The second termination condition only applies for REPE
> + * and REPNE. handle_rep_prefix() macro deals with that.
> + */
> + handle_rep_prefix(c);
> c->regs[VCPU_REGS_RCX]--;
> c->eip = ctxt->vcpu->rip;
> }
> @@ -1570,8 +1598,30 @@ special_insn:
> : c->dst.bytes);
> break;
> case 0xa6 ... 0xa7: /* cmps */
> - DPRINTF("Urk! I don't handle CMPS.\n");
> - goto cannot_emulate;
> + c->src.type = OP_NONE;
>

Shouldn't this be OP_MEM?

> + c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
> + c->src.ptr = (unsigned long *)register_address(
> + ctxt->ds_base,
> + c->regs[VCPU_REGS_RDI]);
> +
> + c->dst.type = OP_NONE;
>

And here?

> + c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
> + c->dst.ptr = (unsigned long *)register_address(
> + ctxt->es_base,
> + c->regs[VCPU_REGS_RSI]);
> +
> + DPRINTF("cmps: mem1=0x%p mem2=0x%p\n", c->src.ptr, c->dst.ptr);
> +
>

Where is the actual memory access?

> + emulate_2op_SrcV("cmp", c->src, c->dst, ctxt->eflags);
> +
> + register_address_increment(c->regs[VCPU_REGS_RDI],
> + (ctxt->eflags & EFLG_DF) ? -c->dst.bytes
> + : c->dst.bytes);
> +
> + register_address_increment(c->regs[VCPU_REGS_RSI],
> + (ctxt->eflags & EFLG_DF) ? -c->dst.bytes
> + : c->dst.bytes);
> + break;
> case 0xaa ... 0xab: /* stos */
> c->dst.type = OP_MEM;
> c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>

--
Any sufficiently difficult bug is indistinguishable from a feature.