On Thu, Jun 06, 2013 at 05:33:52PM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> On Thu, Jun 6, 2013 at 3:45 PM, Gleb Natapov <g...@redhat.com> wrote:
> > On Thu, Jun 06, 2013 at 03:42:56PM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> >> On Thu, Jun 6, 2013 at 3:01 PM, Gleb Natapov <g...@redhat.com> wrote:
> >> > On Thu, Jun 06, 2013 at 02:47:49PM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> >> >> On Thu, Jun 6, 2013 at 1:45 PM, Gleb Natapov <g...@redhat.com> 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
> >> >> > trick. I
> >> >> > think it is time to make the code more generic. Create a function 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 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.
> >> If I commit the patch to merge these test cases, should the patch base
> >> on what I have commit before (after patched of this mail), or base on
> >> the master thread?
> >>
> > Master. First patch provides the infrastructure. Second converts
> > existing users. Third adds new tests.
> There are some problems packaging it. For some test cases some
> registers should be set before call alt_insn_page and some registers
> should be returned after it (such as test_movabs). If the emulator
> part is packed, the initialization before and result got after it may
> be invalid. Add two functions designed by caller to initialize and get
> result may cause the parameter table too long. Do you have any
> suggestions about this?
>
You can do what realmode.c does. It has inregs and outregs structures,
registers are set to inregs values before a test and saved to outregs
after. You can examine outregs to check correctness instead of checking
HW registers directly.
> >
> >> Arthur
> >>
> >> >
> >> >> >
> >> >> >> Signed-off-by: Arthur Chunqi Li <yzt...@gmail.com>
> >> >> >> ---
> >> >> >> 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 == 0x8400);
> >> >> >> }
> >> >> >>
> >> >> >> +static void test_bpl_modrm(uint64_t *mem, uint8_t *insn_page,
> >> >> >> + uint8_t *alt_insn_page, void *insn_ram)
> >> >> >> +{
> >> >> >> + ulong *cr3 = (ulong *)read_cr3();
> >> >> >> + uint16_t cx = 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] = 0x66; // mov $0x4321, %cx
> >> >> >> + insn_page[1] = 0xb9;
> >> >> >> + insn_page[2] = 0x21;
> >> >> >> + insn_page[3] = 0x43;
> >> >> >> + insn_page[4] = 0x89; // mov %eax, (%rax)
> >> >> >> + insn_page[5] = 0x00;
> >> >> >> + insn_page[6] = 0x90; // nop
> >> >> >> + // Place mov %cl, %bpl in alt_insn_page for emulator to
> >> >> >> execuate
> >> >> >> + // If emulator mistaken addressing %bpl, %cl may be moved to
> >> >> >> %ch
> >> >> >> + // %cx will be broken to 0x2121, not 0x4321
> >> >> >> + alt_insn_page[4] = 0x40;
> >> >> >> + alt_insn_page[5] = 0x88;
> >> >> >> + alt_insn_page[6] = 0xcd;
> >> >> >> +
> >> >> >> + // Load the code TLB with insn_page, but point the page tables
> >> >> >> at
> >> >> >> + // alt_insn_page (and keep the data TLB clear, for AMD decode
> >> >> >> 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("":"=c"(cx));
> >> >> > Why not add the constrain to previous asm?
> >> >> I will merge them in next version.
> >> >>
> >> >> >
> >> >> >> + report("access bpl in modr/m", cx == 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.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
