From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [RFC PATCH] kvm-unit-tests : Basic architecture of VMX nested test case Date: Wed, 24 Jul 2013 11:16:56 +0200 Message-ID: <51EF9B88.5040001@redhat.com> References: <1374087242-6125-1-git-send-email-yzt356@gmail.com> <51E78295.2010700@redhat.com> <20130718072652.GB11772@redhat.com> <51E7C7D2.5040303@redhat.com> <20130718110639.GA26173@redhat.com> <51E7DAD3.8000906@redhat.com> <20130718195712.GN13732@redhat.com> <51E8DFCC.8060108@redhat.com> <20130719094027.GY13732@redhat.com> <51E92BE2.1080600@redhat.com> <51EF76E2.2030201@redhat.com> <51EF78B9.9090908@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , kvm , Jan Kiszka To: Arthur Chunqi Li Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33328 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750979Ab3GXJRN (ORCPT ); Wed, 24 Jul 2013 05:17:13 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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 wrote: >> Il 24/07/2013 08:46, Arthur Chunqi Li ha scritto: >>> On Wed, Jul 24, 2013 at 2:40 PM, Paolo Bonzini 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 > > >