Re: [PATCH 1/2] x86/mm/pat: Change pat_disable() to emulate PAT table

[Borislav Petkov]

Ok, let's start again.

So what's wrong with this simpler, cleaner version below?

* It makes sure to run the rendezvous handler on the BP on init.

* It disables PAT otherwise.

* Oh, and it boots fine with Paul's reproducer, X is there (even though
vncviewer dies when X starts with "Rect too large: 640x480 at (0, 0)"
and I have to reconnect to see the X window).

In dmesg I see "x86/PAT: Disabled by MTRR".

---
diff --git a/arch/x86/include/asm/pat.h b/arch/x86/include/asm/pat.h
index ca6c228d5e62..3f3b1bc67391 100644
--- a/arch/x86/include/asm/pat.h
+++ b/arch/x86/include/asm/pat.h
@@ -5,7 +5,8 @@
 #include <asm/pgtable_types.h>
 
 bool pat_enabled(void);
-extern void pat_init(void);
+void pat_disable(const char *reason);
+void pat_setup(void);
 void pat_init_cache_modes(u64);
 
 extern int reserve_memtype(u64 start, u64 end,
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index 19f57360dfd2..44bad29ebc9b 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -444,11 +444,24 @@ static void __init print_mtrr_state(void)
 		pr_debug("TOM2: %016llx aka %lldM\n", mtrr_tom2, mtrr_tom2>>20);
 }
 
+void __init mtrr_pat_setup_bp(void)
+{
+	unsigned long flags;
+
+	/* PAT setup for BP. We need to go through sync steps here */
+	local_irq_save(flags);
+	prepare_set();
+
+	pat_setup();
+
+	post_set();
+	local_irq_restore(flags);
+}
+
 /* Grab all of the MTRR state for this CPU into *state */
 bool __init get_mtrr_state(void)
 {
 	struct mtrr_var_range *vrs;
-	unsigned long flags;
 	unsigned lo, dummy;
 	unsigned int i;
 
@@ -481,15 +494,6 @@ bool __init get_mtrr_state(void)
 
 	mtrr_state_set = 1;
 
-	/* PAT setup for BP. We need to go through sync steps here */
-	local_irq_save(flags);
-	prepare_set();
-
-	pat_init();
-
-	post_set();
-	local_irq_restore(flags);
-
 	return !!(mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED);
 }
 
@@ -788,7 +792,7 @@ static void generic_set_all(void)
 	mask = set_mtrr_state();
 
 	/* also set PAT */
-	pat_init();
+	pat_setup();
 
 	post_set();
 	local_irq_restore(flags);
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 10f8d4796240..5c442b4bd52a 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -752,6 +752,8 @@ void __init mtrr_bp_init(void)
 			/* BIOS may override */
 			__mtrr_enabled = get_mtrr_state();
 
+			mtrr_pat_setup_bp();
+
 			if (mtrr_cleanup(phys_addr)) {
 				changed_by_mtrr_cleanup = 1;
 				mtrr_if->set_all();
@@ -759,8 +761,11 @@ void __init mtrr_bp_init(void)
 		}
 	}
 
-	if (!mtrr_enabled())
+	if (!__mtrr_enabled) {
 		pr_info("MTRR: Disabled\n");
+		pat_disable("PAT disabled by MTRR");
+		pat_setup();
+	}
 }
 
 void mtrr_ap_init(void)
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h
index 951884dcc433..e60d87cceae5 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.h
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
@@ -52,6 +52,7 @@ void set_mtrr_prepare_save(struct set_mtrr_context *ctxt);
 void fill_mtrr_var_range(unsigned int index,
 		u32 base_lo, u32 base_hi, u32 mask_lo, u32 mask_hi);
 bool get_mtrr_state(void);
+void __init mtrr_pat_setup_bp(void);
 
 extern void set_mtrr_ops(const struct mtrr_ops *ops);
 
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index faec01e7a17d..ac8283bcf417 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -41,8 +41,11 @@ static bool boot_cpu_done;
 
 static int __read_mostly __pat_enabled = IS_ENABLED(CONFIG_X86_PAT);
 
-static inline void pat_disable(const char *reason)
+void pat_disable(const char *reason)
 {
+	if (!__pat_enabled)
+		return;
+
 	__pat_enabled = 0;
 	pr_info("x86/PAT: %s\n", reason);
 }
@@ -238,7 +241,7 @@ static void pat_ap_init(u64 pat)
 	wrmsrl(MSR_IA32_CR_PAT, pat);
 }
 
-void pat_init(void)
+void pat_setup(void)
 {
 	u64 pat;
 	struct cpuinfo_x86 *c = &boot_cpu_data;


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.