> > + struct vmx_msr_entry *vm_enter_load;
> > + struct vmx_msr_entry *vm_exit_load;
> > + struct vmx_msr_entry *vm_exit_store;
>
> Is it possible to re-use the existing pointers in vmx_tests.c,
>
> struct vmx_msr_entry *exit_msr_store, *entry_msr_load, *exit_msr_load;
>
>
> instead of using local pointers ?
Done.
> > + int max_allowed = max_msr_list_size();
> > + int byte_capacity = 1ul << (msr_list_page_order + PAGE_SHIFT);
> > + /* Exceeding the max MSR list size at exit trigers KVM to abort. */
> > + int exit_count = count > max_allowed ? max_allowed : count;
> > + int cleanup_count = count > max_allowed ? 2 : 1;
> > + int i;
> > +
> > + /*
> > + * Check for the IA32_TSC MSR,
> > + * available with the "TSC flag" and used to populate the MSR lists.
> > + */
> > + if (!(cpuid(1).d & (1 << 4))) {
> > + report_skip(__func__);
> > + return;
> > + }
> > +
> > + /* Set L2 guest. */
> > + test_set_guest(atomic_switch_msr_limit_test_guest);
>
> Is it possible to directly pass the vmcall() function instead of
> creating a wrapper ?
Done.
> > + if (count <= max_allowed) {
> > + enter_guest();
> > + assert_exit_reason(VMX_VMCALL);
>
> This is redundant because skip_exit_vmcall() calls this also.
Done.
> > + free_pages_by_order(vm_enter_load, msr_list_page_order);
> > + free_pages_by_order(vm_exit_load, msr_list_page_order);
> > + free_pages_by_order(vm_exit_store, msr_list_page_order);
>
> Since the 2nd argument to the function is not changing, is there any
> particular reason for keeping it ?
The second argument has to match what was passed into the respective
alloc_pages() call, per my understanding.
> > + /* Atomic MSR switch tests. */
> > + TEST(atomic_switch_max_msrs_test),
> > + TEST(atomic_switch_overflow_msrs_test),
>
> Actually, it should be a single executable.
> 'atomic_switch_max_msrs_test' is the positive version and
> 'atomic_switch_overflow_msrs_test' is the negative version of the
> MSR-lists, and so these should be enveloped in the same test.
They're separated because the negative version,
atomic_switch_overflow_msrs_test, may behave unexpectedly on real
hardware. This way, the negative version can easily be off by default
in the unittests.cfg file, and opted into as desired. Let me know if
there's a different, preferred, way to do this (e.g., via a flag) with
the two scenarios enveloped within a single test. However, this is
what Sean suggested in a prior code review and it seems like a good
solution to me.
