Re: [kvm-unit-tests PATCH 1/3] x86: Add routines to set/clear PT_USER_MASK for all pages

[Sean Christopherson <seanjc@google.com>]

On Thu, Feb 17, 2022, Shukla, Manali wrote:
> 
> On 2/15/2022 1:00 AM, Sean Christopherson wrote:
> > On Mon, Feb 07, 2022, Manali Shukla wrote:
> >> Add following 2 routines :
> >> 1) set_user_mask_all() - set PT_USER_MASK for all the levels of page tables
> >> 2) clear_user_mask_all - clear PT_USER_MASK for all the levels of page tables
> >>
> >> commit 916635a813e975600335c6c47250881b7a328971
> >> (nSVM: Add test for NPT reserved bit and #NPF error code behavior)
> >> clears PT_USER_MASK for all svm testcases. Any tests that requires
> >> usermode access will fail after this commit.
> > 
> > Gah, I took the easy route and it burned us.  I would rather we start breaking up
> > the nSVM and nVMX monoliths, e.g. add a separate NPT test and clear the USER flag
> > only in that test, not the "common" nSVM test.
> 
> Yeah. I agree. I will try to set/clear User flag in svm_npt_rsvd_bits_test() and 
> set User flag by default for all the test cases by calling setup_vm()
> and use walk_pte() to set/clear User flag in svm_npt_rsvd_bits_test().

I was thinking of something more drastic.  The only reason the nSVM tests are
"incompatible" with usermode is this snippet in main():

  int main(int ac, char **av)
  {
	/* Omit PT_USER_MASK to allow tested host.CR4.SMEP=1. */
	pteval_t opt_mask = 0;
	int i = 0;

	ac--;
	av++;

	__setup_vm(&opt_mask);

	...
  }

Change that to setup_vm() and KUT will build the test with PT_USER_MASK set on
all PTEs.  My thought (might be a bad one) is to move the nNPT tests to their own
file/test so that the tests don't need to fiddle with page tables midway through.

The quick and dirty approach would be to turn the current main() into a small
helper, minus its call to __setup_vm().

Longer term, I think it'd make sense to add svm/ +  vmx/ subdirectories, and turn
much of the common code into proper libraries, e.g. test_wanted() can and should
be common helper, probably with more glue code to allow declaring a set of subtests.
But for now I think we can just add svm_npt.c or whatever.