From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/4] kvm-unit-tests: VMX: Add test cases for I/O bitmaps Date: Thu, 15 Aug 2013 09:58:42 +0200 Message-ID: <520C8A32.9080201@web.de> References: <1376409368-7016-1-git-send-email-yzt356@gmail.com> <1376409368-7016-4-git-send-email-yzt356@gmail.com> <520C860B.9050609@web.de> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="nCpStxlBJ755bxVxVsactdtqShJmMeFBa" Cc: kvm , Gleb Natapov , Paolo Bonzini To: Arthur Chunqi Li Return-path: Received: from mout.web.de ([212.227.17.12]:54861 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754179Ab3HOH6p (ORCPT ); Thu, 15 Aug 2013 03:58:45 -0400 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb103) with ESMTPSA (Nemesis) id 0Lrryk-1W6gQu34rW-013bCi for ; Thu, 15 Aug 2013 09:58:43 +0200 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nCpStxlBJ755bxVxVsactdtqShJmMeFBa Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2013-08-15 09:51, Arthur Chunqi Li wrote: > On Thu, Aug 15, 2013 at 3:40 PM, Jan Kiszka wrote: >> On 2013-08-13 17:56, Arthur Chunqi Li wrote: >>> Add test cases for I/O bitmaps, including corner cases. >> >> Would be good to briefly list the corner cases here. >> >>> >>> Signed-off-by: Arthur Chunqi Li >>> --- >>> x86/vmx.h | 6 +- >>> x86/vmx_tests.c | 167 +++++++++++++++++++++++++++++++++++++++++++++= ++++++++++ >>> 2 files changed, 170 insertions(+), 3 deletions(-) >>> >>> diff --git a/x86/vmx.h b/x86/vmx.h >>> index 18961f1..dba8b20 100644 >>> --- a/x86/vmx.h >>> +++ b/x86/vmx.h >>> @@ -417,15 +417,15 @@ enum Ctrl1 { >>> "popf\n\t" >>> >>> #define VMX_IO_SIZE_MASK 0x7 >>> -#define _VMX_IO_BYTE 1 >>> -#define _VMX_IO_WORD 2 >>> +#define _VMX_IO_BYTE 0 >>> +#define _VMX_IO_WORD 1 >>> #define _VMX_IO_LONG 3 >>> #define VMX_IO_DIRECTION_MASK (1ul << 3) >>> #define VMX_IO_IN (1ul << 3) >>> #define VMX_IO_OUT 0 >>> #define VMX_IO_STRING (1ul << 4) >>> #define VMX_IO_REP (1ul << 5) >>> -#define VMX_IO_OPRAND_DX (1ul << 6) >>> +#define VMX_IO_OPRAND_IMM (1ul << 6) >>> #define VMX_IO_PORT_MASK 0xFFFF0000 >>> #define VMX_IO_PORT_SHIFT 16 >>> >>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c >>> index 44be3f4..ad28c4c 100644 >>> --- a/x86/vmx_tests.c >>> +++ b/x86/vmx_tests.c >>> @@ -2,10 +2,13 @@ >>> #include "msr.h" >>> #include "processor.h" >>> #include "vm.h" >>> +#include "io.h" >>> >>> u64 ia32_pat; >>> u64 ia32_efer; >>> u32 stage; >>> +void *io_bitmap_a, *io_bitmap_b; >>> +u16 ioport; >>> >>> static inline void vmcall() >>> { >>> @@ -473,6 +476,168 @@ static int cr_shadowing_exit_handler() >>> return VMX_TEST_VMEXIT; >>> } >>> >>> +static void iobmp_init() >>> +{ >>> + u32 ctrl_cpu0; >>> + >>> + io_bitmap_a =3D alloc_page(); >>> + io_bitmap_a =3D alloc_page(); >>> + memset(io_bitmap_a, 0x0, PAGE_SIZE); >>> + memset(io_bitmap_b, 0x0, PAGE_SIZE); >>> + ctrl_cpu0 =3D vmcs_read(CPU_EXEC_CTRL0); >>> + ctrl_cpu0 |=3D CPU_IO_BITMAP; >>> + ctrl_cpu0 &=3D (~CPU_IO); >>> + vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0); >>> + vmcs_write(IO_BITMAP_A, (u64)io_bitmap_a); >>> + vmcs_write(IO_BITMAP_B, (u64)io_bitmap_b); >>> +} >>> + >>> +static void iobmp_main() >>> +{ >>> +/* >>> + data =3D (u8 *)io_bitmap_b; >>> + ioport =3D 0xffff; >>> + data[(ioport - 0x8000) /8] |=3D (1 << (ioport % 8)); >>> + inb(ioport); >>> + outb(0, ioport); >>> +*/ >> >> Forgotten debug code? >> >>> + // stage 0, test IO pass >>> + set_stage(0); >>> + inb(0x5000); >>> + outb(0x0, 0x5000); >>> + if (stage !=3D 0) >>> + report("I/O bitmap - I/O pass", 0); >>> + else >>> + report("I/O bitmap - I/O pass", 1); >>> + // test IO width, in/out >>> + ((u8 *)io_bitmap_a)[0] =3D 0xFF; >>> + set_stage(2); >>> + inb(0x0); >>> + if (stage !=3D 3) >>> + report("I/O bitmap - trap in", 0); >>> + else >>> + report("I/O bitmap - trap in", 1); >>> + set_stage(3); >>> + outw(0x0, 0x0); >>> + if (stage !=3D 4) >>> + report("I/O bitmap - trap out", 0); >>> + else >>> + report("I/O bitmap - trap out", 1); >>> + set_stage(4); >>> + inl(0x0); >> >> Forgot to check the progress? >> >>> + // test low/high IO port >>> + set_stage(5); >>> + ((u8 *)io_bitmap_a)[0x5000 / 8] =3D (1 << (0x5000 % 8)); >>> + inb(0x5000); >>> + if (stage =3D=3D 6) >>> + report("I/O bitmap - I/O port, low part", 1); >>> + else >>> + report("I/O bitmap - I/O port, low part", 0); >>> + set_stage(6); >>> + ((u8 *)io_bitmap_b)[0x1000 / 8] =3D (1 << (0x1000 % 8)); >>> + inb(0x9000); >>> + if (stage =3D=3D 7) >>> + report("I/O bitmap - I/O port, high part", 1); >>> + else >>> + report("I/O bitmap - I/O port, high part", 0); >>> + // test partial pass >>> + set_stage(7); >>> + inl(0x4FFF); >>> + if (stage =3D=3D 8) >>> + report("I/O bitmap - partial pass", 1); >>> + else >>> + report("I/O bitmap - partial pass", 0); >>> + // test overrun >>> + set_stage(8); >>> + memset(io_bitmap_b, 0xFF, PAGE_SIZE); >>> + inl(0xFFFF); >> >> Let's check the expected stage also here. > The check is below "if (stage =3D=3D 9)", the following "memset" is jus= t > used to prevent I/O mask to printf. Right, there is an i/o instruction missing below after the second memset - or I cannot follow what you are trying to test. The above inl would always trigger, independent of the wrap-around. Only if you clear both bitmaps, we get to the "interesting" scenario. So something is still wrong here, no? >> >>> + memset(io_bitmap_b, 0x0, PAGE_SIZE); >> >> Note that you still have io_bitmap_a[0] !=3D 0 here. You probably want= to >> clear it in order to have a clean setup. >> >>> + if (stage =3D=3D 9) >>> + report("I/O bitmap - overrun", 1); >>> + else >>> + report("I/O bitmap - overrun", 0); >>> + >>> + return; >>> +} >>> + >>> +static int iobmp_exit_handler() >>> +{ >>> + u64 guest_rip; >>> + ulong reason, exit_qual; >>> + u32 insn_len; >>> + //u32 ctrl_cpu0; >>> + >>> + guest_rip =3D vmcs_read(GUEST_RIP); >>> + reason =3D vmcs_read(EXI_REASON) & 0xff; >>> + exit_qual =3D vmcs_read(EXI_QUALIFICATION); >>> + insn_len =3D vmcs_read(EXI_INST_LEN); >>> + switch (reason) { >>> + case VMX_IO: >>> + switch (stage) { >>> + case 2: >>> + if ((exit_qual & VMX_IO_SIZE_MASK) !=3D _VMX_IO= _BYTE) >>> + report("I/O bitmap - I/O width, byte", = 0); >>> + else >>> + report("I/O bitmap - I/O width, byte", = 1); >>> + if (!(exit_qual & VMX_IO_IN)) >>> + report("I/O bitmap - I/O direction, in"= , 0); >>> + else >>> + report("I/O bitmap - I/O direction, in"= , 1); >>> + set_stage(stage + 1); >>> + break; >>> + case 3: >>> + if ((exit_qual & VMX_IO_SIZE_MASK) !=3D _VMX_IO= _WORD) >>> + report("I/O bitmap - I/O width, word", = 0); >>> + else >>> + report("I/O bitmap - I/O width, word", = 1); >>> + if (!(exit_qual & VMX_IO_IN)) >>> + report("I/O bitmap - I/O direction, out= ", 1); >>> + else >>> + report("I/O bitmap - I/O direction, out= ", 0); >>> + set_stage(stage + 1); >>> + break; >>> + case 4: >>> + if ((exit_qual & VMX_IO_SIZE_MASK) !=3D _VMX_IO= _LONG) >>> + report("I/O bitmap - I/O width, long", = 0); >>> + else >>> + report("I/O bitmap - I/O width, long", = 1); >>> + set_stage(stage + 1); >>> + break; >>> + case 5: >>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_P= ORT_SHIFT) =3D=3D 0x5000) >>> + set_stage(stage + 1); >> >> else >> ...? > We don't need "else" here, if we don't increase "stage" here, it means > test fail, which is the same as if it doesn't cause vmexit. OK. Jan >> >> Here and below. >> >>> + break; >>> + case 6: >>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_P= ORT_SHIFT) =3D=3D 0x9000) >>> + set_stage(stage + 1); >>> + break; >>> + case 7: >>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_P= ORT_SHIFT) =3D=3D 0x4FFF) >>> + set_stage(stage + 1); >>> + //ctrl_cpu0 =3D vmcs_read(CPU_EXEC_CTRL0); >>> + //ctrl_cpu0 &=3D (~CPU_IO_BITMAP); >>> + //vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0); >>> + break; >>> + case 8: >>> + if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_P= ORT_SHIFT) =3D=3D 0xFFFF) >>> + set_stage(stage + 1); >>> + break; >>> + case 0: >>> + case 1: >>> + set_stage(stage + 1); >>> + default: >> >> Unexpected stage? >> >>> + break; >>> + } >>> + vmcs_write(GUEST_RIP, guest_rip + insn_len); >>> + return VMX_TEST_RESUME; >>> + default: >>> + printf("guest_rip =3D 0x%llx\n", guest_rip); >>> + printf("\tERROR : Undefined exit reason, reason =3D %d.= \n", reason); >>> + break; >>> + } >>> + return VMX_TEST_VMEXIT; >>> +} >>> + >>> /* name/init/guest_main/exit_handler/syscall_handler/guest_regs >>> basic_* just implement some basic functions */ >>> struct vmx_test vmx_tests[] =3D { >>> @@ -486,5 +651,7 @@ struct vmx_test vmx_tests[] =3D { >>> test_ctrl_efer_exit_handler, basic_syscall_handler, {0}= }, >>> { "CR shadowing", basic_init, cr_shadowing_main, >>> cr_shadowing_exit_handler, basic_syscall_handler, {0} }= , >>> + { "I/O bitmap", iobmp_init, iobmp_main, iobmp_exit_handler, >>> + basic_syscall_handler, {0} }, >>> { NULL, NULL, NULL, NULL, NULL, {0} }, >>> }; >>> >> >> Jan >> >=20 >=20 >=20 --nCpStxlBJ755bxVxVsactdtqShJmMeFBa 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/ iEYEARECAAYFAlIMijIACgkQitSsb3rl5xTmngCgpg9c3KupkKNmZ8Smgr8GJsSe cvgAoNwtongZE9x4xq+70LJ/HuHvF1o9 =EJBP -----END PGP SIGNATURE----- --nCpStxlBJ755bxVxVsactdtqShJmMeFBa--