Il 18/07/2013 21:57, Gleb Natapov ha scritto:
> On Thu, Jul 18, 2013 at 02:08:51PM +0200, Paolo Bonzini wrote:
>> Il 18/07/2013 13:06, Gleb Natapov ha scritto:
>>> On Thu, Jul 18, 2013 at 12:47:46PM +0200, Paolo Bonzini wrote:
>>>>>> and for a testsuite I'd prefer the latter---which means I'd still favor
>>>>>> setjmp/longjmp.
>>>>>>
>>>>>> Now, here is the long explanation.
>>>>>>
>>>>>> I must admit that the code looks nice. There are some nits I'd like to
>>>>>> see done differently (such as putting vmx_return at the beginning of the
>>>>>> while (1), and the vmresume asm at the end), but it is indeed nice.
>>>>>
>>>>> Why do you prefer setjmp/longjmp then?
>>>>
>>>> Because it is still deceiving, and I dislike the deceit more than I like
>>>> the linear code flow.
>>>>
>>> What is deceiving about it? Of course for someone who has no idea how
>>> vmx works the code will not be obvious, but why should we care. For
>>> someone who knows what is deceiving about returning into the same
>>> function guest was launched from by using VMX mechanism
>>
>> The way the code is written is deceiving. If I see
>>
>> asm("vmlaunch; seta %0")
>> while (ret)
>>
>> I expect HOST_RIP to point at the seta or somewhere near, not at a
>> completely different label somewhere else.
>>
> Why would you expect that assuming you know what vmlaunch is?
Because this is written in C, and I know trying to fool the compiler is
a losing game. So my reaction is "okay, HOST_RIP must be set so that
code will not jump around". If I see
asm("vmlaunch")
exit(-1)
the reaction is the opposite: "hmm, anything that jumps around would
have a hard time with the compiler, there must be some assembly
trampoline somewhere; let's check what HOST_RIP is".
>>>> instead of longjmp()?
>>
>> Look again at the setjmp/longjmp version. longjmp is not used to handle
>> vmexit. It is used to jump back from the vmexit handler to main, which
>> is exactly what setjmp/longjmp is meant for.
>>
> That's because simple return will not work in that version, this is
> artifact of how vmexit was done.
I think it can be made to work without setjmp/longjmp, but the code
would be ugly.
>>>> the compiler, and you rely on the compiler not changing %rsp between the
>>>> vmlaunch and the vmx_return label. Minor nit, you cannot anymore print
>>> HOST_RSP should be loaded on each guest entry.
>>
>> Right, but my point is: without a big asm blob, you don't know the right
>> value to load. It depends on the generated code. And the big asm blob
>> limits a lot the "code looks nice" value of this approach.
>>
> I said it number of times already, this is not about "code looking nice",
> "code looks like KVM" or use less assembler as possible", this is about
> linear data flow. It is not fun chasing code execution path. Yes, you
> can argue that vmlaunch/vmresume inherently non linear, but there is a
> difference between skipping one instruction and remain in the same
> function and on the same stack, or jump completely to a different
> context.
I don't see anything bad in jumping completely to a different context.
The guest and host are sort of like two coroutines, they hardly have any
connection with the code that called vmlaunch.
> The actually differences in asm instruction between both
> version will not be bigger then a couple of lines (if we will not take
> setjmp/longjmp implementation into account :)),
I was waiting for this parenthetical remark to appear. ;)
> but I do not even see
> why we discuss this argument since minimizing asm instructions here is
> not an point. We should not use more then needed to achieve the goal of
> course, but design should not be around number of assembly lines.
I agree, I only mentioned it because you talked about
asm
C
asm
C
and this is not what the setjmp/longjmp code looks like---using inlines
for asm as if they were builtin functions is good, interspersing asm and
C in the same function is bad.
Paolo
--
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
