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.
>> 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.
>> It is still somewhat magic: the "while (ret)" is only there to please
>> the compiler
>
> No, it it there to catch vmlaunch/vmresume errors.
You could change it to "while (0)" and it would still work. That's what
I mean by "only there to please the compiler".
>> 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.
>> different error messages for vmlaunch vs. vmresume failure.
> Just because the same variable is used to store error values :)
> Make vmlaunch use err1 and vmresume err2 and do "while (!err1 & !err2)"
Yeah. :)
>> In the end the choice is between "all in asm" and "small asm trampoline"
>> (which also happens to use setjmp/longjmp, but perhaps Arthur can
>> propagate return codes without using setjmp/longjmp, too).
>>
>>> Rewriting the whole guest entry exit path in asm like you suggest bellow
>>> will eliminate the magic too.
>>
>>> I much prefer one big asm statement than many small asm statement
>>> scattered among two or three C lines.
>>
>> It's not many asm statements, it's just a dozen lines:
>>
> I am not talking about overall file, but the how vmx_run() is written:
> asm()
> C code
> asm()
> C code
> ...
>
> I much prefer:
> C code
>
> big asm() blob
>
> C code.
Me too. But the trampoline version is neither, it is
static inline void vmlaunch() { asm("vmlaunch") }
static inline void vmresume() { asm("vmresume") }
small asm() blob
C code
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
