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@xxxxxxxxx>
> > ---
> > 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
