On Mon, Aug 5, 2013 at 12:54 AM, Jan Kiszka <jan.kiszka@xxxxxx> 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@xxxxxxxxx>
>> ---
>> 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
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
