On 2/17/2022 8:50 PM, Sean Christopherson wrote:
> 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().
Yeah this seems to be a better idea. Based on your suggestions, I am planning to do
following.
1) Move svm_npt_rsvd_bits_test() to file svm_npt.c and run it with __setup_vm()
2) There are 7 more npt test cases in svm_tests.c, move them to svm_npt.c
Below are the test cases:
npt_nx, npt_np, npt_us, npt_rw, npt_rw_pfwalk, npt_l1mmio, npt_rw_l1mmio
3) Change __setup_vm() to setup_vm() in svm_tests.c ( after doing this #AC
exception test will work fine without the need of set_user_mask_all and
clear_user_mask_all)
>
> 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.
I agree, we can do something similar in future.
