[PATCH] Fix Makefile rule for compiling emulate.c

2009-08-12 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/Makefile |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index afaaa76..0e7fe78 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -9,7 +9,7 @@ kvm-y   += $(addprefix ../../../virt/kvm/, 
kvm_main.o ioapic.o \
coalesced_mmio.o irq_comm.o eventfd.o)
 kvm-$(CONFIG_IOMMU_API)+= $(addprefix ../../../virt/kvm/, iommu.o)
 
-kvm-y  += x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
+kvm-y  += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
   i8254.o timer.o
 kvm-intel-y+= vmx.o
 kvm-amd-y  += svm.o
-- 
1.6.0.4

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


[PATCH] x86 emulator: Add 'push es' instruction (opcode 0x06)

2009-08-15 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   13 -
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..6305a12 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,7 +92,7 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, ImplicitOps | Stack, 0,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -1186,6 +1186,14 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   c->src.ptr = (unsigned long *) &c->regs[seg];
+   emulate_push(ctxt);
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,6 +1715,9 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
-- 
1.6.0.4

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


[PATCH] Add push es instruction test in test harness

2009-08-15 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   16 
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..8db2c7a 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -468,6 +468,21 @@ void test_long_jmp()
print_serial("Long JMP Test: FAIL\n");
 }
 
+void test_push_es()
+{
+   struct regs inregs = { 0 }, outregs;
+   
+   MK_INSN(push_es, "mov $0x1234, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"movw (%esp), %bx \n\t"); 
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if(!regs_equal(&inregs, &outregs, R_AX|R_BX|R_SP) || outregs.eax != 
0x1234)
+   print_serial("Push ES Test: FAIL\n");
+}
+
 void test_null(void)
 {
struct regs inregs = { 0 }, outregs;
@@ -492,6 +507,7 @@ void start(void)
test_call();
/* long jmp test uses call near so test it after testing call */
test_long_jmp();
+   test_push_es();
 
exit(0);
 }
-- 
1.6.0.4


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


[PATCH] x86 emulator: Add 'push es' instruction (opcode 0x06)

2009-08-15 Thread Mohammed Gamal
[Removed an unused variable]

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   12 +++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..065131e 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,7 +92,7 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, ImplicitOps | Stack, 0,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -1186,6 +1186,13 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   c->src.ptr = (unsigned long *) &c->regs[seg];
+   emulate_push(ctxt);
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,6 +1714,9 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
-- 
1.6.0.4

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


[PATCH] Add push es instruction test in test harness

2009-08-16 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   17 +
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..041f0a5 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -468,6 +468,22 @@ void test_long_jmp()
print_serial("Long JMP Test: FAIL\n");
 }
 
+void test_push_es()
+{
+   struct regs inregs = { 0 }, outregs;
+   
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+); 
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if(!regs_equal(&inregs, &outregs, R_AX|R_BX) ||  outregs.ebx != 
outregs.eax)
+   print_serial("Push ES Test: FAIL\n");
+}
+
 void test_null(void)
 {
struct regs inregs = { 0 }, outregs;
@@ -481,6 +497,7 @@ void start(void)
test_null();
 
test_shld();
+   test_push_es();
test_mov_imm();
test_cmp_imm();
test_add_imm();
-- 
1.6.0.4

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


[PATCH] x86 emulator: Add 'push es' instruction (opcode 0x06)

2009-08-16 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..7688c0b 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,7 +92,7 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, ImplicitOps | Stack, 0,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -1186,6 +1186,15 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   c->src.ptr = (unsigned long *) &segment.selector;
+   emulate_push(ctxt);
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,6 +1716,9 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
-- 
1.6.0.4


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


Re: [PATCH] x86 emulator: Add 'push es' instruction (opcode 0x06)

2009-08-17 Thread Mohammed Gamal
On Mon, Aug 17, 2009 at 11:03 AM, Avi Kivity wrote:
> On 08/16/2009 08:51 PM, Mohammed Gamal wrote:
>>
>> +static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
>> +{
>> +       struct decode_cache *c =&ctxt->decode;
>> +       struct kvm_segment segment;
>> +       kvm_x86_ops->get_segment(ctxt->vcpu,&segment, seg);
>> +       c->src.ptr = (unsigned long *)&segment.selector;
>> +       emulate_push(ctxt);
>> +}
>>
>
> This will pick up random junk from segment.type if used in 32-bit mode,
> since segment.selector is only 16 bits wide.
>
> btw, I see that emulate_push() uses src.val, not src.ptr.  Have you tested
> this?
>
Hmmm, there are no test cases that test conventional push/pop
instructions. I'll write a test case and see if the function behaves
correctly.

>
> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Add push instructions test in test harness

2009-08-17 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   40 ++--
 1 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..a9208ff 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -467,6 +467,41 @@ void test_long_jmp()
if(!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x1234)
print_serial("Long JMP Test: FAIL\n");
 }
+void test_push()
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(push32, "mov $0x12345678, %eax\n\t"
+   "push %eax\n\t"
+   "pop %ebx\n\t");
+   MK_INSN(push16, "mov $0x1234, %ax\n\t"
+   "push %ax\n\t"
+   "pop %bx\n\t");
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov $0x123, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+);
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push32,
+ insn_push32_end - insn_push32);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x12345678)
+   print_serial("Push Test 1: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push16,
+ insn_push16_end - insn_push16);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x1234)
+   print_serial("Push Test 2: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax || outregs.eax != 0x123)
+   print_serial("Push Test 3: FAIL\n");
+}
 
 void test_null(void)
 {
@@ -479,8 +514,9 @@ void test_null(void)
 void start(void)
 {
test_null();
-
+   
test_shld();
+   test_push();
test_mov_imm();
test_cmp_imm();
test_add_imm();
@@ -492,7 +528,7 @@ void start(void)
test_call();
/* long jmp test uses call near so test it after testing call */
test_long_jmp();
-
+   
exit(0);
 }
 
-- 
1.6.0.4

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


[PATCH] x86 emulator: Add 'push es' instruction (opcode 0x06)

2009-08-17 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..fc8a745 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,7 +92,7 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, ImplicitOps | Stack, 0,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -1186,6 +1186,17 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   
+   c->src.val = segment.selector;
+   emulate_push(ctxt);
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,6 +1718,9 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
-- 
1.6.0.4

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


[PATCH] x86 emulator: Add 'push/pop sreg' instructions

2009-08-18 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   60 ---
 1 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..438d423 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,19 +92,20 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -1186,6 +1187,30 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   
+   c->src.val = segment.selector;
+   emulate_push(ctxt);
+}
+
+static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
+struct x86_emulate_ops *ops, int seg)
+{
+   struct kvm_segment segment;
+   int rc;
+
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   rc = emulate_pop(ctxt, ops, &segment.selector, sizeof(uint16_t));
+   kvm_x86_ops->set_segment(ctxt->vcpu, &segment, seg);
+
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,18 +1732,45 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
+   case 0x07:  /* pop es */
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
+   if (rc != 0)
+   goto done;
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
+   case 0x0e:  /* push cs */
+   emulate_push_sreg(ctxt, VCPU_SREG_CS);
+   break;
case 0x10 ... 0x15:
  adc:  /* adc */
emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
break;
+   case 0x16:  /* push ss */
+   emulate_push_sreg(ctxt, VCPU_SREG_SS);
+   break;
+   case 0x17:  /* pop ss */
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
+   if (rc != 0)
+   goto done;
+   break;
case 0x18 ... 0x1d:
  sbb:  /* sbb */
emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
break;
+   case 0x1e:  /* push ds */
+   emulate_push_sreg(ctxt, VCPU_SREG_DS);
+   break;
+   case 0x1f:  /* pop ds */
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
+   if (rc != 0)
+   goto done;
+   break;
case 0x20 ... 0x25:
  and:  /* and */
emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);
-- 
1.6.0.4

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


[PATCH] Add push/pop instructions test in test harness

2009-08-18 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   66 -
 1 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..e8d5eef 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -467,6 +467,67 @@ void test_long_jmp()
if(!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x1234)
print_serial("Long JMP Test: FAIL\n");
 }
+void test_push_pop()
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(push32, "mov $0x12345678, %eax\n\t"
+   "push %eax\n\t"
+   "pop %ebx\n\t");
+   MK_INSN(push16, "mov $0x1234, %ax\n\t"
+   "push %ax\n\t"
+   "pop %bx\n\t");
+
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov $0x123, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+);
+   MK_INSN(pop_es, "push %es\n\t"
+   "push %ax\n\t"
+   "pop %es\n\t"
+   "mov %es, %bx\n\t"
+   "pop %es\n\t"
+   );
+   MK_INSN(push_pop_ss, "push %ss\n\t"
+"push %ax\n\t"
+"pop %ss\n\t"
+"mov %ss, %bx\n\t"
+"pop %ss\n\t"
+   );
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push32,
+ insn_push32_end - insn_push32);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x12345678)
+   print_serial("Push/Pop Test 1: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push16,
+ insn_push16_end - insn_push16);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x1234)
+   print_serial("Push/Pop Test 2: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) ||  outregs.ebx != 
outregs.eax || outregs.eax != 0x123)
+   print_serial("Push/Pop Test 3: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pop_es,
+ insn_pop_es_end - insn_pop_es);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 4: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_ss,
+ insn_push_pop_ss_end - insn_push_pop_ss);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 5: FAIL\n");
+}
 
 void test_null(void)
 {
@@ -479,8 +540,9 @@ void test_null(void)
 void start(void)
 {
test_null();
-
+   
test_shld();
+   test_push_pop();
test_mov_imm();
test_cmp_imm();
test_add_imm();
@@ -492,7 +554,7 @@ void start(void)
test_call();
/* long jmp test uses call near so test it after testing call */
test_long_jmp();
-
+   
exit(0);
 }
 
-- 
1.6.0.4

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


[PATCH] Add adc and sbb missing decoder flags

2009-08-18 Thread Mohammed Gamal
Add missing decoder flags for adc and sbb instructions
(opcodes 0x14-0x15, 0x1c-0x1d)

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 438d423..af58e54 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -101,11 +101,13 @@ static u32 opcode_table[256] = {
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, ImplicitOps | Stack, ImplicitOps | Stack,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, ImplicitOps | Stack, ImplicitOps | Stack,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-- 
1.6.0.4

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


Re: [PATCH] x86 emulator: Add 'push/pop sreg' instructions

2009-08-18 Thread Mohammed Gamal
On Tue, Aug 18, 2009 at 5:39 PM, Avi Kivity wrote:
> On 08/18/2009 03:48 PM, Mohammed Gamal wrote:
>>
>> +
>> +static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
>> +                            struct x86_emulate_ops *ops, int seg)
>> +{
>> +       struct kvm_segment segment;
>> +       int rc;
>> +
>> +       kvm_x86_ops->get_segment(ctxt->vcpu,&segment, seg);
>> +       rc = emulate_pop(ctxt, ops,&segment.selector, sizeof(uint16_t));
>>
>
> 'pop seg' is still subject to the operand size (I think).
>
>> +       kvm_x86_ops->set_segment(ctxt->vcpu,&segment, seg);
>>
But we're popping the contents of the stack top to a segment register
which is going to be of 16-bits anyway, so we know the length before
hand, no?

>
> You need to call kvm_load_segment_descriptor() so that the segment cache is
> also loaded correctly.
>
> Note some of these instructions are not encodable in long mode; need to
> check for that instead of emulating the wrong instruction.
>
>> @@ -1707,18 +1732,45 @@ special_insn:
>>              add:              /* add */
>>                emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x06:              /* push es */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_ES);
>> +               break;
>> +       case 0x07:              /* pop es */
>> +               rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
>> +               if (rc != 0)
>> +                       goto done;
>> +               break;
>>        case 0x08 ... 0x0d:
>>              or:               /* or */
>>                emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x0e:              /* push cs */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_CS);
>> +               break;
>>        case 0x10 ... 0x15:
>>              adc:              /* adc */
>>                emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x16:              /* push ss */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_SS);
>> +               break;
>> +       case 0x17:              /* pop ss */
>> +               rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
>> +               if (rc != 0)
>> +                       goto done;
>> +               break;
>>        case 0x18 ... 0x1d:
>>              sbb:              /* sbb */
>>                emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x1e:              /* push ds */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_DS);
>> +               break;
>> +       case 0x1f:              /* pop ds */
>> +               rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
>> +               if (rc != 0)
>> +                       goto done;
>> +               break;
>>        case 0x20 ... 0x25:
>>              and:              /* and */
>>                emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);
>>
>
>
I was under the impression that the emulator doesn't support long mode
yet, is that still the case?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add adc and sbb missing decoder flags

2009-08-18 Thread Mohammed Gamal
On Tue, Aug 18, 2009 at 5:41 PM, Avi Kivity wrote:
> On 08/18/2009 03:59 PM, Mohammed Gamal wrote:
>>
>> Add missing decoder flags for adc and sbb instructions
>> (opcodes 0x14-0x15, 0x1c-0x1d)
>>
>
> This depends on the previous patch, so I'll just wait until that is ready.
>
I'll rebase this on the current tree and resend it.

> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Add adc and sbb missing decoder flags

2009-08-18 Thread Mohammed Gamal
Add missing decoder flags for adc and sbb instructions
(opcodes 0x14-0x15, 0x1c-0x1d)

[Rebased to upstream tree]

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..1be5cd6 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -100,11 +100,11 @@ static u32 opcode_table[256] = {
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-- 
1.6.0.4

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


[PATCH] x86 emulator: Add 'push/pop sreg' instructions

2009-08-18 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   99 +---
 1 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1be5cd6..3d9fb44 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,19 +92,22 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -244,11 +247,13 @@ static u32 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,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM, 0, 0,
/* 0xA8 - 0xAF */
-   0, 0, 0, DstMem | SrcReg | ModRM | BitOp,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM,
ModRM, 0,
@@ -1186,6 +1191,45 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   
+   if (ctxt->mode == X86EMUL_MODE_PROT64 && (seg != VCPU_SREG_FS || 
+  seg != VCPU_SREG_GS)) {
+   kvm_queue_exception(ctxt->vcpu, UD_VECTOR);
+   return;
+   }
+
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   
+   c->src.val = segment.selector;
+   emulate_push(ctxt);
+}
+
+static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
+struct x86_emulate_ops *ops, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   int rc;
+
+   if (ctxt->mode == X86EMUL_MODE_PROT64 && (seg != VCPU_SREG_FS || 
+   seg != VCPU_SREG_GS)) {
+   kvm_queue_exception(ctxt->vcpu, UD_VECTOR);
+   return -1;
+   }
+
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   rc = emulate_pop(ctxt, ops, &segment.selector, c->op_bytes);
+   if (rc != 0)
+   return rc;
+   
+   rc = kvm_load_segment_descriptor(ctxt->vcpu, segment.selector, 1, seg);
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,18 +1751,45 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
+   case 0x07:  /* pop es */
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
+   if (rc != 0)
+   goto done;
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
+   case 0x0e:  /* push cs */
+   emulate_push_sreg(ctxt, VCPU_SREG_CS);
+   break;
case 0x10 ... 0x15:
  adc:  /* adc */
emulate_2op_SrcV("adc", c->src, c->dst, ctxt-&g

[PATCH] Add push/pop instructions test in test harness

2009-08-18 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   78 -
 1 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..698328d 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -467,6 +467,79 @@ void test_long_jmp()
if(!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x1234)
print_serial("Long JMP Test: FAIL\n");
 }
+void test_push_pop()
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(push32, "mov $0x12345678, %eax\n\t"
+   "push %eax\n\t"
+   "pop %ebx\n\t");
+   MK_INSN(push16, "mov $0x1234, %ax\n\t"
+   "push %ax\n\t"
+   "pop %bx\n\t");
+
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov $0x123, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+);
+   MK_INSN(pop_es, "push %ax\n\t"
+   "pop %es\n\t"
+   "mov %es, %bx\n\t"
+   );
+   MK_INSN(push_pop_ss, "push %ss\n\t"
+"pushw %ax\n\t"
+"popw %ss\n\t"
+"mov %ss, %bx\n\t"
+"pop %ss\n\t"
+   );
+   MK_INSN(push_pop_fs, "push %fs\n\t"
+"pushl %eax\n\t"
+"popl %fs\n\t"
+"mov %fs, %ebx\n\t"
+"pop %fs\n\t"
+   );
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push32,
+ insn_push32_end - insn_push32);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x12345678)
+   print_serial("Push/Pop Test 1: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push16,
+ insn_push16_end - insn_push16);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x1234)
+   print_serial("Push/Pop Test 2: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) ||  outregs.ebx != 
outregs.eax || outregs.eax != 0x123)
+   print_serial("Push/Pop Test 3: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pop_es,
+ insn_pop_es_end - insn_pop_es);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 4: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_ss,
+ insn_push_pop_ss_end - insn_push_pop_ss);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 5: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_fs,
+ insn_push_pop_fs_end - insn_push_pop_fs);
+   
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 6: FAIL\n");
+}
 
 void test_null(void)
 {
@@ -479,8 +552,9 @@ void test_null(void)
 void start(void)
 {
test_null();
-
+   
test_shld();
+   test_push_pop();
test_mov_imm();
test_cmp_imm();
test_add_imm();
@@ -492,7 +566,7 @@ void start(void)
test_call();
/* long jmp test uses call near so test it after testing call */
test_long_jmp();
-
+   
exit(0);
 }
 
-- 
1.6.0.4

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


Re: [PATCH] x86 emulator: Add 'push/pop sreg' instructions

2009-08-19 Thread Mohammed Gamal
On Wed, Aug 19, 2009 at 11:32 AM, Avi Kivity wrote:
> On 08/19/2009 07:11 AM, Mohammed Gamal wrote:
>>
>> +static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
>> +{
>> +       struct decode_cache *c =&ctxt->decode;
>> +       struct kvm_segment segment;
>> +
>> +       if (ctxt->mode == X86EMUL_MODE_PROT64&&  (seg != VCPU_SREG_FS ||
>> +                                                  seg != VCPU_SREG_GS)) {
>> +               kvm_queue_exception(ctxt->vcpu, UD_VECTOR);
>> +               return;
>> +       }
>>
>
> It's better to check at the callsite, in case the opcode is ever reused for
> a new instruction.  Or even better, add a new decode flag No64 so we can do
> this during the decode stage.

Good idea, but I believe it'd be better to introduce it in a separate
patch so that we can update all instructions incompatible with long
mode in one go. I'll move the checks to the call site for the time
being.

>
>> +
>> +static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
>> +                            struct x86_emulate_ops *ops, int seg)
>> +{
>> +       struct decode_cache *c =&ctxt->decode;
>> +       struct kvm_segment segment;
>> +       int rc;
>> +
>> +       if (ctxt->mode == X86EMUL_MODE_PROT64&&  (seg != VCPU_SREG_FS ||
>> +                                                   seg != VCPU_SREG_GS))
>> {
>> +               kvm_queue_exception(ctxt->vcpu, UD_VECTOR);
>> +               return -1;
>> +       }
>> +
>> +       kvm_x86_ops->get_segment(ctxt->vcpu,&segment, seg);
>> +       rc = emulate_pop(ctxt, ops,&segment.selector, c->op_bytes);
>> +       if (rc != 0)
>> +               return rc;
>> +
>> +       rc = kvm_load_segment_descriptor(ctxt->vcpu, segment.selector, 1,
>> seg);
>> +       return rc;
>> +}
>>
>
> Why do the ->get_segment() at all?  pop into a temporary variable, and call
> kvm_load_segment_descriptor() with that.
>
>
> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] x86 emulator: Add 'push/pop sreg' instructions

2009-08-19 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |  107 +---
 1 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1be5cd6..283237f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,19 +92,22 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -244,11 +247,13 @@ static u32 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,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM, 0, 0,
/* 0xA8 - 0xAF */
-   0, 0, 0, DstMem | SrcReg | ModRM | BitOp,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM,
ModRM, 0,
@@ -1186,6 +1191,32 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   
+   c->src.val = segment.selector;
+   emulate_push(ctxt);
+}
+
+static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
+struct x86_emulate_ops *ops, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   u16 selector;
+   int rc;
+
+   rc = emulate_pop(ctxt, ops, &selector, c->op_bytes);
+   if (rc != 0)
+   return rc;
+   
+   rc = kvm_load_segment_descriptor(ctxt->vcpu, selector, 1, seg);
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,18 +1738,66 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   if (ctxt->mode == X86EMUL_MODE_PROT64)
+   goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
+   case 0x07:  /* pop es */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
+   if (rc != 0)
+   goto done;
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
+   case 0x0e:  /* push cs */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_CS);
+   break;
case 0x10 ... 0x15:
  adc:  /* adc */
emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
break;
+   case 0x16:  /* push ss */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_SS);
+   break;
+   case 0x17:  /* pop 

[PATCH] x86 emulator: Introduce No64 decode option

2009-08-19 Thread Mohammed Gamal
Introduces a new decode option "No64", which is used for instructions that are
invalid in long mode.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   42 ++
 1 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 283237f..cc0fe39 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -75,6 +75,8 @@
 #define Group   (1<<14) /* Bits 3:5 of modrm byte extend opcode */
 #define GroupDual   (1<<15) /* Alternate decoding of mod == 3 */
 #define GroupMask   0xff/* Group number stored in bits 0:7 */
+/* Misc flags */
+#define No64   (1<<28)
 /* Source 2 operand type */
 #define Src2None(0<<29)
 #define Src2CL  (1<<29)
@@ -93,21 +95,21 @@ static u32 opcode_table[256] = {
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, ImplicitOps | Stack, 0,
+   0, 0, ImplicitOps | Stack | No64, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -161,7 +163,7 @@ static u32 opcode_table[256] = {
/* 0x90 - 0x97 */
DstReg, DstReg, DstReg, DstReg, DstReg, DstReg, DstReg, DstReg,
/* 0x98 - 0x9F */
-   0, 0, SrcImm | Src2Imm16, 0,
+   0, 0, SrcImm | Src2Imm16 | No64, 0,
ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
/* 0xA0 - 0xA7 */
ByteOp | DstReg | SrcMem | Mov | MemAbs, DstReg | SrcMem | Mov | MemAbs,
@@ -188,7 +190,7 @@ static u32 opcode_table[256] = {
ByteOp | DstMem | SrcImm | ModRM | Mov, DstMem | SrcImm | ModRM | Mov,
/* 0xC8 - 0xCF */
0, 0, 0, ImplicitOps | Stack,
-   ImplicitOps, SrcImmByte, ImplicitOps, ImplicitOps,
+   ImplicitOps, SrcImmByte, ImplicitOps | No64, ImplicitOps,
/* 0xD0 - 0xD7 */
ByteOp | DstMem | SrcImplicit | ModRM, DstMem | SrcImplicit | ModRM,
ByteOp | DstMem | SrcImplicit | ModRM, DstMem | SrcImplicit | ModRM,
@@ -201,7 +203,7 @@ static u32 opcode_table[256] = {
ByteOp | SrcImmUByte, SrcImmUByte,
/* 0xE8 - 0xEF */
SrcImm | Stack, SrcImm | ImplicitOps,
-   SrcImmU | Src2Imm16, SrcImmByte | ImplicitOps,
+   SrcImmU | Src2Imm16 | No64, SrcImmByte | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
/* 0xF0 - 0xF7 */
@@ -967,6 +969,11 @@ done_prefixes:
}
}
 
+   if (mode == X86EMUL_MODE_PROT64 && (c->d & No64)) {
+   kvm_report_emulation_failure(ctxt->vcpu, "invalid x86/64 
instruction");;
+   return -1;
+   }
+
if (c->d & Group) {
group = c->d & GroupMask;
c->modrm = insn_fetch(u8, 1, c->eip);
@@ -1739,15 +1746,9 @@ special_insn:
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
case 0x06:  /* push es */
-   if (ctxt->mode == X86EMUL_MODE_PROT64)
-   goto cannot_emulate;
-
emulate_push_sreg(ctxt, VCPU_SREG_ES);
break;
case 0x07:  /* pop es */
-if (ctxt->mode == X86EMUL_MODE_PROT64)
-goto cannot_emulate;
-
rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
if (rc != 0)
goto done;
@@ -1757,9 +1758,6 @@ special_insn:
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
case 0x0e:  /* push cs */
-if (ctxt->mode == X86EMUL_MODE_PROT64)
-goto cannot_emulate;
-
  

[PATCH] x86 emulator: Report unhandlead instructions

2009-08-19 Thread Mohammed Gamal
Report unhandled instructions in the syslog on emulation failure

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index cc0fe39..52a4b7d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2194,6 +2194,7 @@ writeback:
 
 done:
if (rc == X86EMUL_UNHANDLEABLE) {
+   kvm_report_emulation_failure(ctxt->vcpu, "unhandled 
instruction");
c->eip = saved_eip;
return -1;
}
@@ -2467,7 +2468,7 @@ twobyte_insn:
goto writeback;
 
 cannot_emulate:
-   DPRINTF("Cannot emulate %02x\n", c->b);
+   kvm_report_emulation_failure(ctxt->vcpu, "unhandled instruction");
c->eip = saved_eip;
return -1;
 }
-- 
1.6.0.4

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


[PATCH][RESEND] Add push/pop instructions test in test harness

2009-08-19 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   74 ++
 1 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..04c1452 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -467,6 +467,79 @@ void test_long_jmp()
if(!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x1234)
print_serial("Long JMP Test: FAIL\n");
 }
+void test_push_pop()
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(push32, "mov $0x12345678, %eax\n\t"
+   "push %eax\n\t"
+   "pop %ebx\n\t");
+   MK_INSN(push16, "mov $0x1234, %ax\n\t"
+   "push %ax\n\t"
+   "pop %bx\n\t");
+
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov $0x123, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+);
+   MK_INSN(pop_es, "push %ax\n\t"
+   "pop %es\n\t"
+   "mov %es, %bx\n\t"
+   );
+   MK_INSN(push_pop_ss, "push %ss\n\t"
+"pushw %ax\n\t"
+"popw %ss\n\t"
+"mov %ss, %bx\n\t"
+"pop %ss\n\t"
+   );
+   MK_INSN(push_pop_fs, "push %fs\n\t"
+"pushl %eax\n\t"
+"popl %fs\n\t"
+"mov %fs, %ebx\n\t"
+"pop %fs\n\t"
+   );
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push32,
+ insn_push32_end - insn_push32);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x12345678)
+   print_serial("Push/Pop Test 1: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push16,
+ insn_push16_end - insn_push16);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x1234)
+   print_serial("Push/Pop Test 2: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) ||  outregs.ebx != 
outregs.eax || outregs.eax != 0x123)
+   print_serial("Push/Pop Test 3: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pop_es,
+ insn_pop_es_end - insn_pop_es);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 4: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_ss,
+ insn_push_pop_ss_end - insn_push_pop_ss);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 5: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_fs,
+ insn_push_pop_fs_end - insn_push_pop_fs);
+   
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 6: FAIL\n");
+}
 
 void test_null(void)
 {
@@ -481,6 +554,7 @@ void start(void)
test_null();
 
test_shld();
+   test_push_pop();
test_mov_imm();
test_cmp_imm();
test_add_imm();
-- 
1.6.0.4

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


[PATCH][RESEND] x86 emulator: Add 'push/pop sreg' instructions

2009-08-20 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |  107 +---
 1 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1be5cd6..283237f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,19 +92,22 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -244,11 +247,13 @@ static u32 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,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM, 0, 0,
/* 0xA8 - 0xAF */
-   0, 0, 0, DstMem | SrcReg | ModRM | BitOp,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM,
ModRM, 0,
@@ -1186,6 +1191,32 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   
+   c->src.val = segment.selector;
+   emulate_push(ctxt);
+}
+
+static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
+struct x86_emulate_ops *ops, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   u16 selector;
+   int rc;
+
+   rc = emulate_pop(ctxt, ops, &selector, c->op_bytes);
+   if (rc != 0)
+   return rc;
+   
+   rc = kvm_load_segment_descriptor(ctxt->vcpu, selector, 1, seg);
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,18 +1738,66 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   if (ctxt->mode == X86EMUL_MODE_PROT64)
+   goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
+   case 0x07:  /* pop es */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
+   if (rc != 0)
+   goto done;
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
+   case 0x0e:  /* push cs */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_CS);
+   break;
case 0x10 ... 0x15:
  adc:  /* adc */
emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
break;
+   case 0x16:  /* push ss */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_SS);
+   break;
+   case 0x17:  /* pop 

[PATCH][RESEND] x86 emulator: Introduce No64 decode option

2009-08-20 Thread Mohammed Gamal
Introduces a new decode option "No64", which is used for instructions that are
invalid in long mode.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   42 ++
 1 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 283237f..cc0fe39 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -75,6 +75,8 @@
 #define Group   (1<<14) /* Bits 3:5 of modrm byte extend opcode */
 #define GroupDual   (1<<15) /* Alternate decoding of mod == 3 */
 #define GroupMask   0xff/* Group number stored in bits 0:7 */
+/* Misc flags */
+#define No64   (1<<28)
 /* Source 2 operand type */
 #define Src2None(0<<29)
 #define Src2CL  (1<<29)
@@ -93,21 +95,21 @@ static u32 opcode_table[256] = {
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, ImplicitOps | Stack, 0,
+   0, 0, ImplicitOps | Stack | No64, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -161,7 +163,7 @@ static u32 opcode_table[256] = {
/* 0x90 - 0x97 */
DstReg, DstReg, DstReg, DstReg, DstReg, DstReg, DstReg, DstReg,
/* 0x98 - 0x9F */
-   0, 0, SrcImm | Src2Imm16, 0,
+   0, 0, SrcImm | Src2Imm16 | No64, 0,
ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
/* 0xA0 - 0xA7 */
ByteOp | DstReg | SrcMem | Mov | MemAbs, DstReg | SrcMem | Mov | MemAbs,
@@ -188,7 +190,7 @@ static u32 opcode_table[256] = {
ByteOp | DstMem | SrcImm | ModRM | Mov, DstMem | SrcImm | ModRM | Mov,
/* 0xC8 - 0xCF */
0, 0, 0, ImplicitOps | Stack,
-   ImplicitOps, SrcImmByte, ImplicitOps, ImplicitOps,
+   ImplicitOps, SrcImmByte, ImplicitOps | No64, ImplicitOps,
/* 0xD0 - 0xD7 */
ByteOp | DstMem | SrcImplicit | ModRM, DstMem | SrcImplicit | ModRM,
ByteOp | DstMem | SrcImplicit | ModRM, DstMem | SrcImplicit | ModRM,
@@ -201,7 +203,7 @@ static u32 opcode_table[256] = {
ByteOp | SrcImmUByte, SrcImmUByte,
/* 0xE8 - 0xEF */
SrcImm | Stack, SrcImm | ImplicitOps,
-   SrcImmU | Src2Imm16, SrcImmByte | ImplicitOps,
+   SrcImmU | Src2Imm16 | No64, SrcImmByte | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
/* 0xF0 - 0xF7 */
@@ -967,6 +969,11 @@ done_prefixes:
}
}
 
+   if (mode == X86EMUL_MODE_PROT64 && (c->d & No64)) {
+   kvm_report_emulation_failure(ctxt->vcpu, "invalid x86/64 
instruction");;
+   return -1;
+   }
+
if (c->d & Group) {
group = c->d & GroupMask;
c->modrm = insn_fetch(u8, 1, c->eip);
@@ -1739,15 +1746,9 @@ special_insn:
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
case 0x06:  /* push es */
-   if (ctxt->mode == X86EMUL_MODE_PROT64)
-   goto cannot_emulate;
-
emulate_push_sreg(ctxt, VCPU_SREG_ES);
break;
case 0x07:  /* pop es */
-if (ctxt->mode == X86EMUL_MODE_PROT64)
-goto cannot_emulate;
-
rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
if (rc != 0)
goto done;
@@ -1757,9 +1758,6 @@ special_insn:
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
case 0x0e:  /* push cs */
-if (ctxt->mode == X86EMUL_MODE_PROT64)
-goto cannot_emulate;
-
  

[PATCH][RESEND] x86 emulator: Report unhandled instructions

2009-08-20 Thread Mohammed Gamal
Report unhandled instructions in the syslog on emulation failure

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index cc0fe39..52a4b7d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2194,6 +2194,7 @@ writeback:
 
 done:
if (rc == X86EMUL_UNHANDLEABLE) {
+   kvm_report_emulation_failure(ctxt->vcpu, "unhandled 
instruction");
c->eip = saved_eip;
return -1;
}
@@ -2467,7 +2468,7 @@ twobyte_insn:
goto writeback;
 
 cannot_emulate:
-   DPRINTF("Cannot emulate %02x\n", c->b);
+   kvm_report_emulation_failure(ctxt->vcpu, "unhandled instruction");
c->eip = saved_eip;
return -1;
 }
-- 
1.6.0.4

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


[PATCH][RESEND] Add push/pop instructions test in test harness

2009-08-20 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   74 ++
 1 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..04c1452 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -467,6 +467,79 @@ void test_long_jmp()
if(!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x1234)
print_serial("Long JMP Test: FAIL\n");
 }
+void test_push_pop()
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(push32, "mov $0x12345678, %eax\n\t"
+   "push %eax\n\t"
+   "pop %ebx\n\t");
+   MK_INSN(push16, "mov $0x1234, %ax\n\t"
+   "push %ax\n\t"
+   "pop %bx\n\t");
+
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov $0x123, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+);
+   MK_INSN(pop_es, "push %ax\n\t"
+   "pop %es\n\t"
+   "mov %es, %bx\n\t"
+   );
+   MK_INSN(push_pop_ss, "push %ss\n\t"
+"pushw %ax\n\t"
+"popw %ss\n\t"
+"mov %ss, %bx\n\t"
+"pop %ss\n\t"
+   );
+   MK_INSN(push_pop_fs, "push %fs\n\t"
+"pushl %eax\n\t"
+"popl %fs\n\t"
+"mov %fs, %ebx\n\t"
+"pop %fs\n\t"
+   );
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push32,
+ insn_push32_end - insn_push32);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x12345678)
+   print_serial("Push/Pop Test 1: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push16,
+ insn_push16_end - insn_push16);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x1234)
+   print_serial("Push/Pop Test 2: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) ||  outregs.ebx != 
outregs.eax || outregs.eax != 0x123)
+   print_serial("Push/Pop Test 3: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pop_es,
+ insn_pop_es_end - insn_pop_es);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 4: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_ss,
+ insn_push_pop_ss_end - insn_push_pop_ss);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 5: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_fs,
+ insn_push_pop_fs_end - insn_push_pop_fs);
+   
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 6: FAIL\n");
+}
 
 void test_null(void)
 {
@@ -481,6 +554,7 @@ void start(void)
test_null();
 
test_shld();
+   test_push_pop();
test_mov_imm();
test_cmp_imm();
test_add_imm();
-- 
1.6.0.4

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


[PATCH][RESEND] x86 emulator: Add 'push/pop sreg' instructions

2009-08-23 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |  107 +---
 1 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1be5cd6..283237f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,19 +92,22 @@ static u32 opcode_table[256] = {
/* 0x00 - 0x07 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   0, 0, ImplicitOps | Stack, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack, ImplicitOps | Stack,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -244,11 +247,13 @@ static u32 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,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM, 0, 0,
/* 0xA8 - 0xAF */
-   0, 0, 0, DstMem | SrcReg | ModRM | BitOp,
+   ImplicitOps | Stack, ImplicitOps | Stack,
+   0, DstMem | SrcReg | ModRM | BitOp,
DstMem | SrcReg | Src2ImmByte | ModRM,
DstMem | SrcReg | Src2CL | ModRM,
ModRM, 0,
@@ -1186,6 +1191,32 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   struct kvm_segment segment;
+   
+   kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+   
+   c->src.val = segment.selector;
+   emulate_push(ctxt);
+}
+
+static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
+struct x86_emulate_ops *ops, int seg)
+{
+   struct decode_cache *c = &ctxt->decode;
+   u16 selector;
+   int rc;
+
+   rc = emulate_pop(ctxt, ops, &selector, c->op_bytes);
+   if (rc != 0)
+   return rc;
+   
+   rc = kvm_load_segment_descriptor(ctxt->vcpu, selector, 1, seg);
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1707,18 +1738,66 @@ special_insn:
  add:  /* add */
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
+   case 0x06:  /* push es */
+   if (ctxt->mode == X86EMUL_MODE_PROT64)
+   goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_ES);
+   break;
+   case 0x07:  /* pop es */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
+   if (rc != 0)
+   goto done;
+   break;
case 0x08 ... 0x0d:
  or:   /* or */
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
+   case 0x0e:  /* push cs */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_CS);
+   break;
case 0x10 ... 0x15:
  adc:  /* adc */
emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
break;
+   case 0x16:  /* push ss */
+if (ctxt->mode == X86EMUL_MODE_PROT64)
+goto cannot_emulate;
+
+   emulate_push_sreg(ctxt, VCPU_SREG_SS);
+   break;
+   case 0x17:  /* pop 

[PATCH][RESEND] Add push/pop instructions test in test harness

2009-08-23 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   74 ++
 1 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 755b5d1..04c1452 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -467,6 +467,79 @@ void test_long_jmp()
if(!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x1234)
print_serial("Long JMP Test: FAIL\n");
 }
+void test_push_pop()
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(push32, "mov $0x12345678, %eax\n\t"
+   "push %eax\n\t"
+   "pop %ebx\n\t");
+   MK_INSN(push16, "mov $0x1234, %ax\n\t"
+   "push %ax\n\t"
+   "pop %bx\n\t");
+
+   MK_INSN(push_es, "mov $0x231, %bx\n\t" //Just write a dummy value to 
see if it gets overwritten
+"mov $0x123, %ax\n\t"
+"mov %ax, %es\n\t"
+"push %es\n\t"
+"pop %bx \n\t"
+);
+   MK_INSN(pop_es, "push %ax\n\t"
+   "pop %es\n\t"
+   "mov %es, %bx\n\t"
+   );
+   MK_INSN(push_pop_ss, "push %ss\n\t"
+"pushw %ax\n\t"
+"popw %ss\n\t"
+"mov %ss, %bx\n\t"
+"pop %ss\n\t"
+   );
+   MK_INSN(push_pop_fs, "push %fs\n\t"
+"pushl %eax\n\t"
+"popl %fs\n\t"
+"mov %fs, %ebx\n\t"
+"pop %fs\n\t"
+   );
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push32,
+ insn_push32_end - insn_push32);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x12345678)
+   print_serial("Push/Pop Test 1: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push16,
+ insn_push16_end - insn_push16);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.eax != 
outregs.ebx || outregs.eax != 0x1234)
+   print_serial("Push/Pop Test 2: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_es,
+ insn_push_es_end - insn_push_es);
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) ||  outregs.ebx != 
outregs.eax || outregs.eax != 0x123)
+   print_serial("Push/Pop Test 3: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pop_es,
+ insn_pop_es_end - insn_pop_es);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 4: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_ss,
+ insn_push_pop_ss_end - insn_push_pop_ss);
+
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 5: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_push_pop_fs,
+ insn_push_pop_fs_end - insn_push_pop_fs);
+   
+   if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
+   print_serial("Push/Pop Test 6: FAIL\n");
+}
 
 void test_null(void)
 {
@@ -481,6 +554,7 @@ void start(void)
test_null();
 
test_shld();
+   test_push_pop();
test_mov_imm();
test_cmp_imm();
test_add_imm();
-- 
1.6.0.4

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


[PATCH][RESEND] x86 emulator: Report unhandled instructions

2009-08-23 Thread Mohammed Gamal
Report unhandled instructions in the syslog on emulation failure

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index cc0fe39..52a4b7d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2194,6 +2194,7 @@ writeback:
 
 done:
if (rc == X86EMUL_UNHANDLEABLE) {
+   kvm_report_emulation_failure(ctxt->vcpu, "unhandled 
instruction");
c->eip = saved_eip;
return -1;
}
@@ -2467,7 +2468,7 @@ twobyte_insn:
goto writeback;
 
 cannot_emulate:
-   DPRINTF("Cannot emulate %02x\n", c->b);
+   kvm_report_emulation_failure(ctxt->vcpu, "unhandled instruction");
c->eip = saved_eip;
return -1;
 }
-- 
1.6.0.4

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


[PATCH][RESEND] x86 emulator: Introduce No64 decode option

2009-08-23 Thread Mohammed Gamal
Introduces a new decode option "No64", which is used for instructions that are
invalid in long mode.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   42 ++
 1 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 283237f..cc0fe39 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -75,6 +75,8 @@
 #define Group   (1<<14) /* Bits 3:5 of modrm byte extend opcode */
 #define GroupDual   (1<<15) /* Alternate decoding of mod == 3 */
 #define GroupMask   0xff/* Group number stored in bits 0:7 */
+/* Misc flags */
+#define No64   (1<<28)
 /* Source 2 operand type */
 #define Src2None(0<<29)
 #define Src2CL  (1<<29)
@@ -93,21 +95,21 @@ static u32 opcode_table[256] = {
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, ImplicitOps | Stack, 0,
+   0, 0, ImplicitOps | Stack | No64, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x18 - 0x1F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
-   ImplicitOps | Stack, ImplicitOps | Stack,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
/* 0x20 - 0x27 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -161,7 +163,7 @@ static u32 opcode_table[256] = {
/* 0x90 - 0x97 */
DstReg, DstReg, DstReg, DstReg, DstReg, DstReg, DstReg, DstReg,
/* 0x98 - 0x9F */
-   0, 0, SrcImm | Src2Imm16, 0,
+   0, 0, SrcImm | Src2Imm16 | No64, 0,
ImplicitOps | Stack, ImplicitOps | Stack, 0, 0,
/* 0xA0 - 0xA7 */
ByteOp | DstReg | SrcMem | Mov | MemAbs, DstReg | SrcMem | Mov | MemAbs,
@@ -188,7 +190,7 @@ static u32 opcode_table[256] = {
ByteOp | DstMem | SrcImm | ModRM | Mov, DstMem | SrcImm | ModRM | Mov,
/* 0xC8 - 0xCF */
0, 0, 0, ImplicitOps | Stack,
-   ImplicitOps, SrcImmByte, ImplicitOps, ImplicitOps,
+   ImplicitOps, SrcImmByte, ImplicitOps | No64, ImplicitOps,
/* 0xD0 - 0xD7 */
ByteOp | DstMem | SrcImplicit | ModRM, DstMem | SrcImplicit | ModRM,
ByteOp | DstMem | SrcImplicit | ModRM, DstMem | SrcImplicit | ModRM,
@@ -201,7 +203,7 @@ static u32 opcode_table[256] = {
ByteOp | SrcImmUByte, SrcImmUByte,
/* 0xE8 - 0xEF */
SrcImm | Stack, SrcImm | ImplicitOps,
-   SrcImmU | Src2Imm16, SrcImmByte | ImplicitOps,
+   SrcImmU | Src2Imm16 | No64, SrcImmByte | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
SrcNone | ByteOp | ImplicitOps, SrcNone | ImplicitOps,
/* 0xF0 - 0xF7 */
@@ -967,6 +969,11 @@ done_prefixes:
}
}
 
+   if (mode == X86EMUL_MODE_PROT64 && (c->d & No64)) {
+   kvm_report_emulation_failure(ctxt->vcpu, "invalid x86/64 
instruction");;
+   return -1;
+   }
+
if (c->d & Group) {
group = c->d & GroupMask;
c->modrm = insn_fetch(u8, 1, c->eip);
@@ -1739,15 +1746,9 @@ special_insn:
emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
break;
case 0x06:  /* push es */
-   if (ctxt->mode == X86EMUL_MODE_PROT64)
-   goto cannot_emulate;
-
emulate_push_sreg(ctxt, VCPU_SREG_ES);
break;
case 0x07:  /* pop es */
-if (ctxt->mode == X86EMUL_MODE_PROT64)
-goto cannot_emulate;
-
rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
if (rc != 0)
goto done;
@@ -1757,9 +1758,6 @@ special_insn:
emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
break;
case 0x0e:  /* push cs */
-if (ctxt->mode == X86EMUL_MODE_PROT64)
-goto cannot_emulate;
-
  

[PATCH] VMX: Return to userspace on invalid state emulation failure

2009-08-23 Thread Mohammed Gamal
Return to userspace instead of repeatedly trying to emulate
instructions that have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1ee811c..6030671 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3341,6 +3341,8 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu,
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
+   kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   kvm_run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
break;
}
 
@@ -3612,7 +3614,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct 
kvm_run *kvm_run)
vmx->entry_time = ktime_get();
 
/* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
+   if (vmx->emulation_required && emulate_invalid_guest_state
+   && kvm_run->internal.suberror != KVM_INTERNAL_ERROR_EMULATION) {
handle_invalid_guest_state(vcpu, kvm_run);
return;
}
-- 
1.6.0.4

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


[PATCH] VMX: Return to userspace on invalid state emulation failure

2009-08-24 Thread Mohammed Gamal
Return to userspace instead of repeatedly trying to emulate
instructions that have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1ee811c..423e44f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3341,6 +3341,8 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu,
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
+   kvm_run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   kvm_run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
break;
}
 
@@ -3612,7 +3614,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu, struct 
kvm_run *kvm_run)
vmx->entry_time = ktime_get();
 
/* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
+   if (vmx->emulation_required && emulate_invalid_guest_state
+   && !(kvm_run->exit_reason == KVM_EXIT_INTERNAL_ERROR && 
+ kvm_run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION)) {
handle_invalid_guest_state(vcpu, kvm_run);
return;
}
-- 
1.6.0.4

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


[PATCH] VMX: Return to userspace on invalid state emulation failure

2009-08-24 Thread Mohammed Gamal
Return to userspace instead of repeatedly trying to emulate
instructions that have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6b57eed..c559bb7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3337,6 +3337,8 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
+   vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
break;
}
 
@@ -3607,7 +3609,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
vmx->entry_time = ktime_get();
 
/* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
+   if (vmx->emulation_required && emulate_invalid_guest_state
+   && !(vcpu->run->exit_reason == KVM_EXIT_INTERNAL_ERROR &&
+ vcpu->run->internal.suberror == 
KVM_INTERNAL_ERROR_EMULATION)) {
handle_invalid_guest_state(vcpu);
return;
}
-- 
1.6.0.4

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


Re: [PATCH] VMX: Return to userspace on invalid state emulation failure

2009-08-26 Thread Mohammed Gamal
On Wed, Aug 26, 2009 at 12:02 PM, Avi Kivity wrote:
> On 08/25/2009 01:37 AM, Mohammed Gamal wrote:
>>
>> Return to userspace instead of repeatedly trying to emulate
>> instructions that have already failed
>>
>> Signed-off-by: Mohammed Gamal
>> ---
>>  arch/x86/kvm/vmx.c |    6 +-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 6b57eed..c559bb7 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3337,6 +3337,8 @@ static void handle_invalid_guest_state(struct
>> kvm_vcpu *vcpu)
>>
>>                if (err != EMULATE_DONE) {
>>                        kvm_report_emulation_failure(vcpu, "emulation
>> failure");
>> +                       vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>> +                       vcpu->run->internal.suberror =
>> KVM_INTERNAL_ERROR_EMULATION;
>>                        break;
>>                }
>>
>> @@ -3607,7 +3609,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
>>                vmx->entry_time = ktime_get();
>>
>>        /* Handle invalid guest state instead of entering VMX */
>> -       if (vmx->emulation_required&&  emulate_invalid_guest_state) {
>> +       if (vmx->emulation_required&&  emulate_invalid_guest_state
>> +               &&  !(vcpu->run->exit_reason == KVM_EXIT_INTERNAL_ERROR&&
>> +                 vcpu->run->internal.suberror ==
>> KVM_INTERNAL_ERROR_EMULATION)) {
>>                handle_invalid_guest_state(vcpu);
>>                return;
>>        }
>>
>
> Still suffers from the same problem.  You don't always update
> vcpu->run->exit_reason, so you can't test it.  Best to return a value from
> handle_invalid_guest_state() (the standard return codes for exit handlers
> are 1 for return-to-guest, 0 for return-to-host, and -errno to return with
> an error).
>
I was thinking of the same idea since I was also concerned about
vcpu->run->exit_reason not being updated. But how can we interpret the
return values of handle_invalid_guest_state() inside vmx_vcpu_run()
since it doesn't have a return value. Or would it be better to move
handle_invalid_guest_state() to the standard vmx exit handlers?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] x86 emulator: Add missing decoder flags for 'or' instructions

2009-08-28 Thread Mohammed Gamal
Add missing decoder flags for or instructions (0xc-0xd)

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 0644d3d..db0820d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -99,7 +99,8 @@ static u32 opcode_table[256] = {
/* 0x08 - 0x0F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, ImplicitOps | Stack | No64, 0,
+   ByteOp | DstAcc | SrcImm, DstAcc | SrcImm,
+   ImplicitOps | Stack | No64, 0,
/* 0x10 - 0x17 */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-- 
1.6.0.4

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


[PATCH 1/2] Update kvm.h in qemu-kvm tree

2009-08-28 Thread Mohammed Gamal
Update struct kvm_run and KVM_EXIT_* constants to match the one in the
kernel-space tree

Signed-off-by: Mohammed Gamal 
---
 kvm/include/linux/kvm.h |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kvm/include/linux/kvm.h b/kvm/include/linux/kvm.h
index ff1025d..801865d 100644
--- a/kvm/include/linux/kvm.h
+++ b/kvm/include/linux/kvm.h
@@ -135,6 +135,10 @@ struct kvm_pit_config {
 #define KVM_EXIT_S390_RESET   14
 #define KVM_EXIT_DCR  15
 #define KVM_EXIT_NMI  16
+#define KVM_EXIT_INTERNAL_ERROR   17
+
+/* For KVM_EXIT_INTERNAL_ERROR */
+#define KVM_INTERNAL_ERROR_EMULATION 1
 
 /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
 struct kvm_run {
@@ -221,6 +225,9 @@ struct kvm_run {
__u32 data;
__u8  is_write;
} dcr;
+   struct {
+   __u32 suberror;
+   } internal;
/* Fix the size of the union. */
char padding[256];
};
-- 
1.6.0.4

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


[PATCH 2/2] Handle emulation failure in userspace

2009-08-28 Thread Mohammed Gamal
Since we return to userspace from KVM on invalid state emulation failure, let
qemu handle it.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index b59e403..a1648e0 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1029,6 +1029,14 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
 r = kvm_s390_handle_reset(kvm, vcpu, run);
 break;
 #endif
+   case KVM_EXIT_INTERNAL_ERROR:
+   kvm_show_regs(vcpu);
+   fprintf(stderr, "\nKVM internal error. Suberror: %d\n",
+   run->internal.suberror);
+   if (run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION)
+   fprintf(stderr, "emulation failure, check dmesg for details\n");
+   abort();
+   break;
 default:
 if (kvm_arch_run(vcpu)) {
 fprintf(stderr, "unhandled vm exit: 0x%x\n", run->exit_reason);
-- 
1.6.0.4

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


[PATCH 3/3] VMX: Enhance invalid guest state emulation

2009-08-28 Thread Mohammed Gamal
- Change returned handle_invalid_guest_state() to return relevant exit codes
- Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
- Return to userspace instead of repeatedly trying to emulate
instructions that have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   24 ++--
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 78101dd..e422470 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3318,10 +3318,11 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu)
return 1;
 }
 
-static void handle_invalid_guest_state(struct kvm_vcpu *vcpu)
+static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
enum emulation_result err = EMULATE_DONE;
+   int ret = 1;
 
local_irq_enable();
preempt_enable();
@@ -3329,11 +3330,16 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
while (!guest_state_valid(vcpu)) {
err = emulate_instruction(vcpu, 0, 0, 0);
 
-   if (err == EMULATE_DO_MMIO)
+   if (err == EMULATE_DO_MMIO) {
+   ret = 0;
break;
+   }
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
+   vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
+   ret = 0;
break;
}
 
@@ -3347,6 +3353,7 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
local_irq_disable();
 
vmx->invalid_state_emulation_result = err;
+   return ret;
 }
 
 /*
@@ -3405,9 +3412,12 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
/* If we need to emulate an MMIO from handle_invalid_guest_state
 * we just return 0 */
if (vmx->emulation_required && emulate_invalid_guest_state) {
-   if (guest_state_valid(vcpu))
+   if (guest_state_valid(vcpu)) {
vmx->emulation_required = 0;
-   return vmx->invalid_state_emulation_result != EMULATE_DO_MMIO;
+   return vmx->invalid_state_emulation_result != 
EMULATE_DO_MMIO;  
+   } else {
+   return handle_invalid_guest_state(vcpu);
+   }
}
 
/* Access CR3 don't cause VMExit in paging mode, so we need
@@ -3603,12 +3613,6 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
vmx->entry_time = ktime_get();
 
-   /* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
-   handle_invalid_guest_state(vcpu);
-   return;
-   }
-
if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
if (test_bit(VCPU_REGS_RIP, (unsigned long *)&vcpu->arch.regs_dirty))
-- 
1.6.0.4

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


[PATCH] x86 emulator: Add pusha and popa instructions

2009-08-29 Thread Mohammed Gamal
This adds pusha and popa instructions (opcodes 0x60-0x61), this enables booting
MINIX with invalid guest state emulation on.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   52 +++-
 1 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index db0820d..9be2e6e 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -139,7 +139,8 @@ static u32 opcode_table[256] = {
DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
/* 0x60 - 0x67 */
-   0, 0, 0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
+   0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
0, 0, 0, 0,
/* 0x68 - 0x6F */
SrcImm | Mov | Stack, 0, SrcImmByte | Mov | Stack, 0,
@@ -1225,6 +1226,47 @@ static int emulate_pop_sreg(struct x86_emulate_ctxt 
*ctxt,
return rc;
 }
 
+static void emulate_pusha(struct x86_emulate_ctxt *ctxt)
+{
+   struct decode_cache *c = &ctxt->decode;
+   unsigned long old_esp = c->regs[VCPU_REGS_RSP];
+   int reg = VCPU_REGS_RAX;
+
+   while (reg <= VCPU_REGS_RDI) {
+   (reg == VCPU_REGS_RSP) ? 
+   (c->src.val = old_esp) : (c->src.val = c->regs[reg]);
+
+   emulate_push(ctxt);
+   ++reg;
+   }
+}
+
+static int emulate_popa(struct x86_emulate_ctxt *ctxt,
+   struct x86_emulate_ops *ops)
+{
+   struct decode_cache *c = &ctxt->decode;
+   unsigned long old_esp = c->regs[VCPU_REGS_RSP];
+   int rc = 0;
+   int reg = VCPU_REGS_RDI;
+
+   while (reg >= VCPU_REGS_RAX) {
+   if (reg == VCPU_REGS_RSP) {
+   register_address_increment(c, &c->regs[VCPU_REGS_RSP],
+   c->op_bytes);
+   --reg;
+   }
+
+   rc = emulate_pop(ctxt, ops, &c->regs[reg], c->op_bytes);
+   if (rc != 0) {
+   c->regs[VCPU_REGS_RSP] = old_esp;
+   break;
+   }
+
+   --reg;
+   }
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1816,6 +1858,14 @@ special_insn:
if (rc != 0)
goto done;
break;
+   case 0x60:  /* pusha */
+   emulate_pusha(ctxt);
+   break;
+   case 0x61:  /* popa */
+   rc = emulate_popa(ctxt, ops);
+   if (rc != 0)
+   goto done;
+   break; 
case 0x63:  /* movsxd */
if (ctxt->mode != X86EMUL_MODE_PROT64)
goto cannot_emulate;
-- 
1.6.0.4

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


[PATCH] Add pusha/popa instructions to the realmode test harness

2009-08-29 Thread Mohammed Gamal
This adds tests for pusha/popa to the test harness. Added some new typedefs
while as well.

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 0db09b8..20d038e 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -4,6 +4,10 @@ typedef unsigned char u8;
 typedef unsigned short u16;
 typedef unsigned u32;
 typedef unsigned long long u64;
+typedef signed char s8;
+typedef signed short s16;
+typedef signed s32;
+typedef signed long long s64;
 
 void test_function(void);
 
@@ -470,6 +474,7 @@ void test_long_jmp()
 void test_push_pop()
 {
struct regs inregs = { 0 }, outregs;
+
MK_INSN(push32, "mov $0x12345678, %eax\n\t"
"push %eax\n\t"
"pop %ebx\n\t");
@@ -499,6 +504,10 @@ void test_push_pop()
 "mov %fs, %ebx\n\t"
 "pop %fs\n\t"
);
+   MK_INSN(pusha,  "pushaw\n\t");
+   MK_INSN(pushad, "pushal\n\t");
+   MK_INSN(popa, "popaw\n\t");
+   MK_INSN(popad, "popal\n\t");
 
exec_in_big_real_mode(&inregs, &outregs,
  insn_push32,
@@ -539,6 +548,34 @@ void test_push_pop()
 
if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
print_serial("Push/Pop Test 6: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+insn_pusha,
+insn_pusha_end - insn_pusha);
+   if (!regs_equal(&inregs, &outregs, R_SP) 
+   || (s16)outregs.esp != (s16)(inregs.esp - 16))
+   print_serial("Push/Pop Test 7: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popa,
+ insn_popa_end - insn_popa);
+   if (outregs.esp != (inregs.esp + 16))
+   print_serial("Push/Pop Test 8: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pushad,
+ insn_pushad_end - insn_pushad);
+
+   if (!regs_equal(&inregs, &outregs, R_SP)
+   || (s16)outregs.esp != (s16)(inregs.esp - 32))
+   print_serial("Push/Pop Test 9: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popad,
+ insn_popad_end - insn_popad);
+
+   if (outregs.esp != (inregs.esp + 32))
+   print_serial("Push/Pop Test 10: FAIL\n");
 }
 
 void test_null(void)
-- 
1.6.0.4

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


[PATCH] Add pusha/popa instructions to the realmode test harness

2009-08-29 Thread Mohammed Gamal
This adds tests for pusha/popa to the test harness. Added some new typedefs
while as well.

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 0db09b8..20d038e 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -4,6 +4,10 @@ typedef unsigned char u8;
 typedef unsigned short u16;
 typedef unsigned u32;
 typedef unsigned long long u64;
+typedef signed char s8;
+typedef signed short s16;
+typedef signed s32;
+typedef signed long long s64;
 
 void test_function(void);
 
@@ -470,6 +474,7 @@ void test_long_jmp()
 void test_push_pop()
 {
struct regs inregs = { 0 }, outregs;
+
MK_INSN(push32, "mov $0x12345678, %eax\n\t"
"push %eax\n\t"
"pop %ebx\n\t");
@@ -499,6 +504,10 @@ void test_push_pop()
 "mov %fs, %ebx\n\t"
 "pop %fs\n\t"
);
+   MK_INSN(pusha,  "pushaw\n\t");
+   MK_INSN(pushad, "pushal\n\t");
+   MK_INSN(popa, "popaw\n\t");
+   MK_INSN(popad, "popal\n\t");
 
exec_in_big_real_mode(&inregs, &outregs,
  insn_push32,
@@ -539,6 +548,34 @@ void test_push_pop()
 
if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
print_serial("Push/Pop Test 6: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+insn_pusha,
+insn_pusha_end - insn_pusha);
+   if (!regs_equal(&inregs, &outregs, R_SP) 
+   || (s16)outregs.esp != (s16)(inregs.esp - 16))
+   print_serial("Push/Pop Test 7: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popa,
+ insn_popa_end - insn_popa);
+   if (outregs.esp != (inregs.esp + 16))
+   print_serial("Push/Pop Test 8: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pushad,
+ insn_pushad_end - insn_pushad);
+
+   if (!regs_equal(&inregs, &outregs, R_SP)
+   || (s16)outregs.esp != (s16)(inregs.esp - 32))
+   print_serial("Push/Pop Test 9: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popad,
+ insn_popad_end - insn_popad);
+
+   if (outregs.esp != (inregs.esp + 32))
+   print_serial("Push/Pop Test 10: FAIL\n");
 }
 
 void test_null(void)
-- 
1.6.0.4

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


[PATCH] Add pusha/popa instructions to the realmode test harness

2009-08-29 Thread Mohammed Gamal
This adds tests for pusha/popa to the test harness. Added some new typedefs
while as well.

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 0db09b8..20d038e 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -4,6 +4,10 @@ typedef unsigned char u8;
 typedef unsigned short u16;
 typedef unsigned u32;
 typedef unsigned long long u64;
+typedef signed char s8;
+typedef signed short s16;
+typedef signed s32;
+typedef signed long long s64;
 
 void test_function(void);
 
@@ -470,6 +474,7 @@ void test_long_jmp()
 void test_push_pop()
 {
struct regs inregs = { 0 }, outregs;
+
MK_INSN(push32, "mov $0x12345678, %eax\n\t"
"push %eax\n\t"
"pop %ebx\n\t");
@@ -499,6 +504,10 @@ void test_push_pop()
 "mov %fs, %ebx\n\t"
 "pop %fs\n\t"
);
+   MK_INSN(pusha,  "pushaw\n\t");
+   MK_INSN(pushad, "pushal\n\t");
+   MK_INSN(popa, "popaw\n\t");
+   MK_INSN(popad, "popal\n\t");
 
exec_in_big_real_mode(&inregs, &outregs,
  insn_push32,
@@ -539,6 +548,34 @@ void test_push_pop()
 
if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
print_serial("Push/Pop Test 6: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+insn_pusha,
+insn_pusha_end - insn_pusha);
+   if (!regs_equal(&inregs, &outregs, R_SP) 
+   || (s16)outregs.esp != (s16)(inregs.esp - 16))
+   print_serial("Push/Pop Test 7: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popa,
+ insn_popa_end - insn_popa);
+   if (outregs.esp != (inregs.esp + 16))
+   print_serial("Push/Pop Test 8: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pushad,
+ insn_pushad_end - insn_pushad);
+
+   if (!regs_equal(&inregs, &outregs, R_SP)
+   || (s16)outregs.esp != (s16)(inregs.esp - 32))
+   print_serial("Push/Pop Test 9: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popad,
+ insn_popad_end - insn_popad);
+
+   if (outregs.esp != (inregs.esp + 32))
+   print_serial("Push/Pop Test 10: FAIL\n");
 }
 
 void test_null(void)
-- 
1.6.0.4

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


[PATCH] Add pusha/popa instructions to the realmode test harness

2009-08-29 Thread Mohammed Gamal
This adds tests for pusha/popa to the test harness. Added some new typedefs
as well.

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 0db09b8..20d038e 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -4,6 +4,10 @@ typedef unsigned char u8;
 typedef unsigned short u16;
 typedef unsigned u32;
 typedef unsigned long long u64;
+typedef signed char s8;
+typedef signed short s16;
+typedef signed s32;
+typedef signed long long s64;
 
 void test_function(void);
 
@@ -470,6 +474,7 @@ void test_long_jmp()
 void test_push_pop()
 {
struct regs inregs = { 0 }, outregs;
+
MK_INSN(push32, "mov $0x12345678, %eax\n\t"
"push %eax\n\t"
"pop %ebx\n\t");
@@ -499,6 +504,10 @@ void test_push_pop()
 "mov %fs, %ebx\n\t"
 "pop %fs\n\t"
);
+   MK_INSN(pusha,  "pushaw\n\t");
+   MK_INSN(pushad, "pushal\n\t");
+   MK_INSN(popa, "popaw\n\t");
+   MK_INSN(popad, "popal\n\t");
 
exec_in_big_real_mode(&inregs, &outregs,
  insn_push32,
@@ -539,6 +548,34 @@ void test_push_pop()
 
if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
print_serial("Push/Pop Test 6: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+insn_pusha,
+insn_pusha_end - insn_pusha);
+   if (!regs_equal(&inregs, &outregs, R_SP) 
+   || (s16)outregs.esp != (s16)(inregs.esp - 16))
+   print_serial("Push/Pop Test 7: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popa,
+ insn_popa_end - insn_popa);
+   if (outregs.esp != (inregs.esp + 16))
+   print_serial("Push/Pop Test 8: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pushad,
+ insn_pushad_end - insn_pushad);
+
+   if (!regs_equal(&inregs, &outregs, R_SP)
+   || (s16)outregs.esp != (s16)(inregs.esp - 32))
+   print_serial("Push/Pop Test 9: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popad,
+ insn_popad_end - insn_popad);
+
+   if (outregs.esp != (inregs.esp + 32))
+   print_serial("Push/Pop Test 10: FAIL\n");
 }
 
 void test_null(void)
-- 
1.6.0.4

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


[[PATCH v2]] VMX: Enhance invalid guest state emulation

2009-08-31 Thread Mohammed Gamal
- Change returned handle_invalid_guest_state() to return relevant exit codes
- Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
- Return to userspace instead of repeatedly trying to emulate
instructions that have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   31 +++
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 78101dd..34bfd87 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -107,7 +107,6 @@ struct vcpu_vmx {
} rmode;
int vpid;
bool emulation_required;
-   enum emulation_result invalid_state_emulation_result;
 
/* Support for vnmi-less CPUs */
int soft_vnmi_blocked;
@@ -3318,22 +3317,24 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu)
return 1;
 }
 
-static void handle_invalid_guest_state(struct kvm_vcpu *vcpu)
+static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 {
-   struct vcpu_vmx *vmx = to_vmx(vcpu);
enum emulation_result err = EMULATE_DONE;
-
-   local_irq_enable();
-   preempt_enable();
+   int ret = 1;
 
while (!guest_state_valid(vcpu)) {
err = emulate_instruction(vcpu, 0, 0, 0);
 
-   if (err == EMULATE_DO_MMIO)
+   if (err == EMULATE_DO_MMIO) {
+   ret = 0;
break;
+   }
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
+   vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
+   ret = 0;
break;
}
 
@@ -3343,10 +3344,7 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
schedule();
}
 
-   preempt_disable();
-   local_irq_disable();
-
-   vmx->invalid_state_emulation_result = err;
+   return ret;
 }
 
 /*
@@ -3405,9 +3403,12 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
/* If we need to emulate an MMIO from handle_invalid_guest_state
 * we just return 0 */
if (vmx->emulation_required && emulate_invalid_guest_state) {
-   if (guest_state_valid(vcpu))
+   if (!guest_state_valid(vcpu)) {
+   return handle_invalid_guest_state(vcpu);
+   } else {
vmx->emulation_required = 0;
-   return vmx->invalid_state_emulation_result != EMULATE_DO_MMIO;
+   return 1;
+   }
}
 
/* Access CR3 don't cause VMExit in paging mode, so we need
@@ -3604,10 +3605,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
vmx->entry_time = ktime_get();
 
/* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
-   handle_invalid_guest_state(vcpu);
+   if (vmx->emulation_required && emulate_invalid_guest_state)
return;
-   }
 
if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
-- 
1.6.0.4

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


[PATCH v2] VMX: Enhance invalid guest state emulation

2009-08-31 Thread Mohammed Gamal
- Change returned handle_invalid_guest_state() to return relevant exit codes
- Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
- Return to userspace instead of repeatedly trying to emulate
instructions that have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   31 +++
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 78101dd..34bfd87 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -107,7 +107,6 @@ struct vcpu_vmx {
} rmode;
int vpid;
bool emulation_required;
-   enum emulation_result invalid_state_emulation_result;
 
/* Support for vnmi-less CPUs */
int soft_vnmi_blocked;
@@ -3318,22 +3317,24 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu)
return 1;
 }
 
-static void handle_invalid_guest_state(struct kvm_vcpu *vcpu)
+static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 {
-   struct vcpu_vmx *vmx = to_vmx(vcpu);
enum emulation_result err = EMULATE_DONE;
-
-   local_irq_enable();
-   preempt_enable();
+   int ret = 1;
 
while (!guest_state_valid(vcpu)) {
err = emulate_instruction(vcpu, 0, 0, 0);
 
-   if (err == EMULATE_DO_MMIO)
+   if (err == EMULATE_DO_MMIO) {
+   ret = 0;
break;
+   }
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
+   vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
+   ret = 0;
break;
}
 
@@ -3343,10 +3344,7 @@ static void handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
schedule();
}
 
-   preempt_disable();
-   local_irq_disable();
-
-   vmx->invalid_state_emulation_result = err;
+   return ret;
 }
 
 /*
@@ -3405,9 +3403,12 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
/* If we need to emulate an MMIO from handle_invalid_guest_state
 * we just return 0 */
if (vmx->emulation_required && emulate_invalid_guest_state) {
-   if (guest_state_valid(vcpu))
+   if (!guest_state_valid(vcpu)) {
+   return handle_invalid_guest_state(vcpu);
+   } else {
vmx->emulation_required = 0;
-   return vmx->invalid_state_emulation_result != EMULATE_DO_MMIO;
+   return 1;
+   }
}
 
/* Access CR3 don't cause VMExit in paging mode, so we need
@@ -3604,10 +3605,8 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
vmx->entry_time = ktime_get();
 
/* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
-   handle_invalid_guest_state(vcpu);
+   if (vmx->emulation_required && emulate_invalid_guest_state)
return;
-   }
 
if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
-- 
1.6.0.4

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


[PATCH 1/2][RESEND] Update kvm.h in qemu-kvm tree

2009-08-31 Thread Mohammed Gamal
Update struct kvm_run and KVM_EXIT_* constants to match the one in the
kernel-space tree

Signed-off-by: Mohammed Gamal 
---
 kvm/include/linux/kvm.h |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kvm/include/linux/kvm.h b/kvm/include/linux/kvm.h
index ff1025d..801865d 100644
--- a/kvm/include/linux/kvm.h
+++ b/kvm/include/linux/kvm.h
@@ -135,6 +135,10 @@ struct kvm_pit_config {
 #define KVM_EXIT_S390_RESET   14
 #define KVM_EXIT_DCR  15
 #define KVM_EXIT_NMI  16
+#define KVM_EXIT_INTERNAL_ERROR   17
+
+/* For KVM_EXIT_INTERNAL_ERROR */
+#define KVM_INTERNAL_ERROR_EMULATION 1
 
 /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
 struct kvm_run {
@@ -221,6 +225,9 @@ struct kvm_run {
__u32 data;
__u8  is_write;
} dcr;
+   struct {
+   __u32 suberror;
+   } internal;
/* Fix the size of the union. */
char padding[256];
};
-- 
1.6.0.4

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


[PATCH 2/2][RESEND] Handle emulation failure in userspace

2009-08-31 Thread Mohammed Gamal
Since we return to userspace from KVM on invalid state emulation failure, let
qemu handle it.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index b59e403..a1648e0 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1029,6 +1029,14 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
 r = kvm_s390_handle_reset(kvm, vcpu, run);
 break;
 #endif
+   case KVM_EXIT_INTERNAL_ERROR:
+   kvm_show_regs(vcpu);
+   fprintf(stderr, "\nKVM internal error. Suberror: %d\n",
+   run->internal.suberror);
+   if (run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION)
+   fprintf(stderr, "emulation failure, check dmesg for details\n");
+   abort();
+   break;
 default:
 if (kvm_arch_run(vcpu)) {
 fprintf(stderr, "unhandled vm exit: 0x%x\n", run->exit_reason);
-- 
1.6.0.4

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


[PATCH][RESEND] x86 emulator: Add pusha and popa instructions

2009-08-31 Thread Mohammed Gamal
This adds pusha and popa instructions (opcodes 0x60-0x61), this enables booting
MINIX with invalid guest state emulation on.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   52 +++-
 1 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index db0820d..9be2e6e 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -139,7 +139,8 @@ static u32 opcode_table[256] = {
DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
/* 0x60 - 0x67 */
-   0, 0, 0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
+   0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
0, 0, 0, 0,
/* 0x68 - 0x6F */
SrcImm | Mov | Stack, 0, SrcImmByte | Mov | Stack, 0,
@@ -1225,6 +1226,47 @@ static int emulate_pop_sreg(struct x86_emulate_ctxt 
*ctxt,
return rc;
 }
 
+static void emulate_pusha(struct x86_emulate_ctxt *ctxt)
+{
+   struct decode_cache *c = &ctxt->decode;
+   unsigned long old_esp = c->regs[VCPU_REGS_RSP];
+   int reg = VCPU_REGS_RAX;
+
+   while (reg <= VCPU_REGS_RDI) {
+   (reg == VCPU_REGS_RSP) ? 
+   (c->src.val = old_esp) : (c->src.val = c->regs[reg]);
+
+   emulate_push(ctxt);
+   ++reg;
+   }
+}
+
+static int emulate_popa(struct x86_emulate_ctxt *ctxt,
+   struct x86_emulate_ops *ops)
+{
+   struct decode_cache *c = &ctxt->decode;
+   unsigned long old_esp = c->regs[VCPU_REGS_RSP];
+   int rc = 0;
+   int reg = VCPU_REGS_RDI;
+
+   while (reg >= VCPU_REGS_RAX) {
+   if (reg == VCPU_REGS_RSP) {
+   register_address_increment(c, &c->regs[VCPU_REGS_RSP],
+   c->op_bytes);
+   --reg;
+   }
+
+   rc = emulate_pop(ctxt, ops, &c->regs[reg], c->op_bytes);
+   if (rc != 0) {
+   c->regs[VCPU_REGS_RSP] = old_esp;
+   break;
+   }
+
+   --reg;
+   }
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1816,6 +1858,14 @@ special_insn:
if (rc != 0)
goto done;
break;
+   case 0x60:  /* pusha */
+   emulate_pusha(ctxt);
+   break;
+   case 0x61:  /* popa */
+   rc = emulate_popa(ctxt, ops);
+   if (rc != 0)
+   goto done;
+   break; 
case 0x63:  /* movsxd */
if (ctxt->mode != X86EMUL_MODE_PROT64)
goto cannot_emulate;
-- 
1.6.0.4

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


[PATCHv3] VMX: Enhance invalid guest state emulation

2009-09-01 Thread Mohammed Gamal
- Change returned handle_invalid_guest_state() to return relevant exit codes
- Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
- Return to userspace instead of repeatedly trying to emulate instructions that 
have already failed

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   44 
 1 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 78101dd..6265098 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -107,7 +107,6 @@ struct vcpu_vmx {
} rmode;
int vpid;
bool emulation_required;
-   enum emulation_result invalid_state_emulation_result;
 
/* Support for vnmi-less CPUs */
int soft_vnmi_blocked;
@@ -3318,35 +3317,37 @@ static int handle_nmi_window(struct kvm_vcpu *vcpu)
return 1;
 }
 
-static void handle_invalid_guest_state(struct kvm_vcpu *vcpu)
+static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx = to_vmx(vcpu);
enum emulation_result err = EMULATE_DONE;
-
-   local_irq_enable();
-   preempt_enable();
+   int ret = 1;
 
while (!guest_state_valid(vcpu)) {
err = emulate_instruction(vcpu, 0, 0, 0);
 
-   if (err == EMULATE_DO_MMIO)
-   break;
+   if (err == EMULATE_DO_MMIO) {
+   ret = 0;
+   goto out;
+   }
 
if (err != EMULATE_DONE) {
kvm_report_emulation_failure(vcpu, "emulation failure");
-   break;
+   vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+   vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
+   ret = 0;
+   goto out;
}
 
if (signal_pending(current))
-   break;
+   goto out;
if (need_resched())
schedule();
}
 
-   preempt_disable();
-   local_irq_disable();
-
-   vmx->invalid_state_emulation_result = err;
+   vmx->emulation_required = 0;
+out:
+   return ret;
 }
 
 /*
@@ -3402,13 +3403,9 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
 
trace_kvm_exit(exit_reason, kvm_rip_read(vcpu));
 
-   /* If we need to emulate an MMIO from handle_invalid_guest_state
-* we just return 0 */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
-   if (guest_state_valid(vcpu))
-   vmx->emulation_required = 0;
-   return vmx->invalid_state_emulation_result != EMULATE_DO_MMIO;
-   }
+   /* If guest state is invalid, start emulating */
+   if (vmx->emulation_required && emulate_invalid_guest_state)
+   return handle_invalid_guest_state(vcpu);
 
/* Access CR3 don't cause VMExit in paging mode, so we need
 * to sync with guest real CR3. */
@@ -3603,11 +3600,10 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
vmx->entry_time = ktime_get();
 
-   /* Handle invalid guest state instead of entering VMX */
-   if (vmx->emulation_required && emulate_invalid_guest_state) {
-   handle_invalid_guest_state(vcpu);
+   /* Don't enter VMX if guest state is invalid, let the exit handler
+  start emulation until we arrive back to a valid state */
+   if (vmx->emulation_required && emulate_invalid_guest_state)
return;
-   }
 
if (test_bit(VCPU_REGS_RSP, (unsigned long *)&vcpu->arch.regs_dirty))
vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
-- 
1.6.0.4

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


Re: [PATCHv3] VMX: Enhance invalid guest state emulation

2009-09-01 Thread Mohammed Gamal
On Tue, Sep 1, 2009 at 1:48 PM, Marcelo Tosatti wrote:
> On Tue, Sep 01, 2009 at 12:48:18PM +0200, Mohammed Gamal wrote:
>> - Change returned handle_invalid_guest_state() to return relevant exit codes
>> - Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
>> - Return to userspace instead of repeatedly trying to emulate instructions 
>> that have already failed
>>
>> Signed-off-by: Mohammed Gamal 
>
> Mohammed,
>
> The handle_invalid_guest_state loop is potentially problematic. It would
> be more appropriate to use the __vcpu_run loop.
>
> Can't you set vmx->emulation_required depending on the result
> of one call to emulate_instruction and get rid of the while
> (!guest_state_valid(vcpu)) loop?
>

Invalid state emulation is VMX-specfic, while the __vcpu_run loop is
independent of the virtualization extension (defined in x86.c), no?
AMD SVM can comforably run hosts in big-real mode and thus it doesn't
have the notion of a guest going to an invalid state because of mode
switching, so I don't think it'd be a good idea to move emulation into
a generic layer. Please correct me if I am wrong
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] Handle emulation failure in userspace

2009-09-01 Thread Mohammed Gamal
On Tue, Sep 1, 2009 at 2:31 PM, Marcelo Tosatti wrote:
> On Fri, Aug 28, 2009 at 04:48:53PM +0200, Mohammed Gamal wrote:
>> Since we return to userspace from KVM on invalid state emulation failure, let
>> qemu handle it.
>>
>> Signed-off-by: Mohammed Gamal 
>> ---
>>  qemu-kvm.c |    8 
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index b59e403..a1648e0 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -1029,6 +1029,14 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
>>              r = kvm_s390_handle_reset(kvm, vcpu, run);
>>              break;
>>  #endif
>> +     case KVM_EXIT_INTERNAL_ERROR:
>> +         kvm_show_regs(vcpu);
>> +         fprintf(stderr, "\nKVM internal error. Suberror: %d\n",
>> +                 run->internal.suberror);
>> +         if (run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION)
>> +             fprintf(stderr, "emulation failure, check dmesg for 
>> details\n");
>> +         abort();
>> +         break;
>>          default:
>>              if (kvm_arch_run(vcpu)) {
>>                  fprintf(stderr, "unhandled vm exit: 0x%x\n", 
>> run->exit_reason);
>> --
>> 1.6.0.4
>
> The common practice is to print msg first and then kvm_show_regs?
True. I just thought the message would be more visible this way. Will resend

> Applied the kvm.h update, thanks.
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH][RESEND] x86 emulator: Add pusha and popa instructions

2009-09-01 Thread Mohammed Gamal
On Tue, Sep 1, 2009 at 2:57 PM, Marcelo Tosatti wrote:
> On Tue, Sep 01, 2009 at 03:26:30AM +0200, Mohammed Gamal wrote:
>> This adds pusha and popa instructions (opcodes 0x60-0x61), this enables 
>> booting
>> MINIX with invalid guest state emulation on.
>>
>> Signed-off-by: Mohammed Gamal 
>> ---
>>  arch/x86/kvm/emulate.c |   52 
>> +++-
>>  1 files changed, 51 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index db0820d..9be2e6e 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -139,7 +139,8 @@ static u32 opcode_table[256] = {
>>       DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
>>       DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
>>       /* 0x60 - 0x67 */
>> -     0, 0, 0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
>> +     ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
>> +     0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
>>       0, 0, 0, 0,
>>       /* 0x68 - 0x6F */
>>       SrcImm | Mov | Stack, 0, SrcImmByte | Mov | Stack, 0,
>> @@ -1225,6 +1226,47 @@ static int emulate_pop_sreg(struct x86_emulate_ctxt 
>> *ctxt,
>>       return rc;
>>  }
>>
>> +static void emulate_pusha(struct x86_emulate_ctxt *ctxt)
>> +{
>> +     struct decode_cache *c = &ctxt->decode;
>> +     unsigned long old_esp = c->regs[VCPU_REGS_RSP];
>> +     int reg = VCPU_REGS_RAX;
>> +
>> +     while (reg <= VCPU_REGS_RDI) {
>> +             (reg == VCPU_REGS_RSP) ?
>> +             (c->src.val = old_esp) : (c->src.val = c->regs[reg]);
>> +
>> +             emulate_push(ctxt);
>> +             ++reg;
>> +     }
>> +}
>> +
>> +static int emulate_popa(struct x86_emulate_ctxt *ctxt,
>> +                     struct x86_emulate_ops *ops)
>> +{
>> +     struct decode_cache *c = &ctxt->decode;
>> +     unsigned long old_esp = c->regs[VCPU_REGS_RSP];
>> +     int rc = 0;
>> +     int reg = VCPU_REGS_RDI;
>> +
>> +     while (reg >= VCPU_REGS_RAX) {
>> +             if (reg == VCPU_REGS_RSP) {
>> +                     register_address_increment(c, &c->regs[VCPU_REGS_RSP],
>> +                                                     c->op_bytes);
>> +                     --reg;
>> +             }
>> +
>> +             rc = emulate_pop(ctxt, ops, &c->regs[reg], c->op_bytes);
>> +             if (rc != 0) {
>> +                     c->regs[VCPU_REGS_RSP] = old_esp;
>
> Why is it necessary to restore the local VCPU_REGS_RSP copy on failure?

Right. Perhaps this is a little too paranoid since we'll fail anyway
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv3] VMX: Enhance invalid guest state emulation

2009-09-01 Thread Mohammed Gamal
On Tue, Sep 1, 2009 at 2:18 PM, Marcelo Tosatti wrote:
> On Tue, Sep 01, 2009 at 02:14:17PM +0200, Mohammed Gamal wrote:
>> On Tue, Sep 1, 2009 at 1:48 PM, Marcelo Tosatti wrote:
>> > On Tue, Sep 01, 2009 at 12:48:18PM +0200, Mohammed Gamal wrote:
>> >> - Change returned handle_invalid_guest_state() to return relevant exit 
>> >> codes
>> >> - Move triggering the emulation from vmx_vcpu_run() to vmx_handle_exit()
>> >> - Return to userspace instead of repeatedly trying to emulate 
>> >> instructions that have already failed
>> >>
>> >> Signed-off-by: Mohammed Gamal 
>> >
>> > Mohammed,
>> >
>> > The handle_invalid_guest_state loop is potentially problematic. It would
>> > be more appropriate to use the __vcpu_run loop.
>> >
>> > Can't you set vmx->emulation_required depending on the result
>> > of one call to emulate_instruction and get rid of the while
>> > (!guest_state_valid(vcpu)) loop?
>> >
>>
>> Invalid state emulation is VMX-specfic, while the __vcpu_run loop is
>> independent of the virtualization extension (defined in x86.c), no?
>> AMD SVM can comforably run hosts in big-real mode and thus it doesn't
>> have the notion of a guest going to an invalid state because of mode
>> switching, so I don't think it'd be a good idea to move emulation into
>> a generic layer. Please correct me if I am wrong
>
> Right. But all i am asking is to emulate one instruction at a
> time in handle_invalid_guest_state, instead of looping until
> guest_state_valid(vcpu).
>
> So you get rid of schedule(), the check for signal_pending, etc.

But we'll still need to enter the guest when it's in a valid state, so
we need to move that loop somewhere, and now that we still have a loop
we'll also still need to do the pending signals and scheduling checks,
no?

I'd appreciate any suggestions you have to alleviate this.
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] x86 emulator: Add pusha and popa instructions

2009-09-01 Thread Mohammed Gamal
This adds pusha and popa instructions (opcodes 0x60-0x61), this enables booting
MINIX with invalid guest state emulation on.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |   49 +++-
 1 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index db0820d..7ca39f8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -139,7 +139,8 @@ static u32 opcode_table[256] = {
DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
DstReg | Stack, DstReg | Stack, DstReg | Stack, DstReg | Stack,
/* 0x60 - 0x67 */
-   0, 0, 0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
+   ImplicitOps | Stack | No64, ImplicitOps | Stack | No64,
+   0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
0, 0, 0, 0,
/* 0x68 - 0x6F */
SrcImm | Mov | Stack, 0, SrcImmByte | Mov | Stack, 0,
@@ -1225,6 +1226,44 @@ static int emulate_pop_sreg(struct x86_emulate_ctxt 
*ctxt,
return rc;
 }
 
+static void emulate_pusha(struct x86_emulate_ctxt *ctxt)
+{
+   struct decode_cache *c = &ctxt->decode;
+   unsigned long old_esp = c->regs[VCPU_REGS_RSP];
+   int reg = VCPU_REGS_RAX;
+
+   while (reg <= VCPU_REGS_RDI) {
+   (reg == VCPU_REGS_RSP) ? 
+   (c->src.val = old_esp) : (c->src.val = c->regs[reg]);
+
+   emulate_push(ctxt);
+   ++reg;
+   }
+}
+
+static int emulate_popa(struct x86_emulate_ctxt *ctxt,
+   struct x86_emulate_ops *ops)
+{
+   struct decode_cache *c = &ctxt->decode;
+   unsigned long old_esp = c->regs[VCPU_REGS_RSP];
+   int rc = 0;
+   int reg = VCPU_REGS_RDI;
+
+   while (reg >= VCPU_REGS_RAX) {
+   if (reg == VCPU_REGS_RSP) {
+   register_address_increment(c, &c->regs[VCPU_REGS_RSP],
+   c->op_bytes);
+   --reg;
+   }
+
+   rc = emulate_pop(ctxt, ops, &c->regs[reg], c->op_bytes);
+   if (rc != 0)
+   break;
+   --reg;
+   }
+   return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
 {
@@ -1816,6 +1855,14 @@ special_insn:
if (rc != 0)
goto done;
break;
+   case 0x60:  /* pusha */
+   emulate_pusha(ctxt);
+   break;
+   case 0x61:  /* popa */
+   rc = emulate_popa(ctxt, ops);
+   if (rc != 0)
+   goto done;
+   break; 
case 0x63:  /* movsxd */
if (ctxt->mode != X86EMUL_MODE_PROT64)
goto cannot_emulate;
-- 
1.6.0.4

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


Re: [PATCHv3] VMX: Enhance invalid guest state emulation

2009-09-01 Thread Mohammed Gamal
On Tue, Sep 1, 2009 at 3:29 PM, Marcelo Tosatti wrote:
> On Tue, Sep 01, 2009 at 03:08:55PM +0200, Mohammed Gamal wrote:
>> On Tue, Sep 1, 2009 at 2:18 PM, Marcelo Tosatti wrote:
>> > On Tue, Sep 01, 2009 at 02:14:17PM +0200, Mohammed Gamal wrote:
>> >> On Tue, Sep 1, 2009 at 1:48 PM, Marcelo Tosatti 
>> >> wrote:
>> >> > On Tue, Sep 01, 2009 at 12:48:18PM +0200, Mohammed Gamal wrote:
>> >> >> - Change returned handle_invalid_guest_state() to return relevant exit 
>> >> >> codes
>> >> >> - Move triggering the emulation from vmx_vcpu_run() to 
>> >> >> vmx_handle_exit()
>> >> >> - Return to userspace instead of repeatedly trying to emulate 
>> >> >> instructions that have already failed
>> >> >>
>> >> >> Signed-off-by: Mohammed Gamal 
>> >> >
>> >> > Mohammed,
>> >> >
>> >> > The handle_invalid_guest_state loop is potentially problematic. It would
>> >> > be more appropriate to use the __vcpu_run loop.
>> >> >
>> >> > Can't you set vmx->emulation_required depending on the result
>> >> > of one call to emulate_instruction and get rid of the while
>> >> > (!guest_state_valid(vcpu)) loop?
>> >> >
>> >>
>> >> Invalid state emulation is VMX-specfic, while the __vcpu_run loop is
>> >> independent of the virtualization extension (defined in x86.c), no?
>> >> AMD SVM can comforably run hosts in big-real mode and thus it doesn't
>> >> have the notion of a guest going to an invalid state because of mode
>> >> switching, so I don't think it'd be a good idea to move emulation into
>> >> a generic layer. Please correct me if I am wrong
>> >
>> > Right. But all i am asking is to emulate one instruction at a
>> > time in handle_invalid_guest_state, instead of looping until
>> > guest_state_valid(vcpu).
>> >
>> > So you get rid of schedule(), the check for signal_pending, etc.
>>
>> But we'll still need to enter the guest when it's in a valid state, so
>> we need to move that loop somewhere,
>
> Sure, just set vmx->emulation_required = guest_state_valid(vcpu). When
> the state is good, the entry handler will vmentry.
>
>> and now that we still have a loop
>> we'll also still need to do the pending signals and scheduling checks,
>> no?
>
> Point is you can use the __vcpu_run loop.
>
> In the latest patch you do:
>
> +       /* Don't enter VMX if guest state is invalid, let the exit handler
> +          start emulation until we arrive back to a valid state */
> +       if (vmx->emulation_required && emulate_invalid_guest_state)
>                return;
>
> And then emulate in the exit handler.
>
>> I'd appreciate any suggestions you have to alleviate this.
>
> I fail to see why you need an internal loop if you can use the external
> (__vcpu_run) one.

Because it's not just used by VMX. So I don't think it'd be wise to
use it for something that's VMX-specific.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Handle emulation failure in userspace

2009-09-01 Thread Mohammed Gamal
Since we return to userspace from KVM on invalid state emulation failure, let
qemu handle it.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index b59e403..090a3ae 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1029,6 +1029,14 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
 r = kvm_s390_handle_reset(kvm, vcpu, run);
 break;
 #endif
+   case KVM_EXIT_INTERNAL_ERROR:
+   fprintf(stderr, "KVM internal error. Suberror: %d\n",
+   run->internal.suberror);
+   kvm_show_regs(vcpu);
+   if (run->internal.suberror == KVM_INTERNAL_ERROR_EMULATION)
+   fprintf(stderr, "emulation failure, check dmesg for details\n");
+   abort();
+   break;
 default:
 if (kvm_arch_run(vcpu)) {
 fprintf(stderr, "unhandled vm exit: 0x%x\n", run->exit_reason);
-- 
1.6.0.4

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


[PATCH] Add pusha/popa instructions to the realmode test harness

2009-09-01 Thread Mohammed Gamal
This adds tests for pusha/popa to the test harness. Added some new typedefs
as well.

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 0db09b8..20d038e 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -4,6 +4,10 @@ typedef unsigned char u8;
 typedef unsigned short u16;
 typedef unsigned u32;
 typedef unsigned long long u64;
+typedef signed char s8;
+typedef signed short s16;
+typedef signed s32;
+typedef signed long long s64;
 
 void test_function(void);
 
@@ -470,6 +474,7 @@ void test_long_jmp()
 void test_push_pop()
 {
struct regs inregs = { 0 }, outregs;
+
MK_INSN(push32, "mov $0x12345678, %eax\n\t"
"push %eax\n\t"
"pop %ebx\n\t");
@@ -499,6 +504,10 @@ void test_push_pop()
 "mov %fs, %ebx\n\t"
 "pop %fs\n\t"
);
+   MK_INSN(pusha,  "pushaw\n\t");
+   MK_INSN(pushad, "pushal\n\t");
+   MK_INSN(popa, "popaw\n\t");
+   MK_INSN(popad, "popal\n\t");
 
exec_in_big_real_mode(&inregs, &outregs,
  insn_push32,
@@ -539,6 +548,34 @@ void test_push_pop()
 
if (!regs_equal(&inregs, &outregs, R_AX|R_BX) || outregs.ebx != 
outregs.eax)
print_serial("Push/Pop Test 6: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+insn_pusha,
+insn_pusha_end - insn_pusha);
+   if (!regs_equal(&inregs, &outregs, R_SP) 
+   || (s16)outregs.esp != (s16)(inregs.esp - 16))
+   print_serial("Push/Pop Test 7: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popa,
+ insn_popa_end - insn_popa);
+   if (outregs.esp != (inregs.esp + 16))
+   print_serial("Push/Pop Test 8: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_pushad,
+ insn_pushad_end - insn_pushad);
+
+   if (!regs_equal(&inregs, &outregs, R_SP)
+   || (s16)outregs.esp != (s16)(inregs.esp - 32))
+   print_serial("Push/Pop Test 9: FAIL\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_popad,
+ insn_popad_end - insn_popad);
+
+   if (outregs.esp != (inregs.esp + 32))
+   print_serial("Push/Pop Test 10: FAIL\n");
 }
 
 void test_null(void)
-- 
1.6.0.4

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


[PATCH] Show registers and exit on unhandled errors

2009-09-21 Thread Mohammed Gamal
Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 0afdb56..c22c28a 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1015,6 +1015,8 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
 switch (run->exit_reason) {
 case KVM_EXIT_UNKNOWN:
 r = handle_unhandled(run->hw.hardware_exit_reason);
+kvm_show_regs(vcpu);
+abort();
 break;
 case KVM_EXIT_FAIL_ENTRY:
 r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
-- 
1.6.0.4

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


Re: [PATCH] Show registers and exit on unhandled errors

2009-09-21 Thread Mohammed Gamal
On Mon, Sep 21, 2009 at 1:29 PM, Jan Kiszka  wrote:
> Mohammed Gamal wrote:
>> Signed-off-by: Mohammed Gamal 
>> ---
>>  qemu-kvm.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index 0afdb56..c22c28a 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -1015,6 +1015,8 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
>>          switch (run->exit_reason) {
>>          case KVM_EXIT_UNKNOWN:
>>              r = handle_unhandled(run->hw.hardware_exit_reason);
>> +            kvm_show_regs(vcpu);
>> +            abort();
>>              break;
>>          case KVM_EXIT_FAIL_ENTRY:
>>              r = 
>> handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
>
> Don't we currently suspend the VM on unknown exits? This is more useful
> than aborting as it allows to
>  - disassemble the problematic code
>  - poke around in the RAM
>  - look at other VCPUs
>  - attach a debugger to qemu
>  - ...
>

Good point. But at least we can still show registers, since that also
can give some diagnostic information, no?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Show registers and exit on unhandled errors

2009-09-21 Thread Mohammed Gamal
>> Good point. But at least we can still show registers, since that also
>> can give some diagnostic information, no?
>
> No fundamental concerns. Just move the call into handle_unhandled.
>
handle_unhandled() doesn't get the vcpu passed to it, so it'd be
better we keep kvm_show_regs() at the caller.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Show registers and exit on unhandled errors

2009-09-21 Thread Mohammed Gamal
On Mon, Sep 21, 2009 at 1:42 PM, Jan Kiszka  wrote:
> Mohammed Gamal wrote:
>> On Mon, Sep 21, 2009 at 1:29 PM, Jan Kiszka  wrote:
>>> Mohammed Gamal wrote:
>>>> Signed-off-by: Mohammed Gamal 
>>>> ---
>>>>  qemu-kvm.c |    2 ++
>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>>>> index 0afdb56..c22c28a 100644
>>>> --- a/qemu-kvm.c
>>>> +++ b/qemu-kvm.c
>>>> @@ -1015,6 +1015,8 @@ int kvm_run(kvm_vcpu_context_t vcpu, void *env)
>>>>          switch (run->exit_reason) {
>>>>          case KVM_EXIT_UNKNOWN:
>>>>              r = handle_unhandled(run->hw.hardware_exit_reason);
>>>> +            kvm_show_regs(vcpu);
>>>> +            abort();
>>>>              break;
>>>>          case KVM_EXIT_FAIL_ENTRY:
>>>>              r = 
>>>> handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
>>> Don't we currently suspend the VM on unknown exits? This is more useful
>>> than aborting as it allows to
>>>  - disassemble the problematic code
>>>  - poke around in the RAM
>>>  - look at other VCPUs
>>>  - attach a debugger to qemu
>>>  - ...
>>>
>>
>> Good point. But at least we can still show registers, since that also
>> can give some diagnostic information, no?
>
> No fundamental concerns. Just move the call into handle_unhandled.
>
> And maybe some note like "kvm_run returned XX - VM stopped" in
> kvm_cpu_exec() would clarify the situation a bit more.
>
> Jan

That's already the case if we don't exit.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Add GDT, IDT and RFLAGS guest state validity checks

2009-09-21 Thread Mohammed Gamal
With emulate_invalid_guest_state=1 Windows XP exits with an invalid guest state
due to rflags not being in a VMX-compliant state. This patch fixes this issue,
although Windows XP doesn't boot yet with invalid state emulation on.

Also added GDT and IDT checks while we're at it.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   57 +++-
 1 files changed, 56 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 3fe0d42..eaec4a5 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2022,6 +2022,23 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
return true;
 }
 
+static bool gdtr_idtr_valid(struct kvm_vcpu *vcpu)
+{
+   struct descriptor_table gdt;
+   struct descriptor_table idt;
+
+   vmx_get_gdt(vcpu, &gdt);
+   vmx_get_idt(vcpu, &idt);
+
+   if (gdt.limit & 0x)
+   return false;
+
+   if (idt.limit & 0x)
+   return false;
+
+   return true;
+}
+
 static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
 {
struct kvm_segment cs, ss;
@@ -2033,6 +2050,41 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
 (ss.selector & SELECTOR_RPL_MASK));
 }
 
+static bool rflags_valid(struct kvm_vcpu *vcpu)
+{
+   unsigned long rflags;
+   u32 entry_intr_info;
+
+   rflags = vmcs_readl(GUEST_RFLAGS);
+   entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
+#ifdef CONFIG_X86_64
+   if (rflags & 0xffc0)
+   return false;
+   if (is_long_mode(vcpu))
+   if (rflags & X86_EFLAGS_VM)
+   return false;   
+#else
+   if (rflags & 0xffc0)
+   return false;
+#endif
+   if (rflags & 0x8000)
+   return false;
+   if (rflags & 0x20)
+   return false;
+   if (rflags & 0x8)
+   return false;
+   if (!(rflags & 0x2))
+   return false;
+
+   if ((entry_intr_info & INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_EXT_INTR 
+   && (entry_intr_info & INTR_INFO_VALID_MASK)) {
+   if (!(rflags & X86_EFLAGS_IF))
+   return false;
+   }
+
+   return true;
+}
+
 /*
  * Check if guest state is valid. Returns true if valid, false if
  * not.
@@ -2077,8 +2129,11 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
}
/* TODO:
 * - Add checks on RIP
-* - Add checks on RFLAGS
 */
+   if (!rflags_valid(vcpu))
+   return false;
+   if (!gdtr_idtr_valid(vcpu))
+   return false;
 
return true;
 }
-- 
1.6.0.4

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


Re: [PATCH] Add GDT, IDT and RFLAGS guest state validity checks

2009-09-22 Thread Mohammed Gamal
On Tue, Sep 22, 2009 at 9:13 AM, Avi Kivity  wrote:
> On 09/21/2009 06:33 PM, Mohammed Gamal wrote:
>>
>> With emulate_invalid_guest_state=1 Windows XP exits with an invalid guest
>> state
>> due to rflags not being in a VMX-compliant state. This patch fixes this
>> issue,
>> although Windows XP doesn't boot yet with invalid state emulation on.
>>
>> Also added GDT and IDT checks while we're at it.
>>
>> Signed-off-by: Mohammed Gamal
>> ---
>>  arch/x86/kvm/vmx.c |   57
>> +++-
>>  1 files changed, 56 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 3fe0d42..eaec4a5 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2022,6 +2022,23 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
>>        return true;
>>  }
>>
>> +static bool gdtr_idtr_valid(struct kvm_vcpu *vcpu)
>> +{
>> +       struct descriptor_table gdt;
>> +       struct descriptor_table idt;
>> +
>> +       vmx_get_gdt(vcpu,&gdt);
>> +       vmx_get_idt(vcpu,&idt);
>> +
>> +       if (gdt.limit&  0x)
>> +               return false;
>> +
>> +       if (idt.limit&  0x)
>> +               return false;
>> +
>> +       return true;
>> +}
>>
>
> gdt and idt limits cannot be > 0x, since the intstructions to load them
> always use a 16 bit quantity.
>
>> +
>>  static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
>>  {
>>        struct kvm_segment cs, ss;
>> @@ -2033,6 +2050,41 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
>>                 (ss.selector&  SELECTOR_RPL_MASK));
>>  }
>>
>> +static bool rflags_valid(struct kvm_vcpu *vcpu)
>> +{
>> +       unsigned long rflags;
>> +       u32 entry_intr_info;
>> +
>> +       rflags = vmcs_readl(GUEST_RFLAGS);
>> +       entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
>> +#ifdef CONFIG_X86_64
>> +       if (rflags&  0xffc0)
>> +               return false;
>> +       if (is_long_mode(vcpu))
>> +               if (rflags&  X86_EFLAGS_VM)
>> +                       return false;
>> +#else
>> +       if (rflags&  0xffc0)
>> +               return false;
>> +#endif
>> +       if (rflags&  0x8000)
>> +               return false;
>> +       if (rflags&  0x20)
>> +               return false;
>> +       if (rflags&  0x8)
>> +               return false;
>> +       if (!(rflags&  0x2))
>> +               return false;
>> +
>> +       if ((entry_intr_info&  INTR_INFO_INTR_TYPE_MASK) ==
>> INTR_TYPE_EXT_INTR
>> +               &&  (entry_intr_info&  INTR_INFO_VALID_MASK)) {
>> +               if (!(rflags&  X86_EFLAGS_IF))
>> +                       return false;
>> +       }
>> +
>> +       return true;
>> +}
>>
>
> It's really difficult to tell what you are doing here since you're using
> numbers instead of symbolic constants.  But I think some of these are
> generally illegal.  !guest_state_valid() means "the state is valid for x86
> but not valid for vmx entry"; if it's generally invalid all bets are off.
>

The checks are based on those mentioned in the Intel Software
Developer Manual Vol. 2 Section 22.3.1.3 and 22.3.1.4
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-19 Thread Mohammed Gamal
On Tue, Apr 20, 2010 at 12:54 AM, jvrao  wrote:
> Mohammed Gamal wrote:
>> On Tue, Apr 13, 2010 at 9:08 PM, jvrao  wrote:
>>> jvrao wrote:
>>>> Alexander Graf wrote:
>>>>> On 12.04.2010, at 13:58, Jamie Lokier wrote:
>>>>>
>>>>>> Mohammed Gamal wrote:
>>>>>>> On Mon, Apr 12, 2010 at 12:29 AM, Jamie Lokier  
>>>>>>> wrote:
>>>>>>>> Javier Guerra Giraldez wrote:
>>>>>>>>> On Sat, Apr 10, 2010 at 7:42 AM, Mohammed Gamal 
>>>>>>>>>  wrote:
>>>>>>>>>> On Sat, Apr 10, 2010 at 2:12 PM, Jamie Lokier  
>>>>>>>>>> wrote:
>>>>>>>>>>> To throw a spanner in, the most widely supported filesystem across
>>>>>>>>>>> operating systems is probably NFS, version 2 :-)
>>>>>>>>>> Remember that Windows usage on a VM is not some rare use case, and
>>>>>>>>>> it'd be a little bit of a pain from a user's perspective to have to
>>>>>>>>>> install a third party NFS client for every VM they use. Having
>>>>>>>>>> something supported on the VM out of the box is a better option IMO.
>>>>>>>>> i don't think virtio-CIFS has any more support out of the box (on any
>>>>>>>>> system) than virtio-9P.
>>>>>>>> It doesn't, but at least network-CIFS tends to work ok and is the
>>>>>>>> method of choice for Windows VMs - when you can setup Samba on the
>>>>>>>> host (which as previously noted you cannot always do non-disruptively
>>>>>>>> with current Sambas).
>>>>>>>>
>>>>>>>> -- Jamie
>>>>>>>>
>>>>>>> I think having support for both 9p and CIFS would be the best option.
>>>>>>> In that case the user will have the option to use either one,
>>>>>>> depending on how their guests support these filesystems. In that case
>>>>>>> I'd prefer to work on CIFS support while the 9p effort can still go
>>>>>>> on. I don't think both efforts are mutually exclusive.
>>>>>>>
>>>>>>> What do the rest of you guys think?
>>>>>> I only noted NFS because most old OSes do not support CIFS or 9P -
>>>>>> especially all the old unixes.
>>>>>>
>>>>>> I don't think old versions of MS-DOS and Windows (95, 98, ME, Nt4?)
>>>>>> even support current CIFS.  They need extra server settings to work
>>>>>> - such as setting passwords on the server to non-encrypted and other 
>>>>>> quirks.
>>>>>>
>>>>>> Meanwhile Windows Vista/2008/7 works better with SMB2, not CIFS, to
>>>>>> properly see symlinks and hard links.
>>>>>>
>>>>>> So there is no really nice out of the box file service which works
>>>>>> easily with all guest OSes.
>>>>>>
>>>>>> I'm guessing that out of all the filesystems, CIFS is the most widely
>>>>>> supported in recent OSes (released in the last 10 years).  But I'm not
>>>>>> really sure what the state of CIFS is for non-Windows, non-Linux,
>>>>>> non-BSD guests.
>>>>> So what? If you want to have direct host fs access, install guest 
>>>>> drivers. If you can't, set up networking and use CIFS or NFS or whatever.
>>>>>
>>>>>> I'm not sure why 9P is being pursued.  Does anything much support it,
>>>>>> or do all OSes except quite recent Linux need a custom driver for 9P?
>>>>>> Even Linux only got the first commit in the kernel 5 years ago, so
>>>>>> probably it was only about 3 years ago that it will have begun
>>>>>> appearing in stable distros, if at all.  Filesystem passthrough to
>>>>>> Linux guests installed in the last couple of years is a useful
>>>>>> feature, and I know that for many people that is their only use of
>>>>>> KVM, but compared with CIFS' broad support it seems like quite a
>>>>>> narrow goal.
>>>>> The goal is to have something simple and fast. We can fine-tune 9P to 
>>>>> align with the Linux VFS structures, making it really 

Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-20 Thread Mohammed Gamal
On Tue, Apr 20, 2010 at 8:36 PM, jvrao  wrote:

...  ...

>> This'd be something interesting to do. I wonder if that would fit in
>> the GSoC timeframe, or whether it'd be a little too short. So how long
>> you'd estimate something like that would take?
>
> I think it would take ~3PM for someone with decent VFS/NFS knowledge.
> They key is fh-to-dentry mapping. In the loose cache mode client caches
> this information .. but even in this mode we can't assume that it will be 
> cached
> forever. Need protocol amendments, client/server side changes to implement
> this in the no-cache mode which can be used even in the loose cache mode when
> we get a cache-miss.
>
> Thanks,
> JV

I think I'd be glad to go for virtio-9p in GSoC. The roadmap is a
little bit hazy for me at the moment but I think we can set the goals.
I'd appreciate some pointers as to where to get more info on what to
do and if there is any relevant documentation on that matter.

Regards,
Mohammed
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: OPCODE Emulation

2010-05-06 Thread Mohammed Gamal
On Thu, May 6, 2010 at 11:37 PM, Matteo Signorini
 wrote:
>
> Dear Yaniv, Dear Avi,
>
> I would like to add the "sidt emulation" feature in kvm, but in order to
> implement it I need to know the details on how the OPCODE works and how 
> exactly opcodes are emulated within kvm.
> For example let's take the SIDT instruction.
> I know the LIDT opcode is "0F 01 /1" but what does 0F, 01 and /1 mean?
> I also know that this instruction has only the operand "ModRM:r/m (w)"
> but where is this operand stored and how can I access it in emulation?
>  Could you please suggest to me where can I found some detailed docs on the 
> subject?
> (I have already read the Intel Volume 2B Instruction Set Reference N-Z pag. 
> 4-440 but I have not found enough detailed information)
>
> Thank you
>
> Matteo Signorini

Hi Matteo,
arch/x86/kvm/emulate.c is the best place to start. All you need to
look at is there.

Regards,
Mohammed
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] VMX: Invalid guest state detection enhancements and bug fixes

2010-05-09 Thread Mohammed Gamal
- Correct unusable flag check on SS, DS, ES, FS, GS, and LDTR
- Add IDTR and GDTR checks
- Add rflags checks

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   64 ---
 1 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 777e00d..f736008 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2122,7 +2122,7 @@ static bool stack_segment_valid(struct kvm_vcpu *vcpu)
ss_rpl = ss.selector & SELECTOR_RPL_MASK;
 
if (ss.unusable)
-   return true;
+   return false;
if (ss.type != 3 && ss.type != 7)
return false;
if (!ss.s)
@@ -2144,7 +2144,7 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, int 
seg)
rpl = var.selector & SELECTOR_RPL_MASK;
 
if (var.unusable)
-   return true;
+   return false;
if (!var.s)
return false;
if (!var.present)
@@ -2185,7 +2185,7 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
vmx_get_segment(vcpu, &ldtr, VCPU_SREG_LDTR);
 
if (ldtr.unusable)
-   return true;
+   return false;
if (ldtr.selector & SELECTOR_TI_MASK)   /* TI = 1 */
return false;
if (ldtr.type != 2)
@@ -2196,6 +2196,23 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
return true;
 }
 
+static bool gdtr_idtr_valid(struct kvm_vcpu *vcpu)
+{
+   struct desc_ptr gdt;
+   struct desc_ptr idt;
+
+   vmx_get_gdt(vcpu, &gdt);
+   vmx_get_idt(vcpu, &idt);
+
+   if (gdt.size & 0x)
+   return false;
+
+   if (idt.size & 0x)
+   return false;
+
+   return true;
+}
+
 static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
 {
struct kvm_segment cs, ss;
@@ -2207,6 +2224,41 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
 (ss.selector & SELECTOR_RPL_MASK));
 }
 
+static bool rflags_valid(struct kvm_vcpu *vcpu)
+{
+   unsigned long rflags;
+   u32 entry_intr_info;
+
+   rflags = vmcs_readl(GUEST_RFLAGS);
+   entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
+#ifdef CONFIG_X86_64
+   if (rflags & 0xffc0)
+   return false;
+   if (is_long_mode(vcpu))
+   if (rflags & X86_EFLAGS_VM)
+   return false;   
+#else
+   if (rflags & 0xffc0)
+   return false;
+#endif
+   if (rflags & 0x8000)
+   return false;
+   if (rflags & 0x20)
+   return false;
+   if (rflags & 0x8)
+   return false;
+   if (!(rflags & 0x2))
+   return false;
+
+   if ((entry_intr_info & INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_EXT_INTR 
+   && (entry_intr_info & INTR_INFO_VALID_MASK)) {
+   if (!(rflags & X86_EFLAGS_IF))
+   return false;
+   }
+
+   return true;
+}
+
 /*
  * Check if guest state is valid. Returns true if valid, false if
  * not.
@@ -2251,8 +2303,11 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
}
/* TODO:
 * - Add checks on RIP
-* - Add checks on RFLAGS
 */
+   if (!rflags_valid(vcpu))
+   return false;
+   if (!gdtr_idtr_valid(vcpu))
+   return false;
 
return true;
 }
@@ -3559,6 +3614,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
}
 
if (err != EMULATE_DONE) {
+   kvm_report_emulation_failure(vcpu, "invalid guest state 
handler");
vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
vcpu->run->internal.ndata = 0;
-- 
1.7.0.4

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


[PATCH] VMX: Invalid guest state detection enhancements and bug fixes

2010-05-10 Thread Mohammed Gamal
- Correct unusable flag check on SS, DS, ES, FS, GS, and LDTR
- Add rflags checks
- Report failed instruction on emulation failure

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   31 +++
 1 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 777e00d..968384b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2122,7 +2122,7 @@ static bool stack_segment_valid(struct kvm_vcpu *vcpu)
ss_rpl = ss.selector & SELECTOR_RPL_MASK;
 
if (ss.unusable)
-   return true;
+   return false;
if (ss.type != 3 && ss.type != 7)
return false;
if (!ss.s)
@@ -2144,7 +2144,7 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, int 
seg)
rpl = var.selector & SELECTOR_RPL_MASK;
 
if (var.unusable)
-   return true;
+   return false;
if (!var.s)
return false;
if (!var.present)
@@ -2185,7 +2185,7 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
vmx_get_segment(vcpu, &ldtr, VCPU_SREG_LDTR);
 
if (ldtr.unusable)
-   return true;
+   return false;
if (ldtr.selector & SELECTOR_TI_MASK)   /* TI = 1 */
return false;
if (ldtr.type != 2)
@@ -2207,6 +2207,27 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
 (ss.selector & SELECTOR_RPL_MASK));
 }
 
+static bool rflags_valid(struct kvm_vcpu *vcpu)
+{
+   unsigned long rflags;
+   u32 entry_intr_info;
+
+   rflags = vmcs_readl(GUEST_RFLAGS);
+   entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
+#ifdef CONFIG_X86_64
+   if (is_long_mode(vcpu))
+   if (rflags & X86_EFLAGS_VM)
+   return false;   
+#endif
+   if ((entry_intr_info & INTR_INFO_INTR_TYPE_MASK) == INTR_TYPE_EXT_INTR 
+   && (entry_intr_info & INTR_INFO_VALID_MASK)) {
+   if (!(rflags & X86_EFLAGS_IF))
+   return false;
+   }
+
+   return true;
+}
+
 /*
  * Check if guest state is valid. Returns true if valid, false if
  * not.
@@ -2251,8 +2272,9 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
}
/* TODO:
 * - Add checks on RIP
-* - Add checks on RFLAGS
 */
+   if (!rflags_valid(vcpu))
+   return false;
 
return true;
 }
@@ -3559,6 +3581,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu 
*vcpu)
}
 
if (err != EMULATE_DONE) {
+   kvm_report_emulation_failure(vcpu, "invalid guest state 
handler");
vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
vcpu->run->internal.suberror = 
KVM_INTERNAL_ERROR_EMULATION;
vcpu->run->internal.ndata = 0;
-- 
1.7.0.4

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


Re: [PATCHv2] KVM: inject #UD if instruction emulation fails and exit to userspace

2010-05-10 Thread Mohammed Gamal
On Mon, May 10, 2010 at 1:25 PM, Gleb Natapov  wrote:
> On Mon, May 10, 2010 at 11:16:56AM +0300, Gleb Natapov wrote:
>> Do not kill VM when instruction emulation fails. Inject #UD and report
>> failure to userspace instead. Userspace may choose to reenter guest if
>> vcpu is in userspace (cpl == 3) in which case guest OS will kill
>> offending process and continue running.
>>

I am curious to know what'd happen in case the vcpu is in kernel space
(cpl == 0). Is that case handled?

> Please use this one instead. Compilation warning fixed.
>
> Signed-off-by: Gleb Natapov 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ed48904..5aa0944 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -575,7 +575,6 @@ enum emulation_result {
>  #define EMULTYPE_SKIP              (1 << 2)
>  int emulate_instruction(struct kvm_vcpu *vcpu,
>                        unsigned long cr2, u16 error_code, int emulation_type);
> -void kvm_report_emulation_failure(struct kvm_vcpu *cvpu, const char 
> *context);
>  void realmode_lgdt(struct kvm_vcpu *vcpu, u16 size, unsigned long address);
>  void realmode_lidt(struct kvm_vcpu *vcpu, u16 size, unsigned long address);
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 95bee9d..41d2de8 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2788,11 +2788,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t 
> cr2, u32 error_code)
>                return 1;
>        case EMULATE_DO_MMIO:
>                ++vcpu->stat.mmio_exits;
> -               return 0;
> +               /* fall through */
>        case EMULATE_FAIL:
> -               vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> -               vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
> -               vcpu->run->internal.ndata = 0;
>                return 0;
>        default:
>                BUG();
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index cea916f..58c91f5 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1449,7 +1449,7 @@ static int io_interception(struct vcpu_svm *svm)
>        string = (io_info & SVM_IOIO_STR_MASK) != 0;
>        in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
>        if (string || in)
> -               return !(emulate_instruction(vcpu, 0, 0, 0) == 
> EMULATE_DO_MMIO);
> +               return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
>
>        port = io_info >> 16;
>        size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT;
> @@ -2300,16 +2300,12 @@ static int iret_interception(struct vcpu_svm *svm)
>
>  static int invlpg_interception(struct vcpu_svm *svm)
>  {
> -       if (emulate_instruction(&svm->vcpu, 0, 0, 0) != EMULATE_DONE)
> -               pr_unimpl(&svm->vcpu, "%s: failed\n", __func__);
> -       return 1;
> +       return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE;
>  }
>
>  static int emulate_on_interception(struct vcpu_svm *svm)
>  {
> -       if (emulate_instruction(&svm->vcpu, 0, 0, 0) != EMULATE_DONE)
> -               pr_unimpl(&svm->vcpu, "%s: failed\n", __func__);
> -       return 1;
> +       return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE;
>  }
>
>  static int cr8_write_interception(struct vcpu_svm *svm)
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 777e00d..e35c479 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3074,7 +3074,7 @@ static int handle_io(struct kvm_vcpu *vcpu)
>        ++vcpu->stat.io_exits;
>
>        if (string || in)
> -               return !(emulate_instruction(vcpu, 0, 0, 0) == 
> EMULATE_DO_MMIO);
> +               return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
>
>        port = exit_qualification >> 16;
>        size = (exit_qualification & 7) + 1;
> @@ -3331,22 +3331,7 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu)
>
>  static int handle_apic_access(struct kvm_vcpu *vcpu)
>  {
> -       unsigned long exit_qualification;
> -       enum emulation_result er;
> -       unsigned long offset;
> -
> -       exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> -       offset = exit_qualification & 0xffful;
> -
> -       er = emulate_instruction(vcpu, 0, 0, 0);
> -
> -       if (er !=  EMULATE_DONE) {
> -               printk(KERN_ERR
> -                      "Fail to handle apic access vmexit! Offset is 0x%lx\n",
> -                      offset);
> -               return -ENOEXEC;
> -       }
> -       return 1;
> +       return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
>  }
>
>  static int handle_task_switch(struct kvm_vcpu *vcpu)
> @@ -3558,13 +3543,8 @@ static int handle_invalid_guest_state(struct kvm_vcpu 
> *vcpu)
>                        goto out;
>                }
>
> -               if (err != EMULATE_DONE) {
> -                       vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> -                       vcpu->run->internal.suberror = 
> KVM_INTERNAL_ERROR_EMULATION;
> -          

Re: [PATCH] VMX: Invalid guest state detection enhancements and bug fixes

2010-05-11 Thread Mohammed Gamal
On Tue, May 11, 2010 at 12:24 PM, Avi Kivity  wrote:
> On 05/10/2010 06:51 PM, Mohammed Gamal wrote:
>>
>> - Correct unusable flag check on SS, DS, ES, FS, GS, and LDTR
>> - Add rflags checks
>> - Report failed instruction on emulation failure
>>
>>
>
> Please post as separate patches.
>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 777e00d..968384b 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2122,7 +2122,7 @@ static bool stack_segment_valid(struct kvm_vcpu
>> *vcpu)
>>        ss_rpl = ss.selector&  SELECTOR_RPL_MASK;
>>
>>        if (ss.unusable)
>> -               return true;
>> +               return false;
>>
>
> Where does it say that ss must be usable?

Oh, I must have misread it. The unusable bit check is for virtual 8086
mode. However, with this check returning true when the segment is
unusable we still return prematurely as we don't do the other checks.

>
>>        if (ss.type != 3&&  ss.type != 7)
>>                return false;
>>        if (!ss.s)
>> @@ -2144,7 +2144,7 @@ static bool data_segment_valid(struct kvm_vcpu
>> *vcpu, int seg)
>>        rpl = var.selector&  SELECTOR_RPL_MASK;
>>
>>        if (var.unusable)
>> -               return true;
>> +               return false;
>>        if (!var.s)
>>                return false;
>>        if (!var.present)
>>
>
> Ditto.
>
>> @@ -2185,7 +2185,7 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
>>        vmx_get_segment(vcpu,&ldtr, VCPU_SREG_LDTR);
>>
>>        if (ldtr.unusable)
>> -               return true;
>> +               return false;
>>        if (ldtr.selector&  SELECTOR_TI_MASK)   /* TI = 1 */
>>                return false;
>>        if (ldtr.type != 2)
>>
>
> Ditto.
>
>> @@ -2207,6 +2207,27 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
>>                 (ss.selector&  SELECTOR_RPL_MASK));
>>  }
>>
>> +static bool rflags_valid(struct kvm_vcpu *vcpu)
>> +{
>> +       unsigned long rflags;
>> +       u32 entry_intr_info;
>> +
>> +       rflags = vmcs_readl(GUEST_RFLAGS);
>> +       entry_intr_info = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
>> +#ifdef CONFIG_X86_64
>> +       if (is_long_mode(vcpu))
>> +               if (rflags&  X86_EFLAGS_VM)
>> +                       return false;
>> +#endif
>>
>
> This is architecturally illegal, not just for vmx entries.  The check should
> be made when emulating setting rflags and in KVM_SET_REGS.
>
> These checks should assume the state is architecturally legal, and only
> check if they are legal for vmx entries.
>

>> +       if ((entry_intr_info&  INTR_INFO_INTR_TYPE_MASK) ==
>> INTR_TYPE_EXT_INTR
>> +               &&  (entry_intr_info&  INTR_INFO_VALID_MASK)) {
>> +               if (!(rflags&  X86_EFLAGS_IF))
>> +                       return false;
>> +       }
>> +
>>
>
> Ditto.
>
>> @@ -3559,6 +3581,7 @@ static int handle_invalid_guest_state(struct
>> kvm_vcpu *vcpu)
>>                }
>>
>>                if (err != EMULATE_DONE) {
>> +                       kvm_report_emulation_failure(vcpu, "invalid guest
>> state handler");
>>                        vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
>>                        vcpu->run->internal.suberror =
>> KVM_INTERNAL_ERROR_EMULATION;
>>                        vcpu->run->internal.ndata = 0;
>>
>
> Uneeded, userspace can report it.

Userspace does report the address at which a VM fails, it doesn't for
instance show the contents of RIP which is necessary for knowing which
instruction failed, this is at least needed for testing purposes. IIRC
I've seen some trace_kvm_failed_insn() functions somewhere. But I
don't know how to use tracing utilities so I'd be delighted if someone
could point me to some relevant documentation or something.

>
> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] VMX: Fix and improve guest state validity checks

2010-05-11 Thread Mohammed Gamal
- Add 's' and 'g' field checks on segment registers
- Correct SS checks for request and descriptor privilege levels

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |   73 +++
 1 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 777e00d..9805c2a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2121,16 +2121,30 @@ static bool stack_segment_valid(struct kvm_vcpu *vcpu)
vmx_get_segment(vcpu, &ss, VCPU_SREG_SS);
ss_rpl = ss.selector & SELECTOR_RPL_MASK;
 
-   if (ss.unusable)
+   if (ss.dpl != ss_rpl) /* DPL != RPL */
+   return false;
+
+   if (ss.unusable) /* Short-circuit */
return true;
+
if (ss.type != 3 && ss.type != 7)
return false;
if (!ss.s)
return false;
-   if (ss.dpl != ss_rpl) /* DPL != RPL */
-   return false;
if (!ss.present)
return false;
+   if (ss.limit & 0xfff0) {
+if ((ss.limit & 0xfff) < 0xfff)
+return false;
+if (!ss.g)
+return false;
+} else {
+if ((ss.limit & 0xfff) == 0xfff)
+return false;
+if (ss.g)
+return false;
+}
+
 
return true;
 }
@@ -2143,8 +2157,15 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, 
int seg)
vmx_get_segment(vcpu, &var, seg);
rpl = var.selector & SELECTOR_RPL_MASK;
 
-   if (var.unusable)
+   if (var.unusable)  /* Short-circuit */
return true;
+   if (!(var.type & AR_TYPE_ACCESSES_MASK))
+   return false;
+   if (var.type & AR_TYPE_CODE_MASK) {
+   if (!(var.type & AR_TYPE_READABLE_MASK))
+   return false;
+   }
+
if (!var.s)
return false;
if (!var.present)
@@ -2154,6 +2175,18 @@ static bool data_segment_valid(struct kvm_vcpu *vcpu, 
int seg)
return false;
}
 
+   if (var.limit & 0xfff0) {
+   if ((var.limit & 0xfff) < 0xfff)
+   return false;
+   if (!var.g)
+   return false;
+   } else {
+   if ((var.limit & 0xfff) == 0xfff)
+   return false;
+   if (var.g)
+   return false;
+   }
+
/* TODO: Add other members to kvm_segment_field to allow checking for 
other access
 * rights flags
 */
@@ -2172,6 +2205,21 @@ static bool tr_valid(struct kvm_vcpu *vcpu)
return false;
if (tr.type != 3 && tr.type != 11) /* TODO: Check if guest is in IA32e 
mode */
return false;
+   if (tr.s)
+   return false;
+   if (tr.limit & 0xfff0) {
+if ((tr.limit & 0xfff) < 0xfff)
+return false;
+if (!tr.g)
+return false;
+} else {
+if ((tr.limit & 0xfff) == 0xfff)
+return false;
+if (tr.g)
+return false;
+}
+
+
if (!tr.present)
return false;
 
@@ -2184,7 +2232,7 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
 
vmx_get_segment(vcpu, &ldtr, VCPU_SREG_LDTR);
 
-   if (ldtr.unusable)
+   if (ldtr.unusable)  /* Short-circuit */
return true;
if (ldtr.selector & SELECTOR_TI_MASK)   /* TI = 1 */
return false;
@@ -2192,6 +2240,20 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
return false;
if (!ldtr.present)
return false;
+   if (ldtr.s)
+   return false;
+   if (ldtr.limit & 0xfff0) {
+if ((ldtr.limit & 0xfff) < 0xfff)
+return false;
+if (!ldtr.g)
+return false;
+} else {
+if ((ldtr.limit & 0xfff) == 0xfff)
+return false;
+if (ldtr.g)
+return false;
+}
+
 
return true;
 }
@@ -2251,7 +2313,6 @@ static bool guest_state_valid(struct kvm_vcpu *vcpu)
}
/* TODO:
 * - Add checks on RIP
-* - Add checks on RFLAGS
 */
 
return true;
-- 
1.7.0.4

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


[PATCH] x86 emulator: Add test acc, imm instruction (opcodes 0xA8 - 0xA9)

2010-05-11 Thread Mohammed Gamal
This adds test acc, imm instruction to the x86 emulator

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index b43ac98..be5e78d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -181,7 +181,7 @@ static u32 opcode_table[256] = {
ByteOp | SrcSI | DstDI | Mov | String, SrcSI | DstDI | Mov | String,
ByteOp | SrcSI | DstDI | String, SrcSI | DstDI | String,
/* 0xA8 - 0xAF */
-   0, 0, ByteOp | DstDI | Mov | String, DstDI | Mov | String,
+   DstAcc | SrcImmByte | ByteOp, DstAcc | SrcImm, ByteOp | DstDI | Mov | 
String, DstDI | Mov | String,
ByteOp | SrcSI | DstAcc | Mov | String, SrcSI | DstAcc | Mov | String,
ByteOp | DstDI | String, DstDI | String,
/* 0xB0 - 0xB7 */
@@ -2754,6 +2754,7 @@ special_insn:
}
break;
case 0x84 ... 0x85:
+   test:
emulate_2op_SrcV("test", c->src, c->dst, ctxt->eflags);
break;
case 0x86 ... 0x87: /* xchg */
@@ -2852,6 +2853,8 @@ special_insn:
c->dst.type = OP_NONE; /* Disable writeback. */
DPRINTF("cmps: mem1=0x%p mem2=0x%p\n", c->src.ptr, c->dst.ptr);
goto cmp;
+   case 0xa8 ... 0xa9: /* test ax, imm */
+   goto test;  
case 0xaa ... 0xab: /* stos */
c->dst.val = c->regs[VCPU_REGS_RAX];
break;
-- 
1.7.0.4

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


[PATCH 1/2] x86 emulator: Add missing decoder flags for sub instruction

2010-05-11 Thread Mohammed Gamal
This adds missing decoder flags for sub instructions (opcodes 0x2c - 0x2d)

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index be5e78d..5c523ed 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -126,7 +126,7 @@ static u32 opcode_table[256] = {
/* 0x28 - 0x2F */
ByteOp | DstMem | SrcReg | ModRM | Lock, DstMem | SrcReg | ModRM | Lock,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   ByteOp | DstAcc | SrcImmByte, DstAcc | SrcImm, 0, 0,
/* 0x30 - 0x37 */
ByteOp | DstMem | SrcReg | ModRM | Lock, DstMem | SrcReg | ModRM | Lock,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-- 
1.7.0.4

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


[PATCH 2/2] x86 emulator: Add missing decoder flags for xor instructions

2010-05-11 Thread Mohammed Gamal
This adds missing decoder flags for xor instructions (opcodes 0x34 - 0x35)

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/emulate.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 5c523ed..1650fe5 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -130,7 +130,7 @@ static u32 opcode_table[256] = {
/* 0x30 - 0x37 */
ByteOp | DstMem | SrcReg | ModRM | Lock, DstMem | SrcReg | ModRM | Lock,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-   0, 0, 0, 0,
+   ByteOp | DstAcc | SrcImmByte, DstAcc | SrcImm, 0, 0,
/* 0x38 - 0x3F */
ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-- 
1.7.0.4

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


[PATCH 1/2] test: Add test for sub acc,imm

2010-05-12 Thread Mohammed Gamal
Adds tests fot sub acc, imm

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   44 ++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index bc4ed97..41e2aea 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -307,6 +307,49 @@ void test_mov_imm(void)
print_serial("mov test 5: PASS\n");
 }
 
+void test_sub_imm(void)
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(sub_r32_imm_1, "mov $1234567890, %eax\n\t" "sub $10, %eax\n\t");
+   MK_INSN(sub_r16_imm_1, "mov $1234, %ax\n\t" "sub $10, %ax\n\t");
+   MK_INSN(sub_r8_imm_1, "mov $0x12, %ah\n\t" "sub $0x10, %ah\n\t");
+   MK_INSN(sub_r8_imm_2, "mov $0x34, %al\n\t" "sub $0x10, %al\n\t");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_sub_r16_imm_1,
+ insn_sub_r16_imm_1_end - insn_sub_r16_imm_1);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 1224)
+   print_serial("sub test 1: FAIL\n");
+   else
+   print_serial("sub test 1: PASS\n");
+
+   /* test mov $imm, %eax */
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_sub_r32_imm_1,
+ insn_sub_r32_imm_1_end - insn_sub_r32_imm_1);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 1234567880)
+   print_serial("sub test 2: FAIL\n");
+   else
+   print_serial("sub test 2: PASS\n");
+
+   /* test mov $imm, %al/%ah */
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_sub_r8_imm_1,
+ insn_sub_r8_imm_1_end - insn_sub_r8_imm_1);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x0200)
+   print_serial("sub test 3: FAIL\n");
+   else
+   print_serial("sub test 3: PASS\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_sub_r8_imm_2,
+ insn_sub_r8_imm_2_end - insn_sub_r8_imm_2);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0x24)
+   print_serial("sub test 4: FAIL\n");
+   else
+   print_serial("sub test 4: PASS\n");
+}
+
 void test_cmp_imm(void)
 {
struct regs inregs = { 0 }, outregs;
@@ -742,6 +785,7 @@ void realmode_start(void)
test_mov_imm();
test_cmp_imm();
test_add_imm();
+   test_sub_imm();
test_io();
test_eflags_insn();
test_jcc_short();
-- 
1.7.0.4

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


[PATCH 2/2] test: Add test for xor acc, imm

2010-05-12 Thread Mohammed Gamal
Adds test for xor acc, imm

Signed-off-by: Mohammed Gamal 
---
 kvm/user/test/x86/realmode.c |   45 ++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/kvm/user/test/x86/realmode.c b/kvm/user/test/x86/realmode.c
index 41e2aea..70a1e05 100644
--- a/kvm/user/test/x86/realmode.c
+++ b/kvm/user/test/x86/realmode.c
@@ -350,6 +350,50 @@ void test_sub_imm(void)
print_serial("sub test 4: PASS\n");
 }
 
+
+void test_xor_imm(void)
+{
+   struct regs inregs = { 0 }, outregs;
+   MK_INSN(xor_r32_imm_1, "mov $1234567890, %eax\n\t" "xor $1234567890, 
%eax\n\t");
+   MK_INSN(xor_r16_imm_1, "mov $1234, %ax\n\t" "xor $1234, %ax\n\t");
+   MK_INSN(xor_r8_imm_1, "mov $0x12, %ah\n\t" "xor $0x12, %ah\n\t");
+   MK_INSN(xor_r8_imm_2, "mov $0x34, %al\n\t" "xor $0x34, %al\n\t");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_xor_r16_imm_1,
+ insn_xor_r16_imm_1_end - insn_xor_r16_imm_1);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0)
+   print_serial("xor test 1: FAIL\n");
+   else
+   print_serial("xor test 1: PASS\n");
+
+   /* test mov $imm, %eax */
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_xor_r32_imm_1,
+ insn_xor_r32_imm_1_end - insn_xor_r32_imm_1);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0)
+   print_serial("xor test 2: FAIL\n");
+   else
+   print_serial("xor test 2: PASS\n");
+
+   /* test mov $imm, %al/%ah */
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_xor_r8_imm_1,
+ insn_xor_r8_imm_1_end - insn_xor_r8_imm_1);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0)
+   print_serial("xor test 3: FAIL\n");
+   else
+   print_serial("xor test 3: PASS\n");
+
+   exec_in_big_real_mode(&inregs, &outregs,
+ insn_xor_r8_imm_2,
+ insn_xor_r8_imm_2_end - insn_xor_r8_imm_2);
+   if (!regs_equal(&inregs, &outregs, R_AX) || outregs.eax != 0)
+   print_serial("xor test 4: FAIL\n");
+   else
+   print_serial("xor test 4: PASS\n");
+}
+
 void test_cmp_imm(void)
 {
struct regs inregs = { 0 }, outregs;
@@ -786,6 +830,7 @@ void realmode_start(void)
test_cmp_imm();
test_add_imm();
test_sub_imm();
+   test_xor_imm();
test_io();
test_eflags_insn();
test_jcc_short();
-- 
1.7.0.4

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


Re: [PATCH] VMX: Fix and improve guest state validity checks

2010-05-13 Thread Mohammed Gamal
On Thu, May 13, 2010 at 9:24 AM, Avi Kivity  wrote:
> On 05/11/2010 07:52 PM, Mohammed Gamal wrote:
>>
>> - Add 's' and 'g' field checks on segment registers
>> - Correct SS checks for request and descriptor privilege levels
>>
>> Signed-off-by: Mohammed Gamal
>> ---
>>  arch/x86/kvm/vmx.c |   73
>> +++
>>  1 files changed, 67 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 777e00d..9805c2a 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2121,16 +2121,30 @@ static bool stack_segment_valid(struct kvm_vcpu
>> *vcpu)
>>        vmx_get_segment(vcpu,&ss, VCPU_SREG_SS);
>>        ss_rpl = ss.selector&  SELECTOR_RPL_MASK;
>>
>> -       if (ss.unusable)
>> +       if (ss.dpl != ss_rpl) /* DPL != RPL */
>> +               return false;
>> +
>> +       if (ss.unusable) /* Short-circuit */
>>                return true;
>>
>
> If ss.unusable, do the dpl and rpl have any meaning?

The idea is that dpl and rpl are checked on vmentry regardless of
whether ss is usable or not. While the other checks are performed only
if ss is usable.

>
>>        if (!ss.present)
>>                return false;
>> +       if (ss.limit&  0xfff0) {
>> +                if ((ss.limit&  0xfff)<  0xfff)
>> +                        return false;
>> +                if (!ss.g)
>> +                        return false;
>> +        } else {
>> +                if ((ss.limit&  0xfff) == 0xfff)
>> +                        return false;
>> +                if (ss.g)
>> +                        return false;
>> +        }
>>
>
> There is no architectural way to break this.  That is, without
> virtualization, there is no way a real cpu will ever have a limit of
> 0x12345678.
>
> We need to distinguish between big real mode and real mode that can be
> virtualized using vm86, but we don't need to consider impossible setups.

I didn't realize this is architecturally impossible. I was simply
implementing the checks specified in the Intel manual. Now that we
know this is redunant, we can just drop these checks.

>
>
>> @@ -2143,8 +2157,15 @@ static bool data_segment_valid(struct kvm_vcpu
>> *vcpu, int seg)
>>        vmx_get_segment(vcpu,&var, seg);
>>        rpl = var.selector&  SELECTOR_RPL_MASK;
>>
>> -       if (var.unusable)
>> +       if (var.unusable)  /* Short-circuit */
>>                return true;
>> +       if (!(var.type&  AR_TYPE_ACCESSES_MASK))
>> +               return false;
>>
>
> Again, there is no architectural way for a segment not to have the accessed
> bit set.
>
>> +       if (var.type&  AR_TYPE_CODE_MASK) {
>> +               if (!(var.type&  AR_TYPE_READABLE_MASK))
>> +                       return false;
>> +       }
>>
>
> About this, I'm not sure.
>
>> +
>>        if (!var.s)
>>                return false;
>>        if (!var.present)
>> @@ -2154,6 +2175,18 @@ static bool data_segment_valid(struct kvm_vcpu
>> *vcpu, int seg)
>>                        return false;
>>        }
>>
>> +       if (var.limit&  0xfff0) {
>> +               if ((var.limit&  0xfff)<  0xfff)
>> +                       return false;
>> +               if (!var.g)
>> +                       return false;
>> +       } else {
>> +               if ((var.limit&  0xfff) == 0xfff)
>> +                       return false;
>> +               if (var.g)
>> +                       return false;
>> +       }
>>
>
> Even disregarding the incorrectness, you shouldn't duplicate code like this.
I was intending to consolidate it into a single function eventually, I
just wasn't sure that this was correct and I needed some comments on
it. It's not needed now anyhow.

>
>> @@ -2192,6 +2240,20 @@ static bool ldtr_valid(struct kvm_vcpu *vcpu)
>>                return false;
>>        if (!ldtr.present)
>>                return false;
>> +       if (ldtr.s)
>> +               return false;
>>
>
> Architecturally impossible.
>
> --
> Do not meddle in the internals of kernels, for they are subtle and quick to
> panic.
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] x86 emulator: Add missing decoder flags for sub instruction

2010-05-13 Thread Mohammed Gamal
On Thu, May 13, 2010 at 9:50 PM, Marcelo Tosatti  wrote:
> On Wed, May 12, 2010 at 01:39:21AM +0300, Mohammed Gamal wrote:
>> This adds missing decoder flags for sub instructions (opcodes 0x2c - 0x2d)
>>
>> Signed-off-by: Mohammed Gamal 
>> ---
>>  arch/x86/kvm/emulate.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> Applied both, thanks.
>
>

Thanks Marcelo, please also take a look at the test cases for these
instructions that I posted to the mailing list.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] VMX: Properly return error to userspace on vmentry failure

2010-05-19 Thread Mohammed Gamal
The vmexit handler returns KVM_EXIT_UNKNOWN since there is no handler
for vmentry failures. This intercepts vmentry failures and returns
KVM_FAIL_ENTRY to userspace instead.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99ae513..4edcffb 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3665,6 +3665,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
if (enable_ept && is_paging(vcpu))
vcpu->arch.cr3 = vmcs_readl(GUEST_CR3);
 
+   if (exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) {
+   vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
+   vcpu->run->fail_entry.hardware_entry_failure_reason
+   = exit_reason & ~VMX_EXIT_REASONS_FAILED_VMENTRY;
+   return 0;
+   }
+
if (unlikely(vmx->fail)) {
vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
vcpu->run->fail_entry.hardware_entry_failure_reason
-- 
1.7.0.4

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


[PATCH] Print a user-friendly message on failed vmentry

2010-05-19 Thread Mohammed Gamal
This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.

Failed vmentries were handled with handle_unhandled() which prints a rather
unfriendly message to the user. This patch separates handling vmentry failures
from unknown exit reasons and prints a friendly message to the user.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 35a4c8a..deb4df8 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -106,6 +106,20 @@ static int handle_unhandled(uint64_t reason)
 return -EINVAL;
 }
 
+static int handle_failed_vmentry(uint64_t reason)
+{
+fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
reason);
+fprintf(stderr, "If you're runnning a guest on an Intel machine, it can 
be\n");
+fprintf(stderr, "most-likely due to the guest going into an invalid 
state\n");
+fprintf(stderr, "for Intel VT. For example, the guest maybe running in 
big\n");
+fprintf(stderr, "real mode which is not supported by Intel VT.\n\n");
+fprintf(stderr, "You may want to try enabling real mode emulation in 
KVM.\n");
+fprintf(stderr, "To Enable it, you may run the following commands as 
root:\n");
+fprintf(stderr, "# rmmod kvm_intel\n");
+fprintf(stderr, "# rmmod kvm\n");
+fprintf(stderr, "# modprobe kvm_intel emulate_invalid_guest_state=1\n");
+return -EINVAL;
+}
 
 static inline void set_gsi(kvm_context_t kvm, unsigned int gsi)
 {
@@ -586,7 +600,7 @@ int kvm_run(CPUState *env)
 r = handle_unhandled(run->hw.hardware_exit_reason);
 break;
 case KVM_EXIT_FAIL_ENTRY:
-r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
+r = 
handle_failed_vmentry(run->fail_entry.hardware_entry_failure_reason);
 break;
 case KVM_EXIT_EXCEPTION:
 fprintf(stderr, "exception %d (%x)\n", run->ex.exception,
-- 
1.7.0.4

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


Re: [PATCH] Print a user-friendly message on failed vmentry

2010-05-19 Thread Mohammed Gamal
On Thu, May 20, 2010 at 4:28 AM, Ryan Harper  wrote:
>
> * Mohammed Gamal  [2010-05-19 16:17]:
> > This patch address bug report in 
> > https://bugs.launchpad.net/qemu/+bug/530077.
> >
> > Failed vmentries were handled with handle_unhandled() which prints a rather
> > unfriendly message to the user. This patch separates handling vmentry 
> > failures
> > from unknown exit reasons and prints a friendly message to the user.
> >
> > Signed-off-by: Mohammed Gamal 
> > ---
> >  qemu-kvm.c |   16 +++-
> >  1 files changed, 15 insertions(+), 1 deletions(-)
> >
> > diff --git a/qemu-kvm.c b/qemu-kvm.c
> > index 35a4c8a..deb4df8 100644
> > --- a/qemu-kvm.c
> > +++ b/qemu-kvm.c
> > @@ -106,6 +106,20 @@ static int handle_unhandled(uint64_t reason)
> >      return -EINVAL;
> >  }
> >
> > +static int handle_failed_vmentry(uint64_t reason)
> > +{
> > +    fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
> > reason);
> > +    fprintf(stderr, "If you're runnning a guest on an Intel machine, it 
> > can be\n");
> > +    fprintf(stderr, "most-likely due to the guest going into an invalid 
> > state\n");
> > +    fprintf(stderr, "for Intel VT. For example, the guest maybe running in 
> > big\n");
> > +    fprintf(stderr, "real mode which is not supported by Intel VT.\n\n");
>
> We might want to qualify this with certain cpu versions.  IIRC, the VMX
> unrestricted mode should handle big real mode correctly, no?   Maybe,
>
> +    fprintf(stderr, "on some Intel processors. For example, the guest maybe 
> running in big\n");
> +    fprintf(stderr, "real mode which is not supported on most Intel 
> processors.\n\n");
>
Good point. Will correct and resend.

> --
> Ryan Harper
> Software Engineer; Linux Technology Center
> IBM Corp., Austin, Tx
> ry...@us.ibm.com
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Print a user-friendly message on failed vmentry

2010-05-19 Thread Mohammed Gamal
This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.

Failed vmentries were handled with handle_unhandled() which prints a rather
unfriendly message to the user. This patch separates handling vmentry failures
from unknown exit reasons and prints a friendly message to the user.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |   19 ++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 35a4c8a..1fdb6fe 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -106,6 +106,23 @@ static int handle_unhandled(uint64_t reason)
 return -EINVAL;
 }
 
+static int handle_failed_vmentry(uint64_t reason)
+{
+fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
reason);
+fprintf(stderr, "If you're runnning a guest on an Intel machine 
without\n");
+fprintf(stderr, "unrestricted mode support, the failure can be most 
likely\n");
+fprintf(stderr, "due to the guest entering an invalid state for Intel 
VT.\n");
+fprintf(stderr, "For example, the guest maybe running in big real mode\n");
+fprintf(stderr, "which is not supported on less recent Intel 
processors.\n\n");
+fprintf(stderr, "You may want to try enabling KVM real mode emulation. 
To\n");
+fprintf(stderr, "enable it, you can run the following commands as 
root:\n");
+fprintf(stderr, "# rmmod kvm_intel\n");
+fprintf(stderr, "# rmmod kvm\n");
+fprintf(stderr, "# modprobe kvm_intel emulate_invalid_guest_state=1\n\n");
+fprintf(stderr, "WARNING: Real mode emulation is still 
work-in-progress\n");
+fprintf(stderr, "and thus it is not always guaranteed to work.\n\n");
+return -EINVAL;
+}
 
 static inline void set_gsi(kvm_context_t kvm, unsigned int gsi)
 {
@@ -586,7 +603,7 @@ int kvm_run(CPUState *env)
 r = handle_unhandled(run->hw.hardware_exit_reason);
 break;
 case KVM_EXIT_FAIL_ENTRY:
-r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
+r = 
handle_failed_vmentry(run->fail_entry.hardware_entry_failure_reason);
 break;
 case KVM_EXIT_EXCEPTION:
 fprintf(stderr, "exception %d (%x)\n", run->ex.exception,
-- 
1.7.0.4

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


Re: [PATCH] Print a user-friendly message on failed vmentry

2010-05-20 Thread Mohammed Gamal
On Thu, May 20, 2010 at 5:37 PM, Chris Lalancette  wrote:
> On 05/19/2010 05:16 PM, Mohammed Gamal wrote:
>> This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.
>>
>> Failed vmentries were handled with handle_unhandled() which prints a rather
>> unfriendly message to the user. This patch separates handling vmentry 
>> failures
>> from unknown exit reasons and prints a friendly message to the user.
>>
>> Signed-off-by: Mohammed Gamal 
>> ---
>>  qemu-kvm.c |   16 +++-
>>  1 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index 35a4c8a..deb4df8 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -106,6 +106,20 @@ static int handle_unhandled(uint64_t reason)
>>      return -EINVAL;
>>  }
>>
>> +static int handle_failed_vmentry(uint64_t reason)
>> +{
>> +    fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
>> reason);
>> +    fprintf(stderr, "If you're runnning a guest on an Intel machine, it can 
>> be\n");
>> +    fprintf(stderr, "most-likely due to the guest going into an invalid 
>> state\n");
>> +    fprintf(stderr, "for Intel VT. For example, the guest maybe running in 
>> big\n");
>> +    fprintf(stderr, "real mode which is not supported by Intel VT.\n\n");
>> +    fprintf(stderr, "You may want to try enabling real mode emulation in 
>> KVM.\n");
>> +    fprintf(stderr, "To Enable it, you may run the following commands as 
>> root:\n");
>> +    fprintf(stderr, "# rmmod kvm_intel\n");
>> +    fprintf(stderr, "# rmmod kvm\n");
>> +    fprintf(stderr, "# modprobe kvm_intel emulate_invalid_guest_state=1\n");
>> +    return -EINVAL;
>> +}
>
> The thing is, there are other valid reasons for vmentry failure.  A while ago 
> I tracked
> down a bug in the Linux kernel that was causing us to vmenter with invalid 
> segments;
> this message would have been very misleading in that case.  I think you'd 
> have to do
> more complete analysis of the vmentry failure code to be more certain about 
> the reason
> for failure.
>
Your point is definitely valid, yet big real mode is usually the most
likely case, and that's why this message is shown. Note also that it
says it's _most likely_ a failure caused by an invalid guest state,
but it doesn't rule out all other reasons. And in any case, it'd be
better than just printing something along the lines of:
" kvm: unhandled exit 8021
  kvm_run returned -22"

> --
> Chris Lalancette
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Print a user-friendly message on failed vmentry

2010-05-20 Thread Mohammed Gamal
On Thu, May 20, 2010 at 6:53 PM, Avi Kivity  wrote:
> On 05/20/2010 05:46 PM, Mohammed Gamal wrote:
>>
>> On Thu, May 20, 2010 at 5:37 PM, Chris Lalancette
>>  wrote:
>>
>>>
>>> On 05/19/2010 05:16 PM, Mohammed Gamal wrote:
>>>
>>>>
>>>> This patch address bug report in
>>>> https://bugs.launchpad.net/qemu/+bug/530077.
>>>>
>>>> Failed vmentries were handled with handle_unhandled() which prints a
>>>> rather
>>>> unfriendly message to the user. This patch separates handling vmentry
>>>> failures
>>>> from unknown exit reasons and prints a friendly message to the user.
>>>>
>>>> Signed-off-by: Mohammed Gamal
>>>> ---
>>>>  qemu-kvm.c |   16 +++-
>>>>  1 files changed, 15 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>>>> index 35a4c8a..deb4df8 100644
>>>> --- a/qemu-kvm.c
>>>> +++ b/qemu-kvm.c
>>>> @@ -106,6 +106,20 @@ static int handle_unhandled(uint64_t reason)
>>>>      return -EINVAL;
>>>>  }
>>>>
>>>> +static int handle_failed_vmentry(uint64_t reason)
>>>> +{
>>>> +    fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64
>>>> "\n\n", reason);
>>>> +    fprintf(stderr, "If you're runnning a guest on an Intel machine, it
>>>> can be\n");
>>>> +    fprintf(stderr, "most-likely due to the guest going into an invalid
>>>> state\n");
>>>> +    fprintf(stderr, "for Intel VT. For example, the guest maybe running
>>>> in big\n");
>>>> +    fprintf(stderr, "real mode which is not supported by Intel
>>>> VT.\n\n");
>>>> +    fprintf(stderr, "You may want to try enabling real mode emulation
>>>> in KVM.\n");
>>>> +    fprintf(stderr, "To Enable it, you may run the following commands
>>>> as root:\n");
>>>> +    fprintf(stderr, "# rmmod kvm_intel\n");
>>>> +    fprintf(stderr, "# rmmod kvm\n");
>>>> +    fprintf(stderr, "# modprobe kvm_intel
>>>> emulate_invalid_guest_state=1\n");
>>>> +    return -EINVAL;
>>>> +}
>>>>
>>>
>>> The thing is, there are other valid reasons for vmentry failure.  A while
>>> ago I tracked
>>> down a bug in the Linux kernel that was causing us to vmenter with
>>> invalid segments;
>>> this message would have been very misleading in that case.  I think you'd
>>> have to do
>>> more complete analysis of the vmentry failure code to be more certain
>>> about the reason
>>> for failure.
>>>
>>>
>>
>> Your point is definitely valid, yet big real mode is usually the most
>> likely case, and that's why this message is shown. Note also that it
>> says it's _most likely_ a failure caused by an invalid guest state,
>> but it doesn't rule out all other reasons. And in any case, it'd be
>> better than just printing something along the lines of:
>> " kvm: unhandled exit 8021
>>   kvm_run returned -22"
>>
>
> However, we're still giving bad advice.  Currently
> emulate_invalid_guest_state=1 is not going to work well (right?).  Once it
> does, we'll simply make it the default and the message will never appear.
>
I already added a warning in the second patch I sent.

> --
> Do not meddle in the internals of kernels, for they are subtle and quick to
> panic.
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] VMX: Properly return error to userspace on vmentry failure

2010-05-23 Thread Mohammed Gamal
The vmexit handler returns KVM_EXIT_UNKNOWN since there is no handler
for vmentry failures. This intercepts vmentry failures and returns
KVM_FAIL_ENTRY to userspace instead.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99ae513..4edcffb 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3665,6 +3665,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
if (enable_ept && is_paging(vcpu))
vcpu->arch.cr3 = vmcs_readl(GUEST_CR3);
 
+   if (exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) {
+   vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
+   vcpu->run->fail_entry.hardware_entry_failure_reason
+   = exit_reason & ~VMX_EXIT_REASONS_FAILED_VMENTRY;
+   return 0;
+   }
+
if (unlikely(vmx->fail)) {
vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
vcpu->run->fail_entry.hardware_entry_failure_reason
-- 
1.7.0.4

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


[PATCH 2/2] VMX: Add constant for invalid guest state exit reason

2010-05-23 Thread Mohammed Gamal
For the sake of completeness, this patch adds a symbolic
constant for VMX exit reason 0x21 (invalid guest state).

Signed-off-by: Mohammed Gamal 
---
 arch/x86/include/asm/vmx.h |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 9e6779f..104cf86 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -257,6 +257,7 @@ enum vmcs_field {
 #define EXIT_REASON_IO_INSTRUCTION  30
 #define EXIT_REASON_MSR_READ31
 #define EXIT_REASON_MSR_WRITE   32
+#define EXIT_REASON_INVALID_STATE  33
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
 #define EXIT_REASON_MONITOR_INSTRUCTION 39
 #define EXIT_REASON_PAUSE_INSTRUCTION   40
-- 
1.7.0.4

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


[PATCH] Print a user-friendly message on failed vmentry

2010-05-23 Thread Mohammed Gamal
This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.

Failed vmentries were handled with handle_unhandled() which prints a rather
unfriendly message to the user. This patch separates handling vmentry failures
from unknown exit reasons and prints a friendly message to the user.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 35a4c8a..3ef0565 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -106,6 +106,31 @@ static int handle_unhandled(uint64_t reason)
 return -EINVAL;
 }
 
+#define VMX_INVALID_GUEST_STATE 0x21
+
+static int handle_failed_vmentry(uint64_t reason)
+{
+fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
reason);
+
+/* Perhaps we will need to check if this machine is intel since exit 
reason 0x21 
+   has a different interpretation on SVM */
+if (reason == VMX_INVALID_GUEST_STATE) {
+fprintf(stderr, "If you're runnning a guest on an Intel machine 
without\n");
+fprintf(stderr, "unrestricted mode support, the failure can be most 
likely\n");
+fprintf(stderr, "due to the guest entering an invalid state for Intel 
VT.\n");
+fprintf(stderr, "For example, the guest maybe running in big real 
mode\n");
+fprintf(stderr, "which is not supported on less recent Intel 
processors.\n\n");
+fprintf(stderr, "You may want to try enabling KVM real mode emulation. 
To\n");
+fprintf(stderr, "enable it, you can run the following commands as 
root:\n");
+fprintf(stderr, "# rmmod kvm_intel\n");
+fprintf(stderr, "# rmmod kvm\n");
+fprintf(stderr, "# modprobe kvm_intel 
emulate_invalid_guest_state=1\n\n");
+fprintf(stderr, "WARNING: Real mode emulation is still 
work-in-progress\n");
+fprintf(stderr, "and thus it is not always guaranteed to work.\n\n");
+}
+
+return -EINVAL;
+}
 
 static inline void set_gsi(kvm_context_t kvm, unsigned int gsi)
 {
@@ -586,7 +611,7 @@ int kvm_run(CPUState *env)
 r = handle_unhandled(run->hw.hardware_exit_reason);
 break;
 case KVM_EXIT_FAIL_ENTRY:
-r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
+r = 
handle_failed_vmentry(run->fail_entry.hardware_entry_failure_reason);
 break;
 case KVM_EXIT_EXCEPTION:
 fprintf(stderr, "exception %d (%x)\n", run->ex.exception,
-- 
1.7.0.4

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


Re: [PATCH] VMX: Fix and improve guest state validity checks

2010-05-25 Thread Mohammed Gamal
On Tue, May 25, 2010 at 12:37 PM, Avi Kivity  wrote:
> On 05/13/2010 11:15 PM, Mohammed Gamal wrote:
>>
>> On Thu, May 13, 2010 at 9:24 AM, Avi Kivity  wrote:
>>
>>>
>>> On 05/11/2010 07:52 PM, Mohammed Gamal wrote:
>>>
>>>>
>>>> - Add 's' and 'g' field checks on segment registers
>>>> - Correct SS checks for request and descriptor privilege levels
>>>>
>>>> Signed-off-by: Mohammed Gamal
>>>> ---
>>>>  arch/x86/kvm/vmx.c |   73
>>>> +++
>>>>  1 files changed, 67 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>> index 777e00d..9805c2a 100644
>>>> --- a/arch/x86/kvm/vmx.c
>>>> +++ b/arch/x86/kvm/vmx.c
>>>> @@ -2121,16 +2121,30 @@ static bool stack_segment_valid(struct kvm_vcpu
>>>> *vcpu)
>>>>        vmx_get_segment(vcpu,&ss, VCPU_SREG_SS);
>>>>        ss_rpl = ss.selector&    SELECTOR_RPL_MASK;
>>>>
>>>> -       if (ss.unusable)
>>>> +       if (ss.dpl != ss_rpl) /* DPL != RPL */
>>>> +               return false;
>>>> +
>>>> +       if (ss.unusable) /* Short-circuit */
>>>>                return true;
>>>>
>>>>
>>>
>>> If ss.unusable, do the dpl and rpl have any meaning?
>>>
>>
>> The idea is that dpl and rpl are checked on vmentry regardless of
>> whether ss is usable or not. While the other checks are performed only
>> if ss is usable.
>>
>
> Any reference to back this up?  I think rpl is valid regardless of
> ss.unusable (i.e. loading selector 0003 results in an unusable segment with
> rpl=3), but I don't see how dpl can be valid in an unusable segment.
>
Intel 64 and IA-32 Architectures Software Developer’s Manual Volume
3B, System Programming Guide, Part 2, Chapter 22, Section 22.3.1.2:
Checks on Guest Segment Registers.
You'll note that DS, ES, FS, GS checks are done when the segment is
usable. SS checks are not necessarily checked only when the segment is
usable.
> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] VMX: Properly return error to userspace on vmentry failure

2010-05-25 Thread Mohammed Gamal
On Tue, May 25, 2010 at 2:45 PM, Avi Kivity  wrote:
> On 05/24/2010 01:01 AM, Mohammed Gamal wrote:
>>
>> The vmexit handler returns KVM_EXIT_UNKNOWN since there is no handler
>> for vmentry failures. This intercepts vmentry failures and returns
>> KVM_FAIL_ENTRY to userspace instead.
>>
>> Signed-off-by: Mohammed Gamal
>> ---
>>  arch/x86/kvm/vmx.c |    7 +++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 99ae513..4edcffb 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3665,6 +3665,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
>>        if (enable_ept&&  is_paging(vcpu))
>>                vcpu->arch.cr3 = vmcs_readl(GUEST_CR3);
>>
>> +       if (exit_reason&  VMX_EXIT_REASONS_FAILED_VMENTRY) {
>> +               vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
>> +               vcpu->run->fail_entry.hardware_entry_failure_reason
>> +                       = exit_reason&  ~VMX_EXIT_REASONS_FAILED_VMENTRY;
>> +               return 0;
>> +       }
>> +
>>        if (unlikely(vmx->fail)) {
>>                vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
>>                vcpu->run->fail_entry.hardware_entry_failure_reason
>>
>
> How does the user distinguish between KVM_EXIT_FAIL_ENTRY due to an exit
> reason with bit 31 set and vmlauch/vmresume failure (vmx->fail set)?  We
> need separate exit codes (with documentation in api.txt).

In both cases the vm fails entry, and I don't think the hardware entry
failure reason codes would overlap between the vmx->fail case and exit
reasons with bit 31 set, so why should there be such distinction
between both cases?
>
> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] VMX: Properly return error to userspace on vmentry failure

2010-05-25 Thread Mohammed Gamal
On Tue, May 25, 2010 at 3:10 PM, Avi Kivity  wrote:
> On 05/25/2010 03:01 PM, Mohammed Gamal wrote:
>>>
>>> How does the user distinguish between KVM_EXIT_FAIL_ENTRY due to an exit
>>> reason with bit 31 set and vmlauch/vmresume failure (vmx->fail set)?  We
>>> need separate exit codes (with documentation in api.txt).
>>>
>>
>> In both cases the vm fails entry, and I don't think the hardware entry
>> failure reason codes would overlap between the vmx->fail case and exit
>> reasons with bit 31 set, so why should there be such distinction
>> between both cases?
>>
>
> Only 5 more error codes (28->33) and we have overlap.
>
> If you return the new codes with bit 31 still set then we can use the
> existing KVM_EXIT_FAIL_ENTRY.

That'd be a better idea.

>
>
> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] VMX: Properly return error to userspace on vmentry failure

2010-05-25 Thread Mohammed Gamal
The vmexit handler returns KVM_EXIT_UNKNOWN since there is no handler
for vmentry failures. This intercepts vmentry failures and returns
KVM_FAIL_ENTRY to userspace instead.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99ae513..4edcffb 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3665,6 +3665,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
if (enable_ept && is_paging(vcpu))
vcpu->arch.cr3 = vmcs_readl(GUEST_CR3);
 
+   if (exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) {
+   vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
+   vcpu->run->fail_entry.hardware_entry_failure_reason
+   = exit_reason & ~VMX_EXIT_REASONS_FAILED_VMENTRY;
+   return 0;
+   }
+
if (unlikely(vmx->fail)) {
vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
vcpu->run->fail_entry.hardware_entry_failure_reason
-- 
1.7.0.4

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


[PATCH v2] VMX: Properly return error to userspace on vmentry failure

2010-05-25 Thread Mohammed Gamal
The vmexit handler returns KVM_EXIT_UNKNOWN since there is no handler
for vmentry failures. This intercepts vmentry failures and returns
KVM_FAIL_ENTRY to userspace instead.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99ae513..33aa0cc 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3665,6 +3665,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
if (enable_ept && is_paging(vcpu))
vcpu->arch.cr3 = vmcs_readl(GUEST_CR3);
 
+   if (exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) {
+   vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
+   vcpu->run->fail_entry.hardware_entry_failure_reason
+   = exit_reason;
+   return 0;
+   }
+
if (unlikely(vmx->fail)) {
vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
vcpu->run->fail_entry.hardware_entry_failure_reason
-- 
1.7.0.4

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


[PATCH v2] Print a user-friendly message on failed vmentry

2010-05-25 Thread Mohammed Gamal
This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.

Failed vmentries were handled with handle_unhandled() which prints a rather
unfriendly message to the user. This patch separates handling vmentry failures
from unknown exit reasons and prints a friendly message to the user.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 35a4c8a..24c15b4 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -106,6 +106,31 @@ static int handle_unhandled(uint64_t reason)
 return -EINVAL;
 }
 
+#define VMX_INVALID_GUEST_STATE 0x8021
+
+static int handle_failed_vmentry(uint64_t reason)
+{
+fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
reason);
+
+/* Perhaps we will need to check if this machine is intel since exit 
reason 0x21 
+   has a different interpretation on SVM */
+if (reason == VMX_INVALID_GUEST_STATE) {
+fprintf(stderr, "If you're runnning a guest on an Intel machine 
without\n");
+fprintf(stderr, "unrestricted mode support, the failure can be most 
likely\n");
+fprintf(stderr, "due to the guest entering an invalid state for Intel 
VT.\n");
+fprintf(stderr, "For example, the guest maybe running in big real 
mode\n");
+fprintf(stderr, "which is not supported on less recent Intel 
processors.\n\n");
+fprintf(stderr, "You may want to try enabling KVM real mode emulation. 
To\n");
+fprintf(stderr, "enable it, you can run the following commands as 
root:\n");
+fprintf(stderr, "# rmmod kvm_intel\n");
+fprintf(stderr, "# rmmod kvm\n");
+fprintf(stderr, "# modprobe kvm_intel 
emulate_invalid_guest_state=1\n\n");
+fprintf(stderr, "WARNING: Real mode emulation is still 
work-in-progress\n");
+fprintf(stderr, "and thus it is not always guaranteed to work.\n\n");
+}
+
+return -EINVAL;
+}
 
 static inline void set_gsi(kvm_context_t kvm, unsigned int gsi)
 {
@@ -586,7 +611,7 @@ int kvm_run(CPUState *env)
 r = handle_unhandled(run->hw.hardware_exit_reason);
 break;
 case KVM_EXIT_FAIL_ENTRY:
-r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
+r = 
handle_failed_vmentry(run->fail_entry.hardware_entry_failure_reason);
 break;
 case KVM_EXIT_EXCEPTION:
 fprintf(stderr, "exception %d (%x)\n", run->ex.exception,
-- 
1.7.0.4

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


[PATCH][RESEND] Print a user-friendly message on failed vmentry

2010-05-31 Thread Mohammed Gamal
This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.

Failed vmentries were handled with handle_unhandled() which prints a rather
unfriendly message to the user. This patch separates handling vmentry failures
from unknown exit reasons and prints a friendly message to the user.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 9534b31..b8a9278 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -106,6 +106,31 @@ static int handle_unhandled(uint64_t reason)
 return -EINVAL;
 }
 
+#define VMX_INVALID_GUEST_STATE 0x8021
+
+static int handle_failed_vmentry(uint64_t reason)
+{
+fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
reason);
+
+/* Perhaps we will need to check if this machine is intel since exit 
reason 0x21 
+   has a different interpretation on SVM */
+if (reason == VMX_INVALID_GUEST_STATE) {
+fprintf(stderr, "If you're runnning a guest on an Intel machine 
without\n");
+fprintf(stderr, "unrestricted mode support, the failure can be most 
likely\n");
+fprintf(stderr, "due to the guest entering an invalid state for Intel 
VT.\n");
+fprintf(stderr, "For example, the guest maybe running in big real 
mode\n");
+fprintf(stderr, "which is not supported on less recent Intel 
processors.\n\n");
+fprintf(stderr, "You may want to try enabling KVM real mode emulation. 
To\n");
+fprintf(stderr, "enable it, you can run the following commands as 
root:\n");
+fprintf(stderr, "# rmmod kvm_intel\n");
+fprintf(stderr, "# rmmod kvm\n");
+fprintf(stderr, "# modprobe kvm_intel 
emulate_invalid_guest_state=1\n\n");
+fprintf(stderr, "WARNING: Real mode emulation is still 
work-in-progress\n");
+fprintf(stderr, "and thus it is not always guaranteed to work.\n\n");
+}
+
+return -EINVAL;
+}
 
 static inline void set_gsi(kvm_context_t kvm, unsigned int gsi)
 {
@@ -586,7 +611,7 @@ int kvm_run(CPUState *env)
 r = handle_unhandled(run->hw.hardware_exit_reason);
 break;
 case KVM_EXIT_FAIL_ENTRY:
-r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
+r = 
handle_failed_vmentry(run->fail_entry.hardware_entry_failure_reason);
 break;
 case KVM_EXIT_EXCEPTION:
 fprintf(stderr, "exception %d (%x)\n", run->ex.exception,
-- 
1.7.0.4

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


[PATCH][RESEND] VMX: Properly return error to userspace on vmentry failure

2010-05-31 Thread Mohammed Gamal
The vmexit handler returns KVM_EXIT_UNKNOWN since there is no handler
for vmentry failures. This intercepts vmentry failures and returns
KVM_FAIL_ENTRY to userspace instead.

Signed-off-by: Mohammed Gamal 
---
 arch/x86/kvm/vmx.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0e561a5..1b6a3be 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3668,6 +3668,13 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
if (enable_ept && is_paging(vcpu))
vcpu->arch.cr3 = vmcs_readl(GUEST_CR3);
 
+   if (exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) {
+   vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
+   vcpu->run->fail_entry.hardware_entry_failure_reason
+   = exit_reason;
+   return 0;
+   }
+
if (unlikely(vmx->fail)) {
vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
vcpu->run->fail_entry.hardware_entry_failure_reason
-- 
1.7.0.4

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


Re: [PATCH][RESEND] Print a user-friendly message on failed vmentry

2010-06-01 Thread Mohammed Gamal
On Tue, Jun 1, 2010 at 11:59 AM, Avi Kivity  wrote:
> On 05/31/2010 10:40 PM, Mohammed Gamal wrote:
>>
>> This patch address bug report in
>> https://bugs.launchpad.net/qemu/+bug/530077.
>>
>> Failed vmentries were handled with handle_unhandled() which prints a
>> rather
>> unfriendly message to the user. This patch separates handling vmentry
>> failures
>> from unknown exit reasons and prints a friendly message to the user.
>>
>>
>>
>> +#define VMX_INVALID_GUEST_STATE 0x8021
>> +
>> +static int handle_failed_vmentry(uint64_t reason)
>> +{
>> +    fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n",
>> reason);
>> +
>> +    /* Perhaps we will need to check if this machine is intel since exit
>> reason 0x21
>> +       has a different interpretation on SVM */
>> +    if (reason == VMX_INVALID_GUEST_STATE) {
>> +        fprintf(stderr, "If you're runnning a guest on an Intel machine
>> without\n");
>> +        fprintf(stderr, "unrestricted mode support, the failure can be
>> most likely\n");
>> +        fprintf(stderr, "due to the guest entering an invalid state for
>> Intel VT.\n");
>> +        fprintf(stderr, "For example, the guest maybe running in big real
>> mode\n");
>> +        fprintf(stderr, "which is not supported on less recent Intel
>> processors.\n\n");
>> +        fprintf(stderr, "You may want to try enabling KVM real mode
>> emulation. To\n");
>> +        fprintf(stderr, "enable it, you can run the following commands as
>> root:\n");
>> +        fprintf(stderr, "# rmmod kvm_intel\n");
>> +        fprintf(stderr, "# rmmod kvm\n");
>> +        fprintf(stderr, "# modprobe kvm_intel
>> emulate_invalid_guest_state=1\n\n");
>> +        fprintf(stderr, "WARNING: Real mode emulation is still
>> work-in-progress\n");
>> +        fprintf(stderr, "and thus it is not always guaranteed to
>> work.\n\n");
>> +    }
>> +
>> +    return -EINVAL;
>> +}
>>
>>
>
> It's almost guaranteed to fail, isn't it?  Is there any guest which fails
> with emulated_invalid_guest_state=0 but works with e_i_g_s=1?
>
You're right! Perhaps I should remove the e_i_g_s bit from the
message. What do you think?

> --
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Print a user-friendly message on failed vmentry

2010-06-06 Thread Mohammed Gamal
This patch address bug report in https://bugs.launchpad.net/qemu/+bug/530077.

Failed vmentries were handled with handle_unhandled() which prints a rather
unfriendly message to the user. This patch separates handling vmentry failures
from unknown exit reasons and prints a friendly message to the user.

Signed-off-by: Mohammed Gamal 
---
 qemu-kvm.c |   20 +++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 9534b31..1a66f55 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -106,6 +106,24 @@ static int handle_unhandled(uint64_t reason)
 return -EINVAL;
 }
 
+#define VMX_INVALID_GUEST_STATE 0x8021
+
+static int handle_failed_vmentry(uint64_t reason)
+{
+fprintf(stderr, "kvm: vm entry failed with error 0x%" PRIx64 "\n\n", 
reason);
+
+/* Perhaps we will need to check if this machine is intel since exit 
reason 0x21 
+   has a different interpretation on SVM */
+if (reason == VMX_INVALID_GUEST_STATE) {
+fprintf(stderr, "If you're runnning a guest on an Intel machine 
without\n");
+fprintf(stderr, "unrestricted mode support, the failure can be most 
likely\n");
+fprintf(stderr, "due to the guest entering an invalid state for Intel 
VT.\n");
+fprintf(stderr, "For example, the guest maybe running in big real 
mode\n");
+fprintf(stderr, "which is not supported on less recent Intel 
processors.\n\n");
+}
+
+return -EINVAL;
+}
 
 static inline void set_gsi(kvm_context_t kvm, unsigned int gsi)
 {
@@ -586,7 +604,7 @@ int kvm_run(CPUState *env)
 r = handle_unhandled(run->hw.hardware_exit_reason);
 break;
 case KVM_EXIT_FAIL_ENTRY:
-r = 
handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
+r = 
handle_failed_vmentry(run->fail_entry.hardware_entry_failure_reason);
 break;
 case KVM_EXIT_EXCEPTION:
 fprintf(stderr, "exception %d (%x)\n", run->ex.exception,
-- 
1.7.0.4

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


Completing big real mode emulation

2010-03-19 Thread Mohammed Gamal
Hello all,
As some of you might know, I've worked on supporting big real mode
emulation on VMX back in GSoC 2008. Looking at the Qemu GSoC ideas
list for this year, I found it among the possible ideas for a GSoC
project. I'd be interested in driving this feature towards completion,
and I have a few questions about it.

- The kernel-space modifications needed to detect an invalid guest
state on VMX and drive emulation from that point was almost complete.
The part that was missing the most, is that the kvm x86 emulator
wasn't complete and didn't support the entire instruction set. I've
seen that the emulator has been the focus of some recent patches
(namely by Gleb Natapov). Is there anything else required to get big
real mode to work correctly on KVM?

- Do we have other problems supporting big real mode on non-VMX
instruction sets? And do we have problems supporting it on the
userspace side?

- Is there anything I am missing?

Regards,
Mohammed
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Completing big real mode emulation

2010-03-20 Thread Mohammed Gamal
On Sat, Mar 20, 2010 at 3:18 PM, Avi Kivity  wrote:
> On 03/20/2010 10:55 AM, Alexander Graf wrote:
>>>
 I'd say that a GSoC project would rather focus on making a guest OS work
 than working on generic big real mode. Having Windows 98 support is way 
 more
 visible to the users. And hopefully more fun to implement too, as it's a
 visible goal :-).


>>>
>>> Big real mode allows you to boot various OSes, such as that old
>>> Ubuntu/SuSE boot loader which triggered the whole thing.
>>>
>>
>> I thought legacy Windows uses it too?
>>
>
> IIRC even current Windows (last I checked was XP, but it's probably true for
> newer) invokes big real mode inadvertently.  All it takes is not to clear fs
> and gs while switching to real mode.  It works because the real mode code
> never uses gs and fs (i.e. while we are technically in big real mode, the
> guest never relies on this), and because there are enough hacks in vmx.c to
> make it work (restoring fs and gs after the switch back).  IIRC there are
> other cases of invalid guest state that we hack into place during mode
> switches.
>
>> Either way - then we should make the goal of the project to support those
>> old boot loaders. IMHO it should contain visibility. Doing theoretical stuff
>> is just less fun for all parties. Or does that stuff work already?
>>
>
> Mostly those old guests aged beyond usefulness.  They are still broken, but
> nobody installs new images.  Old images installed via workarounds work.
>
> Goals for this task could include:
>
>  - get those older guests working
>  - get emulate_invalid_guest_state=1 to work on all supported guests
>  - switch to emulate_invalid_guest_state=1 as the default
>  - drop the code supporting emulate_invalid_guest_state=0 eventually

To this end I guess the next logical step is to compile a list of
guests that are currently not working/work with hacks only, and get
them working. Here are some suggestions:
- MINIX 3.1.6 (developers have been recently filing bug reports
because of boot failures)
- Win XP with emulation enabled
- FreeDOS with memory extenders

Any other guests you'd like to see on this list?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GSoC 2010] Completing Nested VMX

2010-04-02 Thread Mohammed Gamal
Hello All,
I'm interested in adding nested VMX support to KVM in GSoC 2010 (among
other things). I see that Orit Wasserman has done some work in this
area, but it didn't get merged yet. The last patches were a few months
ago and I have not seen any substantial progress in that front ever
since.

I wonder whether the previous work can be used as a starting ground
for any future effort? What is missing from it? What are the current
limitations of that implementation? And how can it be extended?

And within the scopr of GSoC, what do you think the achievments of
such a project should be?

Regards,
Mohammed
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   >