Re: [kvm-unit-tests PATCH] x86: nvmx: test max atomic switch MSRs

[Marc Orr <marcorr@google.com>]

Thanks for the review! I'll get to work on v2 now.

> > I'll happily apply what you've suggested in v2. But I don't see why
> > it's so terrible to over-allocate here. Leveraging a generic 2 MB page
> > allocator can be reused going forward, and encourages uniformity
> > across tests.
>
> My main concern is avoiding setting 6mb+ of memory.  I like to run the
> tests in L1 and L2 and would prefer to keep overhead at a minimum.
>
> As for the allocation itself, being precise in the allocation size is a
> form of documentation, e.g. it conveys that the size/order was chosen to
> ensure enough space for the maximum theoretical list size.  A purely
> arbitrary size, especially one that corresponds with a large page size,
> can lead to people looking for things that don't exist, e.g. the 2mb size
> is partially what led me to believe that this test was deliberately
> exceeding the limit, otherwise why allocate such a large amount of memory?
> I also didn't know if 2mb was sufficient to handle the maximum theoretical
> list size.

SGTM. I'll make this change in v2.

> > > Distilling things down to the bare minimum yields something like the
> > > following.
> >
> > Looks excellent overall. Still not clear what the consensus is on
> > whether or not to test the VM-entry failure. I think a flag seems like
> > a reasonable compromise. I've never added a flag to a kvm-unit-test,
> > so I'll see if I can figure that out.
>
> No need for a flag if you want to go that route, just put it in a separate
> VMX subtest and exclude said test from the [vmx] config, i.e. make the
> test opt-in.

SGTM, thanks!