[PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

Supersedes: <20240130180400.1698136-1-pbonzini@redhat.com>

MKTME repurposes the high bit of physical address to key id for encryption
key and, even though MAXPHYADDR in CPUID[0x80000008] remains the same,
the valid bits in the MTRR mask register are based on the reduced number
of physical address bits.  This breaks boot on machines that have TME enabled
and do something to cleanup MTRRs, unless "disable_mtrr_cleanup" is
passed on the command line.  The fix is to move the check to early CPU
initialization, which runs before Linux sets up MTRRs.

However, as noticed by Kirill, the patch I sent as v1 actually works only
until Linux 6.6.  In Linux 6.7, commit fbf6449f84bf ("x86/sev-es: Set
x86_virt_bits to the correct value straight away, instead of a two-phase
approach") reorganized the initialization of c->x86_phys_bits in a way
that broke the patch.  But even in 6.7 AMD processors, which did try to
reduce it in this_cpu->c_early_init(c), had their x86_phys_bits value
overwritten by get_cpu_address_sizes(), so that early_identify_cpu()
left the wrong value in x86_phys_bits.  This probably went unnoticed
because on AMD processors you need not apply the reduced MAXPHYADDR to
MTRR masks.

Therefore, this v2 prepends the fix for this issue in commit fbf6449f84bf.
Apologies for the oversight.

Tested on an AMD Epyc machine (where I resorted to dumping mtrr_state) and
on the problematic Intel Emerald Rapids machine.

Thanks,

Paolo

Paolo Bonzini (2):
  x86/cpu: allow reducing x86_phys_bits during early_identify_cpu()
  x86/cpu/intel: Detect TME keyid bits before setting MTRR mask
    registers

 arch/x86/kernel/cpu/common.c |   4 +-
 arch/x86/kernel/cpu/intel.c  | 178 ++++++++++++++++++-----------------
 2 files changed, 93 insertions(+), 89 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] x86/cpu: allow reducing x86_phys_bits during early_identify_cpu()

[Paolo Bonzini]

In commit fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct
value straight away, instead of a two-phase approach"), the initialization
of c->x86_phys_bits was moved after this_cpu->c_early_init(c).  This is
incorrect because early_init_amd() expected to be able to reduce the
value according to the contents of CPUID leaf 0x8000001f.

Fortunately, the bug was negated by init_amd()'s call to early_init_amd(),
which does reduce x86_phys_bits in the end.  However, this is very
late in the boot process and, most notably, the wrong value is used for
x86_phys_bits when setting up MTRRs.

To fix this, call get_cpu_address_sizes() as soon as X86_FEATURE_CPUID is
set/cleared, and c->extended_cpuid_level is retrieved.

Fixes: fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct value straight away, instead of a two-phase approach")
Cc: Adam Dunlap <acdunlap@google.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: x86@kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kernel/cpu/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0b97bcde70c6..fbc4e60d027c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1589,6 +1589,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 		get_cpu_vendor(c);
 		get_cpu_cap(c);
 		setup_force_cpu_cap(X86_FEATURE_CPUID);
+		get_cpu_address_sizes(c);
 		cpu_parse_early_param();
 
 		if (this_cpu->c_early_init)
@@ -1601,10 +1602,9 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 			this_cpu->c_bsp_init(c);
 	} else {
 		setup_clear_cpu_cap(X86_FEATURE_CPUID);
+		get_cpu_address_sizes(c);
 	}
 
-	get_cpu_address_sizes(c);
-
 	setup_force_cpu_cap(X86_FEATURE_ALWAYS);
 
 	cpu_set_bug_bits(c);
-- 
2.43.0

[PATCH v2 2/2] x86/cpu/intel: Detect TME keyid bits before setting MTRR mask registers

[Paolo Bonzini]

MKTME repurposes the high bit of physical address to key id for encryption
key and, even though MAXPHYADDR in CPUID[0x80000008] remains the same,
the valid bits in the MTRR mask register are based on the reduced number
of physical address bits.

detect_tme() in arch/x86/kernel/cpu/intel.c detects TME and subtracts
it from the total usable physical bits, but it is called too late.
Move the call to early_init_intel() so that it is called in setup_arch(),
before MTRRs are setup.

This fixes boot on TDX-enabled systems, which until now only worked with
"disable_mtrr_cleanup".  Without the patch, the values written to the
MTRRs mask registers were 52-bit wide (e.g. 0x000fffff_80000800) and
the writes failed; with the patch, the values are 46-bit wide, which
matches the reduced MAXPHYADDR that is shown in /proc/cpuinfo.

Reported-by: Zixi Chen <zixchen@redhat.com>
Cc: Adam Dunlap <acdunlap@google.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Kai Huang <kai.huang@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: x86@kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kernel/cpu/intel.c | 178 ++++++++++++++++++------------------
 1 file changed, 91 insertions(+), 87 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index a927a8fc9624..40dec9b56f87 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -184,6 +184,90 @@ static bool bad_spectre_microcode(struct cpuinfo_x86 *c)
 	return false;
 }
 
+#define MSR_IA32_TME_ACTIVATE		0x982
+
+/* Helpers to access TME_ACTIVATE MSR */
+#define TME_ACTIVATE_LOCKED(x)		(x & 0x1)
+#define TME_ACTIVATE_ENABLED(x)		(x & 0x2)
+
+#define TME_ACTIVATE_POLICY(x)		((x >> 4) & 0xf)	/* Bits 7:4 */
+#define TME_ACTIVATE_POLICY_AES_XTS_128	0
+
+#define TME_ACTIVATE_KEYID_BITS(x)	((x >> 32) & 0xf)	/* Bits 35:32 */
+
+#define TME_ACTIVATE_CRYPTO_ALGS(x)	((x >> 48) & 0xffff)	/* Bits 63:48 */
+#define TME_ACTIVATE_CRYPTO_AES_XTS_128	1
+
+/* Values for mktme_status (SW only construct) */
+#define MKTME_ENABLED			0
+#define MKTME_DISABLED			1
+#define MKTME_UNINITIALIZED		2
+static int mktme_status = MKTME_UNINITIALIZED;
+
+static void detect_tme_early(struct cpuinfo_x86 *c)
+{
+	u64 tme_activate, tme_policy, tme_crypto_algs;
+	int keyid_bits = 0, nr_keyids = 0;
+	static u64 tme_activate_cpu0 = 0;
+
+	rdmsrl(MSR_IA32_TME_ACTIVATE, tme_activate);
+
+	if (mktme_status != MKTME_UNINITIALIZED) {
+		if (tme_activate != tme_activate_cpu0) {
+			/* Broken BIOS? */
+			pr_err_once("x86/tme: configuration is inconsistent between CPUs\n");
+			pr_err_once("x86/tme: MKTME is not usable\n");
+			mktme_status = MKTME_DISABLED;
+
+			/* Proceed. We may need to exclude bits from x86_phys_bits. */
+		}
+	} else {
+		tme_activate_cpu0 = tme_activate;
+	}
+
+	if (!TME_ACTIVATE_LOCKED(tme_activate) || !TME_ACTIVATE_ENABLED(tme_activate)) {
+		pr_info_once("x86/tme: not enabled by BIOS\n");
+		mktme_status = MKTME_DISABLED;
+		return;
+	}
+
+	if (mktme_status != MKTME_UNINITIALIZED)
+		goto detect_keyid_bits;
+
+	pr_info("x86/tme: enabled by BIOS\n");
+
+	tme_policy = TME_ACTIVATE_POLICY(tme_activate);
+	if (tme_policy != TME_ACTIVATE_POLICY_AES_XTS_128)
+		pr_warn("x86/tme: Unknown policy is active: %#llx\n", tme_policy);
+
+	tme_crypto_algs = TME_ACTIVATE_CRYPTO_ALGS(tme_activate);
+	if (!(tme_crypto_algs & TME_ACTIVATE_CRYPTO_AES_XTS_128)) {
+		pr_err("x86/mktme: No known encryption algorithm is supported: %#llx\n",
+				tme_crypto_algs);
+		mktme_status = MKTME_DISABLED;
+	}
+detect_keyid_bits:
+	keyid_bits = TME_ACTIVATE_KEYID_BITS(tme_activate);
+	nr_keyids = (1UL << keyid_bits) - 1;
+	if (nr_keyids) {
+		pr_info_once("x86/mktme: enabled by BIOS\n");
+		pr_info_once("x86/mktme: %d KeyIDs available\n", nr_keyids);
+	} else {
+		pr_info_once("x86/mktme: disabled by BIOS\n");
+	}
+
+	if (mktme_status == MKTME_UNINITIALIZED) {
+		/* MKTME is usable */
+		mktme_status = MKTME_ENABLED;
+	}
+
+	/*
+	 * KeyID bits effectively lower the number of physical address
+	 * bits.  Update cpuinfo_x86::x86_phys_bits accordingly.
+	 */
+	c->x86_phys_bits -= keyid_bits;
+}
+
 static void early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
@@ -322,6 +406,13 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 	 */
 	if (detect_extended_topology_early(c) < 0)
 		detect_ht_early(c);
+
+	/*
+	 * Adjust the number of physical bits early because it affects the
+	 * valid bits of the MTRR mask registers.
+	 */
+	if (cpu_has(c, X86_FEATURE_TME))
+		detect_tme_early(c);
 }
 
 static void bsp_init_intel(struct cpuinfo_x86 *c)
@@ -482,90 +573,6 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
 #endif
 }
 
-#define MSR_IA32_TME_ACTIVATE		0x982
-
-/* Helpers to access TME_ACTIVATE MSR */
-#define TME_ACTIVATE_LOCKED(x)		(x & 0x1)
-#define TME_ACTIVATE_ENABLED(x)		(x & 0x2)
-
-#define TME_ACTIVATE_POLICY(x)		((x >> 4) & 0xf)	/* Bits 7:4 */
-#define TME_ACTIVATE_POLICY_AES_XTS_128	0
-
-#define TME_ACTIVATE_KEYID_BITS(x)	((x >> 32) & 0xf)	/* Bits 35:32 */
-
-#define TME_ACTIVATE_CRYPTO_ALGS(x)	((x >> 48) & 0xffff)	/* Bits 63:48 */
-#define TME_ACTIVATE_CRYPTO_AES_XTS_128	1
-
-/* Values for mktme_status (SW only construct) */
-#define MKTME_ENABLED			0
-#define MKTME_DISABLED			1
-#define MKTME_UNINITIALIZED		2
-static int mktme_status = MKTME_UNINITIALIZED;
-
-static void detect_tme(struct cpuinfo_x86 *c)
-{
-	u64 tme_activate, tme_policy, tme_crypto_algs;
-	int keyid_bits = 0, nr_keyids = 0;
-	static u64 tme_activate_cpu0 = 0;
-
-	rdmsrl(MSR_IA32_TME_ACTIVATE, tme_activate);
-
-	if (mktme_status != MKTME_UNINITIALIZED) {
-		if (tme_activate != tme_activate_cpu0) {
-			/* Broken BIOS? */
-			pr_err_once("x86/tme: configuration is inconsistent between CPUs\n");
-			pr_err_once("x86/tme: MKTME is not usable\n");
-			mktme_status = MKTME_DISABLED;
-
-			/* Proceed. We may need to exclude bits from x86_phys_bits. */
-		}
-	} else {
-		tme_activate_cpu0 = tme_activate;
-	}
-
-	if (!TME_ACTIVATE_LOCKED(tme_activate) || !TME_ACTIVATE_ENABLED(tme_activate)) {
-		pr_info_once("x86/tme: not enabled by BIOS\n");
-		mktme_status = MKTME_DISABLED;
-		return;
-	}
-
-	if (mktme_status != MKTME_UNINITIALIZED)
-		goto detect_keyid_bits;
-
-	pr_info("x86/tme: enabled by BIOS\n");
-
-	tme_policy = TME_ACTIVATE_POLICY(tme_activate);
-	if (tme_policy != TME_ACTIVATE_POLICY_AES_XTS_128)
-		pr_warn("x86/tme: Unknown policy is active: %#llx\n", tme_policy);
-
-	tme_crypto_algs = TME_ACTIVATE_CRYPTO_ALGS(tme_activate);
-	if (!(tme_crypto_algs & TME_ACTIVATE_CRYPTO_AES_XTS_128)) {
-		pr_err("x86/mktme: No known encryption algorithm is supported: %#llx\n",
-				tme_crypto_algs);
-		mktme_status = MKTME_DISABLED;
-	}
-detect_keyid_bits:
-	keyid_bits = TME_ACTIVATE_KEYID_BITS(tme_activate);
-	nr_keyids = (1UL << keyid_bits) - 1;
-	if (nr_keyids) {
-		pr_info_once("x86/mktme: enabled by BIOS\n");
-		pr_info_once("x86/mktme: %d KeyIDs available\n", nr_keyids);
-	} else {
-		pr_info_once("x86/mktme: disabled by BIOS\n");
-	}
-
-	if (mktme_status == MKTME_UNINITIALIZED) {
-		/* MKTME is usable */
-		mktme_status = MKTME_ENABLED;
-	}
-
-	/*
-	 * KeyID bits effectively lower the number of physical address
-	 * bits.  Update cpuinfo_x86::x86_phys_bits accordingly.
-	 */
-	c->x86_phys_bits -= keyid_bits;
-}
-
 static void init_cpuid_fault(struct cpuinfo_x86 *c)
 {
 	u64 msr;
@@ -702,9 +709,6 @@ static void init_intel(struct cpuinfo_x86 *c)
 
 	init_ia32_feat_ctl(c);
 
-	if (cpu_has(c, X86_FEATURE_TME))
-		detect_tme(c);
-
 	init_intel_misc_features(c);
 
 	split_lock_init();
-- 
2.43.0

Re: [PATCH v2 1/2] x86/cpu: allow reducing x86_phys_bits during early_identify_cpu()

[Kirill A . Shutemov]

On Thu, Feb 01, 2024 at 12:09:01AM +0100, Paolo Bonzini wrote:
> In commit fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct
> value straight away, instead of a two-phase approach"), the initialization
> of c->x86_phys_bits was moved after this_cpu->c_early_init(c).  This is
> incorrect because early_init_amd() expected to be able to reduce the
> value according to the contents of CPUID leaf 0x8000001f.
> 
> Fortunately, the bug was negated by init_amd()'s call to early_init_amd(),
> which does reduce x86_phys_bits in the end.  However, this is very
> late in the boot process and, most notably, the wrong value is used for
> x86_phys_bits when setting up MTRRs.
> 
> To fix this, call get_cpu_address_sizes() as soon as X86_FEATURE_CPUID is
> set/cleared, and c->extended_cpuid_level is retrieved.
> 
> Fixes: fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct value straight away, instead of a two-phase approach")
> Cc: Adam Dunlap <acdunlap@google.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: x86@kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH v2 2/2] x86/cpu/intel: Detect TME keyid bits before setting MTRR mask registers

[Kirill A . Shutemov]

On Thu, Feb 01, 2024 at 12:09:02AM +0100, Paolo Bonzini wrote:
> MKTME repurposes the high bit of physical address to key id for encryption
> key and, even though MAXPHYADDR in CPUID[0x80000008] remains the same,
> the valid bits in the MTRR mask register are based on the reduced number
> of physical address bits.
> 
> detect_tme() in arch/x86/kernel/cpu/intel.c detects TME and subtracts
> it from the total usable physical bits, but it is called too late.
> Move the call to early_init_intel() so that it is called in setup_arch(),
> before MTRRs are setup.
> 
> This fixes boot on TDX-enabled systems, which until now only worked with
> "disable_mtrr_cleanup".  Without the patch, the values written to the
> MTRRs mask registers were 52-bit wide (e.g. 0x000fffff_80000800) and
> the writes failed; with the patch, the values are 46-bit wide, which
> matches the reduced MAXPHYADDR that is shown in /proc/cpuinfo.
> 
> Reported-by: Zixi Chen <zixchen@redhat.com>
> Cc: Adam Dunlap <acdunlap@google.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: x86@kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH v2 2/2] x86/cpu/intel: Detect TME keyid bits before setting MTRR mask registers

[Huang, Kai]

>   static void early_init_intel(struct cpuinfo_x86 *c)
>   {
>   	u64 misc_enable;
> @@ -322,6 +406,13 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>   	 */
>   	if (detect_extended_topology_early(c) < 0)
>   		detect_ht_early(c);
> +
> +	/*
> +	 * Adjust the number of physical bits early because it affects the
> +	 * valid bits of the MTRR mask registers.
> +	 */
> +	if (cpu_has(c, X86_FEATURE_TME))
> +		detect_tme_early(c);
>   }
>   

early_init_intel() is also called by init_intel(), so IIUC for BSP 
detect_tme_early() will be called twice.

But this is no harm AFAICT, so:

Acked-by: Kai Huang <kai.huang@intel.com>

Re: [PATCH v2 1/2] x86/cpu: allow reducing x86_phys_bits during early_identify_cpu()

[Huang, Kai]

On 1/02/2024 7:09 am, Paolo Bonzini wrote:
> In commit fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct
> value straight away, instead of a two-phase approach"), the initialization
> of c->x86_phys_bits was moved after this_cpu->c_early_init(c).  This is
> incorrect because early_init_amd() expected to be able to reduce the
> value according to the contents of CPUID leaf 0x8000001f.
> 
> Fortunately, the bug was negated by init_amd()'s call to early_init_amd(),
> which does reduce x86_phys_bits in the end.  However, this is very
> late in the boot process and, most notably, the wrong value is used for
> x86_phys_bits when setting up MTRRs.
> 
> To fix this, call get_cpu_address_sizes() as soon as X86_FEATURE_CPUID is
> set/cleared, and c->extended_cpuid_level is retrieved.
> 
> Fixes: fbf6449f84bf ("x86/sev-es: Set x86_virt_bits to the correct value straight away, instead of a two-phase approach")
> Cc: Adam Dunlap <acdunlap@google.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Kai Huang <kai.huang@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: x86@kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Acked-by: Kai Huang <kai.huang@intel.com>

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

On 1/31/24 15:09, Paolo Bonzini wrote:
> However, as noticed by Kirill, the patch I sent as v1 actually works only
> until Linux 6.6.  In Linux 6.7, commit fbf6449f84bf ("x86/sev-es: Set
> x86_virt_bits to the correct value straight away, instead of a two-phase
> approach") reorganized the initialization of c->x86_phys_bits in a way
> that broke the patch.  But even in 6.7 AMD processors, which did try to
> reduce it in this_cpu->c_early_init(c), had their x86_phys_bits value
> overwritten by get_cpu_address_sizes(), so that early_identify_cpu()
> left the wrong value in x86_phys_bits.  This probably went unnoticed
> because on AMD processors you need not apply the reduced MAXPHYADDR to
> MTRR masks.

I really wanted get_cpu_address_sizes() to be the one and only spot
where c->x86_phys_bits is established.  That way, we don't get a bunch
of code all of the place tweaking it and fighting for who "wins".

We're not there yet, but the approach in this patch moves it back in the
wrong direction because it permits the random tweaking of c->x86_phys_bits.

Could we instead do something more like the (completely untested)
attached patch?

BTW, I'm pretty sure the WARN_ON() in the patch won't actually work, but
it'd be nice to work toward a point when it does.

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

On 2/1/24 10:29, Dave Hansen wrote:
> Could we instead do something more like the (completely untested)
> attached patch?

... actually attaching it here

[-- Attachment #2: enc_phys_bits.patch --]
[-- Type: text/x-patch, Size: 2069 bytes --]

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 26620d7642a9..415ae443ef79 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -119,6 +119,7 @@ struct cpuinfo_x86 {
 #endif
 	__u8			x86_virt_bits;
 	__u8			x86_phys_bits;
+	__u8			enc_phys_bits;
 	/* CPUID returned core id bits: */
 	__u8			x86_coreid_bits;
 	/* Max extended CPUID function supported: */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index f3abca334199..40fc44dfa7a4 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -618,11 +618,13 @@ static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
 			goto clear_all;
 
 		/*
-		 * Always adjust physical address bits. Even though this
-		 * will be a value above 32-bits this is still done for
-		 * CONFIG_X86_32 so that accurate values are reported.
+		 * Record the number of physical address bits that
+		 * have been repurposed for memory encryption.  Do
+		 * this even on CONFIG_X86_32 configs that do can
+		 * not support memory encryption so it is still
+		 * reported accurately.
 		 */
-		c->x86_phys_bits -= (cpuid_ebx(0x8000001f) >> 6) & 0x3f;
+		c->enc_phys_bits = (cpuid_ebx(0x8000001f) >> 6) & 0x3f;
 
 		if (IS_ENABLED(CONFIG_X86_32))
 			goto clear_all;
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0b97bcde70c6..b998ae7fbbfb 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1098,6 +1098,10 @@ void get_cpu_address_sizes(struct cpuinfo_x86 *c)
 	u32 eax, ebx, ecx, edx;
 	bool vp_bits_from_cpuid = true;
 
+	WARN_ON(c->x86_clflush_size ||
+		c->x86_phys_bits    ||
+		c->x86_virt_bits);
+
 	if (!cpu_has(c, X86_FEATURE_CPUID) ||
 	    (c->extended_cpuid_level < 0x80000008))
 		vp_bits_from_cpuid = false;
@@ -1122,6 +1126,8 @@ void get_cpu_address_sizes(struct cpuinfo_x86 *c)
 				c->x86_phys_bits = 36;
 		}
 	}
+	c->x86_phys_bits -= c->enc_phys_bits;
+
 	c->x86_cache_bits = c->x86_phys_bits;
 	c->x86_cache_alignment = c->x86_clflush_size;
 }

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

On Thu, Feb 1, 2024 at 7:29 PM Dave Hansen <dave.hansen@intel.com> wrote:
> I really wanted get_cpu_address_sizes() to be the one and only spot
> where c->x86_phys_bits is established.  That way, we don't get a bunch
> of code all of the place tweaking it and fighting for who "wins".
> We're not there yet, but the approach in this patch moves it back in the
> wrong direction because it permits the random tweaking of c->x86_phys_bits.

I see your point; one of my earlier attempts added a
->c_detect_mem_encrypt() callback that basically amounted to either
amd_detect_mem_encrypt() or detect_tme(). I bailed out of it mostly
because these functions do more than adjusting phys_bits, and it
seemed arbitrary to call them from get_cpu_address_sizes(). The two
approaches share the idea of centralizing the computation of
x86_phys_bits in get_cpu_address_sizes().

There is unfortunately an important hurdle for your patch, in that
currently the BSP and AP flows are completely different. For the BSP
the flow is ->c_early_init(), then get_cpu_address_sizes(), then again
->c_early_init() called by ->c_init(), then ->c_init(). For APs it is
get_cpu_address_sizes(), then ->c_early_init() called by ->c_init(),
then the rest of ->c_init(). And let's not even look at
->c_identify().

This difference is bad for your patch, because get_cpu_address_sizes()
is called too early to see enc_phys_bits on APs. But it was also
something that fbf6449f84bf didn't take into account, because it left
behind the tentative initialization of x86_*_bits in identify_cpu(),
while removing it from early_identify_cpu().  And

TBH my first reaction after Kirill pointed me to fbf6449f84bf was to
revert it. It's not like the code before was much less of a dumpster
fire, but that commit made the BSP vs. AP mess even worse. But it
fixed a real-world bug and it did remove most of the "first set then
adjust" logic, at least for the BSP, so a revert wasn't on the table
and patch 1 was what came out of it.

I know that in general in Linux we prefer to fix things for good.
Dancing one step behind and two ahead comes with the the risk that you
only do the step behind. But in the end something like this patch 1
would have to be posted for stable branches (and Greg doesn't like
one-off patches), and I am not even sure it's a step behind because it
removes _some_ of the BSP vs. AP differences introduced by
fbf6449f84bf.

In a nutshell: I don't dislike the idea behind your patch, but the
code is just not ready for it.

I can look into cleaning up cpu/common.c though. I'm a natural
procrastinator, it's my kind of thing to embark on a series of 30
patches to clean up 20 years old code...

Paolo

> Could we instead do something more like the (completely untested)
> attached patch?
>
> BTW, I'm pretty sure the WARN_ON() in the patch won't actually work, but
> it'd be nice to work toward a point when it does.
>

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

On Fri, Feb 2, 2024 at 12:08 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Thu, Feb 1, 2024 at 7:29 PM Dave Hansen <dave.hansen@intel.com> wrote:
> > I really wanted get_cpu_address_sizes() to be the one and only spot
> > where c->x86_phys_bits is established.  That way, we don't get a bunch
> > of code all of the place tweaking it and fighting for who "wins".
> > We're not there yet, but the approach in this patch moves it back in the
> > wrong direction because it permits the random tweaking of c->x86_phys_bits.
>
> I see your point; one of my earlier attempts added a
> ->c_detect_mem_encrypt() callback that basically amounted to either
> amd_detect_mem_encrypt() or detect_tme(). I bailed out of it mostly
> because these functions do more than adjusting phys_bits, and it
> seemed arbitrary to call them from get_cpu_address_sizes(). The two
> approaches share the idea of centralizing the computation of
> x86_phys_bits in get_cpu_address_sizes().
>
> There is unfortunately an important hurdle for your patch, in that
> currently the BSP and AP flows are completely different. For the BSP
> the flow is ->c_early_init(), then get_cpu_address_sizes(), then again
> ->c_early_init() called by ->c_init(), then ->c_init(). For APs it is
> get_cpu_address_sizes(), then ->c_early_init() called by ->c_init(),
> then the rest of ->c_init(). And let's not even look at
> ->c_identify().
>
> This difference is bad for your patch, because get_cpu_address_sizes()
> is called too early to see enc_phys_bits on APs. But it was also
> something that fbf6449f84bf didn't take into account, because it left
> behind the tentative initialization of x86_*_bits in identify_cpu(),
> while removing it from early_identify_cpu().  And
>
> TBH my first reaction after Kirill pointed me to fbf6449f84bf was to
> revert it. It's not like the code before was much less of a dumpster
> fire, but that commit made the BSP vs. AP mess even worse. But it
> fixed a real-world bug and it did remove most of the "first set then
> adjust" logic, at least for the BSP, so a revert wasn't on the table
> and patch 1 was what came out of it.
>
> I know that in general in Linux we prefer to fix things for good.
> Dancing one step behind and two ahead comes with the the risk that you
> only do the step behind. But in the end something like this patch 1
> would have to be posted for stable branches (and Greg doesn't like
> one-off patches), and I am not even sure it's a step behind because it
> removes _some_ of the BSP vs. AP differences introduced by
> fbf6449f84bf.
>
> In a nutshell: I don't dislike the idea behind your patch, but the
> code is just not ready for it.

This is the diffstat before your patch can be applied more or less as is:

$ git diffstat origin/master
 arch/x86/include/asm/processor.h |   1 +
 arch/x86/kernel/cpu/amd.c        |  12 ++--
 arch/x86/kernel/cpu/centaur.c    |  13 ++---
 arch/x86/kernel/cpu/common.c     | 103 +++++++++++++++++++----------------
 arch/x86/kernel/cpu/hygon.c      |   2 -
 arch/x86/kernel/cpu/intel.c      |   4 +-
 arch/x86/kernel/cpu/transmeta.c  |   2 -
 arch/x86/kernel/cpu/zhaoxin.c    |   1 -
 8 files changed, 69 insertions(+), 69 deletions(-)

$ git log --oneline --reverse origin/master..
d639afed02aa x86/cpu/common: move code up to early
get_cpu_address_sizes() to a new function
40d34260a4ba x86/cpu/common: pull get_cpu_*() calls common to BSPs and
APs to a new function
67b9839f9c38 x86/cpu/common: put all setup_force/clear capabilities together
ebeae7f91cbc x86/cpu/centaur: do everything before
early_init_centaur() in early_init_centaur()
d62fa7400885 x86/cpu/cyrix: call early init function from init function
0aa0916cd7e0 x86/cpu/common: call early_init function on APs from common code
d656b651d217 (HEAD) dave


I still haven't tested very well, but anyway, what do you think?

Paolo

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

On Sun, Feb 4, 2024 at 6:21 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> On Fri, Feb 2, 2024 at 12:08 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > On Thu, Feb 1, 2024 at 7:29 PM Dave Hansen <dave.hansen@intel.com> wrote:
> > > I really wanted get_cpu_address_sizes() to be the one and only spot
> > > where c->x86_phys_bits is established.  That way, we don't get a bunch
> > > of code all of the place tweaking it and fighting for who "wins".
> > > We're not there yet, but the approach in this patch moves it back in the
> > > wrong direction because it permits the random tweaking of c->x86_phys_bits.
> >
> > There is unfortunately an important hurdle [...] in that
> > currently the BSP and AP flows are completely different. For the BSP
> > the flow is ->c_early_init(), then get_cpu_address_sizes(), then again
> > ->c_early_init() called by ->c_init(), then ->c_init(). For APs it is
> > get_cpu_address_sizes(), then ->c_early_init() called by ->c_init(),
> > then the rest of ->c_init(). And let's not even look at
> > ->c_identify(). [...] get_cpu_address_sizes()
> > is called too early to see enc_phys_bits on APs. But it was also
> > something that fbf6449f84bf didn't take into account, because it left
> > behind the tentative initialization of x86_*_bits in identify_cpu(),
> > while removing it from early_identify_cpu().  And

Ping, either for applying the original patches or for guidance on how
to proceed.

Paolo

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

On 2/1/24 15:08, Paolo Bonzini wrote:
> There is unfortunately an important hurdle for your patch, in that
> currently the BSP and AP flows are completely different.

Do we even _need_ c->x86_phys_bits for APs?  I need to do a bit more
grepping, but I only see it being read in show_cpuinfo().  Everything
else seems to be boot_cpu_data.x86_phys_bits.

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On 2/13/24 07:45, Paolo Bonzini wrote:
> Ping, either for applying the original patches or for guidance on how
> to proceed.

Gah, all of the gunk that get_cpu_address_sizes() touches are out of
control.

They (phys/virt_bits and clflush) need to get consolidated back to a
single copy that gets set up *once* in early boot and then read by
everyone else.  I've got a series to do that, but it's got its tentacles
in quite a few places.  They're not great backporting material.

Your patches make things a wee bit worse in the meantime, but they pale
in comparison to the random spaghetti that we've already got.  Also, we
probably need the early TME stuff regardless.

I think I'll probably suck it up, apply them, then fix them up along
with the greater mess.

Anybody have any better ideas?

[-- Attachment #2: x86cai.patch --]
[-- Type: text/x-patch, Size: 20362 bytes --]

diff --git a/arch/x86/events/amd/brs.c b/arch/x86/events/amd/brs.c
index 780acd3dff22..80252bbaf0d8 100644
--- a/arch/x86/events/amd/brs.c
+++ b/arch/x86/events/amd/brs.c
@@ -285,7 +285,7 @@ void amd_brs_drain(void)
 	struct perf_branch_entry *br = cpuc->lbr_entries;
 	union amd_debug_extn_cfg cfg;
 	u32 i, nr = 0, num, tos, start;
-	u32 shift = 64 - boot_cpu_data.x86_virt_bits;
+	u32 shift = 64 - x86_virt_bits();
 
 	/*
 	 * BRS event forced on PMC0,
diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
index eb31f850841a..bf89d47607e7 100644
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -89,7 +89,7 @@ static __always_inline u64 amd_pmu_lbr_get_to(unsigned int idx)
 
 static __always_inline u64 sign_ext_branch_ip(u64 ip)
 {
-	u32 shift = 64 - boot_cpu_data.x86_virt_bits;
+	u32 shift = 64 - x86_virt_bits();
 
 	return (u64)(((s64)ip << shift) >> shift);
 }
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 8e2a12235e62..ff04bfee1ec2 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1453,8 +1453,8 @@ static void pt_event_addr_filters_sync(struct perf_event *event)
 			 * canonical addresses does not affect the result of the
 			 * address filter.
 			 */
-			msr_a = clamp_to_ge_canonical_addr(a, boot_cpu_data.x86_virt_bits);
-			msr_b = clamp_to_le_canonical_addr(b, boot_cpu_data.x86_virt_bits);
+			msr_a = clamp_to_ge_canonical_addr(a, x86_virt_bits());
+			msr_b = clamp_to_le_canonical_addr(b, x86_virt_bits());
 			if (msr_b < msr_a)
 				msr_a = msr_b = 0;
 		}
diff --git a/arch/x86/include/asm/kmsan.h b/arch/x86/include/asm/kmsan.h
index 8fa6ac0e2d76..608a3311dde6 100644
--- a/arch/x86/include/asm/kmsan.h
+++ b/arch/x86/include/asm/kmsan.h
@@ -52,7 +52,7 @@ static inline void *arch_kmsan_get_meta_or_null(void *addr, bool is_origin)
 static inline bool kmsan_phys_addr_valid(unsigned long addr)
 {
 	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
-		return !(addr >> boot_cpu_data.x86_phys_bits);
+		return !(addr >> x86_phys_bits());
 	else
 		return true;
 }
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index de3118305838..ac80c32bccf1 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -89,7 +89,7 @@
 #define  MCI_MISC_ADDR_GENERIC	7	/* generic */
 
 /* MCi_ADDR register defines */
-#define MCI_ADDR_PHYSADDR	GENMASK_ULL(boot_cpu_data.x86_phys_bits - 1, 0)
+#define MCI_ADDR_PHYSADDR	GENMASK_ULL(x86_phys_bits() - 1, 0)
 
 /* CTL2 register defines */
 #define MCI_CTL2_CMCI_EN		BIT_ULL(30)
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 26620d7642a9..54673015a602 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -767,4 +767,21 @@ static inline void weak_wrmsr_fence(void)
 	alternative("mfence; lfence", "", ALT_NOT(X86_FEATURE_APIC_MSRS_FENCE));
 }
 
+static inline u8 x86_phys_bits(void)
+{
+	WARN_ON_ONCE(!boot_cpu_data.x86_phys_bits);
+	return boot_cpu_data.x86_phys_bits;
+}
+
+static inline u8 x86_virt_bits(void)
+{
+	WARN_ON_ONCE(!boot_cpu_data.x86_virt_bits);
+	return boot_cpu_data.x86_virt_bits;
+}
+
+static inline u8 x86_clflush_size(void)
+{
+	WARN_ON_ONCE(!boot_cpu_data.x86_clflush_size);
+	return boot_cpu_data.x86_clflush_size;
+}
 #endif /* _ASM_X86_PROCESSOR_H */
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index 345f7d905db6..1fb061ea15a2 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -62,7 +62,7 @@ static void init_c3(struct cpuinfo_x86 *c)
 		set_cpu_cap(c, X86_FEATURE_3DNOW);
 #endif
 	if (c->x86 == 0x6 && c->x86_model >= 0xf) {
-		c->x86_cache_alignment = c->x86_clflush_size * 2;
+		c->x86_cache_alignment = x86_clflush_size() * 2;
 		set_cpu_cap(c, X86_FEATURE_REP_GOOD);
 	}
 
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0b97bcde70c6..fd5d746d7d16 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -954,8 +954,9 @@ void cpu_detect(struct cpuinfo_x86 *c)
 		c->x86_stepping	= x86_stepping(tfms);
 
 		if (cap0 & (1<<19)) {
-			c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
-			c->x86_cache_alignment = c->x86_clflush_size;
+			if (c == &boot_cpu_data)
+				c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
+			c->x86_cache_alignment = x86_clflush_size();
 		}
 	}
 }
@@ -1123,7 +1124,7 @@ void get_cpu_address_sizes(struct cpuinfo_x86 *c)
 		}
 	}
 	c->x86_cache_bits = c->x86_phys_bits;
-	c->x86_cache_alignment = c->x86_clflush_size;
+	c->x86_cache_alignment = x86_clflush_size();
 }
 
 static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c)
@@ -1822,16 +1823,19 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	c->topo.llc_id = BAD_APICID;
 	c->topo.l2c_id = BAD_APICID;
 #ifdef CONFIG_X86_64
-	c->x86_clflush_size = 64;
-	c->x86_phys_bits = 36;
-	c->x86_virt_bits = 48;
+	if (WARN_ON_ONCE(!c->x86_clflush_size))
+		c->x86_clflush_size = 64;
+	if (WARN_ON_ONCE(!c->x86_phys_bits))
+		c->x86_phys_bits = 36;
+	if (WARN_ON_ONCE(!c->x86_virt_bits))
+		c->x86_virt_bits = 48;
 #else
 	c->cpuid_level = -1;	/* CPUID not detected */
 	c->x86_clflush_size = 32;
 	c->x86_phys_bits = 32;
 	c->x86_virt_bits = 32;
 #endif
-	c->x86_cache_alignment = c->x86_clflush_size;
+	c->x86_cache_alignment = x86_clflush_size();
 	memset(&c->x86_capability, 0, sizeof(c->x86_capability));
 #ifdef CONFIG_X86_VMX_FEATURE_NAMES
 	memset(&c->vmx_capability, 0, sizeof(c->vmx_capability));
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index a927a8fc9624..e97626a8e993 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -653,7 +653,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 
 #ifdef CONFIG_X86_64
 	if (c->x86 == 15)
-		c->x86_cache_alignment = c->x86_clflush_size * 2;
+		c->x86_cache_alignment = x86_clflush_size() * 2;
 	if (c->x86 == 6)
 		set_cpu_cap(c, X86_FEATURE_REP_GOOD);
 #else
diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
index 18cf79d6e2c5..3d91219ae13f 100644
--- a/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -170,7 +170,7 @@ set_var_mtrr(unsigned int reg, unsigned long basek, unsigned long sizek,
 		return;
 	}
 
-	mask = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
+	mask = (1ULL << x86_phys_bits()) - 1;
 	mask &= ~((((u64)sizek) << 10) - 1);
 
 	base = ((u64)basek) << 10;
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index d3524778a545..bf29b4170df8 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -660,7 +660,7 @@ static void __init print_mtrr_state(void)
 	}
 	pr_info("MTRR variable ranges %sabled:\n",
 		mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED ? "en" : "dis");
-	high_width = (boot_cpu_data.x86_phys_bits - (32 - PAGE_SHIFT) + 3) / 4;
+	high_width = (x86_phys_bits() - (32 - PAGE_SHIFT) + 3) / 4;
 
 	for (i = 0; i < num_var_ranges; ++i) {
 		if (mtrr_state.var_ranges[i].mask_lo & MTRR_PHYSMASK_V)
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
index 767bf1c71aad..50b1a6c318e6 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.c
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
@@ -253,7 +253,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
 	}
 
 	if ((base | (base + size - 1)) >>
-	    (boot_cpu_data.x86_phys_bits - PAGE_SHIFT)) {
+	    (x86_phys_bits() - PAGE_SHIFT)) {
 		pr_warn("base or size exceeds the MTRR width\n");
 		return -EINVAL;
 	}
@@ -556,7 +556,7 @@ void __init mtrr_bp_init(void)
 	const char *why = "(not available)";
 	unsigned long config, dummy;
 
-	phys_hi_rsvd = GENMASK(31, boot_cpu_data.x86_phys_bits - 32);
+	phys_hi_rsvd = GENMASK(31, x86_phys_bits() - 32);
 
 	if (!generic_mtrrs && mtrr_state.enabled) {
 		/*
diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index e65fae63660e..259bb5e74532 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -130,10 +130,10 @@ static int show_cpuinfo(struct seq_file *m, void *v)
 	if (c->x86_tlbsize > 0)
 		seq_printf(m, "TLB size\t: %d 4K pages\n", c->x86_tlbsize);
 #endif
-	seq_printf(m, "clflush size\t: %u\n", c->x86_clflush_size);
+	seq_printf(m, "clflush size\t: %u\n", x86_clflush_size());
 	seq_printf(m, "cache_alignment\t: %d\n", c->x86_cache_alignment);
 	seq_printf(m, "address sizes\t: %u bits physical, %u bits virtual\n",
-		   c->x86_phys_bits, c->x86_virt_bits);
+		   x86_phys_bits(), x86_virt_bits());
 
 	seq_puts(m, "power management:");
 	for (i = 0; i < 32; i++) {
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 84201071dfac..6206e0fcf3d3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -813,7 +813,14 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	early_reserve_memory();
 
-	iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
+	/*
+	 * This was too big before.  It ended up getting MAX_PHYSMEM_BITS
+	 * even if .x86_phys_bits was eventually lowered below that.
+	 * But that was evidently harmless, so leave it too big, but
+	 * set it explicitly to MAX_PHYSMEM_BITS instead of taking a
+	 * trip through .x86_phys_bits.
+	 */
+	iomem_resource.end = (1ULL << MAX_PHYSMEM_BITS) - 1;
 	e820__memory_setup();
 	parse_setup_data();
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index adba49afb5fe..5426284fcd53 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1236,7 +1236,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		 * the HPAs do not affect GPAs.
 		 */
 		if (!tdp_enabled)
-			g_phys_as = boot_cpu_data.x86_phys_bits;
+			g_phys_as = x86_phys_bits();
 		else if (!g_phys_as)
 			g_phys_as = phys_as;
 
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 60f21bb4c27b..8437f1ee7ae9 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -84,10 +84,10 @@ static inline gfn_t kvm_mmu_max_gfn(void)
 static inline u8 kvm_get_shadow_phys_bits(void)
 {
 	/*
-	 * boot_cpu_data.x86_phys_bits is reduced when MKTME or SME are detected
-	 * in CPU detection code, but the processor treats those reduced bits as
-	 * 'keyID' thus they are not reserved bits. Therefore KVM needs to look at
-	 * the physical address bits reported by CPUID.
+	 * x86_phys_bits() is reduced when MKTME or SME are detected in CPU
+	 * detection code, but the processor treats those reduced bits as 'keyID'
+	 * thus they are not reserved bits. Therefore KVM needs to look at the
+	 * physical address bits reported by CPUID.
 	 */
 	if (likely(boot_cpu_data.extended_cpuid_level >= 0x80000008))
 		return cpuid_eax(0x80000008) & 0xff;
@@ -97,7 +97,7 @@ static inline u8 kvm_get_shadow_phys_bits(void)
 	 * custom CPUID.  Proceed with whatever the kernel found since these features
 	 * aren't virtualizable (SME/SEV also require CPUIDs higher than 0x80000008).
 	 */
-	return boot_cpu_data.x86_phys_bits;
+	return x86_phys_bits();
 }
 
 void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask);
diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
index 4a599130e9c9..d04e041a54a7 100644
--- a/arch/x86/kvm/mmu/spte.c
+++ b/arch/x86/kvm/mmu/spte.c
@@ -468,7 +468,7 @@ void kvm_mmu_reset_all_pte_masks(void)
 	 * the most significant bits of legal physical address space.
 	 */
 	shadow_nonpresent_or_rsvd_mask = 0;
-	low_phys_bits = boot_cpu_data.x86_phys_bits;
+	low_phys_bits = x86_phys_bits();
 	if (boot_cpu_has_bug(X86_BUG_L1TF) &&
 	    !WARN_ON_ONCE(boot_cpu_data.x86_cache_bits >=
 			  52 - SHADOW_NONPRESENT_OR_RSVD_MASK_LEN)) {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e90b429c84f1..7605560b57ac 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5054,7 +5054,7 @@ static __init void svm_adjust_mmio_mask(void)
 		return;
 
 	enc_bit = cpuid_ebx(0x8000001f) & 0x3f;
-	mask_bit = boot_cpu_data.x86_phys_bits;
+	mask_bit = x86_phys_bits();
 
 	/* Increment the mask bit if it is the same as the encryption bit */
 	if (enc_bit == mask_bit)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e262bc2ba4e5..f16023979ad1 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -8444,14 +8444,14 @@ static void __init vmx_setup_me_spte_mask(void)
 	 * kvm_get_shadow_phys_bits() returns shadow_phys_bits.  Use
 	 * the former to avoid exposing shadow_phys_bits.
 	 *
-	 * On pre-MKTME system, boot_cpu_data.x86_phys_bits equals to
+	 * On pre-MKTME system, x86_phys_bits() equals to
 	 * shadow_phys_bits.  On MKTME and/or TDX capable systems,
-	 * boot_cpu_data.x86_phys_bits holds the actual physical address
-	 * w/o the KeyID bits, and shadow_phys_bits equals to MAXPHYADDR
+	 * x86_phys_bits() holds the actual physical address w/o the
+	 * KeyID bits, and shadow_phys_bits equals to MAXPHYADDR
 	 * reported by CPUID.  Those bits between are KeyID bits.
 	 */
-	if (boot_cpu_data.x86_phys_bits != kvm_get_shadow_phys_bits())
-		me_mask = rsvd_bits(boot_cpu_data.x86_phys_bits,
+	if (x86_phys_bits() != kvm_get_shadow_phys_bits())
+		me_mask = rsvd_bits(x86_phys_bits(),
 			kvm_get_shadow_phys_bits() - 1);
 	/*
 	 * Unlike SME, host kernel doesn't support setting up any
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index e3b0985bb74a..405579ff89c5 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -721,7 +721,7 @@ static inline bool vmx_need_pf_intercept(struct kvm_vcpu *vcpu)
 	if (!enable_ept)
 		return true;
 
-	return allow_smaller_maxphyaddr && cpuid_maxphyaddr(vcpu) < boot_cpu_data.x86_phys_bits;
+	return allow_smaller_maxphyaddr && cpuid_maxphyaddr(vcpu) < x86_phys_bits();
 }
 
 static inline bool is_unrestricted_guest(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index e9251b89a9e9..e8e6fdc94771 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -27,13 +27,12 @@
  */
 static void clean_cache_range(void *addr, size_t size)
 {
-	u16 x86_clflush_size = boot_cpu_data.x86_clflush_size;
-	unsigned long clflush_mask = x86_clflush_size - 1;
+	unsigned long clflush_mask = x86_clflush_size() - 1;
 	void *vend = addr + size;
 	void *p;
 
 	for (p = (void *)((unsigned long)addr & ~clflush_mask);
-	     p < vend; p += x86_clflush_size)
+	     p < vend; p += x86_clflush_size())
 		clwb(p);
 }
 
@@ -65,7 +64,7 @@ long __copy_user_flushcache(void *dst, const void __user *src, unsigned size)
 			clean_cache_range(dst, size);
 	} else {
 		if (!IS_ALIGNED(dest, 8)) {
-			dest = ALIGN(dest, boot_cpu_data.x86_clflush_size);
+			dest = ALIGN(dest, x86_clflush_size());
 			clean_cache_range(dst, 1);
 		}
 
diff --git a/arch/x86/mm/maccess.c b/arch/x86/mm/maccess.c
index 6993f026adec..63d9b110060e 100644
--- a/arch/x86/mm/maccess.c
+++ b/arch/x86/mm/maccess.c
@@ -19,11 +19,14 @@ bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
 	 * Allow everything during early boot before 'x86_virt_bits'
 	 * is initialized.  Needed for instruction decoding in early
 	 * exception handlers.
+	 *
+	 * CHECKME: Is this now done early enough that we can remove
+	 * this???
 	 */
-	if (!boot_cpu_data.x86_virt_bits)
+	if (!x86_virt_bits())
 		return true;
 
-	return __is_canonical_address(vaddr, boot_cpu_data.x86_virt_bits);
+	return __is_canonical_address(vaddr, x86_virt_bits());
 }
 #else
 bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index e9b448d1b1b7..8c3c63381a4f 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -314,7 +314,7 @@ static unsigned long __cpa_addr(struct cpa_data *cpa, unsigned long idx)
 
 static void clflush_cache_range_opt(void *vaddr, unsigned int size)
 {
-	const unsigned long clflush_size = boot_cpu_data.x86_clflush_size;
+	const unsigned long clflush_size = x86_clflush_size();
 	void *p = (void *)((unsigned long)vaddr & ~(clflush_size - 1));
 	void *vend = vaddr + size;
 
diff --git a/arch/x86/mm/physaddr.h b/arch/x86/mm/physaddr.h
index 9f6419cafc32..c2d3e1243ab6 100644
--- a/arch/x86/mm/physaddr.h
+++ b/arch/x86/mm/physaddr.h
@@ -4,7 +4,7 @@
 static inline int phys_addr_valid(resource_size_t addr)
 {
 #ifdef CONFIG_PHYS_ADDR_T_64BIT
-	return !(addr >> boot_cpu_data.x86_phys_bits);
+	return !(addr >> x86_phys_bits());
 #else
 	return 1;
 #endif
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index ddb798603201..66b210b07a92 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -480,22 +480,9 @@ void pcibios_scan_root(int busnum)
 
 void __init pcibios_set_cache_line_size(void)
 {
-	struct cpuinfo_x86 *c = &boot_cpu_data;
-
-	/*
-	 * Set PCI cacheline size to that of the CPU if the CPU has reported it.
-	 * (For older CPUs that don't support cpuid, we se it to 32 bytes
-	 * It's also good for 386/486s (which actually have 16)
-	 * as quite a few PCI devices do not support smaller values.
-	 */
-	if (c->x86_clflush_size > 0) {
-		pci_dfl_cache_line_size = c->x86_clflush_size >> 2;
-		printk(KERN_DEBUG "PCI: pci_cache_line_size set to %d bytes\n",
-			pci_dfl_cache_line_size << 2);
-	} else {
- 		pci_dfl_cache_line_size = 32 >> 2;
-		printk(KERN_DEBUG "PCI: Unknown cacheline size. Setting to 32 bytes\n");
-	}
+	pci_dfl_cache_line_size = boot_cpu_data.x86_clflush_size >> 2;
+	printk(KERN_DEBUG "PCI: pci_cache_line_size set to %d bytes\n",
+		pci_dfl_cache_line_size << 2);
 }
 
 int __init pcibios_init(void)
diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
index 271092f2700a..c6fc0e19eddd 100644
--- a/drivers/acpi/acpi_fpdt.c
+++ b/drivers/acpi/acpi_fpdt.c
@@ -151,7 +151,7 @@ static bool fpdt_address_valid(u64 address)
 	 * On some systems the table contains invalid addresses
 	 * with unsuppored high address bits set, check for this.
 	 */
-	return !(address >> boot_cpu_data.x86_phys_bits);
+	return !(address >> x86_phys_bits());
 }
 #else
 static bool fpdt_address_valid(u64 address)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 7051c9c909c2..1d193e87d901 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -52,7 +52,7 @@ drm_clflush_page(struct page *page)
 {
 	uint8_t *page_virtual;
 	unsigned int i;
-	const int size = boot_cpu_data.x86_clflush_size;
+	const int size = x86_clflush_size();
 
 	if (unlikely(page == NULL))
 		return;
@@ -160,7 +160,7 @@ drm_clflush_virt_range(void *addr, unsigned long length)
 {
 #if defined(CONFIG_X86)
 	if (static_cpu_has(X86_FEATURE_CLFLUSH)) {
-		const int size = boot_cpu_data.x86_clflush_size;
+		const int size = x86_clflush_size();
 		void *end = addr + length;
 
 		addr = (void *)(((unsigned long)addr) & -size);
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 2905df83e180..5c8385a69f37 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1203,8 +1203,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		 */
 		remain = length;
 		if (dst_needs_clflush & CLFLUSH_BEFORE)
-			remain = round_up(remain,
-					  boot_cpu_data.x86_clflush_size);
+			remain = round_up(remain, x86_clflush_size());
 
 		ptr = dst;
 		x = offset_in_page(offset);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 92758b6b41f0..4a7f753d81f3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -696,7 +696,7 @@ i915_gem_shmem_pwrite(struct drm_i915_gem_object *obj,
 	 */
 	partial_cacheline_write = 0;
 	if (needs_clflush & CLFLUSH_BEFORE)
-		partial_cacheline_write = boot_cpu_data.x86_clflush_size - 1;
+		partial_cacheline_write = x86_clflush_size() - 1;
 
 	user_data = u64_to_user_ptr(args->data_ptr);
 	remain = args->size;
diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index 074cb785eafc..a5b804cb9f7a 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -1229,7 +1229,7 @@ static void memcpy_flushcache_optimized(void *dest, void *source, size_t size)
 	 */
 #ifdef CONFIG_X86
 	if (static_cpu_has(X86_FEATURE_CLFLUSHOPT) &&
-	    likely(boot_cpu_data.x86_clflush_size == 64) &&
+	    likely(x86_clflush_size() == 64) &&
 	    likely(size >= 768)) {
 		do {
 			memcpy((void *)dest, (void *)source, 64);

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

On Tue, Feb 13, 2024 at 11:02 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 2/13/24 07:45, Paolo Bonzini wrote:
> > Ping, either for applying the original patches or for guidance on how
> > to proceed.
>
> Gah, all of the gunk that get_cpu_address_sizes() touches are out of
> control.
>
> They (phys/virt_bits and clflush) need to get consolidated back to a
> single copy that gets set up *once* in early boot and then read by
> everyone else.
>
> I've got a series to do that, but it's got its tentacles
> in quite a few places.  They're not great backporting material.

Yes, same for mine. I probably digressed in a different direction than
you, but there will be conflicts almost surely. I can post my stuff in
the next few days.

Paolo

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

On Tue, Feb 13, 2024 at 11:02 PM Dave Hansen <dave.hansen@intel.com> wrote:
> Your patches make things a wee bit worse in the meantime, but they pale
> in comparison to the random spaghetti that we've already got.  Also, we
> probably need the early TME stuff regardless.
>
> I think I'll probably suck it up, apply them, then fix them up along
> with the greater mess.
>
> Anybody have any better ideas?

Ping, in the end are we applying these patches for either 6.8 or 6.9?

Paolo

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

On 2/20/24 04:22, Paolo Bonzini wrote:
> On Tue, Feb 13, 2024 at 11:02 PM Dave Hansen <dave.hansen@intel.com> wrote:
>> Your patches make things a wee bit worse in the meantime, but they pale
>> in comparison to the random spaghetti that we've already got.  Also, we
>> probably need the early TME stuff regardless.
>>
>> I think I'll probably suck it up, apply them, then fix them up along
>> with the greater mess.
>>
>> Anybody have any better ideas?
> Ping, in the end are we applying these patches for either 6.8 or 6.9?

Let me poke at them and see if we can stick them in x86/urgent early
next week.  They do fix an actual bug that's biting people, right?

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Paolo Bonzini]

On Thu, Feb 22, 2024 at 7:07 PM Dave Hansen <dave.hansen@intel.com> wrote:
> > Ping, in the end are we applying these patches for either 6.8 or 6.9?
>
> Let me poke at them and see if we can stick them in x86/urgent early
> next week.  They do fix an actual bug that's biting people, right?

Yes, I have gotten reports of {Sapphire,Emerald} Rapids machines that
don't boot at all without either these patches or
"disable_mtrr_cleanup".

Paolo

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Yin Fengwei]

On 2/23/24 02:08, Paolo Bonzini wrote:
> On Thu, Feb 22, 2024 at 7:07 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>> Ping, in the end are we applying these patches for either 6.8 or 6.9?
>>
>> Let me poke at them and see if we can stick them in x86/urgent early
>> next week.  They do fix an actual bug that's biting people, right?
> 
> Yes, I have gotten reports of {Sapphire,Emerald} Rapids machines that
> don't boot at all without either these patches or
> "disable_mtrr_cleanup".
We tried platform other than Sapphire and Emerald. This patchset can fix
boot issues on that platform also.


Regards
Yin, Fengwei

> 
> Paolo
> 
> 

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

On 2/25/24 17:57, Yin Fengwei wrote:
> On 2/23/24 02:08, Paolo Bonzini wrote:
>> On Thu, Feb 22, 2024 at 7:07 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>>> Ping, in the end are we applying these patches for either 6.8 or 6.9?
>>> Let me poke at them and see if we can stick them in x86/urgent early
>>> next week.  They do fix an actual bug that's biting people, right?
>> Yes, I have gotten reports of {Sapphire,Emerald} Rapids machines that
>> don't boot at all without either these patches or
>> "disable_mtrr_cleanup".
> We tried platform other than Sapphire and Emerald. This patchset can fix
> boot issues on that platform also.

Fengwei, could you also test this series on the troublesome hardware,
please?

> https://lore.kernel.org/all/20240222183926.517AFCD2@davehans-spike.ostc.intel.com/

If it _also_ fixes the problem, it'll be a strong indication that it's
the right long-term approach.

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Yin Fengwei]

On 2/27/24 00:21, Dave Hansen wrote:
> On 2/25/24 17:57, Yin Fengwei wrote:
>> On 2/23/24 02:08, Paolo Bonzini wrote:
>>> On Thu, Feb 22, 2024 at 7:07 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>>>> Ping, in the end are we applying these patches for either 6.8 or 6.9?
>>>> Let me poke at them and see if we can stick them in x86/urgent early
>>>> next week.  They do fix an actual bug that's biting people, right?
>>> Yes, I have gotten reports of {Sapphire,Emerald} Rapids machines that
>>> don't boot at all without either these patches or
>>> "disable_mtrr_cleanup".
>> We tried platform other than Sapphire and Emerald. This patchset can fix
>> boot issues on that platform also.
> 
> Fengwei, could you also test this series on the troublesome hardware,
> please?
Sure. I will try it on my env. I just didn't connected your patchset to this
boot issue. :(.


Regards
Yin, Fengwei

> 
>> https://lore.kernel.org/all/20240222183926.517AFCD2@davehans-spike.ostc.intel.com/
> 
> If it _also_ fixes the problem, it'll be a strong indication that it's
> the right long-term approach.
> 

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Yin Fengwei]

Hi Dave,

On 2/27/24 00:21, Dave Hansen wrote:
> On 2/25/24 17:57, Yin Fengwei wrote:
>> On 2/23/24 02:08, Paolo Bonzini wrote:
>>> On Thu, Feb 22, 2024 at 7:07 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>>>> Ping, in the end are we applying these patches for either 6.8 or 6.9?
>>>> Let me poke at them and see if we can stick them in x86/urgent early
>>>> next week.  They do fix an actual bug that's biting people, right?
>>> Yes, I have gotten reports of {Sapphire,Emerald} Rapids machines that
>>> don't boot at all without either these patches or
>>> "disable_mtrr_cleanup".
>> We tried platform other than Sapphire and Emerald. This patchset can fix
>> boot issues on that platform also.
> 
> Fengwei, could you also test this series on the troublesome hardware,
> please?
> 
>> https://lore.kernel.org/all/20240222183926.517AFCD2@davehans-spike.ostc.intel.com/
> 
> If it _also_ fixes the problem, it'll be a strong indication that it's
> the right long-term approach.
I tried your patchset on a Sapphire machine which is the only one broken machine
I can access today. The base commit is 45ec2f5f6ed3 from the latest linus tree.

Without your patchset, the system boot hang.
With your patchset, the system boot successfully.


Regards
Yin, Fengwei

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Dave Hansen]

On 2/26/24 22:08, Yin Fengwei wrote:
>>> https://lore.kernel.org/all/20240222183926.517AFCD2@davehans-spike.ostc.intel.com/
>> If it _also_ fixes the problem, it'll be a strong indication that it's
>> the right long-term approach.
> I tried your patchset on a Sapphire machine which is the only one broken machine
> I can access today. The base commit is 45ec2f5f6ed3 from the latest linus tree.
> 
> Without your patchset, the system boot hang.
> With your patchset, the system boot successfully.

Yay!  Thanks for testing.

Re: [PATCH v2 0/2] x86/cpu: fix invalid MTRR mask values for SEV or TME

[Yin Fengwei]

On 2/27/24 21:30, Dave Hansen wrote:
> On 2/26/24 22:08, Yin Fengwei wrote:
>>>> https://lore.kernel.org/all/20240222183926.517AFCD2@davehans-spike.ostc.intel.com/
>>> If it _also_ fixes the problem, it'll be a strong indication that it's
>>> the right long-term approach.
>> I tried your patchset on a Sapphire machine which is the only one broken machine
>> I can access today. The base commit is 45ec2f5f6ed3 from the latest linus tree.
>>
>> Without your patchset, the system boot hang.
>> With your patchset, the system boot successfully.
> 
> Yay!  Thanks for testing.
My pleasure.


Regards
Yin, Fengwei