[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/2] x86/Intel: use CPUID bit to determine PPIN availability


  • To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 27 Jan 2022 08:56:49 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=pGnjn2YzP8YJVc+76ST39NTXgr/APnCHJQ78huRYelE=; b=C0U8P1VFPiF2rPG58ewAkD9a4lh4ghUP50UvTa9shgeWOSmjVQDdit+SG0BEm4NSWlYIToaD3tkFy4BPnC3GMaYY/SVPbakgHowHmHxyRxNBF/Wn+QEnf6h9xNA7zdxC1nqbbApByz3r7ptNH/rKHZ6O6UgoNZDwwYOfsyj6s0NGs9S5G3ZMbMYPMiOldYwpDXtVavf77n0vQ8sLWwBWKLU4MvtkzeL4+x2H19GiDCsrKWHtCWON0SXNiN/E+7716nFvupaWGMaWUYJX47pCo+zImTGzt/wArdSMFFfl7cvFbA/3aloYEG6u8WQwkpetXDDTrbjFlnzvDqQylH/u3A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IoAmSqZ8fOHIqWqIoJzH5RX9ezTzPE5zm5GnTZZVteBZ07+bCGwRE6pVgMclT2w0RbOOE/yy3cYlFz9WEuEfegDfXEFREQQ5mTrbQd7Lyfn9iDrZ6LfWuvAUfaWrhLWunQw1af9hlGolswTT5oKjCaj1v7FyybaX98YsX+CeS5pesVsUGRZ21YQ61wQL0+SA+PigU1Hz9AbuXyfRqGqbNz9uR2JZwhtrYHreUXhed5J6WyjpwQMPsOAWDDcOU7V6AkKefFUb1zIdYX30PMwFMlvp/xyt2XGex9bK8MRnE6ncAerDsWyxqYaikPs9rgHI/08O/ERFs1N8OnQsp5tLXw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 27 Jan 2022 07:57:04 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27.01.2022 00:30, Andrew Cooper wrote:
> On 20/01/2022 14:17, Jan Beulich wrote:
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -195,6 +195,11 @@ static const char *const str_e21a[32] =
>>      [ 6] = "nscb",
>>  };
>>  
>> +static const char *const str_7b1[32] =
>> +{
>> +    [ 0] = "ppin",
>> +};
> 
> I hadn't realised what a mess we had with the prefixes.
> 
> We have AMD_PPIN rendered as simply "ppin", while we also have
> AMD_{STIBP,SSBD} which are rendered with an amd- prefix.  This patch is
> the first introduction of an INTEL_ prefixed feature.
> 
> We should figure out a consistency rule and fix the logic, before adding
> more confusion.
> 
> Given the AMD MSR_SPEC_CTRL series just posted, use of CPUID bits will
> often be symmetrical and it's awkward to have one or with a prefix and
> the other without.

IOW you suggest I add a kind-of-prereq patch to drop the prefixes?

>> --- a/xen/include/xen/lib/x86/cpuid.h
>> +++ b/xen/include/xen/lib/x86/cpuid.h
>> @@ -16,6 +16,7 @@
>>  #define FEATURESET_7d0    9 /* 0x00000007:0.edx    */
>>  #define FEATURESET_7a1   10 /* 0x00000007:1.eax    */
>>  #define FEATURESET_e21a  11 /* 0x80000021.eax      */
>> +#define FEATURESET_7b1   12 /* 0x00000007:1.ebx    */
>>  
>>  struct cpuid_leaf
>>  {
>> @@ -188,6 +189,10 @@ struct cpuid_policy
>>                  uint32_t _7a1;
>>                  struct { DECL_BITFIELD(7a1); };
>>              };
>> +            union {
>> +                uint32_t _7b1;
>> +                struct { DECL_BITFIELD(7b1); };
>> +            };
>>          };
>>      } feat;
>>  
>>
> 
> Looking at a related patch I've got, at a minimum, you also need:
> * collect the leaf in generic_identify()

I'll need to make a patch first to collect 7a1, as it seems. It was
actually 7a1 that I used as a reference, iirc.

> * extend cpuid_policy_to_featureset() and cpuid_featureset_to_policy()

Yeah, I missed those. Presumably by looking for instances only under
arch/x86/. Especially with per-arch include/ now living under
arch/<arch>/, having separate x86 bits elsewhere is a little unhelpful.

> However I've got an idea to help us split "add new leaf" from "first bit
> in new leaf" which I'd like to experiment with first.  It is rather
> awkward having the two mostly-unrelated changes forced together in a
> single patch.

I'll make the necessary adjustments here anyway. I can always re-base
on top of what you may come up with.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.