diff mbox series

[kvm-unit-tests,4/5] x86: svm: wrong reserved bit in npt_rsvd_pfwalk_prepare

Message ID 20200630094516.22983-5-namit@vmware.com (mailing list archive)
State New, archived
Headers show
Series x86: svm: fixes | expand

Commit Message

Nadav Amit June 30, 2020, 9:45 a.m. UTC 
According to AMD manual bit 8 in PDPE is not reserved, but bit 7.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 x86/svm_tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini June 30, 2020, 11:06 a.m. UTC | #1 
On 30/06/20 11:45, Nadav Amit wrote:
> According to AMD manual bit 8 in PDPE is not reserved, but bit 7.

Indeed.  Maybe it was a problem in the previous versions because I
remember adding this check explicitly.  I've sent a patch.

Paolo

> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  x86/svm_tests.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index 92cefaf..323031f 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
>      vmcb_ident(vmcb);
>  
>      pdpe = npt_get_pdpe();
> -    pdpe[0] |= (1ULL << 8);
> +    pdpe[0] |= (1ULL << 7);
>  }
>  
>  static bool npt_rsvd_pfwalk_check(struct svm_test *test)
>  {
>      u64 *pdpe = npt_get_pdpe();
> -    pdpe[0] &= ~(1ULL << 8);
> +    pdpe[0] &= ~(1ULL << 7);
>  
>      return (vmcb->control.exit_code == SVM_EXIT_NPF)
>              && (vmcb->control.exit_info_1 == 0x20000000eULL);
>
Paolo Bonzini June 30, 2020, 4:54 p.m. UTC | #2 
On 30/06/20 11:45, Nadav Amit wrote:
> According to AMD manual bit 8 in PDPE is not reserved, but bit 7.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  x86/svm_tests.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index 92cefaf..323031f 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
>      vmcb_ident(vmcb);
>  
>      pdpe = npt_get_pdpe();
> -    pdpe[0] |= (1ULL << 8);
> +    pdpe[0] |= (1ULL << 7);
>  }
>  
>  static bool npt_rsvd_pfwalk_check(struct svm_test *test)
>  {
>      u64 *pdpe = npt_get_pdpe();
> -    pdpe[0] &= ~(1ULL << 8);
> +    pdpe[0] &= ~(1ULL << 7);
>  
>      return (vmcb->control.exit_code == SVM_EXIT_NPF)
>              && (vmcb->control.exit_info_1 == 0x20000000eULL);
> 

Wait, bit 7 is not reserved, it's the PS bit.  We need to use the PML4E instead (and
then using bit 7 or bit 8 is irrelevant):

diff --git a/x86/svm.c b/x86/svm.c
index 0fcad8d..62907fd 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -49,6 +49,11 @@ u64 *npt_get_pdpe(void)
 	return pdpe;
 }
 
+u64 *npt_get_pml4e(void)
+{
+	return pml4e;
+}
+
 bool smp_supported(void)
 {
 	return cpu_count() > 1;
diff --git a/x86/svm.h b/x86/svm.h
index 645deb7..8d688b6 100644
--- a/x86/svm.h
+++ b/x86/svm.h
@@ -365,6 +365,7 @@ typedef void (*test_guest_func)(struct svm_test *);
 u64 *npt_get_pte(u64 address);
 u64 *npt_get_pde(u64 address);
 u64 *npt_get_pdpe(void);
+u64 *npt_get_pml4e(void);
 bool smp_supported(void);
 bool default_supported(void);
 void default_prepare(struct svm_test *test);
diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 92cefaf..b540527 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -824,13 +824,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
     u64 *pdpe;
     vmcb_ident(vmcb);
 
-    pdpe = npt_get_pdpe();
+    pdpe = npt_get_pml4e();
     pdpe[0] |= (1ULL << 8);
 }
 
 static bool npt_rsvd_pfwalk_check(struct svm_test *test)
 {
-    u64 *pdpe = npt_get_pdpe();
+    u64 *pdpe = npt_get_pml4e();
     pdpe[0] &= ~(1ULL << 8);
 
     return (vmcb->control.exit_code == SVM_EXIT_NPF)
Nadav Amit June 30, 2020, 5:37 p.m. UTC | #3 
> On Jun 30, 2020, at 9:54 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 30/06/20 11:45, Nadav Amit wrote:
>> According to AMD manual bit 8 in PDPE is not reserved, but bit 7.
>> 
>> Signed-off-by: Nadav Amit <namit@vmware.com>
>> ---
>> x86/svm_tests.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
>> index 92cefaf..323031f 100644
>> --- a/x86/svm_tests.c
>> +++ b/x86/svm_tests.c
>> @@ -825,13 +825,13 @@ static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
>>     vmcb_ident(vmcb);
>> 
>>     pdpe = npt_get_pdpe();
>> -    pdpe[0] |= (1ULL << 8);
>> +    pdpe[0] |= (1ULL << 7);
>> }
>> 
>> static bool npt_rsvd_pfwalk_check(struct svm_test *test)
>> {
>>     u64 *pdpe = npt_get_pdpe();
>> -    pdpe[0] &= ~(1ULL << 8);
>> +    pdpe[0] &= ~(1ULL << 7);
>> 
>>     return (vmcb->control.exit_code == SVM_EXIT_NPF)
>>             && (vmcb->control.exit_info_1 == 0x20000000eULL);
> 
> Wait, bit 7 is not reserved, it's the PS bit.  We need to use the PML4E instead (and
> then using bit 7 or bit 8 is irrelevant):

Err.. I remembered that bit 7 is not zero for no reason.

Now I wonder why it caused #PF at all. To be fair, I did not run it on
bare-metal yet, since I do not have access to such a machine.
diff mbox series

Patch

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index 92cefaf..323031f 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -825,13 +825,13 @@  static void npt_rsvd_pfwalk_prepare(struct svm_test *test)
     vmcb_ident(vmcb);
 
     pdpe = npt_get_pdpe();
-    pdpe[0] |= (1ULL << 8);
+    pdpe[0] |= (1ULL << 7);
 }
 
 static bool npt_rsvd_pfwalk_check(struct svm_test *test)
 {
     u64 *pdpe = npt_get_pdpe();
-    pdpe[0] &= ~(1ULL << 8);
+    pdpe[0] &= ~(1ULL << 7);
 
     return (vmcb->control.exit_code == SVM_EXIT_NPF)
             && (vmcb->control.exit_info_1 == 0x20000000eULL);