Re: [PATCH] kvm-unit-tests : The first version of VMX nested test case

[Gleb Natapov <gleb@redhat.com>]

On Wed, Jul 17, 2013 at 01:13:56AM +0800, Arthur Chunqi Li wrote:
> On Wed, Jul 17, 2013 at 12:45 AM, Gleb Natapov <gleb@redhat.com> 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 <gleb@redhat.com> 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.
LOC in one function is not an issue to be considered at all. Besides you
can put the switch into separate vmx_handler() function, or have an
array of vmexits.

> >
> >> 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. I have two ways. The first is that we just leave vmx.c as the
> VMX instructions and execution routine test suite, and develop other
> test cases in other files. Since all other tests of nested vmx is
> independent to the basic routine and it is hard for us to put all test
> cases for nested VMX in one file, so we just let this file do simple
> things and reuse some of its functions in other test suites of nested
> vmx. Your proposal of adding new test cases can be implemented in
> other test suites.
> 
> The other way is not splitting nested vmx tests cases in contrast.
> This method may cause a HUGE vmx.c file, and tests for different parts
> are not distinctive.
> 
> Actually, I prefer the former solution.
I do not think we need separate infrastructure just to test basic
instructions. Actually testing those are less interesting part (well
just to sorely test vmlaunch/vmresume we will likely have to write
hundred of tests to verify that all things that should cause failure do
that, but we likely settle for only a couple). I am not worrying about
vmx.c become a huge file as long as writing test is easy and more or
less self contained task.

--
			Gleb.