So what about this one. I merged all the exit reason to "ret" and
remove the flag detection after vmlaunch/vmresume (because I think
this detection is useless). Currently we support only one guest, so
variant "launched" is located in vmx_run(). If we want to support
multiple guest, we could move it to some structures (e.g.
environment_ctxt). Now I just put it here.
static int vmx_run()
{
u32 ret = 0;
bool launched = 0;
asm volatile(
"mov %%rsp, %%rsi\n\t"
"mov %2, %%edi\n\t"
"call vmcs_write\n\t"
"0: "
LOAD_GPR_C
"cmp $0, %1\n\t"
"jne 1f\n\t"
"vmlaunch\n\t"
SAVE_GPR_C
/* vmlaunch error, return VMX_TEST_LAUNCH_ERR */
"mov %3, %0\n\t"
"jmp 2f\n\t"
"1: "
"vmresume\n\t"
SAVE_GPR_C
/* vmresume error, return VMX_TEST_RESUME_ERR */
"mov %4, %0\n\t"
"jmp 2f\n\t"
"vmx_return:\n\t"
SAVE_GPR_C
"call exit_handler\n\t"
/* set launched = 1 */
"mov $0x1, %1\n\t"
"cmp %5, %%eax\n\t"
"je 0b\n\t"
"mov %%eax, %0\n\t"
"2: "
: "=r"(ret), "=r"(launched)
: "i"(HOST_RSP), "i"(VMX_TEST_LAUNCH_ERR),
"i"(VMX_TEST_RESUME_ERR), "i"(VMX_TEST_RESUME)
: "rax", "rbx", "rdi", "rsi",
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
"memory", "cc"
);
switch (ret) {
case VMX_TEST_VMEXIT:
return 0;
case VMX_TEST_LAUNCH_ERR:
printf("%s : vmlaunch failed.\n", __func__);
break;
case VMX_TEST_RESUME_ERR:
printf("%s : vmresume failed.\n", __func__);
break;
default:
printf("%s : unhandled ret from exit_handler, ret=%d.\n",
__func__, ret);
break;
}
return 1;
}
On Wed, Jul 24, 2013 at 5:16 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> 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 <pbonzini@xxxxxxxxxx> wrote:
>>> Il 24/07/2013 08:46, Arthur Chunqi Li ha scritto:
>>>> On Wed, Jul 24, 2013 at 2:40 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> 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
>>
>>
>>
>
--
Arthur Chunqi Li
Department of Computer Science
School of EECS
Peking University
Beijing, China
--
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
