From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: Emulation of shld instruction doesn't work Date: Wed, 08 Oct 2008 16:14:00 +0200 Message-ID: <48ECC028.5070807@redhat.com> References: <20081008145245.78c7084d@frecb000711> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm , Mohammed Gamal To: Guillaume Thouvenin Return-path: Received: from mx2.redhat.com ([66.187.237.31]:56374 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754277AbYJHOOq (ORCPT ); Wed, 8 Oct 2008 10:14:46 -0400 In-Reply-To: <20081008145245.78c7084d@frecb000711> Sender: kvm-owner@vger.kernel.org List-ID: Guillaume Thouvenin wrote: > Hello, > > I need to emulate the shld instruction (needed when enabling invalid > guest state framework). > > I added the code that emulates shld in kvm (see the end of this email) > and I also made a test case in user/test/x86/realmode.c. I think that > the code that emulated the instruction is fine but when I run kvmctl > with file realmode.flat it failed. Here is the test that I added in > realmode.c: > Instructions that modify memory are best tested in emulator.c rather than realmode.c. > diff --git a/user/test/x86/realmode.c b/user/test/x86/realmode.c > index 69ded37..8533240 100644 > --- a/user/test/x86/realmode.c > +++ b/user/test/x86/realmode.c > @@ -141,6 +141,22 @@ int regs_equal(const struct regs *r1, const struct regs *r2, int ignore) > ); \ > extern u8 insn_##name[], insn_##name##_end[] > > +void test_shld(const struct regs *inregs, struct regs *outregs) > +{ > + MK_INSN(shld_test, "mov $0xbe, %ebx\n\t" > + "mov $0xef000000, %ecx\n\t" > Do the assignments in C, modifying the inregs parameter rather than in assembly. > @@ -230,7 +230,8 @@ static u16 twobyte_table[256] = { > /* 0x90 - 0x9F */ > 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > /* 0xA0 - 0xA7 */ > - 0, 0, 0, DstMem | SrcReg | ModRM | BitOp, 0, 0, 0, 0, > + 0, 0, 0, DstMem | SrcReg | ModRM | BitOp, > + DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM, 0, 0, > /* 0xA8 - 0xAF */ > 0, 0, 0, DstMem | SrcReg | ModRM | BitOp, 0, 0, ModRM, 0, > /* 0xB0 - 0xB7 */ > @@ -501,6 +502,16 @@ static u16 group2_table[] = { > (_type)_x; \ > }) > > +/* > + * Reflects the parity of the given argument. It is set if > + * the number of ones is even. > + */ > +static int even_parity(uint8_t v) > +{ > + asm ( "test %b0,%b0; setp %b0" : "=a" (v) : "0" (v) ); > + return v; > +} > + > static inline unsigned long ad_mask(struct decode_cache *c) > { > return (1UL << (c->ad_bytes << 3)) - 1; > @@ -1993,6 +2004,46 @@ twobyte_insn: > c->src.val &= (c->dst.bytes << 3) - 1; > emulate_2op_SrcV_nobyte("bt", c->src, c->dst, ctxt->eflags); > break; > + case 0xa4: /* shld imm8, r, r/m */ > + case 0xa5: /* shld cl, r, r/m */ { > + uint8_t size; > + uint8_t count; > + > + size = c->dst.bytes << 3; > + count = (c->b & 1) ? (uint8_t) c->regs[VCPU_REGS_RCX] : insn_fetch(u8, 1, c->eip); > You need to fetch during the decode phase rather than the execution phase. This is probably the source of the problem. shld has three operands, so we need to add s Src2 decode set. I guess we can start with Src2One, Src2CL, and Src2Imm8 to support shld and expand it later. > + > + if (count == 0) /* No operation */ > + break; > + > + if (count > size) { > + printk(KERN_INFO "shld: bad parameters \n"); > + break; > + } > The parameters aren't bad; the processor ignores excess bits. > + > + c->dst.orig_val = c->dst.val; > + c->dst.type = OP_REG; > + c->dst.val <<= count; > + c->dst.val |= ((c->src.val >> (size - count)) & ((1ull << count) - 1)); > + > + /* Flags affected */ > + ctxt->eflags &= ~(EFLG_OF|EFLG_SF|EFLG_ZF|EFLG_PF|EFLG_CF); > + > + /* Set CF with left bit shifted if count is 1 or greater */ > + if ((c->dst.orig_val >> (size - count)) & 1) > + ctxt->eflags |= EFLG_CF; > + > + /* For 1-bit shit, OF is set if a sign change occured, otherwise it > + * is cleared. For shift greater than 1 it is undefined */ > + if (((c->dst.orig_val ^ c->dst.val) >> (size - 1)) & 1) > + ctxt->eflags |= EFLG_OF; > + > + /* SF, ZF, and PF flags are set according to the value of the result */ > + ctxt->eflags |= ((c->dst.val >> (size - 1)) & 1) ? EFLG_SF : 0; > + ctxt->eflags |= (c->dst.val == 0) ? EFLG_ZF : 0; > + ctxt->eflags |= even_parity(c->dst.val) ? EFLG_PF : 0; > + > This needs to be implemented like the rest of the instructions, with the processor calculating the flags and result. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.