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

Re: [PATCH 1/2] x86/cpuid: Infrastructure for leaves 7:1{ecx,edx}


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 3 Mar 2023 18:32:51 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; 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=OT/qFtyL5etcUL4kw9EP+A99YyKOwwvnOc9AHYczEfI=; b=g2Qo/eblGiiMC+WmUgC7dzfRxaOydH8SwZSM9oSMvKy0RScUAD/r+dtL7W8XELqLiPgil6aC4tgMQgBb4ybbqwXVijVq+I7S4a5txU/ugb17bDQ1xooBKY3LE9z7mwm7EyRehkZK3WUJwdqpclX5d+nF5dGUDEvU7EDBzxQ6FC/LAWg3dVG6e90hsjvjTpL0FKDWjduJe0RD0JdqGlVJrjlPblC1/KclQvxtX398v7v+KilJy9flVEw/9UoN0RWVsC3PkrFW0SKBqoCX9ceSjjobBj//QPDVYGlA+B3+Hb3fQ2TmIkNPlllugoVe0s/AKU3lqtm7ZpdayptIwAGpKw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VWaZJT2neiL6UpsqCaoYNRklZdBoNlVyQWjcFJd2gB0QokJbQqf08WHWJtkOnd2E11bsoYuka8bRlofHG0JI0BFDxg+WAZLGs74JLSLD9ZibMaT7DoaSqwFmk/GpLUI5Tv8mu23Ot8TrZgvC0dZ4CQt+1okMnj1c4gdAwiKRYllzmik3uV8xu3hMfsoLHF9LflUFJk9oHVrD1ctcpgxTeI2cB/HW6ulkkfaIOHY1kFfXBgcULAbcqATJNTI41Jh+zz7oe6vmRySACQ1J1pPjQeCox6cKdET8p+gtr1M0EdOU1QdD4iSM+9C+JpzlfatuI4ikeduQpxaFfbtXAFz3zg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 03 Mar 2023 18:33:23 +0000
  • Ironport-data: A9a23:f5CFxqvOJucIlrye5ZPTDFoFuOfnVHRfMUV32f8akzHdYApBsoF/q tZmKT/VP66INDDxKNlwaonk90sG7cXQm4BhT1NoqilhF39H+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg3HVQ+IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4bKj6Fv0gnRkPaoQ5ASGzCFPZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMwKwE/cSiBttyPw6u9dfFHnPwffJjQFdZK0p1g5Wmx4fcOZ7nmGvyPz/kImTA6i4ZJAOrUY NcfZXx3dhPcbhZTO1ARTpUjgOOvgXq5eDpdwL6XjfNvvy6Pk0osgP60bou9lt+iHK25mm6xo G7c8nu/KRYdLNGFkhKO8262h/+JliT+MG4XPOzpqa400ATCroAVIEQaDXuXp6a0tmKZR4oYK WMLw3UnnYFnoSRHSfG4BXVUukWsvBQRRt5RGO0S8xyWx+zf5APxLncAZi5MbpohrsBebSwn0 BqFks3kARRrsaaJUjSN+7GMtzSwNCMJa2gYakc5oRAt5tDipMQ5iELJR9M6Saqt1ISrSHf33 iyAqzU4i/MLl8kX2q6n/FfBxTWxupzOSQ1z7QLSNo640j5EiEeeT9TAwTDmATxodu51knHpU KA4pvWj
  • Ironport-hdrordr: A9a23:o8Ye4KkD8QWAWk81vyP3dMU8W7HpDfIR3DAbv31ZSRFFG/GwvM ql9c5rsiMc7wx8ZJhAo7+90cy7Kk80mqQa3WB8B9aftWvdyQiVxfBZjbcKqgeIc0eSygc379 YDT0ERMqyVMXFKyer8/QmkA5IB7bC8gcaVbD7lvhJQcT0=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 03/03/2023 7:23 am, Jan Beulich wrote:
> On 04.01.2023 12:11, Andrew Cooper wrote:
>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>> @@ -288,6 +288,9 @@ XEN_CPUFEATURE(NSCB,               11*32+ 6) /*A  Null 
>> Selector Clears Base (and
>>  /* Intel-defined CPU features, CPUID level 0x00000007:1.ebx, word 12 */
>>  XEN_CPUFEATURE(INTEL_PPIN,         12*32+ 0) /*   Protected Processor 
>> Inventory Number */
>>  
>> +/* Intel-defined CPU features, CPUID level 0x00000007:1.ecx, word 14 */
>> +/* Intel-defined CPU features, CPUID level 0x00000007:1.edx, word 15 */
> While committing the backports of this (where I normally test-build
> every commit individually) I came to notice that this introduces a
> transient (until the next commit) build breakage: FEATURESET_NR_ENTRIES
> is calculated from the highest entry found; the comments here don't
> matter at all. Therefore ...
>
>> @@ -343,6 +352,8 @@ static inline void cpuid_policy_to_featureset(
>>      fs[FEATURESET_e21a] = p->extd.e21a;
>>      fs[FEATURESET_7b1] = p->feat._7b1;
>>      fs[FEATURESET_7d2] = p->feat._7d2;
>> +    fs[FEATURESET_7c1] = p->feat._7c1;
>> +    fs[FEATURESET_7d1] = p->feat._7d1;
>>  }
>>  
>>  /* Fill in a CPUID policy from a featureset bitmap. */
>> @@ -363,6 +374,8 @@ static inline void cpuid_featureset_to_policy(
>>      p->extd.e21a  = fs[FEATURESET_e21a];
>>      p->feat._7b1  = fs[FEATURESET_7b1];
>>      p->feat._7d2  = fs[FEATURESET_7d2];
>> +    p->feat._7c1  = fs[FEATURESET_7c1];
>> +    p->feat._7d1  = fs[FEATURESET_7d1];
>>  }
> ... the compiler legitimately complains about out-of-bounds array
> accesses here. This is just fyi for the future (to arrange patch
> splitting differently); I've left the backports as they were.

Hmm.  c/s e3662437eb43 was designed to specifically allow CPUID patches
to be split like this.

Which compiler?  I think I agree with your analysis, but I've never seen
a complaint, hence not noticing.

I suspect we actually want FEATURESET_NR_ENTRIES defined in C, next to
the FEATURESET_* defines, and we want to BUILD_BUG_ON() if the autogen
length is larger than the C length.

~Andrew



 


Rackspace

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