[PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[Xi Ruoyao]

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.

So don't disable PCID if the microcode is new enough.  The precise
minimum microcode revision fixing the issue is provided by engineer from
Intel.

Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Michael Kelley <mhklinux@outlook.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Link: https://lore.kernel.org/all/168436059559.404.13934972543631851306.tip-bot2@tip-bot2/
Link: https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files/releases/tag/microcode-20240312
Link: https://cdrdv2.intel.com/v1/dl/getContent/740518 # RPL042, rev. 13
Link: https://cdrdv2.intel.com/v1/dl/getContent/682436 # ADL063, rev. 24
Link: https://lore.kernel.org/all/20240325231300.qrltbzf6twm43ftb@desk/
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 arch/x86/mm/init.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index eb503f53c319..e960196e8058 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -264,27 +264,33 @@ static void __init probe_page_size_mask(void)
 
 /*
  * 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[] = {
-	X86_MATCH_VFM(INTEL_ALDERLAKE,	    0),
-	X86_MATCH_VFM(INTEL_ALDERLAKE_L,    0),
-	X86_MATCH_VFM(INTEL_ATOM_GRACEMONT, 0),
-	X86_MATCH_VFM(INTEL_RAPTORLAKE,	    0),
-	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,   0),
-	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,   0),
+	X86_MATCH_VFM(INTEL_ALDERLAKE,	    0x2e),
+	X86_MATCH_VFM(INTEL_ALDERLAKE_L,    0x42c),
+	X86_MATCH_VFM(INTEL_ATOM_GRACEMONT, 0x11),
+	X86_MATCH_VFM(INTEL_RAPTORLAKE,	    0x118),
+	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,   0x4117),
+	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,   0x2e),
 	{}
 };
 
 static void setup_pcid(void)
 {
+	const struct x86_cpu_id *invlpg_miss_match;
+
 	if (!IS_ENABLED(CONFIG_X86_64))
 		return;
 
 	if (!boot_cpu_has(X86_FEATURE_PCID))
 		return;
 
-	if (x86_match_cpu(invlpg_miss_ids)) {
+	invlpg_miss_match = x86_match_cpu(invlpg_miss_ids);
+
+	if (invlpg_miss_match &&
+	    boot_cpu_data.microcode < invlpg_miss_match->driver_data) {
 		pr_info("Incomplete global flushes, disabling PCID");
 		setup_clear_cpu_cap(X86_FEATURE_PCID);
 		return;
-- 
2.45.1

[PATCH v10 2/2] x86/mm: Don't disable PCID if the kernel is running on a hypervisor

[Xi Ruoyao]

The Intel erratum for "incomplete Global INVLPG flushes" says:

    This erratum does not apply in VMX non-root operation. It applies
    only when PCIDs are enabled and either in VMX root operation or
    outside VMX operation.

So if the kernel is running in a hypervisor, we are in VMX non-root
operation and we should be safe to use PCID.

Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Michael Kelley <mhklinux@outlook.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Link: https://lore.kernel.org/all/168436059559.404.13934972543631851306.tip-bot2@tip-bot2/
Link: https://cdrdv2.intel.com/v1/dl/getContent/740518 # RPL042, rev. 13
Link: https://cdrdv2.intel.com/v1/dl/getContent/682436 # ADL063, rev. 24
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 arch/x86/mm/init.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index e960196e8058..1efd55835fc5 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -279,7 +279,7 @@ static const struct x86_cpu_id invlpg_miss_ids[] = {
 
 static void setup_pcid(void)
 {
-	const struct x86_cpu_id *invlpg_miss_match;
+	const struct x86_cpu_id *invlpg_miss_match = NULL;
 
 	if (!IS_ENABLED(CONFIG_X86_64))
 		return;
@@ -287,7 +287,9 @@ static void setup_pcid(void)
 	if (!boot_cpu_has(X86_FEATURE_PCID))
 		return;
 
-	invlpg_miss_match = x86_match_cpu(invlpg_miss_ids);
+	/* Only bare-metal is affected.  PCIDs in guests are OK.  */
+	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
+		invlpg_miss_match = x86_match_cpu(invlpg_miss_ids);
 
 	if (invlpg_miss_match &&
 	    boot_cpu_data.microcode < invlpg_miss_match->driver_data) {
-- 
2.45.1

Re: [PATCH v10 2/2] x86/mm: Don't disable PCID if the kernel is running on a hypervisor

[Dave Hansen]

On 5/21/24 19:06, Xi Ruoyao wrote:
> -	invlpg_miss_match = x86_match_cpu(invlpg_miss_ids);
> +	/* Only bare-metal is affected.  PCIDs in guests are OK.  */
> +	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> +		invlpg_miss_match = x86_match_cpu(invlpg_miss_ids);

So, surely, the common case is hypervisors that set
X86_FEATURE_HYPERVISOR are running the guest under VMX.  But it doesn't
cover everything, either.

The guest could be running under regular old QEMU without KVM.  Or it
could be one of the hypervisors that has a sense of humor and runs guest
ring0 in hardware ring3.  But those setups aren't vulnerable in the
first place because they don't actually execute INVLPG directly on the
hardware.

That said, if this check goes wrong (like seeing a
X86_FEATURE_HYPERVISOR==0 under VMX) the worst that can happen is that
PCIDs get disabled without a good reason.

So I think the patch is correct, but I don't like the idea that
X86_FEATURE_HYPERVISOR has any kind of strict connection to VMX.

I'd rather just say:

	Hypervisors lie about CPUID making model and microcode version
	checks worthless. Just assume all guests are immune either
	because they can't use INVLPG directly or are running under VMX
	and are unaffected.

Re: [PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[Xi Ruoyao]

Ping.

Ok to queue these two into some branch for integration?

On Wed, 2024-05-22 at 10:06 +0800, Xi Ruoyao wrote:
> 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.
> 
> So don't disable PCID if the microcode is new enough.  The precise
> minimum microcode revision fixing the issue is provided by engineer from
> Intel.
> 
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Michael Kelley <mhklinux@outlook.com>
> Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Link: https://lore.kernel.org/all/168436059559.404.13934972543631851306.tip-bot2@tip-bot2/
> Link: https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files/releases/tag/microcode-20240312
> Link: https://cdrdv2.intel.com/v1/dl/getContent/740518 # RPL042, rev. 13
> Link: https://cdrdv2.intel.com/v1/dl/getContent/682436 # ADL063, rev. 24
> Link: https://lore.kernel.org/all/20240325231300.qrltbzf6twm43ftb@desk/
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  arch/x86/mm/init.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index eb503f53c319..e960196e8058 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -264,27 +264,33 @@ static void __init probe_page_size_mask(void)
>  
>  /*
>   * 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[] = {
> -	X86_MATCH_VFM(INTEL_ALDERLAKE,	    0),
> -	X86_MATCH_VFM(INTEL_ALDERLAKE_L,    0),
> -	X86_MATCH_VFM(INTEL_ATOM_GRACEMONT, 0),
> -	X86_MATCH_VFM(INTEL_RAPTORLAKE,	    0),
> -	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,   0),
> -	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,   0),
> +	X86_MATCH_VFM(INTEL_ALDERLAKE,	    0x2e),
> +	X86_MATCH_VFM(INTEL_ALDERLAKE_L,    0x42c),
> +	X86_MATCH_VFM(INTEL_ATOM_GRACEMONT, 0x11),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE,	    0x118),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,   0x4117),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,   0x2e),
>  	{}
>  };
>  
>  static void setup_pcid(void)
>  {
> +	const struct x86_cpu_id *invlpg_miss_match;
> +
>  	if (!IS_ENABLED(CONFIG_X86_64))
>  		return;
>  
>  	if (!boot_cpu_has(X86_FEATURE_PCID))
>  		return;
>  
> -	if (x86_match_cpu(invlpg_miss_ids)) {
> +	invlpg_miss_match = x86_match_cpu(invlpg_miss_ids);
> +
> +	if (invlpg_miss_match &&
> +	    boot_cpu_data.microcode < invlpg_miss_match->driver_data) {
>  		pr_info("Incomplete global flushes, disabling PCID");
>  		setup_clear_cpu_cap(X86_FEATURE_PCID);
>  		return;

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[Dave Hansen]

On 6/26/24 06:10, Xi Ruoyao wrote:
> Ping.
> 
> Ok to queue these two into some branch for integration?

Please don't top post, and please trim your replies.

The code looks fine, but this has zero acks and I can't find any public
documentation of which microcode versions fix the errata.

Re: [PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[Pawan Gupta]

On Wed, Jun 26, 2024 at 09:50:39AM -0700, Dave Hansen wrote:
> On 6/26/24 06:10, Xi Ruoyao wrote:
> > Ping.
> > 
> > Ok to queue these two into some branch for integration?
> 
> Please don't top post, and please trim your replies.
> 
> The code looks fine, but this has zero acks and I can't find any public
> documentation of which microcode versions fix the errata.

Based on an internal document, I provided the microcode versions used in
this patch.

Acked-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

Re: [PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[Xi Ruoyao]

On Wed, 2024-06-26 at 10:15 -0700, Pawan Gupta wrote:
> On Wed, Jun 26, 2024 at 09:50:39AM -0700, Dave Hansen wrote:
> > On 6/26/24 06:10, Xi Ruoyao wrote:
> > > Ping.
> > > 
> > > Ok to queue these two into some branch for integration?
> > 
> > Please don't top post, and please trim your replies.
> > 
> > The code looks fine, but this has zero acks and I can't find any public
> > documentation of which microcode versions fix the errata.
> 
> Based on an internal document, I provided the microcode versions used in
> this patch.
> 
> Acked-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

I hope this can catch 6.11...

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v10 1/2] x86/mm: Don't disable PCID if "incomplete Global INVLPG flushes" is fixed by microcode

[Xi Ruoyao]

On Wed, 2024-06-26 at 10:15 -0700, Pawan Gupta wrote:
> On Wed, Jun 26, 2024 at 09:50:39AM -0700, Dave Hansen wrote:
> > On 6/26/24 06:10, Xi Ruoyao wrote:
> > > Ping.
> > > 
> > > Ok to queue these two into some branch for integration?
> > 
> > Please don't top post, and please trim your replies.
> > 
> > The code looks fine, but this has zero acks and I can't find any public
> > documentation of which microcode versions fix the errata.
> 
> Based on an internal document, I provided the microcode versions used in
> this patch.
> 
> Acked-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

Still applies and works on Linus' tree.  Ok for 6.12?

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University