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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 16/07/2013 19:13, Arthur Chunqi Li ha scritto:
> 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.
>>
>>> 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