Re: [kvm-unit-tests PATCH] x86: Assign a canonical address before execute invpcid

[Sean Christopherson <seanjc@google.com>]

On Thu, Dec 30, 2021, Sean Christopherson wrote:
> On Thu, Dec 30, 2021, Zhenzhong Duan wrote:
> > Occasionally we see pcid test fail 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."
> > 
> > Assign desc's address which is guaranteed to be a real memory
> > address and canonical.
> > 
> > Fixes: b44d84dae10c ("Add PCID/INVPCID test")
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > ---
> >  x86/pcid.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/x86/pcid.c b/x86/pcid.c
> > index 527a4a9..4828bbc 100644
> > --- a/x86/pcid.c
> > +++ b/x86/pcid.c
> > @@ -75,6 +75,9 @@ static void test_invpcid_enabled(int pcid_enabled)
> >      struct invpcid_desc desc;
> >      desc.rsv = 0;
> >  
> > +    /* Initialize INVPCID_DESC[127:64] with a canonical address */
> > +    desc.addr = (u64)&desc;
> 
> Casting to a u64 is arguably wrong since the address is an unsigned long.  It
> doesn't cause problems because the test is 64-bit only, but it's a bit odd.

I take that back, "struct invpcid_desc" is the one that's "wrong".  Again, doesn't
truly matter as attempting to build on 32-bit would fail due to the bitfield values
exceeding the storage capacity of an unsigned long.  But to be pedantic, maybe this?

diff --git a/x86/pcid.c b/x86/pcid.c
index 527a4a9..fd218dd 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,7 +73,8 @@ 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