Received-SPF: pass (google.com: domain of linux-kernel+bounces-118103-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Michael Kelley <mhklinux@outlook.com>
To: Xi Ruoyao <xry111@xry111.site>, Dave Hansen <dave.hansen@linux.intel.com>
CC: Andy Lutomirski <luto@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	"x86@kernel.org" <x86@kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3] x86/mm: Don't disable INVLPG if "incomplete Global
 INVLPG flushes" is fixed by microcode
Thread-Topic: [PATCH v3] x86/mm: Don't disable INVLPG if "incomplete Global
 INVLPG flushes" is fixed by microcode
Thread-Index: AQHafvtoA+0/e6KloUSoqoP9U8FJHLFJBtDQ
Date: Mon, 25 Mar 2024 22:27:37 +0000
Message-ID:
 <SN6PR02MB4157C6837815BDB62EB9ED91D4362@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240325212818.125053-1-xry111@xry111.site>
In-Reply-To: <20240325212818.125053-1-xry111@xry111.site>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?re6VW2oz+luO9/VcmuAuch3KmNrcWieTGi2zNnqVv+AuK3g9jUNRWfb/Algy?=
 =?us-ascii?Q?s4jJdExPU2YgM/ORWFm5KHJMLUdf6GDDsyIBR+0g/tJsryfLxEIsP8CEUJxH?=
 =?us-ascii?Q?+FO6+jxRtAsCeufPzMTtyqgY1BkBgScCs86HDh0cnXqt0xefxNZSTY+LlVT4?=
 =?us-ascii?Q?cw9FAEQtWT7jE0wR6DBGCj8pv9mECEUcTYvNSo4jM+Rq8e19NUvTJewNPCR7?=
 =?us-ascii?Q?IhELyzY8N9CcxjHd0f3gfxS/TJuUErdnWdwxams8X9hI3iwdgFanFyxnfUHC?=
 =?us-ascii?Q?pefK+hQ6JZVyoChqgcIMd3rrU3QON0wYaRCq7fjmM8Tp7vaEi0+W2vwjoix/?=
 =?us-ascii?Q?XXLmBiCi7xGKk6R1Ly0UAifuMnsLgODJ8LQ3f3gzLxOizq0xo2A+dgePCi63?=
 =?us-ascii?Q?15neXP26w8w5Mrpyiznc3dAZPnOQ46ZW8sGMZkbfOlouVkXGBY2DgSmdzUZQ?=
 =?us-ascii?Q?Cg4TasOH3nlCU1ssJ2V2P+W1lA9nUW1N1eFFXvTrugR9dBmKeS2yGbKUpCSH?=
 =?us-ascii?Q?K79rZFvvZnK5bqyh0l/dd6xOZXUaoc0vS1mvbFuUt/YT9tPxIVI2/3Wjfxmx?=
 =?us-ascii?Q?pWUlP5fTkK/Ijh7ImOscl4hc22AOdOc0op0MqYNmF7T63/gqemktpUsVrmvt?=
 =?us-ascii?Q?/zefz5GHuXkmKX5q69MgDJOKJTWavhUJ6aBYJJUTJu63UzYQX0Rzd+byP2Hw?=
 =?us-ascii?Q?7pKrEF0BIo5tkkU9fp5b5gCHx32kby/QEqD2VFSC3AFdH/zS0dZjYqXgjz9k?=
 =?us-ascii?Q?gnGAwmXfuPER9y8VE2hJNw5+lKtR1/G7cBwWGy89rxSsfu3cyYCQ8vbtytXA?=
 =?us-ascii?Q?xZam0AxAwdfxaIOScZfCkKM30//K5sdycI0ValfFRRAF8lMmgjh2DoSA63QG?=
 =?us-ascii?Q?hUrPQ1dUvFdORdCdNXlSoanSTd62+hRLoRhFQCBXBzMyT7ZHLFQA0kUe6Wik?=
 =?us-ascii?Q?l/asxRjE62W3BDjJ/cFGf6+l9Za+Gov5gLE0GXXerWjLIfCXsa33Thso5TZJ?=
 =?us-ascii?Q?1rLaMqvAvg/ub29PLiQiZyIbB3fKA88iRy1IcF0ETQfOkVb+t2SpRud5crY6?=
 =?us-ascii?Q?RH/HjuyNk0uGSWNW00WpLtwQLvKC3WEYwn/PZFId/zwwCy0O5lSi1zpIWaNU?=
 =?us-ascii?Q?XM0eX53YpbyRoJAj/aGDdaoQ6oSN61ZhFp4b1sl+wB+27Y5BvbbkqBHdMXnf?=
 =?us-ascii?Q?Gvqv3sMgrK7txB0E6xgOcAGX+IAlCQFqDZ5FEg=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-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 0749a7fd-db1a-4745-5a1f-08dc4d1ac65b
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Mar 2024 22:27:37.1631
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR02MB8557

From: Xi Ruoyao <xry111@xry111.site> Sent: Monday, March 25, 2024 2:28 PM
>=20
> Per the "Processor Specification Update" documentations referred by the
> intel-microcode-20240312 release note, this microcode release has fixed
> the issue for all affected models.
>=20
> So don't disable INVLPG if the microcode is new enough.
>=20
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  arch/x86/mm/init.c | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
>=20
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 679893ea5e68..1a7d6a61a4cb 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -261,33 +261,46 @@ static void __init probe_page_size_mask(void)
>  	}
>  }
>=20
> -#define INTEL_MATCH(_model) { .vendor  =3D X86_VENDOR_INTEL,	\
> -			      .family  =3D 6,			\
> -			      .model =3D _model,			\
> -			    }
> +#define INTEL_MATCH(_model, _fixed_microcode)	\
> +    { .vendor		=3D X86_VENDOR_INTEL,	\
> +      .family		=3D 6,			\
> +      .model		=3D _model,		\
> +      .driver_data	=3D _fixed_microcode,	\
> +    }
> +
>  /*
>   * INVLPG may not properly flush Global entries
> - * on these CPUs when PCIDs are enabled.
> + * on these CPUs when PCIDs are enabled and the
> + * microcode is not updated to fix the issue.
>   */
>  static const struct x86_cpu_id invlpg_miss_ids[] =3D {
> -	INTEL_MATCH(INTEL_FAM6_ALDERLAKE   ),
> -	INTEL_MATCH(INTEL_FAM6_ALDERLAKE_L ),
> -	INTEL_MATCH(INTEL_FAM6_ATOM_GRACEMONT ),
> -	INTEL_MATCH(INTEL_FAM6_RAPTORLAKE  ),
> -	INTEL_MATCH(INTEL_FAM6_RAPTORLAKE_P),
> -	INTEL_MATCH(INTEL_FAM6_RAPTORLAKE_S),
> +	INTEL_MATCH(INTEL_FAM6_ALDERLAKE,	0x34),
> +	INTEL_MATCH(INTEL_FAM6_ALDERLAKE_L,	0x432),
> +	INTEL_MATCH(INTEL_FAM6_ATOM_GRACEMONT,	0x15),
> +	INTEL_MATCH(INTEL_FAM6_RAPTORLAKE,	0x122),
> +	INTEL_MATCH(INTEL_FAM6_RAPTORLAKE_P,	0x4121),
> +	INTEL_MATCH(INTEL_FAM6_RAPTORLAKE_S,	0x34),
>  	{}
>  };
>=20
>  static void setup_pcid(void)
>  {
> +	const struct x86_cpu_id *invlpg_miss_match;
> +
>  	if (!IS_ENABLED(CONFIG_X86_64))
>  		return;
>=20
>  	if (!boot_cpu_has(X86_FEATURE_PCID))
>  		return;
>=20
> -	if (x86_match_cpu(invlpg_miss_ids)) {
> +	invlpg_miss_match =3D x86_match_cpu(invlpg_miss_ids);
> +
> +	/* The hypervisor may lie about the microcode revision, conservatively
> +	 * consider the microcode not updated.
> +	 */

Nit:  Except in the "net" subtree, the proper format for multi-line comment=
s
is this:

	/*
	 * The hypervisor may lie about the microcode revision, conservatively
	 * consider the microcode not updated.
	 */

> +	if (invlpg_miss_match &&
> +	    (boot_cpu_has (X86_FEATURE_HYPERVISOR) ||
> +	     invlpg_miss_match->driver_data > boot_cpu_data.microcode)) {
>  		pr_info("Incomplete global flushes, disabling PCID");
>  		setup_clear_cpu_cap(X86_FEATURE_PCID);
>  		return;

Modulo the nit above,

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

I'm good with this. Losing PCID has definite performance impact,
but I lean toward being conservative when we can't verify the
microcode version.

I'd also be interested in other perspectives.  :-)

Michael