Re: [PATCH] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Arthur Chunqi Li
Hi there,
This is a version calling for comments. Some minor changes should be
done before final commitment (TODOs in it), because these places are
related to the bugs I have commited in the previous weeks and the
relevant patches are not accpeted. But these bugs are all about some
unexpected occasions and the hypervisor can run if we simply ignore
them. After all bugs fixed, everything will be OK.

Arthur

On Tue, Jul 16, 2013 at 5:27 PM, Arthur Chunqi Li yzt...@gmail.com wrote:
 This is the first version for VMX nested environment test case. It
 contains the basic VMX instructions test cases, including VMXON/
 VMXOFF/VMXPTRLD/VMXPTRST/VMCLEAR/VMLAUNCH/VMRESUME/VMCALL. This patch
 also tests the basic execution routine in VMX nested environment and
 let the VM print Hello World to inform its successfully run.

 New files added:
 x86/vmx.h : contains all VMX related macro declerations
 x86/vmx.c : main file for VMX nested test case

 Signed-off-by: Arthur Chunqi Li yzt...@gmail.com
 ---
  config-x86-common.mak |2 +
  config-x86_64.mak |1 +
  lib/x86/msr.h |5 +
  x86/cstart64.S|4 +
  x86/unittests.cfg |6 +
  x86/vmx.c |  568 
 +
  x86/vmx.h |  406 +++
  7 files changed, 992 insertions(+)
  create mode 100644 x86/vmx.c
  create mode 100644 x86/vmx.h

 diff --git a/config-x86-common.mak b/config-x86-common.mak
 index 455032b..34a41e1 100644
 --- a/config-x86-common.mak
 +++ b/config-x86-common.mak
 @@ -101,6 +101,8 @@ $(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o

  $(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o

 +$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o
 +
  arch_clean:
 $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
 $(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o
 diff --git a/config-x86_64.mak b/config-x86_64.mak
 index 4e525f5..bb8ee89 100644
 --- a/config-x86_64.mak
 +++ b/config-x86_64.mak
 @@ -9,5 +9,6 @@ tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
   $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
   $(TEST_DIR)/pcid.flat
  tests += $(TEST_DIR)/svm.flat
 +tests += $(TEST_DIR)/vmx.flat

  include config-x86-common.mak
 diff --git a/lib/x86/msr.h b/lib/x86/msr.h
 index 509a421..281255a 100644
 --- a/lib/x86/msr.h
 +++ b/lib/x86/msr.h
 @@ -396,6 +396,11 @@
  #define MSR_IA32_VMX_VMCS_ENUM  0x048a
  #define MSR_IA32_VMX_PROCBASED_CTLS20x048b
  #define MSR_IA32_VMX_EPT_VPID_CAP   0x048c
 +#define MSR_IA32_VMX_TRUE_PIN  0x048d
 +#define MSR_IA32_VMX_TRUE_PROC 0x048e
 +#define MSR_IA32_VMX_TRUE_EXIT 0x048f
 +#define MSR_IA32_VMX_TRUE_ENTRY0x0490
 +

  /* AMD-V MSRs */

 diff --git a/x86/cstart64.S b/x86/cstart64.S
 index 24df5f8..0fe76da 100644
 --- a/x86/cstart64.S
 +++ b/x86/cstart64.S
 @@ -4,6 +4,10 @@
  .globl boot_idt
  boot_idt = 0

 +.globl idt_descr
 +.globl tss_descr
 +.globl gdt64_desc
 +
  ipi_vector = 0x20

  max_cpus = 64
 diff --git a/x86/unittests.cfg b/x86/unittests.cfg
 index bc9643e..e846739 100644
 --- a/x86/unittests.cfg
 +++ b/x86/unittests.cfg
 @@ -149,3 +149,9 @@ extra_params = --append 1000 `date +%s`
  file = pcid.flat
  extra_params = -cpu qemu64,+pcid
  arch = x86_64
 +
 +[vmx]
 +file = vmx.flat
 +extra_params = -cpu Nehalem,+vmx
 +arch = x86_64
 +
 diff --git a/x86/vmx.c b/x86/vmx.c
 new file mode 100644
 index 000..0435746
 --- /dev/null
 +++ b/x86/vmx.c
 @@ -0,0 +1,568 @@
 +#include libcflat.h
 +#include processor.h
 +#include vm.h
 +#include desc.h
 +#include vmx.h
 +#include msr.h
 +#include smp.h
 +#include io.h
 +
 +
 +int fails = 0, tests = 0;
 +u32 *vmxon_region;
 +struct vmcs *vmcs_root;
 +void *io_bmp1, *io_bmp2;
 +void *msr_bmp;
 +u32 vpid_ctr;
 +char *guest_stack, *host_stack;
 +char *guest_syscall_stack, *host_syscall_stack;
 +u32 ctrl_pin, ctrl_enter, ctrl_exit, ctrl_cpu[2];
 +ulong fix_cr0_set, fix_cr0_clr;
 +ulong fix_cr4_set, fix_cr4_clr;
 +struct regs regs;
 +
 +extern u64 gdt64_desc[];
 +extern u64 idt_descr[];
 +extern u64 tss_descr[];
 +extern void *entry_vmx;
 +extern void *entry_sysenter;
 +extern void *entry_guest;
 +
 +void report(const char *name, int result)
 +{
 +   ++tests;
 +   if (result)
 +   printf(PASS: %s\n, name);
 +   else {
 +   printf(FAIL: %s\n, name);
 +   ++fails;
 +   }
 +}
 +
 +inline u64 get_rflags(void)
 +{
 +   u64 r;
 +   asm volatile(pushf; pop %0\n\t : =q(r) : : cc);
 +   return r;
 +}
 +
 +inline void set_rflags(u64 r)
 +{
 +   asm volatile(push %0; popf\n\t : : q(r) : cc);
 +}
 +
 +int vmcs_clear(struct vmcs *vmcs)
 +{
 +   bool ret;
 +   asm volatile (vmclear %1; seta %0 : =q (ret) : m (vmcs) : cc);
 +   return !ret;
 +}
 +
 +u64 vmcs_read(enum Encoding enc)
 +{
 +   u64 val;
 +   asm volatile (vmread 

Re: [PATCH] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Gleb Natapov
On Tue, Jul 16, 2013 at 05:35:16PM +0800, Arthur Chunqi Li wrote:
 Hi there,
 This is a version calling for comments. Some minor changes should be
Add RFC before PATCH for such submission please.

 done before final commitment (TODOs in it), because these places are
 related to the bugs I have commited in the previous weeks and the
 relevant patches are not accpeted. But these bugs are all about some
I am aware of only one patch. Did I miss/forgot something?

 unexpected occasions and the hypervisor can run if we simply ignore
 them. After all bugs fixed, everything will be OK.
 
 Arthur
 

--
Gleb.
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Arthur Chunqi Li
On Tue, Jul 16, 2013 at 5:45 PM, Gleb Natapov g...@redhat.com wrote:
 On Tue, Jul 16, 2013 at 05:35:16PM +0800, Arthur Chunqi Li wrote:
 Hi there,
 This is a version calling for comments. Some minor changes should be
 Add RFC before PATCH for such submission please.

 done before final commitment (TODOs in it), because these places are
 related to the bugs I have commited in the previous weeks and the
 relevant patches are not accpeted. But these bugs are all about some
 I am aware of only one patch. Did I miss/forgot something?
I have only commit one patch related to VMX tests. Other patches cited
here is about the bug fixes such as
[http://www.mail-archive.com/kvm@vger.kernel.org/msg92932.html]
[http://www.mail-archive.com/kvm@vger.kernel.org/msg93046.html], which
will cause some test cases fail in this patch.

I have been discussing with Jan in the past weeks because most of the
questions are technical affairs, which I don't think it's suitable to
discuss in the community. This is a rather mature version and I think
I can commit it with some minor changes.

 unexpected occasions and the hypervisor can run if we simply ignore
 them. After all bugs fixed, everything will be OK.

 Arthur


 --
 Gleb.
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Gleb Natapov
On Tue, Jul 16, 2013 at 05:53:56PM +0800, Arthur Chunqi Li wrote:
 On Tue, Jul 16, 2013 at 5:45 PM, Gleb Natapov g...@redhat.com wrote:
  On Tue, Jul 16, 2013 at 05:35:16PM +0800, Arthur Chunqi Li wrote:
  Hi there,
  This is a version calling for comments. Some minor changes should be
  Add RFC before PATCH for such submission please.
 
  done before final commitment (TODOs in it), because these places are
  related to the bugs I have commited in the previous weeks and the
  relevant patches are not accpeted. But these bugs are all about some
  I am aware of only one patch. Did I miss/forgot something?
 I have only commit one patch related to VMX tests. Other patches cited
 here is about the bug fixes such as
 [http://www.mail-archive.com/kvm@vger.kernel.org/msg92932.html]
 [http://www.mail-archive.com/kvm@vger.kernel.org/msg93046.html], which
 will cause some test cases fail in this patch.
 
Those are applied to queue already. I am aware of one that is not
applied yet but it looks fine to me, waiting for Paolo's review.

 I have been discussing with Jan in the past weeks because most of the
 questions are technical affairs, which I don't think it's suitable to
 discuss in the community. This is a rather mature version and I think
 I can commit it with some minor changes.
 
  unexpected occasions and the hypervisor can run if we simply ignore
  them. After all bugs fixed, everything will be OK.
 
  Arthur
 
 
  --
  Gleb.

--
Gleb.
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Paolo Bonzini
Il 16/07/2013 11:27, Arthur Chunqi Li ha scritto:
 This is the first version for VMX nested environment test case. It
 contains the basic VMX instructions test cases, including VMXON/
 VMXOFF/VMXPTRLD/VMXPTRST/VMCLEAR/VMLAUNCH/VMRESUME/VMCALL. This patch
 also tests the basic execution routine in VMX nested environment and
 let the VM print Hello World to inform its successfully run.
 
 New files added:
 x86/vmx.h : contains all VMX related macro declerations
 x86/vmx.c : main file for VMX nested test case
 
 Signed-off-by: Arthur Chunqi Li yzt...@gmail.com
 ---
  config-x86-common.mak |2 +
  config-x86_64.mak |1 +
  lib/x86/msr.h |5 +
  x86/cstart64.S|4 +
  x86/unittests.cfg |6 +
  x86/vmx.c |  568 
 +
  x86/vmx.h |  406 +++
  7 files changed, 992 insertions(+)
  create mode 100644 x86/vmx.c
  create mode 100644 x86/vmx.h
 
 diff --git a/config-x86-common.mak b/config-x86-common.mak
 index 455032b..34a41e1 100644
 --- a/config-x86-common.mak
 +++ b/config-x86-common.mak
 @@ -101,6 +101,8 @@ $(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
  
  $(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
  
 +$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o
 +
  arch_clean:
   $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
   $(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o
 diff --git a/config-x86_64.mak b/config-x86_64.mak
 index 4e525f5..bb8ee89 100644
 --- a/config-x86_64.mak
 +++ b/config-x86_64.mak
 @@ -9,5 +9,6 @@ tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
 $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
 $(TEST_DIR)/pcid.flat
  tests += $(TEST_DIR)/svm.flat
 +tests += $(TEST_DIR)/vmx.flat
  
  include config-x86-common.mak
 diff --git a/lib/x86/msr.h b/lib/x86/msr.h
 index 509a421..281255a 100644
 --- a/lib/x86/msr.h
 +++ b/lib/x86/msr.h
 @@ -396,6 +396,11 @@
  #define MSR_IA32_VMX_VMCS_ENUM  0x048a
  #define MSR_IA32_VMX_PROCBASED_CTLS20x048b
  #define MSR_IA32_VMX_EPT_VPID_CAP   0x048c
 +#define MSR_IA32_VMX_TRUE_PIN0x048d
 +#define MSR_IA32_VMX_TRUE_PROC   0x048e
 +#define MSR_IA32_VMX_TRUE_EXIT   0x048f
 +#define MSR_IA32_VMX_TRUE_ENTRY  0x0490
 +
  
  /* AMD-V MSRs */
  
 diff --git a/x86/cstart64.S b/x86/cstart64.S
 index 24df5f8..0fe76da 100644
 --- a/x86/cstart64.S
 +++ b/x86/cstart64.S
 @@ -4,6 +4,10 @@
  .globl boot_idt
  boot_idt = 0
  
 +.globl idt_descr
 +.globl tss_descr
 +.globl gdt64_desc
 +
  ipi_vector = 0x20
  
  max_cpus = 64
 diff --git a/x86/unittests.cfg b/x86/unittests.cfg
 index bc9643e..e846739 100644
 --- a/x86/unittests.cfg
 +++ b/x86/unittests.cfg
 @@ -149,3 +149,9 @@ extra_params = --append 1000 `date +%s`
  file = pcid.flat
  extra_params = -cpu qemu64,+pcid
  arch = x86_64
 +
 +[vmx]
 +file = vmx.flat
 +extra_params = -cpu Nehalem,+vmx

Should this use -cpu host instead? (Or -cpu host,+vmx, I don't
remember).

 +arch = x86_64
 +
 diff --git a/x86/vmx.c b/x86/vmx.c
 new file mode 100644
 index 000..0435746
 --- /dev/null
 +++ b/x86/vmx.c
 @@ -0,0 +1,568 @@
 +#include libcflat.h
 +#include processor.h
 +#include vm.h
 +#include desc.h
 +#include vmx.h
 +#include msr.h
 +#include smp.h
 +#include io.h
 +
 +
 +int fails = 0, tests = 0;
 +u32 *vmxon_region;
 +struct vmcs *vmcs_root;
 +void *io_bmp1, *io_bmp2;
 +void *msr_bmp;
 +u32 vpid_ctr;
 +char *guest_stack, *host_stack;
 +char *guest_syscall_stack, *host_syscall_stack;
 +u32 ctrl_pin, ctrl_enter, ctrl_exit, ctrl_cpu[2];
 +ulong fix_cr0_set, fix_cr0_clr;
 +ulong fix_cr4_set, fix_cr4_clr;
 +struct regs regs;
 +
 +extern u64 gdt64_desc[];
 +extern u64 idt_descr[];
 +extern u64 tss_descr[];
 +extern void *entry_vmx;
 +extern void *entry_sysenter;
 +extern void *entry_guest;
 +
 +void report(const char *name, int result)
 +{
 + ++tests;
 + if (result)
 + printf(PASS: %s\n, name);
 + else {
 + printf(FAIL: %s\n, name);
 + ++fails;
 + }
 +}
 +
 +inline u64 get_rflags(void)
 +{
 + u64 r;
 + asm volatile(pushf; pop %0\n\t : =q(r) : : cc);
 + return r;
 +}
 +
 +inline void set_rflags(u64 r)
 +{
 + asm volatile(push %0; popf\n\t : : q(r) : cc);
 +}
 +
 +int vmcs_clear(struct vmcs *vmcs)
 +{
 + bool ret;
 + asm volatile (vmclear %1; seta %0 : =q (ret) : m (vmcs) : cc);

You can use setbe, it's clearer and avoids the ! in the return statement.

We should later add tests for failure conditions, since failing to
detect errors could give rise to L2-L1 attack vectors.  When we do so,
we will have to distinguish CF from ZF.


 + return !ret;
 +}
 +
 +u64 vmcs_read(enum Encoding enc)
 +{
 + u64 val;
 + asm volatile (vmread %1, %0 : =rm (val) : r ((u64)enc) : cc);
 + return val;
 +}
 +
 +int vmcs_write(enum Encoding enc, u64 val)
 

Re: [PATCH] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Arthur Chunqi Li
Hi Paolo,

On Tue, Jul 16, 2013 at 6:28 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 16/07/2013 11:27, Arthur Chunqi Li ha scritto:
 This is the first version for VMX nested environment test case. It
 contains the basic VMX instructions test cases, including VMXON/
 VMXOFF/VMXPTRLD/VMXPTRST/VMCLEAR/VMLAUNCH/VMRESUME/VMCALL. This patch
 also tests the basic execution routine in VMX nested environment and
 let the VM print Hello World to inform its successfully run.

 New files added:
 x86/vmx.h : contains all VMX related macro declerations
 x86/vmx.c : main file for VMX nested test case

 Signed-off-by: Arthur Chunqi Li yzt...@gmail.com
 ---
  config-x86-common.mak |2 +
  config-x86_64.mak |1 +
  lib/x86/msr.h |5 +
  x86/cstart64.S|4 +
  x86/unittests.cfg |6 +
  x86/vmx.c |  568 
 +
  x86/vmx.h |  406 +++
  7 files changed, 992 insertions(+)
  create mode 100644 x86/vmx.c
  create mode 100644 x86/vmx.h

 diff --git a/config-x86-common.mak b/config-x86-common.mak
 index 455032b..34a41e1 100644
 --- a/config-x86-common.mak
 +++ b/config-x86-common.mak
 @@ -101,6 +101,8 @@ $(TEST_DIR)/asyncpf.elf: $(cstart.o) 
 $(TEST_DIR)/asyncpf.o

  $(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o

 +$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o
 +
  arch_clean:
   $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
   $(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o
 diff --git a/config-x86_64.mak b/config-x86_64.mak
 index 4e525f5..bb8ee89 100644
 --- a/config-x86_64.mak
 +++ b/config-x86_64.mak
 @@ -9,5 +9,6 @@ tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
 $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
 $(TEST_DIR)/pcid.flat
  tests += $(TEST_DIR)/svm.flat
 +tests += $(TEST_DIR)/vmx.flat

  include config-x86-common.mak
 diff --git a/lib/x86/msr.h b/lib/x86/msr.h
 index 509a421..281255a 100644
 --- a/lib/x86/msr.h
 +++ b/lib/x86/msr.h
 @@ -396,6 +396,11 @@
  #define MSR_IA32_VMX_VMCS_ENUM  0x048a
  #define MSR_IA32_VMX_PROCBASED_CTLS20x048b
  #define MSR_IA32_VMX_EPT_VPID_CAP   0x048c
 +#define MSR_IA32_VMX_TRUE_PIN0x048d
 +#define MSR_IA32_VMX_TRUE_PROC   0x048e
 +#define MSR_IA32_VMX_TRUE_EXIT   0x048f
 +#define MSR_IA32_VMX_TRUE_ENTRY  0x0490
 +

  /* AMD-V MSRs */

 diff --git a/x86/cstart64.S b/x86/cstart64.S
 index 24df5f8..0fe76da 100644
 --- a/x86/cstart64.S
 +++ b/x86/cstart64.S
 @@ -4,6 +4,10 @@
  .globl boot_idt
  boot_idt = 0

 +.globl idt_descr
 +.globl tss_descr
 +.globl gdt64_desc
 +
  ipi_vector = 0x20

  max_cpus = 64
 diff --git a/x86/unittests.cfg b/x86/unittests.cfg
 index bc9643e..e846739 100644
 --- a/x86/unittests.cfg
 +++ b/x86/unittests.cfg
 @@ -149,3 +149,9 @@ extra_params = --append 1000 `date +%s`
  file = pcid.flat
  extra_params = -cpu qemu64,+pcid
  arch = x86_64
 +
 +[vmx]
 +file = vmx.flat
 +extra_params = -cpu Nehalem,+vmx

 Should this use -cpu host instead? (Or -cpu host,+vmx, I don't
 remember).

 +arch = x86_64
 +
 diff --git a/x86/vmx.c b/x86/vmx.c
 new file mode 100644
 index 000..0435746
 --- /dev/null
 +++ b/x86/vmx.c
 @@ -0,0 +1,568 @@
 +#include libcflat.h
 +#include processor.h
 +#include vm.h
 +#include desc.h
 +#include vmx.h
 +#include msr.h
 +#include smp.h
 +#include io.h
 +
 +
 +int fails = 0, tests = 0;
 +u32 *vmxon_region;
 +struct vmcs *vmcs_root;
 +void *io_bmp1, *io_bmp2;
 +void *msr_bmp;
 +u32 vpid_ctr;
 +char *guest_stack, *host_stack;
 +char *guest_syscall_stack, *host_syscall_stack;
 +u32 ctrl_pin, ctrl_enter, ctrl_exit, ctrl_cpu[2];
 +ulong fix_cr0_set, fix_cr0_clr;
 +ulong fix_cr4_set, fix_cr4_clr;
 +struct regs regs;
 +
 +extern u64 gdt64_desc[];
 +extern u64 idt_descr[];
 +extern u64 tss_descr[];
 +extern void *entry_vmx;
 +extern void *entry_sysenter;
 +extern void *entry_guest;
 +
 +void report(const char *name, int result)
 +{
 + ++tests;
 + if (result)
 + printf(PASS: %s\n, name);
 + else {
 + printf(FAIL: %s\n, name);
 + ++fails;
 + }
 +}
 +
 +inline u64 get_rflags(void)
 +{
 + u64 r;
 + asm volatile(pushf; pop %0\n\t : =q(r) : : cc);
 + return r;
 +}
 +
 +inline void set_rflags(u64 r)
 +{
 + asm volatile(push %0; popf\n\t : : q(r) : cc);
 +}
 +
 +int vmcs_clear(struct vmcs *vmcs)
 +{
 + bool ret;
 + asm volatile (vmclear %1; seta %0 : =q (ret) : m (vmcs) : cc);

 You can use setbe, it's clearer and avoids the ! in the return statement.

 We should later add tests for failure conditions, since failing to
 detect errors could give rise to L2-L1 attack vectors.  When we do so,
 we will have to distinguish CF from ZF.


 + return !ret;
 +}
 +
 +u64 vmcs_read(enum Encoding enc)
 +{
 + u64 val;
 + asm volatile (vmread %1, %0 : =rm (val) : r ((u64)enc) : 

Re: [PATCH] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Paolo Bonzini
Il 16/07/2013 13:47, Arthur Chunqi Li ha scritto:
 setjmp and longjmp is not implemented in our test environment, and
 these two functions are highly depend on architecture. Do you think we
 need to write a general code for both 32bit and 64bit, or just write
 specific one just for this test case?

Supporting it in x86-64 is enough for now.

Paolo
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Arthur Chunqi Li
On Tue, Jul 16, 2013 at 11:20 PM, Gleb Natapov g...@redhat.com wrote:
 On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote:
  +void vmx_exit(void)
  +{
  +   test_vmxoff();
  +   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
  +   exit(fails ? -1 : 0);
  +}

 Can you try to jump back to main, and do test_vmxoff there?  This will
 avoid having to write our tests in callback style, which is a pain.
 Basically something similar to setjmp/longjmp.  In main:

   if (setjmp(jmpbuf) == 0) {
   vmx_run();
   /* Should not reach here */
   report(test vmlaunch, 0);
   }
   test_vmxoff();

 exit:
   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
   return fails ? 1 : 0;

 In vmx_handler:

   case VMX_HLT:
   printf(\nVM exit.\n);
   longjmp(jmpbuf, 1);

 Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It
 will make code much more straightforward and easer to follow.
The concept easier to follow may have different meanings in
different view. This achievement puts all the test cases in main
function instead of scattering everywhere, which is another view to
easy to follow. As this is just a test case, I prefer this one.

Besides, this way we can start another VM following the previous one
simply in main function. This is flexible if we want to test re-enter
to VMX mode or so.

Arthur

 --
 Gleb.



--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Gleb Natapov
On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote:
  +void vmx_exit(void)
  +{
  +   test_vmxoff();
  +   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
  +   exit(fails ? -1 : 0);
  +}
 
 Can you try to jump back to main, and do test_vmxoff there?  This will
 avoid having to write our tests in callback style, which is a pain.
 Basically something similar to setjmp/longjmp.  In main:
 
   if (setjmp(jmpbuf) == 0) {
   vmx_run();
   /* Should not reach here */
   report(test vmlaunch, 0);
   }
   test_vmxoff();
 
 exit:
   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
   return fails ? 1 : 0;
 
 In vmx_handler:
 
   case VMX_HLT:
   printf(\nVM exit.\n);
   longjmp(jmpbuf, 1);
 
Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It
will make code much more straightforward and easer to follow.

--
Gleb.
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Gleb Natapov
On Tue, Jul 16, 2013 at 11:29:20PM +0800, Arthur Chunqi Li wrote:
 On Tue, Jul 16, 2013 at 11:20 PM, Gleb Natapov g...@redhat.com wrote:
  On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote:
   +void vmx_exit(void)
   +{
   +   test_vmxoff();
   +   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
   +   exit(fails ? -1 : 0);
   +}
 
  Can you try to jump back to main, and do test_vmxoff there?  This will
  avoid having to write our tests in callback style, which is a pain.
  Basically something similar to setjmp/longjmp.  In main:
 
if (setjmp(jmpbuf) == 0) {
vmx_run();
/* Should not reach here */
report(test vmlaunch, 0);
}
test_vmxoff();
 
  exit:
printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
return fails ? 1 : 0;
 
  In vmx_handler:
 
case VMX_HLT:
printf(\nVM exit.\n);
longjmp(jmpbuf, 1);
 
  Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It
  will make code much more straightforward and easer to follow.
 The concept easier to follow may have different meanings in
 different view. This achievement puts all the test cases in main
 function instead of scattering everywhere, which is another view to
 easy to follow. As this is just a test case, I prefer this one.
 
I do not see why what I propose will prevent you to put all tests into main.

vmx_run() will looks like that:

   vmlaunch
   while(1) {
   vmresume
  vmexit jumps here
   switch(exit reason) {
  case reason1:
  break;
  case reason2:
  break;
  case HLT
  return;
   }
   }  
  
 Besides, this way we can start another VM following the previous one
 simply in main function. This is flexible if we want to test re-enter
 to VMX mode or so.
 
That's what I am missing. How do one writes more tests now?

I was thinking about interface like that:

guest_func_test1()
{
}

tes1t_exit_handlers[] = {test1_handle_hlt, test1_handle_exception, }

main()
{

   init_vmcs(); /* generic stuff */
   init_vmcs_test1(); /* test1 related stuff */
   r = run_in_guest(guest_func_test1, test1_exit_handlers);
   report(test1, r);
}

--
Gleb.
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Arthur Chunqi Li
On Wed, Jul 17, 2013 at 12:45 AM, Gleb Natapov g...@redhat.com wrote:
 On Tue, Jul 16, 2013 at 11:29:20PM +0800, Arthur Chunqi Li wrote:
 On Tue, Jul 16, 2013 at 11:20 PM, Gleb Natapov g...@redhat.com wrote:
  On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote:
   +void vmx_exit(void)
   +{
   +   test_vmxoff();
   +   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
   +   exit(fails ? -1 : 0);
   +}
 
  Can you try to jump back to main, and do test_vmxoff there?  This will
  avoid having to write our tests in callback style, which is a pain.
  Basically something similar to setjmp/longjmp.  In main:
 
if (setjmp(jmpbuf) == 0) {
vmx_run();
/* Should not reach here */
report(test vmlaunch, 0);
}
test_vmxoff();
 
  exit:
printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
return fails ? 1 : 0;
 
  In vmx_handler:
 
case VMX_HLT:
printf(\nVM exit.\n);
longjmp(jmpbuf, 1);
 
  Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It
  will make code much more straightforward and easer to follow.
 The concept easier to follow may have different meanings in
 different view. This achievement puts all the test cases in main
 function instead of scattering everywhere, which is another view to
 easy to follow. As this is just a test case, I prefer this one.

 I do not see why what I propose will prevent you to put all tests into main.

 vmx_run() will looks like that:

vmlaunch
while(1) {
vmresume
   vmexit jumps here
switch(exit reason) {
   case reason1:
   break;
   case reason2:
   break;
   case HLT
   return;
}
}
Yes, this recalls me some KVM codes I have read before. This mixes
vmlaunch/resume and vmx_handler into one piece of code. It is a good
way to explicitly show the execution sequence though, it increases LOC
in one function.

 Besides, this way we can start another VM following the previous one
 simply in main function. This is flexible if we want to test re-enter
 to VMX mode or so.

 That's what I am missing. How do one writes more tests now?

 I was thinking about interface like that:

 guest_func_test1()
 {
 }

 tes1t_exit_handlers[] = {test1_handle_hlt, test1_handle_exception, }

 main()
 {

init_vmcs(); /* generic stuff */
init_vmcs_test1(); /* test1 related stuff */
r = run_in_guest(guest_func_test1, test1_exit_handlers);
report(test1, r);
 }

I have thought about this question and I'm not quite sure how to solve
it now. I have two ways. The first is that we just leave vmx.c as the
VMX instructions and execution routine test suite, and develop other
test cases in other files. Since all other tests of nested vmx is
independent to the basic routine and it is hard for us to put all test
cases for nested VMX in one file, so we just let this file do simple
things and reuse some of its functions in other test suites of nested
vmx. Your proposal of adding new test cases can be implemented in
other test suites.

The other way is not splitting nested vmx tests cases in contrast.
This method may cause a HUGE vmx.c file, and tests for different parts
are not distinctive.

Actually, I prefer the former solution.
 --
 Gleb.


Besides, there is also another pseudo bug in PATCH 2/2, here:

+int test_vmx_capability(void)
+{
+   struct cpuid r;
+   u64 ret1, ret2;
+   r = cpuid(1);
+   ret1 = ((r.c)  5)  1;
+   ret2 = ((rdmsr(MSR_IA32_FEATURE_CONTROL)  0x5) == 0x5);
+   report(test vmx capability, ret1  ret2);
+   return !(ret1  ret2);
+}

The IA32_FEATURE_CONTROL MSR should be set by seabios. SInce there's
no patch for seabios and in fact software can also set this MSR if its
lock bit is unset. So I prefer to change it like this:

+int test_vmx_capability(void)
+{
+   struct cpuid r;
+   u64 ret1, ret2;
+   u64 ia32_feature_control;
+   r = cpuid(1);
+   ret1 = ((r.c)  5)  1;
+   ia32_feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
+   ret2 = ((ia32_feature_control  0x5) == 0x5);
+   if ((!ret2)  ((ia32_feature_control  0x1) == 0)){
+   wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
+   ia32_feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
+   ret2 = ((ia32_feature_control  0x5) == 0x5);
+   }
+   report(test vmx capability, ret1  ret2);
+   return !(ret1  ret2);
+}


Arthur
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Paolo Bonzini
Il 16/07/2013 19:13, Arthur Chunqi Li ha scritto:
 On Wed, Jul 17, 2013 at 12:45 AM, Gleb Natapov g...@redhat.com wrote:
 On Tue, Jul 16, 2013 at 11:29:20PM +0800, Arthur Chunqi Li wrote:
 On Tue, Jul 16, 2013 at 11:20 PM, Gleb Natapov g...@redhat.com wrote:
 On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote:
 +void vmx_exit(void)
 +{
 +   test_vmxoff();
 +   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
 +   exit(fails ? -1 : 0);
 +}

 Can you try to jump back to main, and do test_vmxoff there?  This will
 avoid having to write our tests in callback style, which is a pain.
 Basically something similar to setjmp/longjmp.  In main:

   if (setjmp(jmpbuf) == 0) {
   vmx_run();
   /* Should not reach here */
   report(test vmlaunch, 0);
   }
   test_vmxoff();

 exit:
   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
   return fails ? 1 : 0;

 In vmx_handler:

   case VMX_HLT:
   printf(\nVM exit.\n);
   longjmp(jmpbuf, 1);

 Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It
 will make code much more straightforward and easer to follow.
 The concept easier to follow may have different meanings in
 different view. This achievement puts all the test cases in main
 function instead of scattering everywhere, which is another view to
 easy to follow. As this is just a test case, I prefer this one.

 I do not see why what I propose will prevent you to put all tests into main.

 vmx_run() will looks like that:

vmlaunch
while(1) {
vmresume
   vmexit jumps here
switch(exit reason) {
   case reason1:
   break;
   case reason2:
   break;
   case HLT
   return;
}
}
 Yes, this recalls me some KVM codes I have read before. This mixes
 vmlaunch/resume and vmx_handler into one piece of code. It is a good
 way to explicitly show the execution sequence though, it increases LOC
 in one function.

 Besides, this way we can start another VM following the previous one
 simply in main function. This is flexible if we want to test re-enter
 to VMX mode or so.

 That's what I am missing. How do one writes more tests now?

 I was thinking about interface like that:

 guest_func_test1()
 {
 }

 tes1t_exit_handlers[] = {test1_handle_hlt, test1_handle_exception, }

 main()
 {

init_vmcs(); /* generic stuff */
init_vmcs_test1(); /* test1 related stuff */
r = run_in_guest(guest_func_test1, test1_exit_handlers);
report(test1, r);
 }

 I have thought about this question and I'm not quite sure how to solve
 it now.

Why can't you just use a different vmx_handler (e.g. with an indirect
call in entry_vmx) for each test (as in Gleb's test1_exit_handlers)?
run_in_guest would prepare the function pointers and do

init_vmcs(vmcs_root);

if (setjmp(env) == 0){
vmx_run();
/* Should not reach here */
report(test vmlaunch, 0);
}

as in your current testcase.

vmx.c would be a library, and testcases could be either grouped in the
same file or spread across many of them, as you see fit.

Paolo
--
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] kvm-unit-tests : The first version of VMX nested test case

2013-07-16 Thread Gleb Natapov
On Wed, Jul 17, 2013 at 01:13:56AM +0800, Arthur Chunqi Li wrote:
 On Wed, Jul 17, 2013 at 12:45 AM, Gleb Natapov g...@redhat.com wrote:
  On Tue, Jul 16, 2013 at 11:29:20PM +0800, Arthur Chunqi Li wrote:
  On Tue, Jul 16, 2013 at 11:20 PM, Gleb Natapov g...@redhat.com wrote:
   On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote:
+void vmx_exit(void)
+{
+   test_vmxoff();
+   printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
+   exit(fails ? -1 : 0);
+}
  
   Can you try to jump back to main, and do test_vmxoff there?  This will
   avoid having to write our tests in callback style, which is a pain.
   Basically something similar to setjmp/longjmp.  In main:
  
 if (setjmp(jmpbuf) == 0) {
 vmx_run();
 /* Should not reach here */
 report(test vmlaunch, 0);
 }
 test_vmxoff();
  
   exit:
 printf(\nSUMMARY: %d tests, %d failures\n, tests, fails);
 return fails ? 1 : 0;
  
   In vmx_handler:
  
 case VMX_HLT:
 printf(\nVM exit.\n);
 longjmp(jmpbuf, 1);
  
   Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It
   will make code much more straightforward and easer to follow.
  The concept easier to follow may have different meanings in
  different view. This achievement puts all the test cases in main
  function instead of scattering everywhere, which is another view to
  easy to follow. As this is just a test case, I prefer this one.
 
  I do not see why what I propose will prevent you to put all tests into main.
 
  vmx_run() will looks like that:
 
 vmlaunch
 while(1) {
 vmresume
    vmexit jumps here
 switch(exit reason) {
case reason1:
break;
case reason2:
break;
case HLT
return;
 }
 }
 Yes, this recalls me some KVM codes I have read before. This mixes
 vmlaunch/resume and vmx_handler into one piece of code. It is a good
 way to explicitly show the execution sequence though, it increases LOC
 in one function.
LOC in one function is not an issue to be considered at all. Besides you
can put the switch into separate vmx_handler() function, or have an
array of vmexits.

 
  Besides, this way we can start another VM following the previous one
  simply in main function. This is flexible if we want to test re-enter
  to VMX mode or so.
 
  That's what I am missing. How do one writes more tests now?
 
  I was thinking about interface like that:
 
  guest_func_test1()
  {
  }
 
  tes1t_exit_handlers[] = {test1_handle_hlt, test1_handle_exception, }
 
  main()
  {
 
 init_vmcs(); /* generic stuff */
 init_vmcs_test1(); /* test1 related stuff */
 r = run_in_guest(guest_func_test1, test1_exit_handlers);
 report(test1, r);
  }
 
 I have thought about this question and I'm not quite sure how to solve
 it now. I have two ways. The first is that we just leave vmx.c as the
 VMX instructions and execution routine test suite, and develop other
 test cases in other files. Since all other tests of nested vmx is
 independent to the basic routine and it is hard for us to put all test
 cases for nested VMX in one file, so we just let this file do simple
 things and reuse some of its functions in other test suites of nested
 vmx. Your proposal of adding new test cases can be implemented in
 other test suites.
 
 The other way is not splitting nested vmx tests cases in contrast.
 This method may cause a HUGE vmx.c file, and tests for different parts
 are not distinctive.
 
 Actually, I prefer the former solution.
I do not think we need separate infrastructure just to test basic
instructions. Actually testing those are less interesting part (well
just to sorely test vmlaunch/vmresume we will likely have to write
hundred of tests to verify that all things that should cause failure do
that, but we likely settle for only a couple). I am not worrying about
vmx.c become a huge file as long as writing test is easy and more or
less self contained task.

--
Gleb.
--
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