* [kvm-unit-tests PATCH 0/2] Couple of SVM fixes
@ 2021-08-06 16:08 Babu Moger
2021-08-06 16:08 ` [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations Babu Moger
2021-08-06 16:08 ` [kvm-unit-tests PATCH 2/2] nSVM: Fix NPT reserved bits test hang Babu Moger
0 siblings, 2 replies; 10+ messages in thread
From: Babu Moger @ 2021-08-06 16:08 UTC (permalink / raw)
Cc: pbonzini, thuth, drjones, kvm, 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(-)
--
^ permalink raw reply [flat|nested] 10+ messages in thread* [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-06 16:08 [kvm-unit-tests PATCH 0/2] Couple of SVM fixes Babu Moger @ 2021-08-06 16:08 ` Babu Moger [not found] ` <YQ1pA9nN6DP0veQ1@google.com> 2021-08-06 16:08 ` [kvm-unit-tests PATCH 2/2] nSVM: Fix NPT reserved bits test hang Babu Moger 1 sibling, 1 reply; 10+ messages in thread From: Babu Moger @ 2021-08-06 16:08 UTC (permalink / raw) Cc: pbonzini, thuth, drjones, kvm, 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; ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <YQ1pA9nN6DP0veQ1@google.com>]
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations [not found] ` <YQ1pA9nN6DP0veQ1@google.com> @ 2021-08-09 19:43 ` Babu Moger 2021-08-10 16:59 ` Babu Moger 0 siblings, 1 reply; 10+ messages in thread From: Babu Moger @ 2021-08-09 19:43 UTC (permalink / raw) To: Sean Christopherson; +Cc: pbonzini, thuth, drjones, kvm 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; >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-09 19:43 ` Babu Moger @ 2021-08-10 16:59 ` Babu Moger 2021-08-10 23:38 ` Babu Moger 0 siblings, 1 reply; 10+ messages in thread From: Babu Moger @ 2021-08-10 16:59 UTC (permalink / raw) To: Sean Christopherson; +Cc: pbonzini, thuth, drjones, kvm 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; >>> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-10 16:59 ` Babu Moger @ 2021-08-10 23:38 ` Babu Moger 2021-08-11 7:09 ` Paolo Bonzini 0 siblings, 1 reply; 10+ messages in thread From: Babu Moger @ 2021-08-10 23:38 UTC (permalink / raw) To: Sean Christopherson; +Cc: pbonzini, thuth, drjones, kvm 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; >>>> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-10 23:38 ` Babu Moger @ 2021-08-11 7:09 ` Paolo Bonzini 2021-08-11 16:03 ` Babu Moger 0 siblings, 1 reply; 10+ messages in thread From: Paolo Bonzini @ 2021-08-11 7:09 UTC (permalink / raw) To: Babu Moger, Sean Christopherson; +Cc: thuth, drjones, kvm 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-11 7:09 ` Paolo Bonzini @ 2021-08-11 16:03 ` Babu Moger 2021-08-11 16:13 ` Sean Christopherson 0 siblings, 1 reply; 10+ messages in thread From: Babu Moger @ 2021-08-11 16:03 UTC (permalink / raw) To: Paolo Bonzini, Sean Christopherson; +Cc: thuth, drjones, kvm 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; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-11 16:03 ` Babu Moger @ 2021-08-11 16:13 ` Sean Christopherson 2021-08-11 16:43 ` Babu Moger 0 siblings, 1 reply; 10+ messages in thread From: Sean Christopherson @ 2021-08-11 16:13 UTC (permalink / raw) To: Babu Moger; +Cc: Paolo Bonzini, thuth, drjones, kvm 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. */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations 2021-08-11 16:13 ` Sean Christopherson @ 2021-08-11 16:43 ` Babu Moger 0 siblings, 0 replies; 10+ messages in thread From: Babu Moger @ 2021-08-11 16:43 UTC (permalink / raw) To: Sean Christopherson; +Cc: Paolo Bonzini, thuth, drjones, kvm 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [kvm-unit-tests PATCH 2/2] nSVM: Fix NPT reserved bits test hang 2021-08-06 16:08 [kvm-unit-tests PATCH 0/2] Couple of SVM fixes Babu Moger 2021-08-06 16:08 ` [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations Babu Moger @ 2021-08-06 16:08 ` Babu Moger 1 sibling, 0 replies; 10+ messages in thread From: Babu Moger @ 2021-08-06 16:08 UTC (permalink / raw) Cc: pbonzini, thuth, drjones, kvm, 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), ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-08-11 16:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06 16:08 [kvm-unit-tests PATCH 0/2] Couple of SVM fixes Babu Moger
2021-08-06 16:08 ` [kvm-unit-tests PATCH 1/2] x86: access: Fix timeout failure by limiting number of flag combinations Babu Moger
[not found] ` <YQ1pA9nN6DP0veQ1@google.com>
2021-08-09 19:43 ` Babu Moger
2021-08-10 16:59 ` Babu Moger
2021-08-10 23:38 ` Babu Moger
2021-08-11 7:09 ` Paolo Bonzini
2021-08-11 16:03 ` Babu Moger
2021-08-11 16:13 ` Sean Christopherson
2021-08-11 16:43 ` Babu Moger
2021-08-06 16:08 ` [kvm-unit-tests PATCH 2/2] nSVM: Fix NPT reserved bits test hang Babu Moger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.