Re: [PATCH 2/2] kvm-unit-tests: Change two cases to use trap_emulator

[Paolo Bonzini <pbonzini@redhat.com>]

Il 06/06/2013 11:24, Arthur Chunqi Li ha scritto:
> Change two functions (test_mmx_movq_mf and test_movabs) using
> unified trap_emulator.
> 
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
>  x86/emulator.c |   66 ++++++++++++--------------------------------------------
>  1 file changed, 14 insertions(+), 52 deletions(-)
> 
> diff --git a/x86/emulator.c b/x86/emulator.c
> index 8ab9904..fa8993f 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -776,72 +776,34 @@ static void test_mmx_movq_mf(uint64_t *mem, uint8_t *insn_page,
>  			     uint8_t *alt_insn_page, void *insn_ram)
>  {
>      uint16_t fcw = 0;  // all exceptions unmasked
> -    ulong *cr3 = (ulong *)read_cr3();
> +    uint8_t alt_insn[] = {0x0f, 0x7f, 0x00}; // movq %mm0, (%rax)
>  
>      write_cr0(read_cr0() & ~6);  // TS, EM
> -    // Place a trapping instruction in the page to trigger a VMEXIT
> -    insn_page[0] = 0x89; // mov %eax, (%rax)
> -    insn_page[1] = 0x00;
> -    insn_page[2] = 0x90; // nop
> -    insn_page[3] = 0xc3; // ret
> -    // Place the instruction we want the hypervisor to see in the alternate page
> -    alt_insn_page[0] = 0x0f; // movq %mm0, (%rax)
> -    alt_insn_page[1] = 0x7f;
> -    alt_insn_page[2] = 0x00;
> -    alt_insn_page[3] = 0xc3; // ret
> -
>      exceptions = 0;
>      handle_exception(MF_VECTOR, advance_rip_by_3_and_note_exception);
> -
> -    // 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);
>      asm volatile("fninit; fldcw %0" : : "m"(fcw));
>      asm volatile("fldz; fldz; fdivp"); // generate exception
> -    invlpg(insn_ram);
> -    // Load code TLB
> -    asm volatile("call *%0" : : "r"(insn_ram + 3));
> -    install_page(cr3, virt_to_phys(alt_insn_page), insn_ram);
> -    // Trap, let hypervisor emulate at alt_insn_page
> -    asm volatile("call *%0" : : "r"(insn_ram), "a"(mem));
> +
> +    inregs = (struct regs){ 0 };
> +    trap_emulator(mem, insn_page, alt_insn_page, insn_ram, 
> +				alt_insn, 3);
>      // exit MMX mode
>      asm volatile("fnclex; emms");
> -    report("movq mmx generates #MF", exceptions == 1);
> +    report("movq mmx generates #MF2", exceptions == 1);

Extra hunk that is not needed?  Otherwise it looks good.

Thanks,

Paolo

>      handle_exception(MF_VECTOR, 0);
>  }
>  
>  static void test_movabs(uint64_t *mem, uint8_t *insn_page,
>  		       uint8_t *alt_insn_page, void *insn_ram)
>  {
> -    uint64_t val = 0;
> -    ulong *cr3 = (ulong *)read_cr3();
> -
> -    // 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] = 0x89; // mov %eax, (%rax)
> -    insn_page[1] = 0x00;
> -    // Place the instruction we want the hypervisor to see in the alternate
> -    // page. A buggy hypervisor will fetch a 32-bit immediate and return
> -    // 0xffffffffc3c3c3c3.
> -    alt_insn_page[0] = 0x48; // mov $0xc3c3c3c3c3c3c3c3, %rcx
> -    alt_insn_page[1] = 0xb9;
> -
> -    // 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 *%1" : "=c"(val) : "r"(insn_ram), "a"(mem), "c"(0));
> -    report("64-bit mov imm", val == 0xc3c3c3c3c3c3c3c3);
> +    // mov $0xc3c3c3c3c3c3c3c3, %rcx
> +    uint8_t alt_insn[] = {0x48, 0xb9, 0xc3, 0xc3, 0xc3,
> +					0xc3, 0xc3, 0xc3, 0xc3, 0xc3};
> +    inregs = (struct regs){ .rcx = 0 };
> +    
> +    trap_emulator(mem, insn_page, alt_insn_page, insn_ram,
> +				alt_insn, 10);
> +    report("64-bit mov imm2", outregs.rcx == 0xc3c3c3c3c3c3c3c3);
>  }
>  
>  static void test_crosspage_mmio(volatile uint8_t *mem)
>