From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] kvm-unit-tests : The first version of VMX nested test case Date: Tue, 16 Jul 2013 19:24:05 +0200 Message-ID: <51E581B5.50200@redhat.com> References: <1373966841-22206-1-git-send-email-yzt356@gmail.com> <51E52035.1050401@redhat.com> <20130716152035.GB8981@redhat.com> <20130716164519.GE8981@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , kvm , Jan Kiszka To: Arthur Chunqi Li Return-path: Received: from mail-qe0-f52.google.com ([209.85.128.52]:42516 "EHLO mail-qe0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933059Ab3GPRYS (ORCPT ); Tue, 16 Jul 2013 13:24:18 -0400 Received: by mail-qe0-f52.google.com with SMTP id i11so548724qej.25 for ; Tue, 16 Jul 2013 10:24:17 -0700 (PDT) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Il 16/07/2013 19:13, Arthur Chunqi Li ha scritto: > On Wed, Jul 17, 2013 at 12:45 AM, Gleb Natapov wrote: >> On Tue, Jul 16, 2013 at 11:29:20PM +0800, Arthur Chunqi Li wrote: >>> On Tue, Jul 16, 2013 at 11:20 PM, Gleb Natapov wrote: >>>> On Tue, Jul 16, 2013 at 12:28:05PM +0200, Paolo Bonzini wrote: >>>>>> +void vmx_exit(void) >>>>>> +{ >>>>>> + test_vmxoff(); >>>>>> + printf("\nSUMMARY: %d tests, %d failures\n", tests, fails); >>>>>> + exit(fails ? -1 : 0); >>>>>> +} >>>>> >>>>> Can you try to jump back to main, and do test_vmxoff there? This will >>>>> avoid having to write our tests in callback style, which is a pain. >>>>> Basically something similar to setjmp/longjmp. In main: >>>>> >>>>> if (setjmp(jmpbuf) == 0) { >>>>> vmx_run(); >>>>> /* Should not reach here */ >>>>> report("test vmlaunch", 0); >>>>> } >>>>> test_vmxoff(); >>>>> >>>>> exit: >>>>> printf("\nSUMMARY: %d tests, %d failures\n", tests, fails); >>>>> return fails ? 1 : 0; >>>>> >>>>> In vmx_handler: >>>>> >>>>> case VMX_HLT: >>>>> printf("\nVM exit.\n"); >>>>> longjmp(jmpbuf, 1); >>>>> >>>> Why not just make vmexit occur after vmlaunch/vmresume like KVM does. It >>>> will make code much more straightforward and easer to follow. >>> The concept "easier to follow" may have different meanings in >>> different view. This achievement puts all the test cases in main >>> function instead of scattering everywhere, which is another view to >>> "easy to follow". As this is just a test case, I prefer this one. >>> >> I do not see why what I propose will prevent you to put all tests into main. >> >> vmx_run() will looks like that: >> >> vmlaunch >> while(1) { >> vmresume >> <---- vmexit jumps here >> switch(exit reason) { >> case reason1: >> break; >> case reason2: >> break; >> case HLT >> return; >> } >> } > Yes, this recalls me some KVM codes I have read before. This mixes > vmlaunch/resume and vmx_handler into one piece of code. It is a good > way to explicitly show the execution sequence though, it increases LOC > in one function. >> >>> Besides, this way we can start another VM following the previous one >>> simply in main function. This is flexible if we want to test re-enter >>> to VMX mode or so. >>> >> That's what I am missing. How do one writes more tests now? >> >> I was thinking about interface like that: >> >> guest_func_test1() >> { >> } >> >> tes1t_exit_handlers[] = {test1_handle_hlt, test1_handle_exception, ....} >> >> main() >> { >> >> init_vmcs(); /* generic stuff */ >> init_vmcs_test1(); /* test1 related stuff */ >> r = run_in_guest(guest_func_test1, test1_exit_handlers); >> report("test1", r); >> } >> > I have thought about this question and I'm not quite sure how to solve > it now. Why can't you just use a different vmx_handler (e.g. with an indirect call in entry_vmx) for each test (as in Gleb's test1_exit_handlers)? run_in_guest would prepare the function pointers and do init_vmcs(&vmcs_root); if (setjmp(env) == 0){ vmx_run(); /* Should not reach here */ report("test vmlaunch", 0); } as in your current testcase. vmx.c would be a "library", and testcases could be either grouped in the same file or spread across many of them, as you see fit. Paolo