Re: [PATCH 2/2] KVM: x86 emulator: add decoding of CMPXCHG8B dst operand.

[Gleb Natapov <gleb@redhat.com>]

On Sun, Mar 21, 2010 at 04:41:24PM +0200, Avi Kivity wrote:
> On 03/21/2010 01:08 PM, Gleb Natapov wrote:
> >Decode CMPXCHG8B destination operand in decoding stage. Fixes regression
> >introduced by "If LOCK prefix is used dest arg should be memory" commit.
> >This commit relies on dst operand be decoded at the beginning of an
> >instruction emulation.
> 
> >@@ -1719,15 +1719,12 @@ static inline int emulate_grp9(struct x86_emulate_ctxt *ctxt,
> >  		c->regs[VCPU_REGS_RAX] = (u32) (old>>  0);
> >  		c->regs[VCPU_REGS_RDX] = (u32) (old>>  32);
> >  		ctxt->eflags&= ~EFLG_ZF;
> >-
> >  	} else {
> >-		new = ((u64)c->regs[VCPU_REGS_RCX]<<  32) |
> >+		c->dst.val = ((u64)c->regs[VCPU_REGS_RCX]<<  32) |
> >  		       (u32) c->regs[VCPU_REGS_RBX];
> >
> >-		rc = ops->cmpxchg_emulated(c->modrm_ea,&old,&new, 8, ctxt->vcpu);
> >-		if (rc != X86EMUL_CONTINUE)
> >-			return rc;
> >  		ctxt->eflags |= EFLG_ZF;
> >+		c->lock_prefix = 1;
> 
> Why is this bit needed?  cmpxchg64b without lock is valid and racy,
> but the guest may know it is safe.
> 
Agree. Before this patch cmpxchg8b emulation always called
cmpxchg_emulated(), so to be extra careful I wanted to preserve old
behaviour. Resend the patch without this line?

--
			Gleb.