Re: [PATCH 1/2] kvm-unit-tests: Add a func to run instruction in emulator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 19, 2013 at 09:26:59AM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> On Wed, Jun 19, 2013 at 12:44 AM, Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> > Send code in a form of a patch.
> >
> > On Wed, Jun 19, 2013 at 12:14:13AM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> >> extern u8 insn_page[], insn_page_end[];
> >> extern u8 test_insn[], test_insn_end[];
> >> extern u8 alt_insn_page[];
> >>
> >> asm(
> >> ".align 4096\n\t"
> >> ".global insn_page\n\t"
> >> ".global insn_page_end\n\t"
> >> ".global test_insn\n\t"
> >> ".global test_insn_end\n\t"
> >> "insn_page:\n\t"
> >>
> >> "ret \n\t"
> >>
> >> "push %rax; push %rbx\n\t"
> >> "push %rcx; push %rdx\n\t"
> >> "push %rsi; push %rdi\n\t"
> >> "push %rbp\n\t"
> >> "push %r8; push %r9\n\t"
> >> "push %r10; push %r11\n\t"
> >> "push %r12; push %r13\n\t"
> >> "push %r14; push %r15\n\t"
> >> "pushf\n\t"
> >>
> >> "push 136+save \n\t"
> >> "popf \n\t"
> >> "mov 0+save, %rax \n\t"
> >> "mov 8+save, %rbx \n\t"
> >> "mov 16+save, %rcx \n\t"
> >> "mov 24+save, %rdx \n\t"
> >> "mov 32+save, %rsi \n\t"
> >> "mov 40+save, %rdi \n\t"
> >> "mov 56+save, %rbp \n\t"
> >> "mov 64+save, %r8 \n\t"
> >> "mov 72+save, %r9 \n\t"
> >> "mov 80+save, %r10  \n\t"
> >> "mov 88+save, %r11 \n\t"
> >> "mov 96+save, %r12 \n\t"
> >> "mov 104+save, %r13 \n\t"
> >> "mov 112+save, %r14 \n\t"
> >> "mov 120+save, %r15 \n\t"
> >>
> >> "test_insn:\n\t"
> >> "in  (%dx),%al\n\t"
> >> ". = . + 31\n\t"
> >> "test_insn_end:\n\t"
> >>
> >> "pushf \n\t"
> >> "pop 136+save \n\t"
> >> "mov %rax, 0+save \n\t"
> >> "mov %rbx, 8+save \n\t"
> >> "mov %rcx, 16+save \n\t"
> >> "mov %rdx, 24+save \n\t"
> >> "mov %rsi, 32+save \n\t"
> >> "mov %rdi, 40+save \n\t"
> >> "mov %rbp, 56+save \n\t"
> >> "mov %r8, 64+save \n\t"
> >> "mov %r9, 72+save \n\t"
> >> "mov %r10, 80+save \n\t"
> >> "mov %r11, 88+save \n\t"
> >> "mov %r12, 96+save \n\t"
> >> "mov %r13, 104+save \n\t"
> >> "mov %r14, 112+save \n\t"
> >> "mov %r15, 120+save \n\t"
> >> "popf \n\t"
> >> "pop %r15; pop %r14 \n\t"
> >> "pop %r13; pop %r12 \n\t"
> >> "pop %r11; pop %r10 \n\t"
> >> "pop %r9; pop %r8 \n\t"
> >> "pop %rbp \n\t"
> >> "pop %rdi; pop %rsi \n\t"
> >> "pop %rdx; pop %rcx \n\t"
> >> "pop %rbx; pop %rax \n\t"
> >>
> >> "ret\n\t"
> >> "save:\n\t"
> >> ". = . + 256\n\t"
> >> ".align 4096\n\t"
> >> "alt_insn_page:\n\t"
> >> ". = . + 4096\n\t"
> >> );
> >>
> >>
> >> static void mk_insn_page(uint8_t *alt_insn_page,
> >> uint8_t *alt_insn, int alt_insn_length)
> >> {
> >>     int i, emul_offset;
> >>     for (i=1; i<test_insn_end - test_insn; i++)
> >>         test_insn[i] = 0x90; // nop
> > Why? Gcc should pad it with nops.
> >
> >>     emul_offset = test_insn - insn_page;
> >>     for (i=0; i<alt_insn_length; i++)
> >>         alt_insn_page[i+emul_offset] = alt_insn[i];
> >> }
> >>
> >> static void trap_emulator(uint64_t *mem, uint8_t* alt_insn, int alt_insn_length)
> >> {
> >>     ulong *cr3 = (ulong *)read_cr3();
> >>     int save_offset = (u8 *)(&save) - insn_page;
> >>
> >>     memset(alt_insn_page, 0x90, 4096);
> > alt_insn_page should contains the same instruction as insn_page except
> > between test_insn and test_insn_end. I do not know how you expect it to
> > work otherwise.
> In my oponion, only codes between test_insn and test_insn_end in
> alt_insn_page need to be set, insn_page will be executed in the guest,
> and when trapping into emulator OS will load alt_insn_page (because of
> invlpg(insn_page)), then return to guest with executing insn_page
> (from TLB).
While before trap the code will likely be executed from insn_page,
but after the trap it is very optimistic to assume that tlb cache
will still contain this virtual address since host will execute quite a
lot of code and can even schedule in the middle, so the TLB will not
contain the address and your test will crash. Even the code before test
instruction can be executed from alt_insn_page if guest is scheduled out
after invlpg() and before it executes every instruction until trapping
one. In your case the test will crash too instead of yielding false positive.

> I don't know if this is right, but I use this trick in my
> previous patch and it runs well.
Your previous patches always had c3 (ret) after tested instruction on
alt_insn_page.

>                                  I use "trace-cmd record -e kvm" to
> trace it and found instructions in alt_insn_page are not executed, so
> I suppose that alt_insn_page is not loaded to the right place.
Do you see "in" instruction emulated? Anyway current code is incorrect
since current install_page() implementation cannot handle large pages
and the code is backed up by large pages. You can fix install_page() to
check for that and break large page into small one before installing a
page.

> 
> Arthur
> >
> >>     save = inregs;
> >>     mk_insn_page(alt_insn_page, alt_insn, alt_insn_length);
> >>     // 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_page);
> >>     invlpg(insn_page);
> >>     // Load code TLB
> >>     asm volatile("call *%0" : : "r"(insn_page));
> >>     install_page(cr3, virt_to_phys(alt_insn_page), insn_page);
> >>     // Trap, let hypervisor emulate at alt_insn_page
> >>     asm volatile("call *%0": : "r"(insn_page+1));
> >>
> >>     outregs = *((struct regs *)(&alt_insn_page[save_offset]));
> >> }
> >>
> >> static void test_movabs(uint64_t *mem)
> >> {
> >>     // mov $0xc3c3c3c3c3c3c3c3, %rcx
> >>     uint8_t alt_insn[] = {0x48, 0xb9, 0xc3, 0xc3, 0xc3,
> >>                                 0xc3, 0xc3, 0xc3, 0xc3, 0xc3};
> >>     inregs = (struct regs){ 0 };
> >>     trap_emulator(mem, alt_insn, 10);
> >>     report("64-bit mov imm2", outregs.rcx == 0xc3c3c3c3c3c3c3c3);
> >> }
> >>
> >> On Wed, Jun 19, 2013 at 12:09 AM, Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> >> > On Tue, Jun 18, 2013 at 11:56:24PM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> >> >> On Tue, Jun 18, 2013 at 11:47 PM, Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> >> >> > On Tue, Jun 18, 2013 at 10:28:59PM +0800, Ê??Ê?•Â•? <Arthur Chunqi Li> wrote:
> >> >> >> On Tue, Jun 18, 2013 at 8:45 PM, Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> >> >> >> > On Thu, Jun 13, 2013 at 05:30:03PM +0800, 李春奇 <Arthur Chunqi Li> wrote:
> >> >> >> >> Hi Gleb,
> >> >> >> >> I'm trying to solve these problems in the past days and meet many
> >> >> >> >> difficulties. You want to save all the general registers in calling
> >> >> >> >> insn_page, so registers should be saved to (save) in insn_page.
> >> >> >> >> Because all the instructions should be generated outside and copy to
> >> >> >> >> insn_page, and the instructions generated outside is RIP-relative, so
> >> >> >> >> inside insn_page (save) will be wrong pointed with RIP-relative code.
> >> >> >> >>
> >> >> >> > They do not have to be generated outside. You can write code into
> >> >> >> > insn_page directly. Something like this outside of any functions:
> >> >> >> >
> >> >> >> > asm(".align 4096\n\t"
> >> >> >> >     ".global insn_page\n\t"
> >> >> >> >     ".global insn_page_end\n\t"
> >> >> >> >     ".global test_insn\n\t"
> >> >> >> >     ".global test_insn_end\n\t"
> >> >> >> >     "insn_page:"
> >> >> >> >     "mov %%rax, outregs \n\t"
> >> >> >> >     ...
> >> >> >> >     "test_insn:\n\t"
> >> >> >> >     "in (%ds), %al\n\t"
> >> >> >> >     ". = . + 31\n\t"
> >> >> >> >     "test_insn_end:\n\t"
> >> >> >> >     "mov outregs, %%rax\n\t"
> >> >> >> >     ...
> >> >> >> >     "ret\n\t"
> >> >> >> >     ".align 4096\n\t"
> >> >> >> >     "insn_page_end:\n\t");
> >> >> >> >
> >> >> >> > Now you copy that into alt_insn_page, put instruction you want to test
> >> >> >> > into test_insn offset and remap alt_insn_page into "insn_page" virtual address.
> >> >> >> I used such codes:
> >> >> >>
> >> >> >> invlpg((void *)virt_to_phys(insn_page));
> >> >> > virt_to_phys?
> >> >> This is a mistake, I changed it to "invlpg(insn_page)" but the result
> >> >> is the same.
> >> >> >
> >> >> >> asm volatile("call *%0" : : "r"(insn_page));
> >> >> >> install_page(cr3, virt_to_phys(alt_insn_page), insn_page);
> >> >> >> asm volatile("call *%0": : "r"(insn_page+1));
> >> >> > +1?
> >> >> Here I put "ret" on the first byte of insn_page, so the first call of
> >> >> "insn_page" can just return, and the second call of "insn_page+1“ will
> >> >> directly call the second byte, which is the real content of insn_page.
> >> > Send the code.
> >> >
> >> > --
> >> >                         Gleb.
> >>
> >>
> >>
> >> --
> >> Arthur Chunqi Li
> >> Department of Computer Science
> >> School of EECS
> >> Peking University
> >> Beijing, China
> >
> > --
> >                         Gleb.
> 
> 
> 
> --
> Arthur Chunqi Li
> Department of Computer Science
> School of EECS
> Peking University
> Beijing, China

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux