Received-SPF: pass (google.com: domain of linux-kernel+bounces-150381-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Wang, Wei W" <wei.w.wang@intel.com>
To: "Wang, Wei W" <wei.w.wang@intel.com>, Sean Christopherson
	<seanjc@google.com>
CC: "pbonzini@redhat.com" <pbonzini@redhat.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [RFC PATCH v1] KVM: x86: Introduce macros to simplify KVM_X86_OPS
 static calls
Thread-Topic: [RFC PATCH v1] KVM: x86: Introduce macros to simplify
 KVM_X86_OPS static calls
Thread-Index: AQHakNiYqtvKEMFbS0SvTLh/C3Jv+7FsplwAgACN6vCAANskAIAAAkcQgAATVbA=
Date: Thu, 18 Apr 2024 15:22:09 +0000
Message-ID: <DS0PR11MB63736F3EF472087F84F0523DDC0E2@DS0PR11MB6373.namprd11.prod.outlook.com>
References: <20240417150354.275353-1-wei.w.wang@intel.com>
 <Zh_4QN7eFxyu9hgA@google.com>
 <DS0PR11MB63739BE4347EC6369ED22EBADC0E2@DS0PR11MB6373.namprd11.prod.outlook.com>
 <ZiEnIFW3ZQhDwdZ-@google.com>
 <DS0PR11MB6373B4170ECA01D31830CA20DC0E2@DS0PR11MB6373.namprd11.prod.outlook.com>
In-Reply-To: <DS0PR11MB6373B4170ECA01D31830CA20DC0E2@DS0PR11MB6373.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ef7IZGtJg4jV2psroufNJMIsaZCzDrAa9AUAarp4liyaZ8Tdm4UxIPyPC5VA?=
 =?us-ascii?Q?RI5l+ZXIJeVecTS/QZawIdmITJqHdFBKNqcq3VFv2LyuqWigawl/01rS1zeg?=
 =?us-ascii?Q?1++/R6LAy/MnbABoTUk45eLF2I1Ih9r2IF2uvC2I3xdiKclfZirDPyyxKdIH?=
 =?us-ascii?Q?9sM/9y8/eQ3i5SOQyYDLC/qaijSv5G1Pcztyuo+as9rlIGVneuS1X/qVdzo+?=
 =?us-ascii?Q?SCI4WiTQdaDbuC57x36bTblaCNCJ3OhEcEE6r+wnT3+hD94fp+6WHXzrguuY?=
 =?us-ascii?Q?udHpZ5L/nR9QW8Svpm2dG9QkB7U/COY4ZJ/g0jEkBrZxVQes0XsnQPuVH4YN?=
 =?us-ascii?Q?XJFXffMyKI5G1+7nmOW/Sjwx1OzXzenkrdvNdIxXrYDtgUHFa8hAn7n5fiX6?=
 =?us-ascii?Q?iw8HS9N+UJx5U/aFn/h4HKGo6hyehsfqvXPUT3St2AC6kzaEqa2cOinVRJqi?=
 =?us-ascii?Q?7Xzk6qoc4GJ+bKrzWCygusl/ftAgzJDNCNpBoc0CYnbvXUe6RO/JZSEhpoHZ?=
 =?us-ascii?Q?OYru+UHge4OzNish8kx5dJwgtfPRawXZF5WV5wsgMdxwdPA1Uk1MNCsqWwCD?=
 =?us-ascii?Q?YZ0rKisMV8NOGhurAnz2h86f/3s/E69DiKXfL/iobriYqkxgd0b6wQGYv6L7?=
 =?us-ascii?Q?A5IHgZxfgQfzQegfXAHijN8EN9o0PGVVbGq3/1kho5PSKh+L4nkxxgC9UCwh?=
 =?us-ascii?Q?Ro181j7+ts8LU6Vt88AXm4cUdJuefuAuOLqdSz1YAYaWptshLg7LzJkgHQAF?=
 =?us-ascii?Q?IVJULbu1kbJ6rkkiPIXgrEHAksfFMPmOuxIRQK3bxO8QWYMStGFNRiQtnWQR?=
 =?us-ascii?Q?tvSpcNvJmbfOtMuni279TdKQ462rm+G06078TFNckXABV0hz+Ngn/MI0NLk9?=
 =?us-ascii?Q?1Clcfwd4qkKm6EBVTqUK0BPDBPfZejJmKqdciYK0AfkHDzaJLeuCesboj6ux?=
 =?us-ascii?Q?uZLqMf9bAqDMMh5+XlZolSa9Uam5giVIdaSQVUE/x2NGn+XpoQwBx+CS8gNL?=
 =?us-ascii?Q?sYtHZkoQk+24zzq7OrHMC3V67BhXHB5KSfF0hW39zR/J2lPdv0xD7PdDC93U?=
 =?us-ascii?Q?nbsV47ZZC782AeWbtXHiGVbBtOgAYyRF0Fp65ugbGVdd3tIndUXcS8IIciqg?=
 =?us-ascii?Q?shK9U8buw0z7GPiwKXzXx9vnzM4osEUkFKq9qJyaPoomXIXanrCPe9+xEUw7?=
 =?us-ascii?Q?HDGdjlME5xGvV48JMQl3o0KtlqPe6qwxjOt/Id2IuHsGsxp8i+BmWb2ZzQqI?=
 =?us-ascii?Q?dWTkoAUDn2jkFcyqpZQ0uht0WPfsRpVkvYDUHElOI8EHtVn7RiDPiF3+e9Ln?=
 =?us-ascii?Q?v/gZWgEmi40gJzvBlOVeDq7x0Hi6YNTEiE61gyyhTUave2m0cwjEQzVgZzar?=
 =?us-ascii?Q?+WSczxOLRBzzfMV3TQ6NG1LM1Ga0Gear13RWkK7xYQX0wDpHyvGNXsgOZjlX?=
 =?us-ascii?Q?+6TZ1y7vyrDA05Iyg1hv/4VqoFHeNMqVk+FO7m2hciYvmVUdnEQKOshWmmTA?=
 =?us-ascii?Q?HUlqbI2u6EbpK/Fou0gzvKbelL4qH7ny/EfIARW2AAaZFQYMkbR49eFs7w?=
 =?us-ascii?Q?=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB6373.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 65e81b45-07a2-4190-fb37-08dc5fbb5092
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2024 15:22:09.4941
 (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: wdlY+3whIJR7bkCUlxShWLME72HQozkSyM0dwf5U9XzZiN3kJ4FamdG5GSACwZ8Kxe9yB8Y16QkZLGtBVr6oUg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6008
X-OriginatorOrg: intel.com

On Thursday, April 18, 2024 10:20 PM, Wang, Wei W wrote:
> On Thursday, April 18, 2024 9:59 PM, Sean Christopherson wrote:
> > On Thu, Apr 18, 2024, Wei W Wang wrote:
> > > On Thursday, April 18, 2024 12:27 AM, Sean Christopherson wrote:
> > > > On Wed, Apr 17, 2024, Wei Wang wrote:
> > > > > Introduces two new macros, KVM_X86_SC() and KVM_X86_SCC(), to
> > > > > streamline the usage of KVM_X86_OPS static calls. The current
> > > > > implementation of these calls is verbose and can lead to
> > > > > alignment challenges due to the two pairs of parentheses. This
> > > > > makes the code susceptible to exceeding the "80 columns per singl=
e
> line of code"
> > > > > limit as defined in the coding-style document. The two macros
> > > > > are added to improve code readability and maintainability, while
> > > > > adhering to
> > > > the coding style guidelines.
> > > >
> > > > Heh, I've considered something similar on multiple occasionsi.
> > > > Not because the verbosity bothers me, but because I often search
> > > > for exact "word" matches when looking for function usage and the
> > > > kvm_x86_
> > prefix trips me up.
> > >
> > > Yeah, that's another compelling reason for the improvement.
> > >
> > > > IIRC, static_call_cond() is essentially dead code, i.e. it's the
> > > > exact same as static_call().  I believe there's details buried in
> > > > a proposed series to remove it[*].  And to not lead things astray,
> > > > I verified that invoking a NULL kvm_x86_op with static_call() does
> > > > no harm
> > (well, doesn't explode at least).
> > > >
> > > > So if we add wrapper macros, I would be in favor in removing all
> > > > static_call_cond() as a prep patch so that we can have a single mac=
ro.
> > >
> > > Sounds good. Maybe KVM_X86_OP_OPTIONAL could now also be removed?
> >
> > No, KVM_X86_OP_OPTIONAL() is what allow KVM to WARN if a mandatory
> > hook isn't defined.  Without the OPTIONAL and OPTIONAL_RET variants,
> > KVM would need to assume every hook is optional, and thus couldn't WARN=
.
>=20
> Yes, KVM_X86_OP_OPTIONAL is used to enforce the definition of mandatory
> hooks with WARN_ON().=20

I meant the KVM_X86_OP in the current implementation as you shared.
If we don't need KVM_X86_OP_OPTIONAL(), the WARN_ON() from KVM_X86_OP
will need to be removed to allow that all the hooks could be optional.

> But the distinction between mandatory and optional
> hooks has now become ambiguous. For example, all the hooks, whether
> defined or undefined (NULL), are invoked via static_call() without issues=
 now.
> In some sense, all hooks could potentially be deemed as optional, and the
> undefined ones just lead to NOOP when unconditionally invoked by the
> kvm/x86 core code.
> (the KVM_X86_OP_RET0 is needed)
> Would you see any practical issues without that WARN_ON?