Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations

[Babu Moger <babu.moger@amd.com>]

On 8/11/21 11:13 AM, Sean Christopherson wrote:
> On Wed, Aug 11, 2021, Babu Moger wrote:
>>
>> On 8/11/21 2:09 AM, Paolo Bonzini wrote:
>>> On 11/08/21 01:38, Babu Moger wrote:
>>>> No. This will not work. The PKU feature flag is bit 30. That is 2^30
>>>> iterations to cover the tests for this feature. Looks like I need to split
>>>> the tests into PKU and non PKU tests. For PKU tests I may need to change
>>>> the bump frequency (in ac_test_bump_one) to much higher value. Right now,
>>>> it is 1. Let me try that,
>>>
>>> The simplest way to cut on tests, which is actually similar to this patch,
>>> would be:
>>>
>>> - do not try all combinations of PTE access bits when reserved bits are set
>>>
>>> - do not try combinations with more than one reserved bit set
>>
>> Did you mean this? Just doing this reduces the combination by huge number.
>> I don't need to add your first PTE access combinations.
>>
>> diff --git a/x86/access.c b/x86/access.c
>> index 47807cc..a730b6b 100644
>> --- a/x86/access.c
>> +++ b/x86/access.c
>> @@ -317,9 +317,7 @@ 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)
>> -        return false;
>> -    if ((F(AC_PTE_BIT51) + F(AC_PTE_BIT36)) > 1)
>> +    if ((F(AC_PDE_BIT51) + F(AC_PDE_BIT36) + F(AC_PDE_BIT13) +
>> F(AC_PTE_BIT51) + F(AC_PTE_BIT36)) > 1)
>>          return false;
>>
>>      return true;
> 
> Looks good to me, is it sufficient to keep the overall runtime sane?.  And maybe

It keeps the running time about 2 minutes.

> update the comment too, e.g. something like
> 
> 	/*
> 	 * Skip testing multiple reserved bits to shorten the test.  Reserved
> 	 * bit page faults are terminal and multiple reserved bits do not affect
> 	 * the error code; the odds of a KVM bug are super low, and the odds of
> 	 * actually being able to detect a bug are even lower.
> 	 */
> 

Sure. Will update the commit log.
Thanks
Babu