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: <c716e021-99ad-2cb7-44c0-ec8393a3440d@intel.com>
Date:   Mon, 10 Apr 2023 13:43:10 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: [RFC PATCH] x86/fpu/xstate: Add more diagnostic information on
 inconsistent xstate sizes
Content-Language: en-US
To:     "Chang S. Bae" <chang.seok.bae@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>
From:   Fenghua Yu <fenghua.yu@intel.com>
In-Reply-To: <113918f9-0e44-3d46-8b48-028277ec26bb@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?bFdvVXBOQ3owOFFSTUZOVGphOWpEUTlNRXJCZmlQcGdpcWJoV3J4Sk9LZE5K?=
 =?utf-8?B?eFBUK3BGS2ZWMm0rSzBmbVpMeDFhUkY0SGo0UGFQVkN2YTdOYzd5alNteHB2?=
 =?utf-8?B?a3MyYk82NkRWblhpYmttN29TWGh6ZkVKcTAxVW10M3duc3hTZ0ZVWnFNczhF?=
 =?utf-8?B?bzdMYkxxRkQ0ODEvODVKZzNKYTZ4SjExcmhyL2FiQmUwK2JYRitaSTFoTUIy?=
 =?utf-8?B?Q3NyVW5TWjNSZi9wQ0NtSHVUOWN2REdudXdhSTRBV2JZejh0MWFhTkZYZ284?=
 =?utf-8?B?M1FoYWRYMVIwMlk2MW13RTEzVHhIUkZtOUFISHdOaGFyc0RPR3NQRW9qQkc2?=
 =?utf-8?B?N1NMVk81NHdtZ3Z5dndWL3c1aEFvNW5yMXQ3WUs1QlNNOEJ2VThtQTFrbVpE?=
 =?utf-8?B?dENlSU96UXp4Y0tZVGtlT0x3SlhEeE9xZFZ1akRURURDWUxuTE5MUHZjK3RV?=
 =?utf-8?B?YTk0aDBSTFBPa1NyRVRQa0JjNGR4aGd3L01hcGdxK3pQeFNTNmExQUJVc2dJ?=
 =?utf-8?B?WkQ0MmE5VmxORFdxdXVSVlNieFVkRC9GTHZXZ05KZWlQUTR0ZVZvRUpPSWNo?=
 =?utf-8?B?YXNnT2NQbHdYTXFTMktJQ3RvQlE5amRLUmlFYzdQOWZEcDZBOXhZS2wvNlc4?=
 =?utf-8?B?OUduVXI5TUFMQ3hsaFdQZzR0Z1VHNFBFTzRqSDBqVGtIaE1QOG1PWXRWYnRv?=
 =?utf-8?B?UzZVN2RiblFSNGdYT0NTTVcxcU81RTExY3NRZEI1NGpCd1dKNVJzalJYSkV2?=
 =?utf-8?B?T0h0WmxVeFJ4TlJ4dDZaODhnaHNhRWh4dnlUN1AydGRYaGdGL2FVaFpBOHdR?=
 =?utf-8?B?ZmFhV2x3bWJMTUhMWktya3Y3YXlmTFAwb05meWoxWnpERmFrWC9hMlVqV1BX?=
 =?utf-8?B?TVc3QWlFeERoOEYrT0xMNTNqNTk4OUxFRm5CNi9QY2hBczdLSGJsVG1tbk9i?=
 =?utf-8?B?aG01U091MWZzeUw5dkRadUhCbzV6MENUcmNkRkN1eElOWFZwNnhmam14dUJs?=
 =?utf-8?B?eTVLM0FpakQ3dDZOUFZPT3pqWG8vVU5CYXBVV0xGZmwzeGtFczZUeUJETmRm?=
 =?utf-8?B?Z3IzLzdlTUs2Z1REeXkybjJ4dzlqUXNWREtOMEp2UTU4RnA0eXBjR3VRUVgw?=
 =?utf-8?B?OUo5ZVU1U3A3eXF1M3h6NnR2WGMwMlJhZDFSMmFxU2xtK1N2UHk0U01VMEQ2?=
 =?utf-8?B?ci9qQnlYRzlhaFNVUHNmUU9hZ1d4TmkxazVMVlh0b1RJalg5c2VFZkszeWxR?=
 =?utf-8?B?dmUzVnRYejZUb3RTbVhsTEJvenZzc2RYS3ZZZGpnUnc3NXltVWxWTGhPdXZQ?=
 =?utf-8?B?bGhoMUVDR2h4dVRNK0tnTGtKOU5lV3gzOWFPdVhmSU14K2Uyc01ERDRlVVZz?=
 =?utf-8?B?WlFPclhNTVBSZDVQdVF6aTN4ZFlzaGNuNkZCUXV6cjZYR0ZlaC8yOXorWlZM?=
 =?utf-8?B?dDY0a0gxOWNHejB6b1dkZDV1RzNpNVh6bmcxejExTHNZSjd3YVBWR2tCSmIw?=
 =?utf-8?B?aVA0ck1SNGtOQ1ZBL1RleW02UlFaWUQ1TDVidUhzTCtNeUZ6eFZWUzFRMU1j?=
 =?utf-8?B?WXdIUGc0QjhaYWc0aWZrV1J2SCtTNUZxamhub2s1aExQQU52SlNEUmpEVXBZ?=
 =?utf-8?B?bVA0d2w1eHZ2aVZRSzUwNjF3dXhmNXRvbkN2Rkh3M1BZcllCb2Q3enlGT1Ux?=
 =?utf-8?B?OFlFdG1sellCUmNsaHVTQzdSaURrNkpXUno2eURLVHRvVE9WeW5vbjM1Vk1o?=
 =?utf-8?B?WFMxMFJIdXRFTDB5ZlQ4TmZjaWJUajdkN1UrejlXRmZqNjBzVEVYbWJvYUhJ?=
 =?utf-8?B?d3d5RXJaY1lpa1pjRUY2V0hDOEh3bUs0RGQ2OXRXVVBZeFQ0VTRDdm9tenBo?=
 =?utf-8?B?bnhCc0w4c0s4S0tjQzFOK0I2Nk90WDkwTnQ1UUkwcm5ScDJWeEowazc3UkVR?=
 =?utf-8?B?VmhvZHZZUWo0V1hLVjQ0eG56RjlLOFdMMkVOWXowU1hnYnVmK3JqaG0zMFJR?=
 =?utf-8?B?elQ5Z2t3c05ZcWNZWTJBMFBMZG5CWms0VXRuZUM5cEdCSDFYRFFnUHdSak5B?=
 =?utf-8?B?U1FrazR4Tk11TUdxNWF1NzlXc1M3OU9nNzJDZ3NaRjdKMXRUMW90cytkQmlD?=
 =?utf-8?Q?IxX5tP98x4UbiyNkGNxCvWdrA?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 90edbf5b-8529-44d5-fb26-08db3a042724
X-MS-Exchange-CrossTenant-AuthSource: IA1PR11MB6097.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Apr 2023 20:42:51.6666
 (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: ScgecA9GShjmnRxV26RxhENkbJFBGHSimia8T3MO8dTeyUmz1G2jUA5oFf7SUKIedeAj4AnVGgHtA79WbxAGMA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6663
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.7 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

Hi, Chang,

On 4/7/23 11:22, Chang S. Bae wrote:
> 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?

How about getting IA32_XSS from xfeatures_mask_supervisor()? That's how 
to get kernel_size by setting IA32_XSS without independent features in 
get_xsave_compacted_size().

Thanks.

-Fenghua