Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <0112b446-ee7e-4b78-b3a4-671d3ba67299@intel.com>
Date:   Fri, 1 Dec 2023 16:36:23 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 06/26] x86/fpu/xstate: Create guest fpstate with guest
 specific config
Content-Language: en-US
To:     Maxim Levitsky <mlevitsk@redhat.com>
CC:     <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>,
        <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20231124055330.138870-1-weijiang.yang@intel.com>
 <20231124055330.138870-7-weijiang.yang@intel.com>
 <e1469c732e179dfd7870d0f4ba69f791af0b5d57.camel@redhat.com>
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <e1469c732e179dfd7870d0f4ba69f791af0b5d57.camel@redhat.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?U0EvOGhOU2tEbG40OUZ0dnNRWEpyYkdlZ3Y4SFFackl0VU9ONTI0aVZvUDJp?=
 =?utf-8?B?U2ErV0Q1QnB2UWt3NEdDL1B3Q0pFMEYwUTNObVkvQXZhR3lack5LUXliTXlI?=
 =?utf-8?B?dG0rZGtLZjlnMkR2Q1JwOW13UFg1elJIa0Q5c1RPdzIyZWtFNVhxSlU0UHZj?=
 =?utf-8?B?RnNYZnJKTys5eWN2Z0dLdThLb0dleUlJcTRUcEdHRC9SWVZtemV2NnFhU3NC?=
 =?utf-8?B?Rlp4a3VJQ0Nwcmc0UmJqQlBHblhsV3JGa1NReVZrSUdCRlMxSHZrdWVnTHRy?=
 =?utf-8?B?NDVSYU96Z0llVzJNTXpJQmRQMW1DSlorMEZUY0tMSVRZWWE1UTNtalRjaTF3?=
 =?utf-8?B?S2UrYm03MUk3aVltMW82K2IycXViVjdpc1NLYXRLY3p0Zm5IdnM2bWV0SU1Y?=
 =?utf-8?B?SHllaTM2ejRUdHJQWkc5VWROUHVoaTRoalBJb3dTNnVQZTRIbG5ncGFOK2FB?=
 =?utf-8?B?d2NWMytqWWZrN2dTZC8yMzg0N3gvUjZab3loQ3hWVE9BQXV2YmFDYzZMcUl6?=
 =?utf-8?B?V3lOMEJBUUU3VkV6L0lxZG1pMkZBQ0dtR2lqOG9SK0ttVisrcUYyekswV2dl?=
 =?utf-8?B?a1hqYmQwT3BidlYyYzJDL0dINFNuMzV3ano1R3lzZDAvUjNOa21UNGJFL1lz?=
 =?utf-8?B?OW1mb3ZSeThYano0RDl4ZmdHVGwzYjhhNEw1SlROMHRtT3lidW1rVFZuMVlw?=
 =?utf-8?B?RWxheTJnckNvUG90RU9CeW9LQVBJZUVqRHFTbVRPamtTS3U5OVRJVEZlREg3?=
 =?utf-8?B?U0tHcXN1U3l5SngxY0lhMmVHblBEeWRlSWVPVGtTa2JYU2V2ZmttN0xacVZ6?=
 =?utf-8?B?K1BZdFhqcW5aYVFsdksxK01oZWpzK2hLUWNoTGpHeW9oS25nZEVCaTdDQmV0?=
 =?utf-8?B?dHZ1Y0FPYlZ2bGVQL015cFp0dkQ2NVVJSG5MdmRpc1lQeitWMms5ZnAzNFNs?=
 =?utf-8?B?MmtGdTJSQkVsMmJ1QklibFd2YTZwa0lzaUtjVGxrNWtqVEQ3SWkzSHlzMWFS?=
 =?utf-8?B?enNaYnVSU1VCdG1HNGxmMzZUb3RHWVhFWnhqcHYvOCtsUEljL09rR0tJSWQ0?=
 =?utf-8?B?bU5hMlZVejl0REsvZU9xUHhsRllzczRyY1ZFR2cvUFJOaWw3Qlp5aXNBTVNx?=
 =?utf-8?B?N1VTS242cG1HUnE2aHYrNlQwdE9SZVhxOC80UXpCRXpiTEU0dGRWMGpMTE5v?=
 =?utf-8?B?MDFBK2J2NUVoYzlneGdRV1lHU1o3bnd6cUhjbmNQMm9QcTRySnNyRjJKR2ZN?=
 =?utf-8?B?N1VJYUpGSlZqeDUrV0dxTnJOOXZ3V2hSM1RpbnNoazhkemhYRkpRYTJLM3Ba?=
 =?utf-8?B?MnZkZkZBdFFiOWNOM3BwcWZHcit2ZTBqUUFZVjEyckt0aFhVZSt2MzFCT24y?=
 =?utf-8?B?R2lFVGprTUFRaXNSbDAwcHA0azJlUSs3UEtRa0FkYmU1RDdiTDJlRXB6MlA0?=
 =?utf-8?B?TE4xMHExa1JHblVQN0VRekN5b0J5UEdQUDlnZk4rSlZSWWFPMFl0SEY4RktZ?=
 =?utf-8?B?bS9Iak9Dakdib1dUT2JnV3VQdFNWUm8ycDd4WkhGTmZmZkFvMndMS1hISHQ1?=
 =?utf-8?B?Z2IrNHZuc05McTdwU3lKcWYvR3lVYU9KNTF1dURBR3pzdEJUSVB1T0hyVmZk?=
 =?utf-8?B?SXdJa004NTRZTWVHajV6Y2JnUmlNOUJnVmhaS3M3NHlwK0RITE5WRjRPck0x?=
 =?utf-8?B?ZDQvVWtqUzZ1SUFwTXgyd2hiYm5SZmxtbkJYMjVYK3pXcUtPbTV3SnpLalYz?=
 =?utf-8?B?Vk1melBpMkgwTDBRSWlyMjlWYnEwdHRUU2Y0REtjd2dHaHU5U3MvR1B2bGMy?=
 =?utf-8?B?RC9PbUxUSUR2YzVKcUQvc2QvUG5RMUdxMzJVUVpKUlB0dVdYbnh6ZDBXUXpu?=
 =?utf-8?B?ODJ2aFhuZDN3TVpJcDNaOUwxbVl4cmNBRTRDVUlGMExsZFlEbkg3YUFyREw5?=
 =?utf-8?B?bmQrQ1ZOQ3I3RFNQVVNRc2NCd0NxR0t3YlRKS3hlRWFHMm1iL2pPVzRwSW84?=
 =?utf-8?B?UEk1b3FLTGVReUJIMzdiVGoyWlp0TzIvdFdRR0pvclErQWZUcUcwdC9aeVFJ?=
 =?utf-8?B?VVR2ZmZCTlV1SnRvOWZBcGhjMGFTSW8wbHVxc0cyZnZrdW9INVl1NWlTMGFW?=
 =?utf-8?B?MktyK1laWDFSTmFGc2xXekR3QkEyY2pzVEx4aWhTVFNPd3ltQ0orK0VmYUJK?=
 =?utf-8?B?Y1E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 11b98808-49c5-4894-b95c-08dbf2489f69
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2023 08:36:33.6413
 (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: ecfrUcTKwG3DBY2b5/b0x7X3pggVVxGx+xn0lxeMeWlq5I/pK4kO5TqKCnHVVvWoY/kP7HLMmLVIOK870RIk6g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB7907
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Fri, 01 Dec 2023 00:37:02 -0800 (PST)

On 12/1/2023 1:36 AM, Maxim Levitsky wrote:

[...]

>> +	fpstate->user_size	= fpu_user_cfg.default_size;
>> +	fpstate->user_xfeatures	= fpu_user_cfg.default_features;
> The whole thing makes my head spin like the good old CD/DVD writers used to ....
>
> So just to summarize this is what we have:
>
>
> KERNEL FPU CONFIG
>
> /*
>     all known and CPU supported user and supervisor features except
>     - "dynamic" kernel features" (CET_S)
>     - "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 (arch LBRs)
>      - "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;
>
>
> 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;
>
>
> 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 (arch LBRs)
>      - "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;

Good suggestion! Thanks!
how about adding them in patch 5 to make the summaries manifested?

> ---
>
>
> So in essence, guest FPU config is guest kernel fpu config and that is why
> 'fpu_user_cfg.default_size' had to be used above.
>
> How about that we have fpu_guest_kernel_config and fpu_guest_user_config instead
> to make the whole horrible thing maybe even more complicated but at least a bit more orthogonal?

I think it becomes necessary when there were more guest user/kernel xfeaures requiring
special handling like CET-S MSRs, then it looks much reasonable to split guest config into two,
but now we only have one single outstanding xfeature for guest. IMHO, existing definitions still
work with a few comments.

But I really like your ideas of making things clean and tidy :-)