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 10:23:31 +0200 Message-ID: <520C9003.1060600@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> <520C8A32.9080201@web.de> <520C8D98.6060805@web.de> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vX0m7VVDhuw2bM56SOGBRahRgr5es8jue" Cc: kvm , Gleb Natapov , Paolo Bonzini To: Arthur Chunqi Li Return-path: Received: from mout.web.de ([212.227.15.3]:52359 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754056Ab3HOIXf (ORCPT ); Thu, 15 Aug 2013 04:23:35 -0400 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb102) with ESMTPSA (Nemesis) id 0MRCnT-1VXvVz28YZ-00Uaxn for ; Thu, 15 Aug 2013 10:23:32 +0200 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --vX0m7VVDhuw2bM56SOGBRahRgr5es8jue Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2013-08-15 10:20, Arthur Chunqi Li wrote: > On Thu, Aug 15, 2013 at 4:13 PM, Jan Kiszka wrote: >> On 2013-08-15 10:09, Arthur Chunqi Li wrote: >>> On Thu, Aug 15, 2013 at 3:58 PM, Jan Kiszka wrote= : >>>> On 2013-08-15 09:51, Arthur Chunqi Li wrote: >>>>> On Thu, Aug 15, 2013 at 3:40 PM, Jan Kiszka wro= te: >>>>>> 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= just >>>>> used to prevent I/O mask to printf. >>>> >>>> Right, there is an i/o instruction missing below after the second me= mset >>>> - or I cannot follow what you are trying to test. The above inl woul= d >>>> always trigger, independent of the wrap-around. Only if you clear bo= th >>>> bitmaps, we get to the "interesting" scenario. So something is still= >>>> wrong here, no? >>> Yes, we need to memset io_bit_map_a to 0 here. The above inl and the >>> test "if (stage =3D=3D 9)" are cooperatively used to test I/O overrun= : >>> test 4 bits width "in" to 0xFFFF. >> >> The point is that, according to our understanding of the SDM, we shoul= d >> even see a trap in this wrap-around scenario if both bitmaps are clear= ed. > Well, yep. I get the same understanding when I had first glance at > SDM, but currently IO will pass if every bits cleared. This is the > only pending problem that I asked Paolo and Gleb in a previous mail > thread, and they are both too busy as you told me and no response > until now :) That is strange when looking at kvm: static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) =2E.. if (port < 0x8000) bitmap =3D vmcs12->io_bitmap_a; else if (port < 0x10000) bitmap =3D vmcs12->io_bitmap_b; else return 1; Are you testing with kvm.git next? Jan --vX0m7VVDhuw2bM56SOGBRahRgr5es8jue 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/ iEYEARECAAYFAlIMkAMACgkQitSsb3rl5xRyDgCeOyAXVXxaEeFs4shE5Bps0DhJ uiAAn2JwYKu5JUpxjDF2IGFTQpqR7rpx =IFYk -----END PGP SIGNATURE----- --vX0m7VVDhuw2bM56SOGBRahRgr5es8jue--