Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <113918f9-0e44-3d46-8b48-028277ec26bb@intel.com>
Date:   Fri, 7 Apr 2023 11:22:37 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [RFC PATCH] x86/fpu/xstate: Add more diagnostic information on
 inconsistent xstate sizes
To:     Fenghua Yu <fenghua.yu@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>
CC:     linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
        Chintan M Patel <chintan.m.patel@intel.com>,
        Thiago Macieira <thiago.macieira@intel.com>
References: <20230405183942.734019-1-fenghua.yu@intel.com>
Content-Language: en-US
From:   "Chang S. Bae" <chang.seok.bae@intel.com>
In-Reply-To: <20230405183942.734019-1-fenghua.yu@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VDhPSVhPWkxZcEpYbWYwRFVxT3pMNTJ5UWg3VUF5VGZNSXIzS2VuVzl3VVFy?=
 =?utf-8?B?d0tCV2RFN05RTEVzK1NHZ1NBemZ4MlFscWVOd1pYN2NVV2MxVVJUQkJUazF3?=
 =?utf-8?B?UmpjYzZqaHJVQUNSVWpSMXM1ak1VRGxacytMNUMyWTROWWtaRWl6VmJKT1Bq?=
 =?utf-8?B?V2ZaMlkyZ2JoMXZzWnlKZFBKakI4OE9GOFhoV1NpaUdNYXRiSHlCVGFEeTFz?=
 =?utf-8?B?bEI5TENQTVA3UjNBYVBYbERMZHl6L3ZNWnYxL1RFNWJXWVE2WGtvTmFyaHQx?=
 =?utf-8?B?UWFnaW5GekNLSmhFZmxMbWtkWWh2NzZ6dEpzc1ZPM09zRUwxa3hXdzUyc003?=
 =?utf-8?B?VTZ2VGw5SWl2V3lydUJKWWc0NnliS3ZRY1YzSjlqV1JyVkZhQTNUNVJXaXA1?=
 =?utf-8?B?M0JyWFJ2eVNORTF5bjB5YmpJRHFpaHA1TmJqeWhJd2ZEZE11M1hUR3JRYStD?=
 =?utf-8?B?V3FzYUhZOVdRMlJYYXY5dG9GOFhzL1lyYnhtYUtLT0pMazRLUktmVkpKdHcz?=
 =?utf-8?B?K2pKcTRqcDl2Z3pJbmdhUkp3aW5lWkQweGNJOEFpN3Nadll5ZDNTNjdkMmhv?=
 =?utf-8?B?ajJFSzRZU0s2MG4xamRKenVDa05nU3NWUzU5bld1UkhqcU1rcWV2SGgrQzNL?=
 =?utf-8?B?Ykxmdko1K0Ura3hIZHhCeDhqODZBaUk3cU1sYXpiMVNZUU92N1RNb0MyRklM?=
 =?utf-8?B?TEQyVjlHT0hrRTdmSkJOZS9xV09qVmNPSUl0cWpFN1BPbnI1ek5tK3Nrd2hn?=
 =?utf-8?B?QTlVeGFxTzJaV2IydkJDeks3cStFY0FYektRQVg3MEh1bytEeTdlcFVTempC?=
 =?utf-8?B?eU0rVkFIWHNPOFBUMkcvWDA0MDNTODYxZXhFTC8rQmZmdG4rYVhJRWxRL2ov?=
 =?utf-8?B?djc2cmxpK1ZZeHFCZTVFTkJoTmhzN3ZDNjdzNlRoSDBVRGMvWUpNYlZBVDNy?=
 =?utf-8?B?d0Fyb2xxZlh0NnAxck52TkpCUDF1U0ZuczlYVEN6RzY0TEo3RXd6ZzZURFpQ?=
 =?utf-8?B?eHBleHpuNDJLakhPdENLdHJLTG5kekc4aERIUXJQb0JrWEFDOFFWWmN0ZUhN?=
 =?utf-8?B?bjhJNzhNTUVXL1o4MHZHTDZIdmV5NWZDKzg3a0xvVUdUQVdCTGswcGx5SzEw?=
 =?utf-8?B?cmZUdzhnbGhpYkVmUmFmWmErVUFiVlJSYm52YjVyTlpzZ1U4WHRxKzRQRGsw?=
 =?utf-8?B?bFVKZ1NETjlkMzR3ajdNemVhK0NRVVFxM3F4Si9TWlJSakpQakxzM1lOaDRj?=
 =?utf-8?B?dU1hU216OFJ5MVV5elRkTVFKaWJEZ2xNaC8ySzdCNXJWZEI0cjIzMzVZcyty?=
 =?utf-8?B?NFlSaFJTME9rWTVOaUtRc2VVSElhOWhQR0l2MjF5NVlmMzJjRUJwRWVFaVAx?=
 =?utf-8?B?RG5GYWZTdG9wU21rUDV3SkdreVQrNTJyWXN1M3VDZGg2THhVc29qa3JmdHJv?=
 =?utf-8?B?dU04dGlHODZmUWxSRS9MMDdUVUp3cnVIdXc3RzVpZ2ljREs1K3djTU1UaUEz?=
 =?utf-8?B?NG1MUTFQYVVDUURKN3lKQitHcFBzcVZMS1RtQ0hBOTEzRFgxVCtLY2tEUld2?=
 =?utf-8?B?dnZjUDcrVzBJUzNudzlUOCszdi9aQzNsVU1xZzR6V2xlazNzdWhaTXVCVGRJ?=
 =?utf-8?B?aXJHcVJDN3RzeGtERVpCLzBaRDQxK1lQS1ZMN3pvay9BUEluZU5nZVA3Ykhs?=
 =?utf-8?B?K2pOekIrUVlRaUhSY3oveHc3bEJRc1ZHQ2dTK3N3WDZTZ1JESVZseGpnKzJs?=
 =?utf-8?B?bDE2NDFvSHY3UHpjcHcrMEQ0RUx6ZkdMQzdybk5Bb0VFOVA3RklZTjd6ZHJn?=
 =?utf-8?B?VUVMZzZ3czNWNS9ZUFBaZFRqaHJxd0Y1amd0cE1nblRSNE15bFhaRzdhZG9I?=
 =?utf-8?B?UHl4WXU4Q1lXdUVOeFU3eWp1VGtDUEdXbUNBdkNDdG5vS3VQSlBkbEJjTWcr?=
 =?utf-8?B?cFo1MEp0QUlhVmtNbU5KUG5tL2k0SWVEK0VpeDhQNEZqLzh2Ynk5Z01CVnJ5?=
 =?utf-8?B?WTNXR3VObUZ1dVl5M3hXMDZsMi85WG9HWFk2VUlWZGwzd2diQkN0dWFzazA3?=
 =?utf-8?B?MHFaMDBmRjRuZnpuTTU1U0RrMkJOTjh3Zm5lbVJHYjc1VTMrZkhxdU5TaXRR?=
 =?utf-8?B?WXV0UDg4WE9jSDJFZE9BQmhLVldjWHhCM2IyT2hmNHRqbFdpRkYrSWxkeExP?=
 =?utf-8?B?cHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 062fe9eb-7e8b-482d-509c-08db37951244
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4855.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Apr 2023 18:22:40.1469
 (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: 6gYUBLw46ptiKoROZuuaPmJeqmiEKDrCWLrk8ktXQW4y3kl2a060SR7jz35hVI0vzZdu5N9caNkcWZ3Dmq87IU1LxZogiyPTbV4/ofOztHM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB6126
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 4/5/2023 11:39 AM, Fenghua Yu wrote:
> A warning is emitted when xstate sizes are found inconsistent.
> But the warning message doesn't show enough information to diagnose
> the issue.
> 
> Provide more detailed xstate size information to help debug the issue.
> As a hypothetical example, on a platform that may report incorrect
> xstate size in CPUID.0xd.1:EBX, the diagnostic information after
> the warning will show:
> [    0.000000] x86/fpu: max_features=0x407
> [    0.000000] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
> [    0.000000] x86/fpu: xstate_offset[10]: 832, xstate_sizes[10]:    8
> [    0.000000] x86/fpu: total size: 840 bytes
> [    0.000000] x86/fpu: XCR0=0x7, IA32_XSS=0x400
> [    0.000000] x86/fpu: kernel_size from CPUID.0xd.0x1:EBX: 576 bytes
> 
> XCR0 | IA32_XSS is 0x407 which is consistent with max_features.
> CPUID.0xd.0x1:EBX should report the size of the xsave area
> containing xstate components corresponding to bits set in
> XCR0 | IA32_XSS. But it only reports 576 bytes of xsave area which
> doesn't count xstate sizes for AVX (offset 2 and 256 bytes) and
> PASID (offset 10 and 8 bytes). This confirms that the platform
> reports xstate size incorrectly through the CPUID bits.
> 
> Suggest-by: Dave Hansen <dave.hansen@linux.intel.com>
> Tested-by: Chintan M Patel <chintan.m.patel@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>   arch/x86/kernel/fpu/xstate.c | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 0bab497c9436..5f27fcdc6c90 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -602,8 +602,37 @@ static bool __init paranoid_xstate_size_valid(unsigned int kernel_size)
>   		}
>   	}
>   	size = xstate_calculate_size(fpu_kernel_cfg.max_features, compacted);
> -	XSTATE_WARN_ON(size != kernel_size,
> -		       "size %u != kernel_size %u\n", size, kernel_size);
> +	if (size != kernel_size) {
> +		u64 xcr0, ia32_xss;
> +
> +		XSTATE_WARN_ON(1, "size %u != kernel_size %u\n",
> +			       size, kernel_size);
> +
> +		/* Show more information to help diagnose the size issue. */
> +		pr_info("x86/fpu: max_features=0x%llx\n",
> +			fpu_kernel_cfg.max_features);
> +		print_xstate_offset_size();
> +		pr_info("x86/fpu: total size: %u bytes\n", size);
> +		xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
> +		if (compacted) {
> +			rdmsrl(MSR_IA32_XSS, ia32_xss);

This shouldn't be directly read here because of the LBR state component.

See the function comment:

  * Independent XSAVE features allocate their own buffers and are not
  * covered by these checks. Only the size of the buffer for task->fpu
  * is checked here.

But, isn't that max_features bitmask pretty much about it?

> +			pr_info("x86/fpu: XCR0=0x%llx, IA32_XSS=0x%llx\n",
> +				xcr0, ia32_xss);
> +		} else {
> +			pr_info("x86/fpu: XCR0=0x%llx\n", xcr0);
> +		}
> +		/*
> +		 * In compact case, CPUID.0xd.0x1:EBX reports the size of
> +		 * the XSAVE size containing all the state components
> +		 * corresponding to bits set in XCR0 | IA32_XSS.
> +		 *
> +		 * Otherwise, CPUID.0xd.0x0:EBX reports the size of an XSAVE
> +		 * area containing all the *user* state components
> +		 * corresponding to bits set in XCR0.
> +		 */
> +		pr_info("x86/fpu: kernel_size from CPUID.0xd.0x%x:EBX: %u bytes\n",
> +			compacted ? 1 : 0, kernel_size);

Include Thiago who asked this.

Thanks,
Chang