* [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file
@ 2013-07-31 9:22 Arthur Chunqi Li
2013-08-04 16:54 ` Jan Kiszka
0 siblings, 1 reply; 4+ messages in thread
From: Arthur Chunqi Li @ 2013-07-31 9:22 UTC (permalink / raw)
To: kvm; +Cc: jan.kiszka, gleb, pbonzini, Arthur Chunqi Li
Reconstruct VMX codes and put all VMX test suites in x86/vmx_tests.c.
Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
---
config-x86-common.mak | 2 +-
x86/vmx.c | 71 +++----------------------------------------------
x86/vmx.h | 43 ++++++++++++++++++++++++------
x86/vmx_tests.c | 43 ++++++++++++++++++++++++++++++
x86/vmx_tests.h | 7 +++++
5 files changed, 90 insertions(+), 76 deletions(-)
create mode 100644 x86/vmx_tests.c
create mode 100644 x86/vmx_tests.h
diff --git a/config-x86-common.mak b/config-x86-common.mak
index 34a41e1..bf88c67 100644
--- a/config-x86-common.mak
+++ b/config-x86-common.mak
@@ -101,7 +101,7 @@ $(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
+$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
arch_clean:
$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
diff --git a/x86/vmx.c b/x86/vmx.c
index 082c3bb..397554a 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -6,19 +6,7 @@
#include "msr.h"
#include "smp.h"
#include "io.h"
-
-int fails = 0, tests = 0;
-u32 *vmxon_region;
-struct vmcs *vmcs_root;
-u32 vpid_cnt;
-void *guest_stack, *guest_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;
-struct vmx_test *current;
-u64 hypercall_field = 0;
-bool launched;
+#include "vmx_tests.h"
extern u64 gdt64_desc[];
extern u64 idt_descr[];
@@ -27,17 +15,6 @@ extern void *vmx_return;
extern void *entry_sysenter;
extern void *guest_entry;
-static void report(const char *name, int result)
-{
- ++tests;
- if (result)
- printf("PASS: %s\n", name);
- else {
- printf("FAIL: %s\n", name);
- ++fails;
- }
-}
-
static int make_vmcs_current(struct vmcs *vmcs)
{
bool ret;
@@ -80,7 +57,7 @@ static inline int vmx_off()
return ret;
}
-static void print_vmexit_info()
+void print_vmexit_info()
{
u64 guest_rip, guest_rsp;
ulong reason = vmcs_read(EXI_REASON) & 0xff;
@@ -587,48 +564,6 @@ static void basic_syscall_handler(u64 syscall_no)
{
}
-static void vmenter_main()
-{
- u64 rax;
- u64 rsp, resume_rsp;
-
- report("test vmlaunch", 1);
-
- asm volatile(
- "mov %%rsp, %0\n\t"
- "mov %3, %%rax\n\t"
- "vmcall\n\t"
- "mov %%rax, %1\n\t"
- "mov %%rsp, %2\n\t"
- : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
- : "g"(0xABCD));
- report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
-}
-
-static int vmenter_exit_handler()
-{
- u64 guest_rip;
- ulong reason;
-
- guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
- switch (reason) {
- case VMX_VMCALL:
- if (current->guest_regs.rax != 0xABCD) {
- report("test vmresume", 0);
- return VMX_TEST_VMEXIT;
- }
- current->guest_regs.rax = 0xFFFF;
- vmcs_write(GUEST_RIP, guest_rip + 3);
- return VMX_TEST_RESUME;
- default:
- report("test vmresume", 0);
- print_vmexit_info();
- }
- return VMX_TEST_VMEXIT;
-}
-
-
/* name/init/guest_main/exit_handler/syscall_handler/guest_regs
basic_* just implement some basic functions */
static struct vmx_test vmx_tests[] = {
@@ -644,6 +579,8 @@ int main(void)
setup_vm();
setup_idt();
+ fails = tests = 0;
+ hypercall_field = 0;
if (test_vmx_capability() != 0) {
printf("ERROR : vmx not supported, check +vmx option\n");
diff --git a/x86/vmx.h b/x86/vmx.h
index d80e000..f82bf5a 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -1,5 +1,5 @@
-#ifndef __HYPERVISOR_H
-#define __HYPERVISOR_H
+#ifndef __VMX_H
+#define __VMX_H
#include "libcflat.h"
@@ -41,7 +41,7 @@ struct vmx_test {
int exits;
};
-static union vmx_basic {
+union vmx_basic {
u64 val;
struct {
u32 revision;
@@ -55,35 +55,35 @@ static union vmx_basic {
};
} basic;
-static union vmx_ctrl_pin {
+union vmx_ctrl_pin {
u64 val;
struct {
u32 set, clr;
};
} ctrl_pin_rev;
-static union vmx_ctrl_cpu {
+union vmx_ctrl_cpu {
u64 val;
struct {
u32 set, clr;
};
} ctrl_cpu_rev[2];
-static union vmx_ctrl_exit {
+union vmx_ctrl_exit {
u64 val;
struct {
u32 set, clr;
};
} ctrl_exit_rev;
-static union vmx_ctrl_ent {
+union vmx_ctrl_ent {
u64 val;
struct {
u32 set, clr;
};
} ctrl_enter_rev;
-static union vmx_ept_vpid {
+union vmx_ept_vpid {
u64 val;
struct {
u32:16,
@@ -432,6 +432,20 @@ enum Ctrl1 {
#define HYPERCALL_MASK 0xFFF
#define HYPERCALL_VMEXIT 0x1
+
+u32 *vmxon_region;
+struct vmcs *vmcs_root;
+void *guest_stack, *guest_syscall_stack;
+int fails, tests;
+u64 hypercall_field;
+u32 vpid_cnt;
+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;
+struct vmx_test *current;
+bool launched;
+
static inline int vmcs_clear(struct vmcs *vmcs)
{
bool ret;
@@ -462,5 +476,18 @@ static inline int vmcs_save(struct vmcs **vmcs)
return ret;
}
+static inline void report(const char *name, int result)
+{
+ ++tests;
+ if (result)
+ printf("PASS: %s\n", name);
+ else {
+ printf("FAIL: %s\n", name);
+ ++fails;
+ }
+}
+
+void print_vmexit_info();
+
#endif
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
new file mode 100644
index 0000000..6bd36b0
--- /dev/null
+++ b/x86/vmx_tests.c
@@ -0,0 +1,43 @@
+#include "vmx.h"
+
+void vmenter_main()
+{
+ u64 rax;
+ u64 rsp, resume_rsp;
+
+ report("test vmlaunch", 1);
+
+ asm volatile(
+ "mov %%rsp, %0\n\t"
+ "mov %3, %%rax\n\t"
+ "vmcall\n\t"
+ "mov %%rax, %1\n\t"
+ "mov %%rsp, %2\n\t"
+ : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
+ : "g"(0xABCD));
+ report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
+}
+
+int vmenter_exit_handler()
+{
+ u64 guest_rip;
+ ulong reason;
+
+ guest_rip = vmcs_read(GUEST_RIP);
+ reason = vmcs_read(EXI_REASON) & 0xff;
+ switch (reason) {
+ case VMX_VMCALL:
+ if (current->guest_regs.rax != 0xABCD) {
+ report("test vmresume", 0);
+ return VMX_TEST_VMEXIT;
+ }
+ current->guest_regs.rax = 0xFFFF;
+ vmcs_write(GUEST_RIP, guest_rip + 3);
+ return VMX_TEST_RESUME;
+ default:
+ report("test vmresume", 0);
+ print_vmexit_info();
+ }
+ return VMX_TEST_VMEXIT;
+}
+
diff --git a/x86/vmx_tests.h b/x86/vmx_tests.h
new file mode 100644
index 0000000..41580ff
--- /dev/null
+++ b/x86/vmx_tests.h
@@ -0,0 +1,7 @@
+#ifndef __VMX_TESTS_H
+#define __VMX_TESTS_H
+
+void vmenter_main();
+int vmenter_exit_handler();
+
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file
2013-07-31 9:22 [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file Arthur Chunqi Li
@ 2013-08-04 16:54 ` Jan Kiszka
2013-08-04 17:18 ` Arthur Chunqi Li
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2013-08-04 16:54 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: kvm, gleb, pbonzini
[-- Attachment #1: Type: text/plain, Size: 7849 bytes --]
On 2013-07-31 11:22, Arthur Chunqi Li wrote:
> Reconstruct VMX codes and put all VMX test suites in x86/vmx_tests.c.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> config-x86-common.mak | 2 +-
> x86/vmx.c | 71 +++----------------------------------------------
> x86/vmx.h | 43 ++++++++++++++++++++++++------
> x86/vmx_tests.c | 43 ++++++++++++++++++++++++++++++
> x86/vmx_tests.h | 7 +++++
> 5 files changed, 90 insertions(+), 76 deletions(-)
> create mode 100644 x86/vmx_tests.c
> create mode 100644 x86/vmx_tests.h
>
> diff --git a/config-x86-common.mak b/config-x86-common.mak
> index 34a41e1..bf88c67 100644
> --- a/config-x86-common.mak
> +++ b/config-x86-common.mak
> @@ -101,7 +101,7 @@ $(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
> +$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
>
> arch_clean:
> $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 082c3bb..397554a 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -6,19 +6,7 @@
> #include "msr.h"
> #include "smp.h"
> #include "io.h"
> -
> -int fails = 0, tests = 0;
> -u32 *vmxon_region;
> -struct vmcs *vmcs_root;
> -u32 vpid_cnt;
> -void *guest_stack, *guest_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;
> -struct vmx_test *current;
> -u64 hypercall_field = 0;
> -bool launched;
> +#include "vmx_tests.h"
>
> extern u64 gdt64_desc[];
> extern u64 idt_descr[];
> @@ -27,17 +15,6 @@ extern void *vmx_return;
> extern void *entry_sysenter;
> extern void *guest_entry;
>
> -static void report(const char *name, int result)
> -{
> - ++tests;
> - if (result)
> - printf("PASS: %s\n", name);
> - else {
> - printf("FAIL: %s\n", name);
> - ++fails;
> - }
> -}
> -
> static int make_vmcs_current(struct vmcs *vmcs)
> {
> bool ret;
> @@ -80,7 +57,7 @@ static inline int vmx_off()
> return ret;
> }
>
> -static void print_vmexit_info()
> +void print_vmexit_info()
> {
> u64 guest_rip, guest_rsp;
> ulong reason = vmcs_read(EXI_REASON) & 0xff;
> @@ -587,48 +564,6 @@ static void basic_syscall_handler(u64 syscall_no)
> {
> }
>
> -static void vmenter_main()
> -{
> - u64 rax;
> - u64 rsp, resume_rsp;
> -
> - report("test vmlaunch", 1);
> -
> - asm volatile(
> - "mov %%rsp, %0\n\t"
> - "mov %3, %%rax\n\t"
> - "vmcall\n\t"
> - "mov %%rax, %1\n\t"
> - "mov %%rsp, %2\n\t"
> - : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
> - : "g"(0xABCD));
> - report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
> -}
> -
> -static int vmenter_exit_handler()
> -{
> - u64 guest_rip;
> - ulong reason;
> -
> - guest_rip = vmcs_read(GUEST_RIP);
> - reason = vmcs_read(EXI_REASON) & 0xff;
> - switch (reason) {
> - case VMX_VMCALL:
> - if (current->guest_regs.rax != 0xABCD) {
> - report("test vmresume", 0);
> - return VMX_TEST_VMEXIT;
> - }
> - current->guest_regs.rax = 0xFFFF;
> - vmcs_write(GUEST_RIP, guest_rip + 3);
> - return VMX_TEST_RESUME;
> - default:
> - report("test vmresume", 0);
> - print_vmexit_info();
> - }
> - return VMX_TEST_VMEXIT;
> -}
> -
> -
> /* name/init/guest_main/exit_handler/syscall_handler/guest_regs
> basic_* just implement some basic functions */
> static struct vmx_test vmx_tests[] = {
> @@ -644,6 +579,8 @@ int main(void)
>
> setup_vm();
> setup_idt();
> + fails = tests = 0;
> + hypercall_field = 0;
>
> if (test_vmx_capability() != 0) {
> printf("ERROR : vmx not supported, check +vmx option\n");
> diff --git a/x86/vmx.h b/x86/vmx.h
> index d80e000..f82bf5a 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -1,5 +1,5 @@
> -#ifndef __HYPERVISOR_H
> -#define __HYPERVISOR_H
> +#ifndef __VMX_H
> +#define __VMX_H
>
> #include "libcflat.h"
>
> @@ -41,7 +41,7 @@ struct vmx_test {
> int exits;
> };
>
> -static union vmx_basic {
> +union vmx_basic {
> u64 val;
> struct {
> u32 revision;
> @@ -55,35 +55,35 @@ static union vmx_basic {
> };
> } basic;
extern union vmx_basic basic, and stick the definition in vmx.c. Same
pattern for the other cases.
>
> -static union vmx_ctrl_pin {
> +union vmx_ctrl_pin {
> u64 val;
> struct {
> u32 set, clr;
> };
> } ctrl_pin_rev;
>
> -static union vmx_ctrl_cpu {
> +union vmx_ctrl_cpu {
> u64 val;
> struct {
> u32 set, clr;
> };
> } ctrl_cpu_rev[2];
>
> -static union vmx_ctrl_exit {
> +union vmx_ctrl_exit {
> u64 val;
> struct {
> u32 set, clr;
> };
> } ctrl_exit_rev;
>
> -static union vmx_ctrl_ent {
> +union vmx_ctrl_ent {
> u64 val;
> struct {
> u32 set, clr;
> };
> } ctrl_enter_rev;
>
> -static union vmx_ept_vpid {
> +union vmx_ept_vpid {
> u64 val;
> struct {
> u32:16,
> @@ -432,6 +432,20 @@ enum Ctrl1 {
> #define HYPERCALL_MASK 0xFFF
> #define HYPERCALL_VMEXIT 0x1
>
> +
> +u32 *vmxon_region;
> +struct vmcs *vmcs_root;
> +void *guest_stack, *guest_syscall_stack;
> +int fails, tests;
> +u64 hypercall_field;
> +u32 vpid_cnt;
> +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;
> +struct vmx_test *current;
> +bool launched;
> +
extern <type> <varname>
But do all these variables need to be exported to the test cases? Better
confine it to a minimum, exporting more when test cases come along that
actually need them.
> static inline int vmcs_clear(struct vmcs *vmcs)
> {
> bool ret;
> @@ -462,5 +476,18 @@ static inline int vmcs_save(struct vmcs **vmcs)
> return ret;
> }
>
> +static inline void report(const char *name, int result)
> +{
> + ++tests;
> + if (result)
> + printf("PASS: %s\n", name);
> + else {
> + printf("FAIL: %s\n", name);
> + ++fails;
> + }
> +}
> +
Why static inline? Better leave this as service of the vmx core.
> +void print_vmexit_info();
> +
> #endif
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> new file mode 100644
> index 0000000..6bd36b0
> --- /dev/null
> +++ b/x86/vmx_tests.c
> @@ -0,0 +1,43 @@
> +#include "vmx.h"
> +
> +void vmenter_main()
> +{
> + u64 rax;
> + u64 rsp, resume_rsp;
> +
> + report("test vmlaunch", 1);
> +
> + asm volatile(
> + "mov %%rsp, %0\n\t"
> + "mov %3, %%rax\n\t"
> + "vmcall\n\t"
> + "mov %%rax, %1\n\t"
> + "mov %%rsp, %2\n\t"
> + : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
> + : "g"(0xABCD));
> + report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
> +}
> +
> +int vmenter_exit_handler()
> +{
> + u64 guest_rip;
> + ulong reason;
> +
> + guest_rip = vmcs_read(GUEST_RIP);
> + reason = vmcs_read(EXI_REASON) & 0xff;
> + switch (reason) {
> + case VMX_VMCALL:
> + if (current->guest_regs.rax != 0xABCD) {
> + report("test vmresume", 0);
> + return VMX_TEST_VMEXIT;
> + }
> + current->guest_regs.rax = 0xFFFF;
> + vmcs_write(GUEST_RIP, guest_rip + 3);
> + return VMX_TEST_RESUME;
> + default:
> + report("test vmresume", 0);
> + print_vmexit_info();
> + }
> + return VMX_TEST_VMEXIT;
> +}
> +
> diff --git a/x86/vmx_tests.h b/x86/vmx_tests.h
> new file mode 100644
> index 0000000..41580ff
> --- /dev/null
> +++ b/x86/vmx_tests.h
> @@ -0,0 +1,7 @@
> +#ifndef __VMX_TESTS_H
> +#define __VMX_TESTS_H
> +
> +void vmenter_main();
> +int vmenter_exit_handler();
I'd rather move vmx_tests to, well, vmx_tests.c
> +
> +#endif
>
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file
2013-08-04 16:54 ` Jan Kiszka
@ 2013-08-04 17:18 ` Arthur Chunqi Li
2013-08-04 17:23 ` Jan Kiszka
0 siblings, 1 reply; 4+ messages in thread
From: Arthur Chunqi Li @ 2013-08-04 17:18 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm, Gleb Natapov, Paolo Bonzini
On Mon, Aug 5, 2013 at 12:54 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2013-07-31 11:22, Arthur Chunqi Li wrote:
>> Reconstruct VMX codes and put all VMX test suites in x86/vmx_tests.c.
>>
>> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
>> ---
>> config-x86-common.mak | 2 +-
>> x86/vmx.c | 71 +++----------------------------------------------
>> x86/vmx.h | 43 ++++++++++++++++++++++++------
>> x86/vmx_tests.c | 43 ++++++++++++++++++++++++++++++
>> x86/vmx_tests.h | 7 +++++
>> 5 files changed, 90 insertions(+), 76 deletions(-)
>> create mode 100644 x86/vmx_tests.c
>> create mode 100644 x86/vmx_tests.h
>>
>> diff --git a/config-x86-common.mak b/config-x86-common.mak
>> index 34a41e1..bf88c67 100644
>> --- a/config-x86-common.mak
>> +++ b/config-x86-common.mak
>> @@ -101,7 +101,7 @@ $(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
>> +$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
>>
>> arch_clean:
>> $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>> diff --git a/x86/vmx.c b/x86/vmx.c
>> index 082c3bb..397554a 100644
>> --- a/x86/vmx.c
>> +++ b/x86/vmx.c
>> @@ -6,19 +6,7 @@
>> #include "msr.h"
>> #include "smp.h"
>> #include "io.h"
>> -
>> -int fails = 0, tests = 0;
>> -u32 *vmxon_region;
>> -struct vmcs *vmcs_root;
>> -u32 vpid_cnt;
>> -void *guest_stack, *guest_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;
>> -struct vmx_test *current;
>> -u64 hypercall_field = 0;
>> -bool launched;
>> +#include "vmx_tests.h"
>>
>> extern u64 gdt64_desc[];
>> extern u64 idt_descr[];
>> @@ -27,17 +15,6 @@ extern void *vmx_return;
>> extern void *entry_sysenter;
>> extern void *guest_entry;
>>
>> -static void report(const char *name, int result)
>> -{
>> - ++tests;
>> - if (result)
>> - printf("PASS: %s\n", name);
>> - else {
>> - printf("FAIL: %s\n", name);
>> - ++fails;
>> - }
>> -}
>> -
>> static int make_vmcs_current(struct vmcs *vmcs)
>> {
>> bool ret;
>> @@ -80,7 +57,7 @@ static inline int vmx_off()
>> return ret;
>> }
>>
>> -static void print_vmexit_info()
>> +void print_vmexit_info()
>> {
>> u64 guest_rip, guest_rsp;
>> ulong reason = vmcs_read(EXI_REASON) & 0xff;
>> @@ -587,48 +564,6 @@ static void basic_syscall_handler(u64 syscall_no)
>> {
>> }
>>
>> -static void vmenter_main()
>> -{
>> - u64 rax;
>> - u64 rsp, resume_rsp;
>> -
>> - report("test vmlaunch", 1);
>> -
>> - asm volatile(
>> - "mov %%rsp, %0\n\t"
>> - "mov %3, %%rax\n\t"
>> - "vmcall\n\t"
>> - "mov %%rax, %1\n\t"
>> - "mov %%rsp, %2\n\t"
>> - : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
>> - : "g"(0xABCD));
>> - report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
>> -}
>> -
>> -static int vmenter_exit_handler()
>> -{
>> - u64 guest_rip;
>> - ulong reason;
>> -
>> - guest_rip = vmcs_read(GUEST_RIP);
>> - reason = vmcs_read(EXI_REASON) & 0xff;
>> - switch (reason) {
>> - case VMX_VMCALL:
>> - if (current->guest_regs.rax != 0xABCD) {
>> - report("test vmresume", 0);
>> - return VMX_TEST_VMEXIT;
>> - }
>> - current->guest_regs.rax = 0xFFFF;
>> - vmcs_write(GUEST_RIP, guest_rip + 3);
>> - return VMX_TEST_RESUME;
>> - default:
>> - report("test vmresume", 0);
>> - print_vmexit_info();
>> - }
>> - return VMX_TEST_VMEXIT;
>> -}
>> -
>> -
>> /* name/init/guest_main/exit_handler/syscall_handler/guest_regs
>> basic_* just implement some basic functions */
>> static struct vmx_test vmx_tests[] = {
>> @@ -644,6 +579,8 @@ int main(void)
>>
>> setup_vm();
>> setup_idt();
>> + fails = tests = 0;
>> + hypercall_field = 0;
>>
>> if (test_vmx_capability() != 0) {
>> printf("ERROR : vmx not supported, check +vmx option\n");
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index d80e000..f82bf5a 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -1,5 +1,5 @@
>> -#ifndef __HYPERVISOR_H
>> -#define __HYPERVISOR_H
>> +#ifndef __VMX_H
>> +#define __VMX_H
>>
>> #include "libcflat.h"
>>
>> @@ -41,7 +41,7 @@ struct vmx_test {
>> int exits;
>> };
>>
>> -static union vmx_basic {
>> +union vmx_basic {
>> u64 val;
>> struct {
>> u32 revision;
>> @@ -55,35 +55,35 @@ static union vmx_basic {
>> };
>> } basic;
>
> extern union vmx_basic basic, and stick the definition in vmx.c. Same
> pattern for the other cases.
Here you mean put definition in vmx.c and "extern" definition in vmx.h?
>
>>
>> -static union vmx_ctrl_pin {
>> +union vmx_ctrl_pin {
>> u64 val;
>> struct {
>> u32 set, clr;
>> };
>> } ctrl_pin_rev;
>>
>> -static union vmx_ctrl_cpu {
>> +union vmx_ctrl_cpu {
>> u64 val;
>> struct {
>> u32 set, clr;
>> };
>> } ctrl_cpu_rev[2];
>>
>> -static union vmx_ctrl_exit {
>> +union vmx_ctrl_exit {
>> u64 val;
>> struct {
>> u32 set, clr;
>> };
>> } ctrl_exit_rev;
>>
>> -static union vmx_ctrl_ent {
>> +union vmx_ctrl_ent {
>> u64 val;
>> struct {
>> u32 set, clr;
>> };
>> } ctrl_enter_rev;
>>
>> -static union vmx_ept_vpid {
>> +union vmx_ept_vpid {
>> u64 val;
>> struct {
>> u32:16,
>> @@ -432,6 +432,20 @@ enum Ctrl1 {
>> #define HYPERCALL_MASK 0xFFF
>> #define HYPERCALL_VMEXIT 0x1
>>
>> +
>> +u32 *vmxon_region;
>> +struct vmcs *vmcs_root;
>> +void *guest_stack, *guest_syscall_stack;
>> +int fails, tests;
>> +u64 hypercall_field;
>> +u32 vpid_cnt;
>> +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;
>> +struct vmx_test *current;
>> +bool launched;
>> +
>
> extern <type> <varname>
>
> But do all these variables need to be exported to the test cases? Better
> confine it to a minimum, exporting more when test cases come along that
> actually need them.
>
>> static inline int vmcs_clear(struct vmcs *vmcs)
>> {
>> bool ret;
>> @@ -462,5 +476,18 @@ static inline int vmcs_save(struct vmcs **vmcs)
>> return ret;
>> }
>>
>> +static inline void report(const char *name, int result)
>> +{
>> + ++tests;
>> + if (result)
>> + printf("PASS: %s\n", name);
>> + else {
>> + printf("FAIL: %s\n", name);
>> + ++fails;
>> + }
>> +}
>> +
>
> Why static inline? Better leave this as service of the vmx core.
You mean put it in vmx.c and "extern" it here?
>
>> +void print_vmexit_info();
>> +
>> #endif
>>
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> new file mode 100644
>> index 0000000..6bd36b0
>> --- /dev/null
>> +++ b/x86/vmx_tests.c
>> @@ -0,0 +1,43 @@
>> +#include "vmx.h"
>> +
>> +void vmenter_main()
>> +{
>> + u64 rax;
>> + u64 rsp, resume_rsp;
>> +
>> + report("test vmlaunch", 1);
>> +
>> + asm volatile(
>> + "mov %%rsp, %0\n\t"
>> + "mov %3, %%rax\n\t"
>> + "vmcall\n\t"
>> + "mov %%rax, %1\n\t"
>> + "mov %%rsp, %2\n\t"
>> + : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
>> + : "g"(0xABCD));
>> + report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
>> +}
>> +
>> +int vmenter_exit_handler()
>> +{
>> + u64 guest_rip;
>> + ulong reason;
>> +
>> + guest_rip = vmcs_read(GUEST_RIP);
>> + reason = vmcs_read(EXI_REASON) & 0xff;
>> + switch (reason) {
>> + case VMX_VMCALL:
>> + if (current->guest_regs.rax != 0xABCD) {
>> + report("test vmresume", 0);
>> + return VMX_TEST_VMEXIT;
>> + }
>> + current->guest_regs.rax = 0xFFFF;
>> + vmcs_write(GUEST_RIP, guest_rip + 3);
>> + return VMX_TEST_RESUME;
>> + default:
>> + report("test vmresume", 0);
>> + print_vmexit_info();
>> + }
>> + return VMX_TEST_VMEXIT;
>> +}
>> +
>> diff --git a/x86/vmx_tests.h b/x86/vmx_tests.h
>> new file mode 100644
>> index 0000000..41580ff
>> --- /dev/null
>> +++ b/x86/vmx_tests.h
>> @@ -0,0 +1,7 @@
>> +#ifndef __VMX_TESTS_H
>> +#define __VMX_TESTS_H
>> +
>> +void vmenter_main();
>> +int vmenter_exit_handler();
>
> I'd rather move vmx_tests to, well, vmx_tests.c
I have tried to put vmx_tests in vmx_tests.c and extern definition in
vmx.h. But it is impossible to get its size statically when compiling,
thus the codes "for (i = 1; i < ARRAY_SIZE(vmx_tests); ++i) {" in main
of vmx.c is invalid. If we use ARRAY_SIZE to get its size, we should
make it static.
An alternative solution is put vmx_tests in vmx_tests.h, which will be
included by vmx.c
Arthur
>
>> +
>> +#endif
>>
>
> Jan
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file
2013-08-04 17:18 ` Arthur Chunqi Li
@ 2013-08-04 17:23 ` Jan Kiszka
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kiszka @ 2013-08-04 17:23 UTC (permalink / raw)
To: Arthur Chunqi Li; +Cc: kvm, Gleb Natapov, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 5502 bytes --]
On 2013-08-04 19:18, Arthur Chunqi Li wrote:
>>> diff --git a/x86/vmx.h b/x86/vmx.h
>>> index d80e000..f82bf5a 100644
>>> --- a/x86/vmx.h
>>> +++ b/x86/vmx.h
>>> @@ -1,5 +1,5 @@
>>> -#ifndef __HYPERVISOR_H
>>> -#define __HYPERVISOR_H
>>> +#ifndef __VMX_H
>>> +#define __VMX_H
>>>
>>> #include "libcflat.h"
>>>
>>> @@ -41,7 +41,7 @@ struct vmx_test {
>>> int exits;
>>> };
>>>
>>> -static union vmx_basic {
>>> +union vmx_basic {
>>> u64 val;
>>> struct {
>>> u32 revision;
>>> @@ -55,35 +55,35 @@ static union vmx_basic {
>>> };
>>> } basic;
>>
>> extern union vmx_basic basic, and stick the definition in vmx.c. Same
>> pattern for the other cases.
> Here you mean put definition in vmx.c and "extern" definition in vmx.h?
Yes.
>>
>>>
>>> -static union vmx_ctrl_pin {
>>> +union vmx_ctrl_pin {
>>> u64 val;
>>> struct {
>>> u32 set, clr;
>>> };
>>> } ctrl_pin_rev;
>>>
>>> -static union vmx_ctrl_cpu {
>>> +union vmx_ctrl_cpu {
>>> u64 val;
>>> struct {
>>> u32 set, clr;
>>> };
>>> } ctrl_cpu_rev[2];
>>>
>>> -static union vmx_ctrl_exit {
>>> +union vmx_ctrl_exit {
>>> u64 val;
>>> struct {
>>> u32 set, clr;
>>> };
>>> } ctrl_exit_rev;
>>>
>>> -static union vmx_ctrl_ent {
>>> +union vmx_ctrl_ent {
>>> u64 val;
>>> struct {
>>> u32 set, clr;
>>> };
>>> } ctrl_enter_rev;
>>>
>>> -static union vmx_ept_vpid {
>>> +union vmx_ept_vpid {
>>> u64 val;
>>> struct {
>>> u32:16,
>>> @@ -432,6 +432,20 @@ enum Ctrl1 {
>>> #define HYPERCALL_MASK 0xFFF
>>> #define HYPERCALL_VMEXIT 0x1
>>>
>>> +
>>> +u32 *vmxon_region;
>>> +struct vmcs *vmcs_root;
>>> +void *guest_stack, *guest_syscall_stack;
>>> +int fails, tests;
>>> +u64 hypercall_field;
>>> +u32 vpid_cnt;
>>> +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;
>>> +struct vmx_test *current;
>>> +bool launched;
>>> +
>>
>> extern <type> <varname>
>>
>> But do all these variables need to be exported to the test cases? Better
>> confine it to a minimum, exporting more when test cases come along that
>> actually need them.
>>
>>> static inline int vmcs_clear(struct vmcs *vmcs)
>>> {
>>> bool ret;
>>> @@ -462,5 +476,18 @@ static inline int vmcs_save(struct vmcs **vmcs)
>>> return ret;
>>> }
>>>
>>> +static inline void report(const char *name, int result)
>>> +{
>>> + ++tests;
>>> + if (result)
>>> + printf("PASS: %s\n", name);
>>> + else {
>>> + printf("FAIL: %s\n", name);
>>> + ++fails;
>>> + }
>>> +}
>>> +
>>
>> Why static inline? Better leave this as service of the vmx core.
> You mean put it in vmx.c and "extern" it here?
Yes.
>>
>>> +void print_vmexit_info();
>>> +
>>> #endif
>>>
>>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>>> new file mode 100644
>>> index 0000000..6bd36b0
>>> --- /dev/null
>>> +++ b/x86/vmx_tests.c
>>> @@ -0,0 +1,43 @@
>>> +#include "vmx.h"
>>> +
>>> +void vmenter_main()
>>> +{
>>> + u64 rax;
>>> + u64 rsp, resume_rsp;
>>> +
>>> + report("test vmlaunch", 1);
>>> +
>>> + asm volatile(
>>> + "mov %%rsp, %0\n\t"
>>> + "mov %3, %%rax\n\t"
>>> + "vmcall\n\t"
>>> + "mov %%rax, %1\n\t"
>>> + "mov %%rsp, %2\n\t"
>>> + : "=r"(rsp), "=r"(rax), "=r"(resume_rsp)
>>> + : "g"(0xABCD));
>>> + report("test vmresume", (rax == 0xFFFF) && (rsp == resume_rsp));
>>> +}
>>> +
>>> +int vmenter_exit_handler()
>>> +{
>>> + u64 guest_rip;
>>> + ulong reason;
>>> +
>>> + guest_rip = vmcs_read(GUEST_RIP);
>>> + reason = vmcs_read(EXI_REASON) & 0xff;
>>> + switch (reason) {
>>> + case VMX_VMCALL:
>>> + if (current->guest_regs.rax != 0xABCD) {
>>> + report("test vmresume", 0);
>>> + return VMX_TEST_VMEXIT;
>>> + }
>>> + current->guest_regs.rax = 0xFFFF;
>>> + vmcs_write(GUEST_RIP, guest_rip + 3);
>>> + return VMX_TEST_RESUME;
>>> + default:
>>> + report("test vmresume", 0);
>>> + print_vmexit_info();
>>> + }
>>> + return VMX_TEST_VMEXIT;
>>> +}
>>> +
>>> diff --git a/x86/vmx_tests.h b/x86/vmx_tests.h
>>> new file mode 100644
>>> index 0000000..41580ff
>>> --- /dev/null
>>> +++ b/x86/vmx_tests.h
>>> @@ -0,0 +1,7 @@
>>> +#ifndef __VMX_TESTS_H
>>> +#define __VMX_TESTS_H
>>> +
>>> +void vmenter_main();
>>> +int vmenter_exit_handler();
>>
>> I'd rather move vmx_tests to, well, vmx_tests.c
> I have tried to put vmx_tests in vmx_tests.c and extern definition in
> vmx.h. But it is impossible to get its size statically when compiling,
> thus the codes "for (i = 1; i < ARRAY_SIZE(vmx_tests); ++i) {" in main
> of vmx.c is invalid. If we use ARRAY_SIZE to get its size, we should
> make it static.
>
> An alternative solution is put vmx_tests in vmx_tests.h, which will be
> included by vmx.c
Another alternative is to iterate over vmx_tests until you hit a NULL
entry (one mandatory element of an entry is NULL), i.e. exploring its
size at runtime.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-04 17:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-31 9:22 [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file Arthur Chunqi Li
2013-08-04 16:54 ` Jan Kiszka
2013-08-04 17:18 ` Arthur Chunqi Li
2013-08-04 17:23 ` Jan Kiszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox