Re: [RFC PATCH] kvm-unit-tests : Basic architecture of VMX nested test case

[Gleb Natapov <gleb@redhat.com>]

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 instead of
longjmp()?

> > Agree, I dislike this magic too, but this is fixed by you suggestion
> > above about putting vmx_return at the beginning of while(). So the logic
> > will looks like that:
> > 
> > asm volatile("vmlaunch;setbe %0\n\t" : "=m"(ret));
> > while(ret) {
> 
> while(!ret) if you use setbe, of course.
> 
Yeah, this not meant to be compilable code :)

> >    vmx_return:
> >    SAVE_GPR_C
> >    eax = exit_handler();
> >    switch(eax) {
> >    }
> >    LOAD_GPR_C
> >    asm volatile("vmresume;seta %0\n\t" : "=m"(ret));
> > }
> 
> It is still somewhat magic: the "while (ret)" is only there to please
No, it it there to catch vmlaunch/vmresume errors.

> 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.

> 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)"

> 
> 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.


--
			Gleb.