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: Thu, 18 Jul 2013 14:08:51 +0200 Message-ID: <51E7DAD3.8000906@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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Arthur Chunqi Li , kvm@vger.kernel.org, jan.kiszka@web.de To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:24119 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756731Ab3GRMJC (ORCPT ); Thu, 18 Jul 2013 08:09:02 -0400 In-Reply-To: <20130718110639.GA26173@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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