Received-SPF: pass (google.com: domain of linux-kernel+bounces-27097-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 16 Jan 2024 15:46:25 +0800
From: Chao Gao <chao.gao@intel.com>
To: Xin Li <xin3.li@intel.com>
CC: <linux-kernel@vger.kernel.org>, <kvm@vger.kernel.org>,
	<seanjc@google.com>, <pbonzini@redhat.com>, <tglx@linutronix.de>,
	<mingo@redhat.com>, <bp@alien8.de>, <dave.hansen@linux.intel.com>,
	<x86@kernel.org>, <hpa@zytor.com>, <weijiang.yang@intel.com>,
	<kai.huang@intel.com>
Subject: Re: [PATCH v4 1/2] KVM: VMX: Cleanup VMX basic information defines
 and usages
Message-ID: <ZaY0UbFjwCYh4u/r@chao-email>
References: <20240112093449.88583-1-xin3.li@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240112093449.88583-1-xin3.li@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?fsgVby6SyLEVCWy6x7UmjO08+2jWhJIhwv4GfqsT6LhiY4f/+8egCp4Ws43A?=
 =?us-ascii?Q?rresbvLFsc2LsqTNAf1puOBYGXgr/xPA1gxpapG3r9nCTyWTJVyBsD60DTMh?=
 =?us-ascii?Q?k/K+UIeB5wjglFoD+59MV0WzOGXStv+gdoJDim4E5iByKj28+FI4RFFikwIL?=
 =?us-ascii?Q?Hh5CI34lbdJc6pKv805hvmSt5ZXRuUTcrTCXQ27/ZIS/Wfwq10E/J9cIqJ4c?=
 =?us-ascii?Q?vRM/Jamiw/Gu7zSJtGiAX6X3lBkEjcrsggbO1t6jIMzIRB4DlblCSX9g5fjm?=
 =?us-ascii?Q?rsw3WFnfSN/vsYyLTQW4/5UXgXNav1apJDHUxj6J6LA8CJkJkZwFp6DC+dE3?=
 =?us-ascii?Q?+rfYA8k+Anh3pWMnmTpK4AKJWNozUWq+Wo8gUhFu9k2S+YB0V/9ekrTKPNns?=
 =?us-ascii?Q?MYruKG3ngSjA96Tg73Vgd/eBTdbJ3Sl+5ixQ9E2N6HFhL/rcj235ZgdbJSk0?=
 =?us-ascii?Q?wPzztyTMU97ddaurAuqJWLIvjdgI4FOYzBdGYRIfQfuYJeGoC8so8slpCe0d?=
 =?us-ascii?Q?MTmjHhpyRylZApbOHoKtAPY97Oj7Q1lYHbVIkQp85ycWFjVM+UlEqGfneu24?=
 =?us-ascii?Q?lCk3e/o7S8QNIFTdQzWinMThBerYOmWkftuSY0DyxvVDavSgAgTID/dXlfjo?=
 =?us-ascii?Q?qijfDKmpcMZPkFZMIH8QIDrHFOuar2BywXYnPb4ziLo1Vx1jZ78RTlB3GgO4?=
 =?us-ascii?Q?5v/9q9iOZy1XIJJSHPb3LB8a3U7KD09X4WRo8BWNO2q5qtTNWSyMO79Ui6J2?=
 =?us-ascii?Q?dT+zT68EJqoCVt8iHxKCJ8YWpIpS9Sqg0yf7BUl4lXg2VbPJj4rzmS9hlKfS?=
 =?us-ascii?Q?Dc6Rw9QREKaNFx56QUahIk+3W+NmDUCpuH+hDW06w1dqC1DMJkfvzyf16PK/?=
 =?us-ascii?Q?MRpgI+wwtrpdA6HxZoLgTbcXzUX4b812fadP6uEf6xuIkYH3wEQTzya86+kx?=
 =?us-ascii?Q?7ZK6G0qdUvAQo2xm9SUKachgN0FbAo9KLK7ZeZWHK0jvEk69soEgmniFWxQ+?=
 =?us-ascii?Q?fvtUcQzaSSktMvpIgNQcOIzxNooH9HB8UCCdXWfPx3CPGgtXJ7UTFLrrckmY?=
 =?us-ascii?Q?QxP9nGEZ7yT0+IqQJwAxXwZzlZk0E21JeFwcnb7Npd1HnwpqmZ4kRL10J9/9?=
 =?us-ascii?Q?O0CfNt0+70pa9CxT7uM7tqF1VI74RpyjbdacSiJBkN++TtakZWs6gzO12zRD?=
 =?us-ascii?Q?hkZDaoMTbVbUgy/U1z91gESUgzpD7Nfblj6kZwqO8Dd1lD4CtBuM7xXMRD25?=
 =?us-ascii?Q?rgxPXiFxMSijlNLZypc6kBz9dZVKGB5icLtplb1AFLAjqcFttTLpM6LloKDu?=
 =?us-ascii?Q?0XAnqOjWjrECQQ/iCBQ2bIRAkRL57o1srdnNn45sn5LZWfdhyGk5xzIsvjEm?=
 =?us-ascii?Q?tuafe+wAIireOaVuh+imXgK5qufTVLSinJxTjEQgFZx82Au7eFpPPDuxFRwU?=
 =?us-ascii?Q?ikNfMsUN8zmDjXphIJ5ZVc5RULfl/83iuBn6MVl01SnDSIXfATCyOz3SpiBH?=
 =?us-ascii?Q?c7Twc+mKoW4L5Zybkt5JDGYvQ/91o3/tdGUDSQR/DmTZqIOhC7gQc90iHASO?=
 =?us-ascii?Q?XxrGflx4uk4daaEkppWQDorCFFWOJ9ZvCJwhbbbU?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 19f056fd-08e2-44bd-ed2e-08dc1667439f
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jan 2024 07:46:35.2846
 (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: W5Z75q/EOlpcK7x51h8XheIwKKUvZ9S7OcA/sOEfwURngXjVF4KRLpFAxcdOY28u2BiTdeK8kXTDVZozjotU7w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4727
X-OriginatorOrg: intel.com

On Fri, Jan 12, 2024 at 01:34:48AM -0800, Xin Li wrote:
> 
>+#define VMX_BASIC_DUAL_MONITOR_TREATMENT	BIT_ULL(49)
>+#define VMX_BASIC_TRUE_CTLS			BIT_ULL(55)
>+
>+#define VMX_BASIC_FEATURES_MASK			\
>+	(VMX_BASIC_DUAL_MONITOR_TREATMENT |	\
>+	 VMX_BASIC_INOUT |			\
>+	 VMX_BASIC_TRUE_CTLS)
>+
>+#define VMX_BASIC_RESERVED_BITS			\
>+	(GENMASK_ULL(63, 56) | GENMASK_ULL(47, 45) | BIT_ULL(31))

When we add a new feature (e.g., in CET series, bit 56 is added), the above
two macros need to be modified.

Would it be better to use a macro for bits exempt from the bitwise check below
e.g.,

#define VMX_BASIC_MULTI_BITS_FEATURES_MASK

	(GENMASK_ULL(53, 50) | GENMASK_ULL(44, 32) | GENMASK_ULL(30, 0))

and do
	if (!is_bitwise_subset(vmx_basic, data,
			       ~VMX_BASIC_MULTI_BITS_FEATURES_MASK)

then we don't need to change the macro when adding new features.

>+
> static int vmx_restore_vmx_basic(struct vcpu_vmx *vmx, u64 data)
> {
>-	const u64 feature_and_reserved =
>-		/* feature (except bit 48; see below) */
>-		BIT_ULL(49) | BIT_ULL(54) | BIT_ULL(55) |
>-		/* reserved */
>-		BIT_ULL(31) | GENMASK_ULL(47, 45) | GENMASK_ULL(63, 56);
> 	u64 vmx_basic = vmcs_config.nested.basic;
> 
>-	if (!is_bitwise_subset(vmx_basic, data, feature_and_reserved))
>+	static_assert(!(VMX_BASIC_FEATURES_MASK & VMX_BASIC_RESERVED_BITS));
>+
>+	if (!is_bitwise_subset(vmx_basic, data,
>+			       VMX_BASIC_FEATURES_MASK | VMX_BASIC_RESERVED_BITS))
> 		return -EINVAL;
> 
> 	/*
> 	 * KVM does not emulate a version of VMX that constrains physical
> 	 * addresses of VMX structures (e.g. VMCS) to 32-bits.
> 	 */
>-	if (data & BIT_ULL(48))
>+	if (data & VMX_BASIC_32BIT_PHYS_ADDR_ONLY)
> 		return -EINVAL;

Side topic:

Actually, there is no need to handle bit 48 as a special case. If we add bit 48
to VMX_BASIC_FEATURES_MASK, the bitwise check will fail if bit 48 of @data is 1.