Received-SPF: pass (google.com: domain of linux-kernel+bounces-15329-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <961bb6f9-1d82-44ff-b63a-d7a07e3f6bd2@intel.com>
Date: Wed, 3 Jan 2024 17:17:18 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 05/26] x86/fpu/xstate: Introduce fpu_guest_cfg for
 guest FPU configuration
Content-Language: en-US
To: 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: <20231221140239.4349-1-weijiang.yang@intel.com>
 <20231221140239.4349-6-weijiang.yang@intel.com>
 <4ba9edb3f988314637052321d339e41938dfe196.camel@redhat.com>
From: "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <4ba9edb3f988314637052321d339e41938dfe196.camel@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VTkxTGdkZXBWWm1SUi8wS1YwaHJRSXplWmhoUUVGMW9hbmRyWmxJcHlNQWhB?=
 =?utf-8?B?ZU0rWlZ1L0NDYkF5eHNGWXdPMGZyTS9UWmtxNVpST1lpUTZ2SWhpWU9ReXM0?=
 =?utf-8?B?ajB1NHpQVVRLTzk1SUtMb2p1VmVpak01TDQ2YUlUS0hYV0dCT3RXbHNBOWFn?=
 =?utf-8?B?MVdtRVlHUVBDa2YwdWJkcUU2bzFrN0VQbFl3ZW5zZUx4alRRUUx6Z29HL2hQ?=
 =?utf-8?B?Q3RLak9yR0FXb25Ca1Rhc0FBTFRoeDhBcUMyRHlvZzhFbEhLWkpMV25EWlRi?=
 =?utf-8?B?bEsxbmtoMXJ0RDczNGM4YkxkdnVsM1VocDh1SDZETnM5Z2xITVMwTXJzUU90?=
 =?utf-8?B?YlIzRUdlT0Y4R0JvVlZhejdzUUVxa2FsRU1kZDV2ck1sUmhNQWNZdU1EUTI0?=
 =?utf-8?B?RlM4YUN1SVlPcVBydWp4alB2R3E3QXdLMVE2bzdtRHZ5NzE2M2pRS1NOWHRh?=
 =?utf-8?B?bzVxUmxUTVpRb3JsMW04bzR0NmxTZnFHdW5HVS84YU53SkFVOXFPQVpPWWU2?=
 =?utf-8?B?V2plS1cwRGxuRnNJYkhtNjNFOElEZldiU211aTNvY3pqSkVTb2lPTWkxbkNT?=
 =?utf-8?B?UExkcHFMTUE5R0dDN2FpYnFHWXZVUzJQRFZmQk1BMHZBUUtpWWFUMWRqOFZG?=
 =?utf-8?B?WWY3ZnVZR2tXNEFtN3VvUFZ3bW01am1ML1l1ZmEwZHAwSUZ1aWswSDBwbk8r?=
 =?utf-8?B?TWdNU2R2a3pBN2IyRWV0MmNLdGY1V0YzaW84MFg3Z3J1UytucGExUzdSbmVL?=
 =?utf-8?B?TExMQUtZQ2hlaytSZ0w5QzVER0orK0JxQ0pIeG9PUjdjekRlYkxyRHlWbEt6?=
 =?utf-8?B?OXBQOE9KM0JXRVVNNEpIUXk5ZEpnZUF4eWxBRlVKVXlnWDdJVkMzOWpqd3o5?=
 =?utf-8?B?alJnR2FLbDFjSlZPUFFXcVpDRGZzU1NCdjNZbGVkSGRCNEdRWU5kZkpsN0Q2?=
 =?utf-8?B?aDVLQmN3TVhKRmJLTXFsUEZmeExyUEJxcWVxVk01NFRrc29IMHBsSTcraDB1?=
 =?utf-8?B?RC9uUktiVUFzMG1rNXNkQmVjNm1OZGo0VlFDakpuSDBWQ3JPVXpZM3FQdTgw?=
 =?utf-8?B?Ulc5Qm5OUkVYTHlKbTQ5dlQ1V1VMcmZxOVdxa05BbmRBS05lU0xTM3lodEEr?=
 =?utf-8?B?cGVXbVF4MXRDWnJRQzRCSnpBVTB0U0lCcmNpRFRJTWJ6dFJjbkxIZmRWalVp?=
 =?utf-8?B?WGdhMytjajJUQmp1QVdCQjRSRTdYOWNJSkU3cmhQZDI1VkZDU1dpT2U3dGdT?=
 =?utf-8?B?OGRSREFma21lWldtNm0vallWVXR1bFVOd0JvZ0YzR3kyRkdVL3BPMmFyMXQ4?=
 =?utf-8?B?NU1YOUdjN1RWbUhCejhMbXB6NEluQ3F1K3h6TWY5ZlZvNDJ3YVNYbUxzYjBj?=
 =?utf-8?B?SVVBRG05SUxnYi9Ub1U4TlU4bXN0d24wL0liNGlNMTJyMjYvNE0vNGpJTUlW?=
 =?utf-8?B?WWV2aUZxSkF0azFNVUxHRWdVMnRQUlMwWUYrRWVNbEhKUEg5Sk93azBCdlZQ?=
 =?utf-8?B?eGZ3SEpzNmo0RU80QUdRb0l1Sms0cGNxSi9iRVBFWmhUdTVsSjN5UlhOQ0xM?=
 =?utf-8?B?NlR2V0lJQk8rL3YvL09za2xtaFIxMHpjR3JPbUg1MTBtK0RjT25MQjJOQlRQ?=
 =?utf-8?B?eE1VdERlcG16MWZ5UlFneFYzUGtmNFBkOGJtTkVyYjdWUVM2VjVzWFJpN0h1?=
 =?utf-8?B?UmdsUGRMcWJ6VHVEZDNXSHh1ampXUDlrRHZJYWk4WU9YZ0M1bExVaUUyaWRw?=
 =?utf-8?B?ZDNoQWhRbTRIK3BudFhSaG5kZTJxcnFHM25Xaml0L3NvY0VINXhweCtnRjBz?=
 =?utf-8?B?aDFJWUgzUXFlNTBHZitNNUJoWXNFTTlISXpWTk9neDZFVldFK1BOSXlDOVJU?=
 =?utf-8?B?ZGZzWCt6OWJkNThoaDFmeWZiM0RMZ0ZOVEtqWG1OVHNzZzBPZUhXY1BWTTZm?=
 =?utf-8?B?aWVOWjd5UjFqQjZLemdjUGxhVjBBWURSLy9KV2ovbGJqLzUwSDNHSGdIazFu?=
 =?utf-8?B?ZzFBeGpkVkVyU0FoeU9nN1pZRFhUYkVWTURFZncvbHBIcW9CTHFlNm41b3c0?=
 =?utf-8?B?UzBPbmJOMnRCS3g2a09Uc2dUQW4zb2ZYUjl4SEFIaXlWZUpXQmJvQ0RnS0pu?=
 =?utf-8?B?ODdMTGJyN0d6NGRzTXM0Zjd0bDNsNVJKaG5qZlJLRnRhU0VkNllCSjI4TFpP?=
 =?utf-8?B?Mmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d7e5a3a8-4fd9-4b57-250c-08dc0c3cceb7
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2024 09:17:28.9116
 (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: zZc6k/HWXNd/Epnszf6xhZjhOhY7iwoFtfLqupfeGj0EsTF7Db0xZZbnrEzRNWM7bQirX8AMRS8vBmP5sXGUBg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7608
X-OriginatorOrg: intel.com

On 1/3/2024 6:32 AM, Maxim Levitsky wrote:
> On Thu, 2023-12-21 at 09:02 -0500, Yang Weijiang wrote:
>> Define new fpu_guest_cfg to hold all guest FPU settings so that it can
>> differ from generic kernel FPU settings, e.g., enabling CET supervisor
>> xstate by default for guest fpstate while it's remained disabled in
>> kernel FPU config.
>>
>> The kernel dynamic xfeatures are specifically used by guest fpstate now,
>> add the mask for guest fpstate so that guest_perm.__state_permit ==
>> (fpu_kernel_cfg.default_xfeature | XFEATURE_MASK_KERNEL_DYNAMIC). And
>> if guest fpstate is re-allocated to hold user dynamic xfeatures, the
>> resulting permissions are consumed before calculate new guest fpstate.
>>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>> ---
>>   arch/x86/include/asm/fpu/types.h |  2 +-
>>   arch/x86/kernel/fpu/core.c       | 70 ++++++++++++++++++++++++++++++--
>>   arch/x86/kernel/fpu/xstate.c     | 10 +++++
>>   3 files changed, 78 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
>> index c6fd13a17205..306825ad6bc0 100644
>> --- a/arch/x86/include/asm/fpu/types.h
>> +++ b/arch/x86/include/asm/fpu/types.h
>> @@ -602,6 +602,6 @@ struct fpu_state_config {
>>   };
>>   
>>   /* FPU state configuration information */
>> -extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg;
>> +extern struct fpu_state_config fpu_kernel_cfg, fpu_user_cfg, fpu_guest_cfg;
>>   
>>   #endif /* _ASM_X86_FPU_H */
>> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
>> index a21a4d0ecc34..976f519721e2 100644
>> --- a/arch/x86/kernel/fpu/core.c
>> +++ b/arch/x86/kernel/fpu/core.c
>> @@ -33,10 +33,67 @@ DEFINE_STATIC_KEY_FALSE(__fpu_state_size_dynamic);
>>   DEFINE_PER_CPU(u64, xfd_state);
>>   #endif
>>   
>> -/* The FPU state configuration data for kernel and user space */
>> +/* The FPU state configuration data for kernel, user space and guest. */
>> +/*
>> + * kernel FPU config:
>> + *
>> + * all known and CPU supported user and supervisor features except
>> + *  - independent kernel features (XFEATURE_LBR)
>> + * @fpu_kernel_cfg.max_features;
>> + *
>> + * all known and CPU supported user and supervisor features except
>> + *  - dynamic kernel features (CET_S)
>> + *  - independent kernel features (XFEATURE_LBR)
>> + *  - dynamic userspace features (AMX state)
>> + * @fpu_kernel_cfg.default_features;
>> + *
>> + * size of compacted buffer with 'fpu_kernel_cfg.max_features'
>> + * @fpu_kernel_cfg.max_size;
>> + *
>> + * size of compacted buffer with 'fpu_kernel_cfg.default_features'
>> + * @fpu_kernel_cfg.default_size;
>> + */
>>   struct fpu_state_config	fpu_kernel_cfg __ro_after_init;
>> +
>> +/*
>> + * user FPU config:
>> + *
>> + * all known and CPU supported user features
>> + * @fpu_user_cfg.max_features;
>> + *
>> + * all known and CPU supported user features except
>> + *  - dynamic userspace features (AMX state)
>> + * @fpu_user_cfg.default_features;
>> + *
>> + * size of non-compacted buffer with 'fpu_user_cfg.max_features'
>> + * @fpu_user_cfg.max_size;
>> + *
>> + * size of non-compacted buffer with 'fpu_user_cfg.default_features'
>> + * @fpu_user_cfg.default_size;
>> + */
>>   struct fpu_state_config fpu_user_cfg __ro_after_init;
>>   
>> +/*
>> + * guest FPU config:
>> + *
>> + * all known and CPU supported user and supervisor features except
>> + *  - independent  kernel features (XFEATURE_LBR)
>> + * @fpu_guest_cfg.max_features;
>> + *
>> + * all known and CPU supported user and supervisor features except
>> + *  - independent kernel features (XFEATURE_LBR)
>> + *  - dynamic userspace features (AMX state)
>> + * @fpu_guest_cfg.default_features;
>> + *
>> + * size of compacted buffer with 'fpu_guest_cfg.max_features'
>> + * @fpu_guest_cfg.max_size;
>> + *
>> + * size of compacted buffer with 'fpu_guest_cfg.default_features'
>> + * @fpu_guest_cfg.default_size;
>> + */
>
> IMHO this comment is too verbose. I didn't intend it to be copied verbatim,
> to the kernel, but rather to explain the meaning of the fpu context fields
> to both of us (I also keep on forgetting what each combination means...).
>
> At least this comment should not include examples because xfeatures
> are subject to change.

Yeah, I cannot find a better place to put these annotations, but feel putting them here
is not too bad :-). How about putting them in commit log?

the examples inlined are just to make it clearer for audiences how the fields are used, surely
will remove them later.