[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/vpmu: Simplify is_pmc_quirk
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Wed, 21 Jun 2023 10:30:49 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=tM61P+aXKVH9l6UMGrKDJb2ovIju9mC8czrBkVlSHIA=; b=gjN6mBWMVnWGh5zKahLPVDs+v7PcATG1FNxMh7s50wbDbNoqIYzxF1Pfp4+8ldNGn9gt4D7D4TCaeXA3CMkWhLZZRazRY40ATSmfo8g11wi33i6mRDnGWh1bvjsR0EG5aw+5usI633xCIJuvlWGaH/LEGCt3MIZyK4NXK4HdaberxL0ahLmeRryEcjFLm0C9Baz1Hjd7uheqVMELUizyDSdSGgEVbUTm48BfHnGLWaT76Km5C672gC5iwINCOVZlK5X3wLGPwDdjKkJW9dt9AyGuTGSs08hCnABotY/i6vOt+Gnlmrjb3DX+sRCYNFvy3RzYay1Y0yGakwcOdjJPgw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HlgVZTQCwAznmXsSsuUS8kY5Nimt78g4lBTO6HI2lMxrad7mDG7bkkrtnEBpgS1p2s8TEEDzlA6BSt0qGw0t2pHdfRH0ZIqh79f0wq3lK+aySs3YgKrlaQdKuqyHQmtTTHIydrY5fiUdDDHwfpO7kpTCAyKDHz7qFToNOn1mjM2fRV1P3dWSO2GkyplcKz91R1+0x2qSdudJXw+aCTphZTEb2FJKjFslDEYU0gjj1vQN1Eq5+Y4yxYmMLuZ5jNzp+aDI3I0R4ILkLvbUwBNWN7pb6Rd8KTmwo1YPjUI5PXI83ZbhHfZL9VqMuqZLyIW/AerLWcYWhDOEfmddnxv9gQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Wed, 21 Jun 2023 09:31:15 +0000
- Ironport-data: A9a23:1va+r6Ou8HatxMPvrR2HlsFynXyQoLVcMsEvi/4bfWQNrUorgTcGx 2BLXmyBb/eJNGP8KN91a4+3/U9V6sWGz9M3TAto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CU6jufQAOKnUoYoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGjxSs/rrRC9H5qyo42tG5AVmP5ingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0uRZWllW/ vMAEjIuYy2Zv+657K+jUNA506zPLOGzVG8ekldJ6GmFSNwAEdXESaiM4sJE1jAtgMwIBezZe 8cSdTtoalLHfgFLPVAUTpk5mY9EhFGmK2Ee9A3T+vZxvzi7IA9ZidABNPL8fNCQSNoTtUGfv m/cpEzyAw0ANczZwj2Amp6prraWx3KhAthMRdVU8NZVmXyIxWhICyQpSFuXmfWQsBaEXt9Qf hl8Fi0G6PJaGFaQZtv3UgC8oXWElgUBQNcWGOo/gCmSzoLE7gDfAXILJhZRZdpjuMIoSDgC0 l6Sg8ivFTFpqKeSS3+W6vGTtzzaBMQOBWoLZCtBRw1V5dDm+N03lkiXEoYlF7OphNroHz222 yqNsCU1m7QUi4gMyrm/+lfExTmro/AlUzII2+keZUr9hisRWWJvT9XABYTzhRqYELukcw==
- Ironport-hdrordr: A9a23:PhqWyKx2M6nzb+rqXPHXKrPwl71zdoMgy1knxilNoNJuE/Bw8P re+sjztCWE7wr5PUtLpTnuAsS9qB/nmaKdpLNhXotLsmHdyReVxcJZnPbfKwSJIVyAygcl79 YfT0EdMr3N5ClB/KLHCVKDYq8dKbC8mcjCuQ6d9QYOcegNUc5dBmxCe2Om+yNNKjWuLKBJZa a0145opyeAZX9SVciyHH8DNtKz3eHjpdbJYQMmGxVi0wWFjSqp5LnmeiLopSs2YndgwaoC7W OAqADy5ryiv/anjjfQ2nTe9Y4+oqqQ9vJzQOKNl+kIIXHXhgGkaJ8JYcz7gAwI
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 21/06/2023 9:16 am, Jan Beulich wrote:
> On 20.06.2023 19:45, Andrew Cooper wrote:
>> This should be static, and there's no need for a separate (non-init, even)
>> function to perform a simple equality test. Drop the is_ prefix which is
>> gramatically questionable, and make it __ro_after_init.
>>
>> Leave a TODO, because the behaviour is definitely wrong to be applied to ~all
>> modern Intel CPUs, and has been raised on xen-devel previously without
>> conclusion.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks.
> with one request:
>
>> @@ -967,7 +960,8 @@ const struct arch_vpmu_ops *__init core2_vpmu_init(void)
>> sizeof(uint64_t) * fixed_pmc_cnt +
>> sizeof(struct xen_pmu_cntr_pair) * arch_pmc_cnt;
>>
>> - check_pmc_quirk();
>> + /* TODO: This is surely wrong. */
>> + pmc_quirk = current_cpu_data.x86 == 6;
> In the description you say "~all modern Intel CPUs", which suggests it might
> be correct for old enough ones. Would you mind weakening the comment to
> "This surely isn't universally correct" or some such?
I'm happy to tweak the wording as appropriate, but Kyle (who is a
regular contributor to perf in Linux) questioned that there was an issue.
There's insufficient information to figure out why it was introduced in
the first place, and IIRC he wasn't aware of any errata that manifested
in this way.
It's possible it's entirely bogus, or it may be a misunderstood errata.
It needs looking into by someone with some copious free time.
~Andrew
|
