Re: Emulation of shld instruction doesn't work

2008-10-08 Thread Avi Kivity

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 $0xef00, %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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Emulation of shld instruction doesn't work

2008-10-08 Thread Guillaume Thouvenin
On Wed, 8 Oct 2008 14:52:45 +0200
Guillaume Thouvenin <[EMAIL PROTECTED]> wrote:

> When I launch the test, I have an emulation failure that depends of the
> count operand. With count == 4 (shld $4,...) I have:
> 
>   emulation failed (emulation failure) rip 1aa 04 87 06 c0
> 
> If count == 12 I have
> 
>  emulation failed (emulation failure) rip 1aa 0c 87 06 c0
> 
> and if count == 8 
>  
>  emulation failed (emulation failure) rip 1b1 1e c4 21 66

Just to be more accurate, I added trace in shld to display information
and I can see that destination operand is ok, the failure occurs after
test_shld(). 

Regards,
Guillaume
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Emulation of shld instruction doesn't work

2008-10-08 Thread Guillaume Thouvenin
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:

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 $0xef00, %ecx\n\t"
+  "shld $8,%ecx,%ebx\n\t");
+
+   exec_in_big_real_mode(inregs, outregs,
+ insn_shld_test,
+ insn_shld_test_end - insn_shld_test);
+
+   if ((outregs->ebx != 0xbeef) && (outregs->ecx != 0xef00))
+   print_serial("shld: failed\n");
+   else
+   print_serial("shld: succeded\n");
+}
+
 void test_mov_imm(const struct regs *inregs, struct regs *outregs)
 {
MK_INSN(mov_r32_imm_1, "mov $1234567890, %eax");
@@ -342,6 +358,7 @@ void start(void)
test_call(&inregs, &outregs);
test_mov_imm(&inregs, &outregs);
test_cmp_imm(&inregs, &outregs);
+   test_shld(&inregs, &outregs);
test_io(&inregs, &outregs);
test_eflags_insn(&inregs, &outregs);
exit(0);

When I launch the test, I have an emulation failure that depends of the
count operand. With count == 4 (shld $4,...) I have:

  emulation failed (emulation failure) rip 1aa 04 87 06 c0

If count == 12 I have

 emulation failed (emulation failure) rip 1aa 0c 87 06 c0

and if count == 8 
 
 emulation failed (emulation failure) rip 1b1 1e c4 21 66


If I disassemble the code of realmode.flat I can see 
 
 198:   dc 21   fsubl  (%ecx)
...
 1aa:   66 87 06xchg   %ax,(%esi)
 1ad:   c0 21 66shlb   $0x66,(%ecx)
 1b0:   87 1e   xchg   %ebx,(%esi)
 1b2:   c4 21   les(%ecx),%esp

So rip values reported during the emulation failure are strange. I also
notice that the problem occurs after test_shld() is called. If the test
is the last one the program exit normally. I don't see what is wrong in
the emulation of shld instruction so any hints are welcome.



Best Regards,
Guillaume

diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index a391e21..93eea5f 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -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);
+
+   if (count == 0) /* No operation */
+   break;
+
+   if (count > size) {
+   printk(KERN_INFO "shld: bad parameters \n");
+   break;
+   }
+
+   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 */
+