[kvm-unit-tests PATCH 0/2] Couple of SVM fixes

[kvm-unit-tests PATCH 0/2] Couple of SVM fixes

[Babu Moger]

This series fixes couple of unittest failures for SVM.
1. The test ./x86/access is failing with timeout.
2. The test ./x86/svm failure with infinite loop.
More details in each patch.

---

Babu Moger (2):
      x86: access: Fix timeout failure by limiting number of flag combinations
      nSVM: Fix NPT reserved bits test hang


 lib/x86/processor.h |   10 ++++++++++
 x86/access.c        |    4 ++--
 x86/svm_tests.c     |   25 +++++++++++++++++++------
 3 files changed, 31 insertions(+), 8 deletions(-)

--

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

[Babu Moger]

From: Babu Moger <Babu.Moger@amd.com>

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.

Signed-off-by: Babu Moger <Babu.Moger@amd.com>
---
 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)
         return false;
 
     return true;

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

[Babu Moger]

On 8/6/21 11:53 AM, Sean Christopherson wrote:
> On Fri, Aug 06, 2021, Babu Moger wrote:
>> From: Babu Moger <Babu.Moger@amd.com>
>>
>> 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@amd.com>
>> ---
>>  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;
>>

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

[Babu Moger]

On 8/9/21 2:43 PM, Babu Moger wrote:
> 
> 
> On 8/6/21 11:53 AM, Sean Christopherson wrote:
>> On Fri, Aug 06, 2021, Babu Moger wrote:
>>> From: Babu Moger <Babu.Moger@amd.com>
>>>
>>> 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@amd.com>
>>> ---
>>>  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...

As we know now that we cannot run a huge number of tests without running
into timeout, I was thinking of adding a extra parameter "max_runs" for
these tests and add a check in ac_test_run to limit the number of runs.
The max_runs will be set to default 10000000. But it can be changed in
unittests.cfg. Something like this.

[access]
 file = access.flat
 arch = x86_64
-extra_params = -cpu max
+extra_params = -cpu max -append 10000000
 timeout = 180

Thoughts?

> 
> Let me investigate if we can do that fairly easy. Will let you know.
> Thanks
> Babu
>>
>>>          return false;
>>>  
>>>      return true;
>>>

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

[Babu Moger]

On 8/10/21 11:59 AM, Babu Moger wrote:
> 
> 
> On 8/9/21 2:43 PM, Babu Moger wrote:
>>
>>
>> On 8/6/21 11:53 AM, Sean Christopherson wrote:
>>> On Fri, Aug 06, 2021, Babu Moger wrote:
>>>> From: Babu Moger <Babu.Moger@amd.com>
>>>>
>>>> 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@amd.com>
>>>> ---
>>>>  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...
> 
> As we know now that we cannot run a huge number of tests without running
> into timeout, I was thinking of adding a extra parameter "max_runs" for
> these tests and add a check in ac_test_run to limit the number of runs.
> The max_runs will be set to default 10000000. But it can be changed in
> unittests.cfg. Something like this.
> 
> [access]
>  file = access.flat
>  arch = x86_64
> -extra_params = -cpu max
> +extra_params = -cpu max -append 10000000

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,

>  timeout = 180
> 
> Thoughts?
> 
>>
>> Let me investigate if we can do that fairly easy. Will let you know.
>> Thanks
>> Babu
>>>
>>>>          return false;
>>>>  
>>>>      return true;
>>>>

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

[Paolo Bonzini]

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

Paolo

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

[Babu Moger]

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;

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

[Sean Christopherson]

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
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.
	 */

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

[Babu Moger]

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

[kvm-unit-tests PATCH 2/2] nSVM: Fix NPT reserved bits test hang

[Babu Moger]

From: Babu Moger <Babu.Moger@amd.com>

SVM reserved bits tests hangs in a infinite loop. The test uses the
instruction 'rdtsc' to generate the random reserved bits. It hangs
while generating the valid reserved bits.

The AMD64 Architecture Programmers Manual Volume 2: System
Programming manual says, When using the TSC to measure elapsed time,
programmers must be aware that for some implementations, the rate at
which the TSC is incremented varies based on the processor power
management state (Pstate). For other implementations, the TSC
increment rate is fixed and is not subject to power-management
related changes in processor frequency.

In AMD gen3 machine, the rdtsc value is a P state multiplier.
Here are the rdtsc value in 10 sucessive reads.
0 rdtsc = 0x1ec92919b9710
1 rdtsc = 0x1ec92919c01f0
2 rdtsc = 0x1ec92919c0f70
3 rdtsc = 0x1ec92919c18d0
4 rdtsc = 0x1ec92919c2060
5 rdtsc = 0x1ec92919c28d0
6 rdtsc = 0x1ec92919c30b0
7 rdtsc = 0x1ec92919c5660
8 rdtsc = 0x1ec92919c6150
9 rdtsc = 0x1ec92919c7c80

This test uses the lower nibble and right shifts to generate the
valid reserved bit. It loops forever because the lower nibble is always
zero.

Fixing the issue with replacing rdrand instruction if available or
skipping the test if we cannot generate the valid reserved bits.

Signed-off-by: Babu Moger <Babu.Moger@amd.com>
---
 lib/x86/processor.h |   10 ++++++++++
 x86/svm_tests.c     |   25 +++++++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index a08ea1f..974077a 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -531,6 +531,16 @@ static inline void sti(void)
     asm volatile ("sti");
 }
 
+static inline unsigned long long rdrand(void)
+{
+	long long r;
+
+	asm volatile("1:;\n\
+	              rdrand %0;\n\
+		      jnc 1b;\n":"=r"(r));
+	return r;
+}
+
 static inline unsigned long long rdtsc(void)
 {
 	long long r;
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 79ed48e..a2963c0 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -2704,11 +2704,23 @@ static void _svm_npt_rsvd_bits_test(u64 *pxe, u64 pxe_rsvd_bits,  u64 efer,
 
 static u64 get_random_bits(u64 hi, u64 low)
 {
-	u64 rsvd_bits;
+	unsigned retry = 5;
+	u64 rsvd_bits = 0;
+
+	if (this_cpu_has(X86_FEATURE_RDRAND)) {
+		do {
+			rsvd_bits = (rdrand() << low) & GENMASK_ULL(hi, low);
+			retry--;
+		} while (!rsvd_bits && retry);
+	}
 
-	do {
-		rsvd_bits = (rdtsc() << low) & GENMASK_ULL(hi, low);
-	} while (!rsvd_bits);
+	if (!rsvd_bits) {
+		retry = 5;
+		do {
+			rsvd_bits = (rdtsc() << low) & GENMASK_ULL(hi, low);
+			retry--;
+		} while (!rsvd_bits && retry);
+	}
 
 	return rsvd_bits;
 }
@@ -2733,10 +2745,11 @@ static void svm_npt_rsvd_bits_test(void)
 
 	/*
 	 * 4k PTEs don't have reserved bits if MAXPHYADDR >= 52, just skip the
-	 * sub-test.  The NX test is still valid, but the extra bit of coverage
+	 * sub-test. Also skip if cannot generate the valid random reserved bits.
+	 * The NX test is still valid, but the extra bit of coverage
 	 * isn't worth the extra complexity.
 	 */
-	if (cpuid_maxphyaddr() >= 52)
+	if ((cpuid_maxphyaddr() >= 52) || !get_random_bits(51, cpuid_maxphyaddr()))
 		goto skip_pte_test;
 
 	_svm_npt_rsvd_bits_test(npt_get_pte((u64)basic_guest_main),