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: <388b09ea-7eb4-2878-5292-97475fde8a5a@intel.com>
Date:   Tue, 11 Apr 2023 21:38:50 -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>
 <113918f9-0e44-3d46-8b48-028277ec26bb@intel.com>
 <c716e021-99ad-2cb7-44c0-ec8393a3440d@intel.com>
 <2af114b9-2737-70e5-f534-e60416b52246@intel.com>
 <50e67263-33ba-9921-1bc2-a37b99bc2459@intel.com>
Content-Language: en-US
From:   "Chang S. Bae" <chang.seok.bae@intel.com>
In-Reply-To: <50e67263-33ba-9921-1bc2-a37b99bc2459@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?TXRtemRvQzMwMU9XMUhjTlF0aEkrZnVnUmNMY1hxQTFnT1JkbGQyOG5kLzJY?=
 =?utf-8?B?c3NrWG5zU2wyZUo2VE1yc21kYjdHYnQxbjVrMTBHcEZRaWdYbG4wUzh1N0lP?=
 =?utf-8?B?NCtybHI3RWxGZmZCU2Y1RStqeElOYU41cmhzUXJzTDc0ZExib1VNTmFmNk9M?=
 =?utf-8?B?ZTRraDhmbmVGaXNsdXVYRWhSM056R0tqd3FBbHJOWnpybGNIb0dwb0d6T1hN?=
 =?utf-8?B?M2VsTTZCSWFvV3NyV3FjWHk2akhmWkNIZDJkOU1STW1Fc2ZFQWdBZ3AwUTFD?=
 =?utf-8?B?QlpiYUR2L0pzUHMzMkJLNURDSU5VUXp1cm91ZWRXaGRNOGF6Rnc0WG9GU0F2?=
 =?utf-8?B?WTY3cjlVV3N1K0ZkZUdwNnRkamFXaW5odFY2NXppMytvcWd1RnMxSHBhblFO?=
 =?utf-8?B?d0FLdG5KQzFqdE1BemtxVkZrQWw3T09nYnRJQk1ZQjlMR211Q0N4bFc2dFI5?=
 =?utf-8?B?WFdnUkYydEYvMUQ5WmJUMkVzeFFIdTdJc3IxQ3l0dm9LTFExT2Q4STJtVVI0?=
 =?utf-8?B?aGVPUVdBbUVrZ0d4dUYwZVZKL0FPTlU5dnpUazFVQTVUWEZCQVNLVUlWQVJO?=
 =?utf-8?B?MVNYSFdvd1JZTmJqRWJrQmRPRXQ4WkVka1pIc1FHSHR0RTRkUmQ5Rlk5ZkZq?=
 =?utf-8?B?YlVsRjBCdDlXc21kdnB4RlVTc0R4MysrWEVzd1ZpMGltWlZTOTVMVWFhMURm?=
 =?utf-8?B?RjRyTzRFblM1Y0dWWThHdEZ3TWFod0pqVDFmc2NuakxnaW5PVmJlSUJ0VlEw?=
 =?utf-8?B?anZEeU94UDZGSHE2U1U4WGZ6Vzd4YnNLTHYvQ0NOTDJ5ZWMrK09Obm53NEp2?=
 =?utf-8?B?UkJPYnVFU1hBdEpQTkJtZmVKamVabmZsNjdGdDl6UjFKdTRtUTFwQ2hRcGRw?=
 =?utf-8?B?N2hwV1BFNHJvU0o0dzVMS2RxWGsxZ2M0dzJnMktjcjhSdy8yaXNtL3I5Z01o?=
 =?utf-8?B?Y25nekhFbURPYkkxUTdYWXhVRWFFRExnYVFQOGkrNXZVSHo3RFlrOWxzaFdn?=
 =?utf-8?B?SGowbWNTak5uNSsycmlEUXlFczJOanVNVnlSTzhXanhUQnZ5K0ZYc0dhdXVO?=
 =?utf-8?B?U2twTzNHeUtZSTZ4Q0tWMnA5aEp1d0RJUXFETC9HdXRwQXRUeVR1SGJoV09F?=
 =?utf-8?B?c1NmWnF0VUFpYWN4Uk1aQWpKZ05WT2ZjZ2pJQk4yTnpJeTR3bE1NYlNER0tE?=
 =?utf-8?B?TnRoUVVXbThpVDI1WGdBT1JWSlhuRjNPbllwelFDbDZ2MVV3S0l5Y2YrZTVY?=
 =?utf-8?B?Nk4xU1FjRWYvRzdRZlFtd0ttNjZJUjRNM0R3bnk0c0lxd2NLdWl6OFhYbzhL?=
 =?utf-8?B?RHpjbDNrem0yc0lJaW5FTmkzM0k1SGpUd1g5MWhDOExhSlBTU05qRzBlRGlZ?=
 =?utf-8?B?MXRyZEZzL2JsbDNTWVF1M3dHanJoSHhZL3phNFBGclJ2c3B6NFB0Y2lOeGt0?=
 =?utf-8?B?L1Z4TVoybVo1eUEreXlwSEZpbnlGeDZDNUdQOCtlR1hOQ1NmQTFLS0phMWJk?=
 =?utf-8?B?UEF4azBBSXpxMG9WVWhnQ2FyTGk4STZkSC9YWGhVT1QyL2dDcituUU9RZFZt?=
 =?utf-8?B?RW9YRmlJYWZyMzZqOUFYYXltMkRDZ25vaitmNE11MFNKVkE1ekhaeitzaCtD?=
 =?utf-8?B?L3F1K1U1dkd4VWx6bHFFbm0xU2x4N2d1SWRNakd6OExNb3N1V2RGenQzOVd1?=
 =?utf-8?B?eG1tUGtZVC9wSk5UM1l0eGlwR2xURE5zWThRd0hvUDVXUnhxY2pDVUpublVI?=
 =?utf-8?B?b2IwcTZ6cUwwOEJNcTlDSk4wSWM2NWFCMzFsKzcwWit5WnJIN0M4YmhiWUZp?=
 =?utf-8?B?THdmc3RjWGpWQXdhZVNCWkljSkQzeEI0ZE45NDR4TXNScWl1UlVxcFBGMDBK?=
 =?utf-8?B?aGJjSklHNWk2L1hrVkZCbVMycXh5ckNYSFpPQVNhSENqV0hMVGdLTnd3d3V6?=
 =?utf-8?B?R0sxQkNodW1TZ3RCL1U4Zk1FdXUwWS82UVZTeUFRSlpnU3Y3ZVgvRCtWUGt1?=
 =?utf-8?B?eGI1bXU4MlZ6eXZZUDZoVzV1SUlZTjZhYlN3cnFqSmFnUHlYQ21xelU3eEtG?=
 =?utf-8?B?N1NEQ1ArSDEybzErYzN3S1Q4em1BZS8vdEhFdnZ0Y1ljc0JmTGNZKzJpWjI5?=
 =?utf-8?B?NjNqb011bTZUdStRT0Q4WjV4VHE5YVBJd25HUDNZejBNVUR1ZW1vOTFuc1du?=
 =?utf-8?B?eUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 98b72210-f517-46c1-4897-08db3b0fd1cb
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4855.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2023 04:38:53.5306
 (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: umzs+GdAIPjEn/4gytNyEvczahGS5FJQU76yFrZ4fR9eAVxTdtKjydGfsivrirN9Z7YbupJLuJz51v0HSkAQua/K5bKGtcKVOzwfi943LNM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7810
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-6.6 required=5.0 tests=BAYES_00,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,URIBL_BLOCKED autolearn=ham
        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/11/2023 6:21 PM, Fenghua Yu wrote:
> 
> First of all, max_features is shown already.

Yes.

> Kernel_size from CPUID.0xd.0x1:EBX takes XCR0 | IA32_XSS as input. 
> Platform may take wrong XCR0 or IA32_XSS and get wrong kernel_size. The  > purpose of this patch is to provide more debug info to help debug
> platform/kernel issue. So instead of a whole max_features, xgetbv() to 
> get XCR0 and xfeatures_mask_supervisor() to get IA32_XSS provides more 
> debug info in case platform may have issue in XCR0 or IA32_XSS.
> 
> In other words, splitting max_features into XCR0 and IA32_XSS and 
> showing them individually provide more useful debug info than one single 
> max_features value. >
> Does it make sense?

Hmm, I don't get it. I don't think whether the microcode takes those 
register values wrong or miscalculates the size does matter here.

print_xstate_offset_size() or something can decode the mask and readily 
shows off how it was calculated here. Then, probably that's it.

>> I still expect some acknowledgment of what is coded here for the 
>> kernel calculation details.
> 
> The kernel calculation is shown in
> +        print_xstate_offset_size();
> +        pr_info("x86/fpu: total size: %u bytes\n", size);
> 
> Isn't that detailed enough to show offset and size of each xstate and 
> sum of sizes?
> 
> After that,
> +    pr_info("x86/fpu: kernel_size from CPUID.0xd.0x%x:EBX: %u bytes\n",
> +               compacted ? 1 : 0, kernel_size);
> shows how kernel_size is calculated from CPUID?
> 
> Using the above debug info, a real platform CPUID issue is shown clearly.
> 
> What other details are needed?

I recall it was also asked to show which features are off or mismatched 
as compared to the CPU calculation. I'm not so sure about it.

Thanks,
Chang