Received-SPF: pass (google.com: domain of linux-kernel+bounces-149485-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Wang, Wei W" <wei.w.wang@intel.com>
To: 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+7FsplwAgACN6vA=
Date: Thu, 18 Apr 2024 03:57:52 +0000
Message-ID: <DS0PR11MB63739BE4347EC6369ED22EBADC0E2@DS0PR11MB6373.namprd11.prod.outlook.com>
References: <20240417150354.275353-1-wei.w.wang@intel.com>
 <Zh_4QN7eFxyu9hgA@google.com>
In-Reply-To: <Zh_4QN7eFxyu9hgA@google.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?9vx2iKA7kZewbt5cDRo6nL4SZw0rnEhGPGwtHXiDNnB1M1QCXu/o/de0G/bj?=
 =?us-ascii?Q?noo4bmKbCxINFdTCOEGcg0MEN23xXxFl40fWcSFJ2zVS2cTcaN9cPH2QirK4?=
 =?us-ascii?Q?yWJmFjspPnnAj6b+yn0qjXdnSWL1poCrWddr3Njsm/hDctL1vwfLtEnVCPbS?=
 =?us-ascii?Q?nWcx18docBlNZ2ntsbzk3v4vLkaw0LEUgtgo0Pg/UjmO6uYOHpvUEF202A73?=
 =?us-ascii?Q?5QksWOojVvxVgdkR8VNlCzU3wWNs5if6zbDTwIL8CqA/R7wuCwHiOygu+sAo?=
 =?us-ascii?Q?c4J4ZO2+C4Gp1t2fwZRJcZ9aGcSjclx3H5CR89KNwfQZrottC3ZtP3IZRT0U?=
 =?us-ascii?Q?eQ6hDxTVkH223K8zaHFmBOgqYGZ6dHIGTxJJmPcUHBwNMTfsL4ctM8qnzApG?=
 =?us-ascii?Q?EkVoh3AIHTiKbsyiiDQbruxMutRBOTx2N1TMqBru4PY1JiMrhC77ntGajmB3?=
 =?us-ascii?Q?1iuuAi7CphKfvZboXiFrEz0typEKBFPa2gPXQN/SFyplXw6LAU7QDsYTJlqT?=
 =?us-ascii?Q?r6lf282xyxqBBHJSHealRvVUjgLpjF2dfdvU4/TgEWktcvet7Mv+oycAi5Yi?=
 =?us-ascii?Q?2b6LNu+RNwObMElpu+CV06klZehVn2Nq70PjUXAYvUuBZ+21y8GUD4rGPdXf?=
 =?us-ascii?Q?vXQX3qq4mPPFB3S8qY69hplSDkTmmeiJ8j4rMwlw1+fA/PADKw1w5OGL15uT?=
 =?us-ascii?Q?K37W5XC4q7DNsJQIjVAGsJTGh1SqeK2mGj9hPJYGL0ttIoMkbU+j1eOXLCF9?=
 =?us-ascii?Q?41BgtdNoSniTDR+gqmigdW8kFzOJPF5Pxn8yBVWCarR0x54NHz6s/b5N/v3c?=
 =?us-ascii?Q?RA6GdVq9tThBL6FHXcYsvyZ9JQlCxz4cQcv7liM8MetYxwyF15a649iHvOPE?=
 =?us-ascii?Q?XC65DG9jjWqhKpBoYX3n3hpWiu/zDz6uAI+ndh7E417yDJccirBVoJ409Tkd?=
 =?us-ascii?Q?HgJGx0wUfM8fI9uV/WGfKXp+B6+CgxNeFUtOvwIV1kbujmpY9MSfzhgnobJ4?=
 =?us-ascii?Q?3oeR7NSvdFkrE4z2zpKimjcoVxLWFPiF7c+v/T2ESp0zt/WS7j6fv/EAJyec?=
 =?us-ascii?Q?KCgVTu7bDeYI03vftDIn97fITQPaxvRriSXjhjTOy7mrZboPa8sZVrj9iP9P?=
 =?us-ascii?Q?zcZ3MgppK+JOFtF2yhbQcHOm8cghXUKHi5yYmwY0exO2KwSqABsE6kGdtJKD?=
 =?us-ascii?Q?SA6aRrS8zdkOVpRWKvyrhasWyU+vL+J1AwH+NK3CKgi5qzSkZJAk5kSH9aCI?=
 =?us-ascii?Q?b2rJsidD+i0yrfE6dShJmuro+yItUF9DEXaYsXlC13Aujedfn5ruz3XWBjQB?=
 =?us-ascii?Q?5e4clZHO6hF19GW6cTN6cA+4mFBFWHEN60Qp6k/LYb7Wlquhp7j63mzCchht?=
 =?us-ascii?Q?4Rm051KHJrD8KVn//ijPJ7Oy0C1Xw+LuQE0UQuY+rjeFa/fBphLBQpoEw3Cm?=
 =?us-ascii?Q?i8pMlD+PfZiTiaqRCIUTLh9EM5jKmH73vCLdchg99dCBCSiyJ7G3QM3aDsOA?=
 =?us-ascii?Q?ecH6Fbxv+IbYyuyiwqaVMBUG1NAvZnfiSzs4yuE4cVXay/Mxk/Syo9KtyQ?=
 =?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: 9d371ba2-9a93-40c7-f111-08dc5f5bb879
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2024 03:57:52.1118
 (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: k3Ex2gTcq7tdum1hPW4dOfeqb8ML9pXp7Mcm3EUTdH3MU52nKy3xVb4lKKqpJqLb39sC41RDPbzR+TU1NZ47Xg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB7205
X-OriginatorOrg: intel.com

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 single line of code"
> > limit as defined in the coding-style document. The two macros are
> > added to improve code readability and maintainability, while adhering t=
o
> the coding style guidelines.
>=20
> Heh, I've considered something similar on multiple occasionsi.  Not becau=
se
> the verbosity bothers me, but because I often search for exact "word" mat=
ches
> 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 sa=
me 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 kv=
m_x86_op
> with static_call() does no harm (well, doesn't explode at least).
>=20
> 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 macro.

Sounds good. Maybe KVM_X86_OP_OPTIONAL could now also be removed?=20


> kvm_ops_update() already WARNs if a mandatory hook isn't defined, so doin=
g
> more checks at runtime wouldn't provide any value.

>=20
> As for the name, what about KVM_X86_CALL() instead of KVM_X86_SC()?  Two
> extra characters, but should make it much more obvious what's going on fo=
r
> readers that aren't familiar with the infrastructure.

I thought the macro definition is quite intuitive and those encountering it=
 for the
first time could get familiar with it easily from the definition.
Similarly, KVM_X86_CALL() is fine to me, despite the fact that it doesn't e=
xplicitly
denote "static" calls.

>=20
> And I bet we can get away with KVM_PMU_CALL() for the PMU hooks.

Yes, this can be covered as well.