>-----Original Message-----
>From: Sean Christopherson <seanjc@xxxxxxxxxx>
>Sent: Friday, December 31, 2021 1:50 AM
>To: Duan, Zhenzhong <zhenzhong.duan@xxxxxxxxx>
>Cc: kvm@xxxxxxxxxxxxxxx; pbonzini@xxxxxxxxxx
>Subject: Re: [kvm-unit-tests PATCH] x86: Assign a canonical address before
>execute invpcid
>
>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?
Sorry for late response. Not clear why the mail went into junk box automatically.
Yea, I think your change is better. Will you send formal patch with your change
or you want me to do that?
Thanks
Zhenzhong
>
>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
