From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] kvm-unit-tests: VMX: Split VMX test suites to separate file Date: Sun, 04 Aug 2013 18:54:10 +0200 Message-ID: <51FE8732.6080003@web.de> References: <1375262546-32151-1-git-send-email-yzt356@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="2nl3JMnc4rpvuRqjnTBFbiplBUsFrsRm1" Cc: kvm@vger.kernel.org, gleb@redhat.com, pbonzini@redhat.com To: Arthur Chunqi Li Return-path: Received: from mout.web.de ([212.227.15.4]:53207 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753689Ab3HDQyN (ORCPT ); Sun, 4 Aug 2013 12:54:13 -0400 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb102) with ESMTPSA (Nemesis) id 0LiUG8-1UTOV526to-00cjEr for ; Sun, 04 Aug 2013 18:54:12 +0200 In-Reply-To: <1375262546-32151-1-git-send-email-yzt356@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --2nl3JMnc4rpvuRqjnTBFbiplBUsFrsRm1 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2013-07-31 11:22, Arthur Chunqi Li wrote: > Reconstruct VMX codes and put all VMX test suites in x86/vmx_tests.c. >=20 > Signed-off-by: Arthur Chunqi Li > --- > 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 >=20 > 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)/as= yncpf.o > =20 > $(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o > =20 > -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o > +$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tes= ts.o > =20 > 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 =3D 0, tests =3D 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 =3D 0; > -bool launched; > +#include "vmx_tests.h" > =20 > extern u64 gdt64_desc[]; > extern u64 idt_descr[]; > @@ -27,17 +15,6 @@ extern void *vmx_return; > extern void *entry_sysenter; > extern void *guest_entry; > =20 > -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; > } > =20 > -static void print_vmexit_info() > +void print_vmexit_info() > { > u64 guest_rip, guest_rsp; > ulong reason =3D vmcs_read(EXI_REASON) & 0xff; > @@ -587,48 +564,6 @@ static void basic_syscall_handler(u64 syscall_no) > { > } > =20 > -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" > - : "=3Dr"(rsp), "=3Dr"(rax), "=3Dr"(resume_rsp) > - : "g"(0xABCD)); > - report("test vmresume", (rax =3D=3D 0xFFFF) && (rsp =3D=3D resume_rsp= )); > -} > - > -static int vmenter_exit_handler() > -{ > - u64 guest_rip; > - ulong reason; > - > - guest_rip =3D vmcs_read(GUEST_RIP); > - reason =3D vmcs_read(EXI_REASON) & 0xff; > - switch (reason) { > - case VMX_VMCALL: > - if (current->guest_regs.rax !=3D 0xABCD) { > - report("test vmresume", 0); > - return VMX_TEST_VMEXIT; > - } > - current->guest_regs.rax =3D 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[] =3D { > @@ -644,6 +579,8 @@ int main(void) > =20 > setup_vm(); > setup_idt(); > + fails =3D tests =3D 0; > + hypercall_field =3D 0; > =20 > if (test_vmx_capability() !=3D 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 > =20 > #include "libcflat.h" > =20 > @@ -41,7 +41,7 @@ struct vmx_test { > int exits; > }; > =20 > -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. > =20 > -static union vmx_ctrl_pin { > +union vmx_ctrl_pin { > u64 val; > struct { > u32 set, clr; > }; > } ctrl_pin_rev; > =20 > -static union vmx_ctrl_cpu { > +union vmx_ctrl_cpu { > u64 val; > struct { > u32 set, clr; > }; > } ctrl_cpu_rev[2]; > =20 > -static union vmx_ctrl_exit { > +union vmx_ctrl_exit { > u64 val; > struct { > u32 set, clr; > }; > } ctrl_exit_rev; > =20 > -static union vmx_ctrl_ent { > +union vmx_ctrl_ent { > u64 val; > struct { > u32 set, clr; > }; > } ctrl_enter_rev; > =20 > -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 > =20 > + > +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 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; > } > =20 > +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 > =20 > 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" > + : "=3Dr"(rsp), "=3Dr"(rax), "=3Dr"(resume_rsp) > + : "g"(0xABCD)); > + report("test vmresume", (rax =3D=3D 0xFFFF) && (rsp =3D=3D resume_rsp= )); > +} > + > +int vmenter_exit_handler() > +{ > + u64 guest_rip; > + ulong reason; > + > + guest_rip =3D vmcs_read(GUEST_RIP); > + reason =3D vmcs_read(EXI_REASON) & 0xff; > + switch (reason) { > + case VMX_VMCALL: > + if (current->guest_regs.rax !=3D 0xABCD) { > + report("test vmresume", 0); > + return VMX_TEST_VMEXIT; > + } > + current->guest_regs.rax =3D 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 >=20 Jan --2nl3JMnc4rpvuRqjnTBFbiplBUsFrsRm1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlH+hzIACgkQitSsb3rl5xSqiQCcD8r5E+KHw/7ZWDhqvEAOSElu elUAoLH5vugYfSHBHpCHBUmED4wHVIp7 =+UJB -----END PGP SIGNATURE----- --2nl3JMnc4rpvuRqjnTBFbiplBUsFrsRm1--