[PATCH v2] x86: Assign a canonical address before execute invpcid

[PATCH v2] x86: Assign a canonical address before execute invpcid

[Zhenzhong Duan]

Accidently we see pcid test falied as INVPCID_DESC[127:64] is
uninitialized before execute invpcid.

According to Intel spec: "#GP If INVPCID_TYPE is 0 and the linear
address in INVPCID_DESC[127:64] is not canonical."

By zeroing the whole invpcid_desc structure, ensure the address
canonical and reserved bit zero in desc.

Fixes: b44d84dae10c ("Add PCID/INVPCID test")
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 x86/pcid.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/x86/pcid.c b/x86/pcid.c
index 527a4a9..80a4611 100644
--- a/x86/pcid.c
+++ b/x86/pcid.c
@@ -5,9 +5,9 @@
 #include "desc.h"
 
 struct invpcid_desc {
-    unsigned long pcid : 12;
-    unsigned long rsv  : 52;
-    unsigned long addr : 64;
+    u64 pcid : 12;
+    u64 rsv  : 52;
+    u64 addr : 64;
 };
 
 static int write_cr0_checking(unsigned long val)
@@ -73,12 +73,12 @@ static void test_invpcid_enabled(int pcid_enabled)
     int passed = 0, i;
     ulong cr4 = read_cr4();
     struct invpcid_desc desc;
-    desc.rsv = 0;
+
+    memset(&desc, 0, sizeof(desc));
 
     /* try executing invpcid when CR4.PCIDE=0, desc.pcid=0 and type=0..3
      * no exception expected
      */
-    desc.pcid = 0;
     for (i = 0; i < 4; i++) {
         if (invpcid_checking(i, &desc) != 0)
             goto report;
-- 
2.25.1

Re: [PATCH v2] x86: Assign a canonical address before execute invpcid

[Sean Christopherson]

On Wed, Jan 12, 2022, Zhenzhong Duan wrote:
> Accidently we see pcid test falied as INVPCID_DESC[127:64] is
> uninitialized before execute invpcid.
> 
> According to Intel spec: "#GP If INVPCID_TYPE is 0 and the linear
> address in INVPCID_DESC[127:64] is not canonical."
> 
> By zeroing the whole invpcid_desc structure, ensure the address
> canonical and reserved bit zero in desc.

The changelog should also note the opportunistic change from "unsigned long"
to "u64".  It's all too easy to forget that they're equivalent due to this being
64-bit only.

Reviewed-by: Sean Christopherson <seanjc@google.com>