From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH] kvm-unit-tests: Add test case for accessing bpl via modr/m Date: Thu, 6 Jun 2013 10:45:14 +0300 Message-ID: <20130606074514.GW4725@redhat.com> References: <1370495024-3712-1-git-send-email-yzt356@gmail.com> <20130606054502.GN4725@redhat.com> <20130606070155.GS4725@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm , Paolo Bonzini To: =?utf-8?B?5p2O5pil5aWHIDxBcnRodXIgQ2h1bnFpIExpPg==?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56415 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757684Ab3FFHpT convert rfc822-to-8bit (ORCPT ); Thu, 6 Jun 2013 03:45:19 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Jun 06, 2013 at 03:42:56PM +0800, =E6=9D=8E=E6=98=A5=E5=A5=87 <= Arthur Chunqi Li> wrote: > On Thu, Jun 6, 2013 at 3:01 PM, Gleb Natapov wrote: > > On Thu, Jun 06, 2013 at 02:47:49PM +0800, =E6=9D=8E=E6=98=A5=E5=A5=87= wrote: > >> On Thu, Jun 6, 2013 at 1:45 PM, Gleb Natapov wro= te: > >> > On Thu, Jun 06, 2013 at 01:03:44PM +0800, Arthur Chunqi Li wrote= : > >> >> Test access to %bpl via modr/m addressing mode. This case can t= est another bug in the boot of RHEL5.9 64-bit. > >> >> > >> > We have growing number of instructions tests using the same tlb = trick. I > >> > think it is time to make the code more generic. Create a functio= n that > >> > receives instruction to check and all the tlb games will be done= by that > >> > function. > >> Should I do some work to merge all these test cases? > >> > > It would be nice, yes. I also have an idea on how to improve test > > reliability. Since it relies on tlb to be out of sync with actual p= age > > table a vmexit in a wrong time can break this assumption and wrong > > instruction will be emulated (the one from insn_page instead of > > alt_insn_page). If we make the instruction on insn_page place speci= al > > value somewhere and check it after the test we can see if the wrong > > instruction was executed and rerun the test. > If I commit the patch to merge these test cases, should the patch bas= e > on what I have commit before (after patched of this mail), or base on > the master thread? >=20 Master. First patch provides the infrastructure. Second converts existing users. Third adds new tests. > Arthur >=20 > > > >> > > >> >> Signed-off-by: Arthur Chunqi Li > >> >> --- > >> >> x86/emulator.c | 41 ++++++++++++++++++++++++++++++++++++++++= + > >> >> 1 file changed, 41 insertions(+) > >> >> > >> >> diff --git a/x86/emulator.c b/x86/emulator.c > >> >> index 96576e5..3563971 100644 > >> >> --- a/x86/emulator.c > >> >> +++ b/x86/emulator.c > >> >> @@ -901,6 +901,45 @@ static void test_simplealu(u32 *mem) > >> >> report("test", *mem =3D=3D 0x8400); > >> >> } > >> >> > >> >> +static void test_bpl_modrm(uint64_t *mem, uint8_t *insn_page, > >> >> + uint8_t *alt_insn_page, void *insn_ram) > >> >> +{ > >> >> + ulong *cr3 =3D (ulong *)read_cr3(); > >> >> + uint16_t cx =3D 0; > >> >> + > >> >> + // Pad with RET instructions > >> >> + memset(insn_page, 0xc3, 4096); > >> >> + memset(alt_insn_page, 0xc3, 4096); > >> >> + // Place a trapping instruction in the page to trigger a = VMEXIT > >> >> + insn_page[0] =3D 0x66; // mov $0x4321, %cx > >> >> + insn_page[1] =3D 0xb9; > >> >> + insn_page[2] =3D 0x21; > >> >> + insn_page[3] =3D 0x43; > >> >> + insn_page[4] =3D 0x89; // mov %eax, (%rax) > >> >> + insn_page[5] =3D 0x00; > >> >> + insn_page[6] =3D 0x90; // nop > >> >> + // Place mov %cl, %bpl in alt_insn_page for emulator to e= xecuate > >> >> + // If emulator mistaken addressing %bpl, %cl may be moved= to %ch > >> >> + // %cx will be broken to 0x2121, not 0x4321 > >> >> + alt_insn_page[4] =3D 0x40; > >> >> + alt_insn_page[5] =3D 0x88; > >> >> + alt_insn_page[6] =3D 0xcd; > >> >> + > >> >> + // Load the code TLB with insn_page, but point the page t= ables at > >> >> + // alt_insn_page (and keep the data TLB clear, for AMD de= code assist). > >> >> + // This will make the CPU trap on the insn_page instructi= on but the > >> >> + // hypervisor will see alt_insn_page. > >> >> + install_page(cr3, virt_to_phys(insn_page), insn_ram); > >> >> + // Load code TLB > >> >> + invlpg(insn_ram); > >> >> + asm volatile("call *%0" : : "r"(insn_ram+3)); > >> >> + // Trap, let hypervisor emulate at alt_insn_page > >> >> + install_page(cr3, virt_to_phys(alt_insn_page), insn_ram); > >> >> + asm volatile("call *%0" : : "r"(insn_ram), "a"(mem)); > >> >> + asm volatile("":"=3Dc"(cx)); > >> > Why not add the constrain to previous asm? > >> I will merge them in next version. > >> > >> > > >> >> + report("access bpl in modr/m", cx =3D=3D 0x4321); > >> >> +} > >> >> + > >> >> int main() > >> >> { > >> >> void *mem; > >> >> @@ -964,6 +1003,8 @@ int main() > >> >> > >> >> test_string_io_mmio(mem); > >> >> > >> >> + test_bpl_modrm(mem, insn_page, alt_insn_page, insn_ram); > >> >> + > >> >> printf("\nSUMMARY: %d tests, %d failures\n", tests, fails= ); > >> >> return fails ? 1 : 0; > >> >> } > >> >> -- > >> >> 1.7.9.5 > >> > > >> > -- > >> > Gleb. > > > > -- > > Gleb. -- Gleb.