Il 24/07/2013 10:48, Arthur Chunqi Li ha scritto: > So as what Gleb said, what about the following codes: > > static int vmx_run2() > { > u32 eax; > bool ret; > > asm volatile( > "mov %%rsp, %%rsi\n\t" > "mov %2, %%edi\n\t" > "call vmcs_write\n\t" > "vmlaunch\n\t" > "setbe %0\n\t" > "jne 4f\n\t"
setbe doesn't set the flags, so you need jbe here (and then you can have a "mov $1, %0" at label 4 instead of using setbe). > > "vmx_return:\n\t" > SAVE_GPR_C > "call exit_handler\n\t" > "cmp %3, %%eax\n\t" > "je 2f\n\t" > "cmp %4, %%eax\n\t" > "je 1f\n\t" > "jmp 3f\n\t" > > /* VMX_TEST_RESUME */ > "1:\n\t" > LOAD_GPR_C > "vmresume\n\t" You need a SAVE_GPR_C here. Then it is better to put the jbe at the vmx_return label: ... do vmlaunch or vmreturn as Jan said ... vmx_return: SAVE_GPR_C jbe 4f call exit_handler etc. Here is the relevant code from KVM that Jan was referring to: "jne 1f \n\t" __ex(ASM_VMX_VMLAUNCH) "\n\t" "jmp 2f \n\t" "1: " __ex(ASM_VMX_VMRESUME) "\n\t" "2: " I'd prefer to have a "jbe vmx_return; ud2" after the vmlaunch/vmresume, in order to test that the hypervisor respects HOST_RIP. Paolo > "setbe %0\n\t" > "jne 4f\n\t" > /* VMX_TEST_VMEXIT */ > "2:\n\t" > "mov $0, %1\n\t" > "jmp 5f\n\t" > /* undefined ret from exit_handler */ > "3:\n\t" > "mov $2, %1\n\t" > "jmp 5f\n\t" > /* vmlaunch/vmresume failed, exit */ > "4:\n\t" > "mov $1, %1\n\t" > "5:\n\t" > : "=r"(ret), "=r"(eax) > : "i"(HOST_RSP), "i"(VMX_TEST_VMEXIT), > "i"(VMX_TEST_RESUME) > : "rax", "rbx", "rdi", "rsi", > "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", > "memory", "cc" > ); > switch (eax) { > case 0: > return 0; > case 1: > printf("%s : vmenter failed.\n", __func__); > break; > default: > printf("%s : unhandled ret from exit_handler.\n", __func__); > break; > } > return 1; > } > > On Wed, Jul 24, 2013 at 2:48 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: >> Il 24/07/2013 08:46, Arthur Chunqi Li ha scritto: >>> On Wed, Jul 24, 2013 at 2:40 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: >>>> Il 24/07/2013 08:11, Arthur Chunqi Li ha scritto: >>>>> >>>>> static int vmx_run() >>>>> { >>>>> u32 eax; >>>>> bool ret; >>>>> >>>>> vmcs_write(HOST_RSP, get_rsp()); >>>>> ret = vmlaunch(); >>>> >>>> The compiler can still change rsp between here... >>>> >>>>> while (!ret) { >>>>> asm volatile( >>>>> "vmx_return:\n\t" >>>> >>>> ... and here. >>>> >>>> If you want to write it in C, the only thing that can be after >>>> vmlaunch/vmresume is "exit()". Else it has to be asm. >>> Actually, you mean we need to write all the codes in asm to avoid >>> changing to rsp, right? >> >> Not necessarily all the code. It is also ok to use setjmp/longjmp with >> a small asm trampoline, because this method won't care about the exact >> rsp values that are used. But if you want to do as Gleb said, and put >> vmx_return just after vmlaunch, it has to be all asm as in KVM's >> arch/x86/kvm/vmx.c. >> >> Paolo > > > -- 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