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:01:55 +0300 Message-ID: <20130606070155.GS4725@redhat.com> References: <1370495024-3712-1-git-send-email-yzt356@gmail.com> <20130606054502.GN4725@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]:49963 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754307Ab3FFHB6 convert rfc822-to-8bit (ORCPT ); Thu, 6 Jun 2013 03:01:58 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Jun 06, 2013 at 02:47:49PM +0800, =E6=9D=8E=E6=98=A5=E5=A5=87 <= Arthur Chunqi Li> wrote: > On Thu, Jun 6, 2013 at 1:45 PM, Gleb Natapov wrote: > > 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 test= another bug in the boot of RHEL5.9 64-bit. > >> > > We have growing number of instructions tests using the same tlb tri= ck. I > > think it is time to make the code more generic. Create a function t= hat > > 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? >=20 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 page 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 special value somewhere and check it after the test we can see if the wrong instruction was executed and rerun the test. > > > >> 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 VME= XIT > >> + 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 exec= uate > >> + // 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 tabl= es at > >> + // alt_insn_page (and keep the data TLB clear, for AMD decod= e assist). > >> + // This will make the CPU trap on the insn_page instruction = 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. >=20 > > > >> + 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.