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: <084b98d1-1021-14fd-0d7b-3b85ffee3035@intel.com>
Date:   Thu, 13 Oct 2022 20:53:54 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH] x86/fpu: Remove dynamic features from xcomp_bv for
 init_fpstate
Content-Language: en-CA
To:     Dave Hansen <dave.hansen@intel.com>,
        "Yao, Yuan" <yuan.yao@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "x86@kernel.org" <x86@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>
References: <20221011222425.866137-1-dave.hansen@linux.intel.com>
 <BYAPR11MB3717EDEF2351C958F2C86EED95259@BYAPR11MB3717.namprd11.prod.outlook.com>
 <9b198ed3-4b2d-c857-710b-3f7115bbcf74@intel.com>
 <78f2021e-339f-9dfb-2aee-51f58ea77ec9@intel.com>
 <9be1413b-ce30-acf8-86fb-3e302dc98396@intel.com>
 <dd5815fa-ec61-b7d1-c0b3-70cd0f33bbee@intel.com>
From:   "Chang S. Bae" <chang.seok.bae@intel.com>
In-Reply-To: <dd5815fa-ec61-b7d1-c0b3-70cd0f33bbee@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?eDJ6OTdpb0h3TWcwMXlkRy9TcVAzQmRSVy8rUzNUZ2VTS2pHck55Y1QybkhG?=
 =?utf-8?B?eWVMS0JYV0loVmFjZmxwK0dFTDYwUjQyM2dBVnVUejBPam1JMnlsaDFhdEsv?=
 =?utf-8?B?SFAzRHVIVUlaa3Y0RkVuZlU5TGtqdDNBdWdEQUM4Q2dwUkZtYmNuQU04QWhj?=
 =?utf-8?B?cE10QTNxU0JneEtmZ3lLMmVFRjJSejk4NHlhM3dUZGY0WElVSkYvOXRRM2dV?=
 =?utf-8?B?S2xMeTlXMFh5T0dvSUFVTDFoM2luME5ucFdNOFdod0Q0bjBrMzlCQngycTVM?=
 =?utf-8?B?Y0FRTEoveTBESldlRzlWd2w5Q3VMS2NrMExGdlNGRzZyQ0ZZRkFoSDZGdllY?=
 =?utf-8?B?TGR0OUlmZUIxYVdTQXJYUkpIenNMQVN1b0pZbUpwY1VicDFxOTV0UFp2YUcw?=
 =?utf-8?B?NXh6ZW1ibmVWNGRjcGNGK0syRHd0ZGxjYmNQVzNmQlVrTW1JZnhQVWlPQ1N0?=
 =?utf-8?B?VXFGN0s4YUs1ZUZqRDBMNHlOY1Y1ZEFRV3F6RkszOTgyRmNkSDRYeHIrZFda?=
 =?utf-8?B?aDRwaFpMT0pPT0hpeXhZbGFPRmQ3ZTdFT3ZLcmE2Tm5ndFlWUDVkb2NnWjdF?=
 =?utf-8?B?T01CQlhEbGN0cFlmNWlkUmM3eitmQ1p0NUJYV3hhL0dKM1JJUWMxZTBZek9l?=
 =?utf-8?B?Z0lpZjRVVlBkSGdPUzliVG5NejhCTlJRU3psOU9Pbi9Bajl0S00zRlh4dVJM?=
 =?utf-8?B?enpSUVdZQTRsVU1EOEwrMlFWcU5henBYT2xqK0dKbWljazNVUXFtSDFwV2c0?=
 =?utf-8?B?b0FNRy9XaEpiNWRVVExMbWJhUUNnRHdQcCt6UkZHUUUxeTk3ei9LQlN4UHFX?=
 =?utf-8?B?amtWeWhSTThtUkJueEljQUhnWmYrOU82ZFZ5dlpvM2MrZU9PSXE4WkpqQUgy?=
 =?utf-8?B?TkFUQ25qNStzUit2QjVXU1JHUjVxSUtxQmhGR2lzZXY3SXREbGo5bDJzckZJ?=
 =?utf-8?B?SlFVcXkxWDFyamtvNDNtb3Q5R2VHSGg1Z3ZWSjRjaHgyYzJOTEo1QWNtMDFQ?=
 =?utf-8?B?ZkplMUVpWThCNXZrV25QOVJaMmVVYXM5aFdzL3Z4c3d3N1g1WWtoelhoUUxV?=
 =?utf-8?B?amQvMGlTZ3FRSUJzUTR1NXlUMm45a0JEUHIxU0J6QlV1eldyS0hXbFoxTGhS?=
 =?utf-8?B?ajlqa0VzNUZGVkZvN2ZSTzNFZ3RKS0VOTk1CbjhyRi9oVUZRaER4NVpjU2lR?=
 =?utf-8?B?enJkS1doOXNTQzhJbmtxaFlZbWNyc3YyZHVrTGdReDN2WStSUkV6QzRBMlVh?=
 =?utf-8?B?N2FrSmlyU3FiMk50VGJsTHpFUUx5VlpZS1BFeS9BbkQ3NjA2WmYydHQwSDc5?=
 =?utf-8?B?T2tXOEJXQWtNUzFDM0F2ZHN6bXc0RlM1SGJNb09OMU5HRFJBM2lmNlJMWVg4?=
 =?utf-8?B?akVTeVRUbklMalEyNGpKVDFTUXBFY1JCREhkZXpiTmVaNS9CUk5WeCtJeEJH?=
 =?utf-8?B?SGpGK2lOK1htWGV3UkhEcjVibWFwb0RHVzQxdlZESVJaUUh0YUJMV21qYWFz?=
 =?utf-8?B?L1BYQTB4N042YXUvbHFkR1ZLcTVxRGVtODVtcEplZnhwSWF2UlpCeGNSa1g0?=
 =?utf-8?B?ckRWc0RpOUtsRlhTb25WV1BYblo2eGpJekFWb25YY0JGTW9LK3drb3Q3dis3?=
 =?utf-8?B?TS93TVJBZ1pQYThHMzRvNUpoREVQbFIrcms4dEU4MHVqME5hMFFWMFNQOXVQ?=
 =?utf-8?B?cHF2dzRxZzM4MnJjbUVnVTFHa0FabGZndDBCM2FMYkZxTVpVNk9ucCtJN0Fo?=
 =?utf-8?B?QTBOKzN5OW9DRlZZaDJ4Q0VVVDIreFJUTzJJY1l1WGtPaGYxT3UyMmJjejIr?=
 =?utf-8?B?NTJoSENQbHUvUEdPcW5xaHgzcE1EV0xGS3ZoVUpQVS9nK3pIc1Q0aGxHdmZo?=
 =?utf-8?B?ZHB1N0VhaC83OVVJNDhIVjBjOWJGd3BKVks3YjZJOE04aDJSNlRsd3hyMU9Q?=
 =?utf-8?B?OFkrWm9lN3ZlTnJZWkxYY2JrYWorUm96QUMxMWNMMU4rRWFmVHFrUGozRHpB?=
 =?utf-8?B?SnhJc3RCTC9vT2t5cndGM2k0alZxS0NPOEkwVVpieHVFR1NyM09TYWNnbThD?=
 =?utf-8?B?WVJzY1RqOVVTUnFUMGJFbVVoMGRleXV5ZHMzSG9XT05NdzVLZmFVaVp2aWtU?=
 =?utf-8?B?NExlK2N3MFlIakFGQ3VmVkR2c0hoUWxBR3VPVjBJRi90cy9OVVdRWVYxWmlP?=
 =?utf-8?B?TWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c343ee69-ffc3-493d-6a60-08daad97b7ca
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4855.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Oct 2022 03:53:56.4477
 (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: NOpOCJEbSuOxIx/69sBNGspqtY6Hr5SuTIkqaOqOarJRpIZaISDfFrDo7xSFiSqkOroOP5ZBzr9Yer7wGhwnqaSg8Dp6/ZaU9kEJw+ceAyA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB5826
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-8.3 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_HI,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE 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 10/13/2022 10:44 AM, Dave Hansen wrote:
> 
> A better comment for that would be:
> 
> 	 * Some user_xfeatures may not be present in the fpstate.
> 	 * Remove those from 'mask' to zero those features in the
> 	 * user buffer instead of retrieving them from fpstate.

Yes, indeed!

Also this xstate copy routine looks to need some updates.

If an xfeature is present in fpstate, and in init state, the value is 
retrieved from init_fpstate via copy_feature(). But, it has no space for 
dynamic states. Also, for extended states, the init state is known to be 
zero.

Then, perhaps, init_fpstate is better not to be accessed in the 
for_each_extended_xfeature loop; instead of using copy_feature(), the 
feature can be zeroed like this:

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 13b83b11b3d8..0fdfd03938b6 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1150,11 +1150,11 @@ void __copy_xstate_to_uabi_buf(struct membuf to, 
struct fpstate *fpstate,
                          */
                         pkru.pkru = pkru_val;
                         membuf_write(&to, &pkru, sizeof(pkru));
-               } else {
-                       copy_feature(header.xfeatures & BIT_ULL(i), &to,
-                                    __raw_xsave_addr(xsave, i),
-                                    __raw_xsave_addr(xinit, i),
+               } else if (header.xfeatures & BIT_ULL(i)) {
+                       membuf_write(&to, __raw_xsave_addr(xsave, i),
                                      xstate_sizes[i]);
+               } else {
+                       membuf_zero(&to, xstate_sizes[i]);
                 }

Thanks,
Chang