Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <121cb43b-029d-4d34-8f6c-15f41c2dd0aa@intel.com>
Date:   Thu, 14 Dec 2023 11:12:54 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 02/26] x86/fpu/xstate: Refine CET user xstate bit
 enabling
To:     "Chang S. Bae" <chang.seok.bae@intel.com>,
        Maxim Levitsky <mlevitsk@redhat.com>
CC:     <linux-kernel@vger.kernel.org>, <kvm@vger.kernel.org>,
        <dave.hansen@intel.com>, <pbonzini@redhat.com>,
        <seanjc@google.com>, <peterz@infradead.org>, <chao.gao@intel.com>,
        <rick.p.edgecombe@intel.com>, <john.allen@amd.com>
References: <20231124055330.138870-1-weijiang.yang@intel.com>
 <20231124055330.138870-3-weijiang.yang@intel.com>
 <c22d17ab04bf5f27409518e3e79477d579b55071.camel@redhat.com>
 <cdf53e44-62d0-452d-9c06-5c2d2ce3ce66@intel.com>
 <20d45cb6adaa4a8203822535e069cdbbf3b8ba2d.camel@redhat.com>
 <a3a14562-db72-4c19-9f40-7778f14fc516@intel.com>
 <039eaa7c35020774b74dc5e2d03bb0ecfa7c6d60.camel@redhat.com>
 <eb30c3e0-8e13-402c-b23d-48b21e0a1498@intel.com>
 <e7d7709a5962e8518ccb062e3818811cdbe110f8.camel@redhat.com>
 <917a9dc4-bcae-4a1d-b5b5-d086431e8650@intel.com>
 <62ff5c86-a63a-46f2-88f8-6c1589433a89@intel.com>
Content-Language: en-US
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <62ff5c86-a63a-46f2-88f8-6c1589433a89@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SVNnSjNON0w3bXV2UEMxajg0ZDZ6VWNhd0lLRXFpUW5CSnRCU3IyVTRKYXdN?=
 =?utf-8?B?VkNkTEZpeHpYTVdoWVZJT0ZodHVYR28zZFlzdG1xTHJFblVlUDM3WUp0YWhF?=
 =?utf-8?B?b2Z0dFNOOEFhazQ5TlNTZWhBWTgrZU11bFp6ZFlEeGsvZVRzYURSTFkrZ2U5?=
 =?utf-8?B?bUF6Nk90Q2FKaU1TY29ubVJJUVhxMFlpelkzTExXb0k3Z01zRHBuYWsvb1F2?=
 =?utf-8?B?RlhrTmp1QW5malJqSXo2RXl2amRHM1JQWWYwVE5TS3g5Zzc2VGVSTy95NUVQ?=
 =?utf-8?B?OElJQisrQWdzYy81c0QzTGczajlMcmF2eSs1eis0M1JCZ1Z6SytxT2FXcW5w?=
 =?utf-8?B?YkdhdGF6Q3JMMnJJN1lKQUp5L1hCWFlUVmoxcU1tV28rbjZxUEozTVBQWm96?=
 =?utf-8?B?NTVZdG9NeE9QY09ONmtvVXhPaUR0dXJUczgxbm1DSVd6RTNOUWxtdGRCOFkw?=
 =?utf-8?B?YXJxUXIzdng0MFNsQW9VbEt4eGl3MGpDSVRMQ3diNko5ZlJwZE9YTnhJN0Nh?=
 =?utf-8?B?ckhEWDNaaDJyTWxaNmNrVGF1T2EzNzUwZHgyL3dTWlorMmxxTDFQdjZackN5?=
 =?utf-8?B?Q0NzSHc4NnZzWWI0U2JIczcrR05jU3lZTlNIWCtFM3BuaFozZDVIRitINXBN?=
 =?utf-8?B?SXovQWtCNDJGdUhnNU8zcWttbVpzZjB6RCtzTG1zbnUwcXY1S3ZuRDltWlBi?=
 =?utf-8?B?OEY0WmtQeEo1bGdqSUltM2wxODlKeVo0MmVCQXN1bXpuOVFTOGhVSWN0NzRo?=
 =?utf-8?B?ZlJ1UGpzUFVpRzA1RVpLZkhVbm9PWVJiRmR2SWJHTytEQkhqOFA3MGlva2NQ?=
 =?utf-8?B?L1lHR0RiU0p6TXZnMWdWQkQrSHhNNVFYeitkVnQ1c0VHTzI0VEF6V2JWTWFz?=
 =?utf-8?B?amZPRWFkS0RIUDFnQ09lZkthUlhHZjBiOU50QndtSVFDeU1Yd1ZFRDdBNUpa?=
 =?utf-8?B?akJDdDNTeG4yYXdab0hROHIxUEd4c1NMamFqOWFBSEVjYzBIYmNKRnRyTStW?=
 =?utf-8?B?VXhGVkkzbXFUejYzTno0WDFxcUVwcUFqUFkyOURSazk2WUduZXNWZDJCY3pt?=
 =?utf-8?B?M1hCYVB3VVNxeG1jcDl0eVdXc3V5SzlKT2kvd2YwdTdNS2hEcmxqQ2x1S3pD?=
 =?utf-8?B?RjRvM3lzdFdESDQrWjRKbFlxUnlJOHpaVC9VM0tGQis5cVdjdDdBbXJnM3lP?=
 =?utf-8?B?MGJsbjdabHhPSTFrTWttNGh2Nk1xUHZRU2tDR3pCN3pOTmx6VytacVZOb2FM?=
 =?utf-8?B?STl5TGhqMHh2WDkzOXY3ejBmZVBsZzBnRnZCV01XSG03RS90SGoxOCtjVytk?=
 =?utf-8?B?cnZaL2dqd2lXMVNSWXlOYTE3VzhoYnJOQ3ZXcXdpODhiNVNFVm9aS054ait1?=
 =?utf-8?B?cEVjUU4xM3dhSmNuV215WU10b04weTNYbEhXV3JJQ21lSEQ2R2NrWmRMSnA1?=
 =?utf-8?B?bUlva3lORlNPZE9lU1dKNmlEd0hUc21iVXJIWnBrQTFYbTlGVk5pZHRUb3FZ?=
 =?utf-8?B?V09lekJJQjBQcHBodFZwV045VmZyODkraWxCNUV3Q0tTckxLaFRvYnRBZ3JS?=
 =?utf-8?B?VFJLcDNLeUx2RzVMeEMreXFURnAxa2dyeVBiWmUyVm1vM1FoV09CL1pZR1BR?=
 =?utf-8?B?d3JHQ0lUQWJGZWxkU0FlUFFzYWdKallOaktsUy80VTJIVTVndHQvYmRwV1RK?=
 =?utf-8?B?Z2ZxVnc0dnRaNlhvbUVPWjQ3QlZ4aXdZaXFIb09Wajh4ckY3dmp4QU10Wis0?=
 =?utf-8?B?M1FjME52YjltQVNpNDc5L3AyeFFEY2ozRGNNQXp6dm5ic05UVkZSMFNYK2ZO?=
 =?utf-8?B?ZS9Ub2wrcm5aU2dmNDBJaHNheURTWHhQc3RKUzdGMUQvRGVjRnBROHlrc2M2?=
 =?utf-8?B?ZDkyZUVESDlHemxjOURDWEsrMHhocjg4aW1FS1g4QldFTEgrZFRwa2JkNnVr?=
 =?utf-8?B?czlrSTFXZmlsTko3ekFQNjljWFhRdVBJVDB0eSs0UStlSFJOUjRURG5ORTEy?=
 =?utf-8?B?bHNaVVQ0aEpoVThaaWRxQ24vVUZHOW5mZndXenlxeDVyYXhUUmg1RU9Va09m?=
 =?utf-8?B?VG9kbCt2UGx5emo4dEs0d3lXNCtrY0krR08zZ0kwZzlndlBrOFVnUHlVZEZL?=
 =?utf-8?B?V0NuSjZZSjRaWkp4bXpTVFY1Tms3ams0ckZjTWhNaGtGOVBkVGgzeHIvOGE4?=
 =?utf-8?B?V2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: aad72991-8137-46c2-74cb-08dbfc52962a
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Dec 2023 03:13:04.3043
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3wo41TgC9MgV7YUcAPS/jKh9SjlzL2+gp1EUtxF2oIRVLB5Jb/pgma+f4iDXqghtVQX0mMOtsdvV832TfqT9CA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7668
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Wed, 13 Dec 2023 19:13:21 -0800 (PST)

On 12/14/2023 1:01 AM, Chang S. Bae wrote:
> On 12/13/2023 1:30 AM, Yang, Weijiang wrote:
>>
>> Let me involve Chang, the author of the code in question.
>>
>> Hi, Chang,
>> In commit 70c3f1671b0c ("x86/fpu/xstate: Prepare XSAVE feature table for gaps in state component numbers"),
>> you modified the loop as below:
>>          for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
>> -               if (!boot_cpu_has(xsave_cpuid_features[i]))
>> +               unsigned short cid = xsave_cpuid_features[i];
>> +
>> +               /* Careful: X86_FEATURE_FPU is 0! */
>> +               if ((i != XFEATURE_FP && !cid) || !boot_cpu_has(cid))
>>                          fpu_kernel_cfg.max_features &= ~BIT_ULL(i);
>>          }
>>
>> IMHO the change resulted functional change of the loop, i.e., before that only it only clears the bits without CPUIDs,
>> but after the change, the side-effect of the loop will clear the bits of blank entries ( where xsave_cpuid_features[i] == 0 )
>> since the loop hits (i != XFEATURE_FP && !cid), is it intended or something else?
>
> The code was considered as much *simpler* than the other [1]. Yes, it clears those not listed in the table but they were either non-existed or disabled at that moment.

Thanks Chang, so I assume the "side-effect" is intended.

>
> Thanks,
> Chang
>
> [1] https://lore.kernel.org/lkml/87y2eha576.fsf@nanos.tec.linutronix.de/
>