On 8/6/21 11:53 AM, Sean Christopherson wrote: > On Fri, Aug 06, 2021, Babu Moger wrote: >> From: Babu Moger <Babu.Moger@xxxxxxx> >> >> The test ./x86/access fails with a timeout. This is due to the number test >> combination. The test cases increase exponentially as the features get >> enabled. The new machine adds the feature AC_CPU_CR4_PKE. The default >> timeout is 180 seconds. Seen this problem both on AMD and Intel machines. >> >> #./tests/access >> qemu-system-x86_64: terminating on signal 15 from pid 20050 (timeout) >> FAIL access (timeout; duration=180) >> >> This test can take about 7 minutes without timeout. >> time ./tests/access >> 58982405 tests, 0 failures >> PASS access >> >> real 7m10.063s >> user 7m9.063s >> sys 0m0.309s >> >> Fix the problem by adding few more limit checks. > > Please state somewhere in the changelog what is actually being changed, and the > actual effect of the change. E.g. > > Disallow protection keys testcase in combination with reserved bit > testcasess to further limit the number of tests run to avoid timeouts on > systems with support for protection keys. > > Disallowing this combination reduces the total number of tests from > 58982405 to <???>, and the runtime from ~7 minutes to <???> Sure. Will do. > >> Signed-off-by: Babu Moger <Babu.Moger@xxxxxxx> >> --- >> x86/access.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/x86/access.c b/x86/access.c >> index 47807cc..e371dd5 100644 >> --- a/x86/access.c >> +++ b/x86/access.c >> @@ -317,9 +317,9 @@ static _Bool ac_test_legal(ac_test_t *at) >> /* >> * Shorten the test by avoiding testing too many reserved bit combinations >> */ >> - if ((F(AC_PDE_BIT51) + F(AC_PDE_BIT36) + F(AC_PDE_BIT13)) > 1) >> + if ((F(AC_PDE_BIT51) + F(AC_PDE_BIT36) + F(AC_PDE_BIT13) + F(AC_CPU_CR4_PKE)) > 1) >> return false; >> - if ((F(AC_PTE_BIT51) + F(AC_PTE_BIT36)) > 1) >> + if ((F(AC_PTE_BIT51) + F(AC_PTE_BIT36) + F(AC_CPU_CR4_PKE)) > 1) > > Why are protection keys the sacrifical lamb? Simply because they're the newest? Yes. I added it because it was new ):. > > And before we start killing off arbitrary combinations, what about sharding the > test so that testcases that depend on a specific CR0/CR4/EFER bit, i.e. trigger > a VM-Exit when the configuration changes, are separate runs? Being able to run > a specific combination would also hopefully make it easier to debug issues as > the user could specify which combo to run without having to modify the code and > recompile. > > That probably won't actually reduce the total run time, but it would make each > run a separate test and give developers a warm fuzzy feeling that they're indeed > making progress :-) > > Not sure how this could be automagically expressed this in unittest.cfg though... Let me investigate if we can do that fairly easy. Will let you know. Thanks Babu > >> return false; >> >> return true; >>
