x86: Minimize work done with the caches disabled in mtrr functions.

The code in src/cpu/x86/mtrr/mtrr.c disables caching in a few places when
changing mtrr settings. While I can't find anything that says that's actually
required, I can believe it's necessary. With that said, other code around the
wrmsr instructions which actually modify the settings should be able to run
with caching enabled with no ill effects.

This is particularly true for two calls to printk, one in the fixed mtrr code
and one in the variable, which could result in an arbitrary amount of work
being done without caching. When changing the implementation of the cbmem
console, these two printks caused a significant regression in boot performance
on link of about 70ms which is about 10% of total firmware boot time. When the
window where the cache is disabled is minimized, both this and the new
implementation were about 30ms faster than the original boot time.

For the variable MTRRs, we now store what we want to set the MSRs to and then
write them all at once at the end of commit_var_mtrrs(). This way we don't
have some set and some not, but we still minimize the time we spend with the
caches disabled.

Change-Id: I5139b262bd2d13f79afd88e2e2c0f514fb3e27c9
Signed-off-by: Gabe Black <gabeblack@google.com>
Reviewed-on: https://chromium-review.googlesource.com/187811
Reviewed-by: Ronald Minnich <rminnich@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Commit-Queue: Gabe Black <gabeblack@chromium.org>
Tested-by: Gabe Black <gabeblack@chromium.org>
(cherry picked from commit 31529d6d965676c6cedeb62137eabc26819956fc)
Signed-off-by: Isaac Christensen <isaac.christensen@se-eng.com>
Reviewed-on: http://review.coreboot.org/6952
Tested-by: build bot (Jenkins)
Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com>

1 files changed, 78 insertions, 38 deletions

diff --git a/src/cpu/x86/mtrr/mtrr.c b/src/cpu/x86/mtrr/mtrr.c
index b19b853196..e0392f7978 100644
--- a/src/cpu/x86/mtrr/mtrr.c
+++ b/src/cpu/x86/mtrr/mtrr.c
@@ -50,6 +50,12 @@
 #define BIOS_MTRRS 6
 #define OS_MTRRS   2
 #define MTRRS      (BIOS_MTRRS + OS_MTRRS)
+/*
+ * Static storage size for variable MTRRs. Its sized sufficiently large to
+ * handle different types of CPUs. Empiricially, 16 variable MTRRs has not
+ * yet been observed.
+ */
+#define NUM_MTRR_STATIC_STORAGE 16
 
 static int total_mtrrs = MTRRS;
 static int bios_mtrrs = BIOS_MTRRS;
@@ -61,6 +67,13 @@ static void detect_var_mtrrs(void)
 	msr = rdmsr(MTRRcap_MSR);
 
 	total_mtrrs = msr.lo & 0xff;
+
+	if (total_mtrrs > NUM_MTRR_STATIC_STORAGE) {
+		printk(BIOS_WARNING,
+			"MTRRs detected (%d) > NUM_MTRR_STATIC_STORAGE (%d)\n",
+			total_mtrrs, NUM_MTRR_STATIC_STORAGE);
+		total_mtrrs = NUM_MTRR_STATIC_STORAGE;
+	}
 	bios_mtrrs = total_mtrrs - OS_MTRRS;
 }
 
@@ -314,8 +327,6 @@ static void commit_fixed_mtrrs(void)
 
 	memset(&fixed_msrs, 0, sizeof(fixed_msrs));
 
-	disable_cache();
-
 	msr_num = 0;
 	type_index = 0;
 	for (i = 0; i < ARRAY_SIZE(fixed_mtrr_desc); i++) {
@@ -347,12 +358,13 @@ static void commit_fixed_mtrrs(void)
 		}
 	}
 
-	for (i = 0; i < ARRAY_SIZE(fixed_msrs); i++) {
+	for (i = 0; i < ARRAY_SIZE(fixed_msrs); i++)
 		printk(BIOS_DEBUG, "MTRR: Fixed MSR 0x%lx 0x%08x%08x\n",
 		       msr_index[i], fixed_msrs[i].hi, fixed_msrs[i].lo);
-		wrmsr(msr_index[i], fixed_msrs[i]);
-	}
 
+	disable_cache();
+	for (i = 0; i < ARRAY_SIZE(fixed_msrs); i++)
+		wrmsr(msr_index[i], fixed_msrs[i]);
 	enable_cache();
 }
 
@@ -370,13 +382,28 @@ void x86_setup_fixed_mtrrs(void)
 	enable_fixed_mtrr();
 }
 
+struct var_mtrr_regs {
+	msr_t base;
+	msr_t mask;
+};
+
+struct var_mtrr_solution {
+	int mtrr_default_type;
+	int num_used;
+	struct var_mtrr_regs regs[NUM_MTRR_STATIC_STORAGE];
+};
+
+/* Global storage for variable MTRR solution. */
+static struct var_mtrr_solution mtrr_global_solution;
+
 struct var_mtrr_state {
 	struct memranges *addr_space;
 	int above4gb;
 	int address_bits;
-	int commit_mtrrs;
+	int prepare_msrs;
 	int mtrr_index;
 	int def_mtrr_type;
+	struct var_mtrr_regs *regs;
 };
 
 static void clear_var_mtrr(int index)
@@ -388,11 +415,10 @@ static void clear_var_mtrr(int index)
 	wrmsr(MTRRphysMask_MSR(index), msr_val);
 }
 
-static void write_var_mtrr(struct var_mtrr_state *var_state,
-                           uint32_t base, uint32_t size, int mtrr_type)
+static void prep_var_mtrr(struct var_mtrr_state *var_state,
+                          uint32_t base, uint32_t size, int mtrr_type)
 {
-	msr_t msr_val;
-	unsigned long msr_index;
+	struct var_mtrr_regs *regs;
 	resource_t rbase;
 	resource_t rsize;
 	resource_t mask;
@@ -420,18 +446,15 @@ static void write_var_mtrr(struct var_mtrr_state *var_state,
 	printk(BIOS_DEBUG, "MTRR: %d base 0x%016llx mask 0x%016llx type %d\n",
 	       var_state->mtrr_index, rbase, rsize, mtrr_type);
 
-	msr_val.lo = rbase;
-	msr_val.lo |= mtrr_type;
+	regs = &var_state->regs[var_state->mtrr_index];
 
-	msr_val.hi = rbase >> 32;
-	msr_index = MTRRphysBase_MSR(var_state->mtrr_index);
-	wrmsr(msr_index, msr_val);
+	regs->base.lo = rbase;
+	regs->base.lo |= mtrr_type;
+	regs->base.hi = rbase >> 32;
 
-	msr_val.lo = rsize;
-	msr_val.lo |= MTRRphysMaskValid;
-	msr_val.hi = rsize >> 32;
-	msr_index = MTRRphysMask_MSR(var_state->mtrr_index);
-	wrmsr(msr_index, msr_val);
+	regs->mask.lo = rsize;
+	regs->mask.lo |= MTRRphysMaskValid;
+	regs->mask.hi = rsize >> 32;
 }
 
 static void calc_var_mtrr_range(struct var_mtrr_state *var_state,
@@ -453,8 +476,8 @@ static void calc_var_mtrr_range(struct var_mtrr_state *var_state,
 		else
 			mtrr_size = 1 << addr_lsb;
 
-		if (var_state->commit_mtrrs)
-			write_var_mtrr(var_state, base, mtrr_size, mtrr_type);
+		if (var_state->prepare_msrs)
+			prep_var_mtrr(var_state, base, mtrr_size, mtrr_type);
 
 		size -= mtrr_size;
 		base += mtrr_size;
@@ -614,7 +637,7 @@ static void __calc_var_mtrrs(struct memranges *addr_space,
 	var_state.addr_space = addr_space;
 	var_state.above4gb = above4gb;
 	var_state.address_bits = address_bits;
-	var_state.commit_mtrrs = 0;
+	var_state.prepare_msrs = 0;
 
 	wb_deftype_count = 0;
 	uc_deftype_count = 0;
@@ -711,20 +734,21 @@ static int calc_var_mtrrs(struct memranges *addr_space,
 	return MTRR_TYPE_UNCACHEABLE;
 }
 
-static void commit_var_mtrrs(struct memranges *addr_space, int def_type,
-                             int above4gb, int address_bits)
+static void prepare_var_mtrrs(struct memranges *addr_space, int def_type,
+				int above4gb, int address_bits,
+				struct var_mtrr_solution *sol)
 {
 	struct range_entry *r;
 	struct var_mtrr_state var_state;
-	int i;
 
 	var_state.addr_space = addr_space;
 	var_state.above4gb = above4gb;
 	var_state.address_bits = address_bits;
-	/* Write the MSRs. */
-	var_state.commit_mtrrs = 1;
+	/* Prepare the MSRs. */
+	var_state.prepare_msrs = 1;
 	var_state.mtrr_index = 0;
 	var_state.def_mtrr_type = def_type;
+	var_state.regs = &sol->regs[0];
 
 	memranges_each_entry(r, var_state.addr_space) {
 		if (range_entry_mtrr_type(r) == def_type)
@@ -737,30 +761,46 @@ static void commit_var_mtrrs(struct memranges *addr_space, int def_type,
 			calc_var_mtrrs_without_hole(&var_state, r);
 	}
 
-	/* Clear all remaining variable MTRRs. */
-	for (i = var_state.mtrr_index; i < total_mtrrs; i++)
+	/* Update the solution. */
+	sol->num_used = var_state.mtrr_index;
+}
+
+static void commit_var_mtrrs(const struct var_mtrr_solution *sol)
+{
+	int i;
+
+	/* Write out the variable MTTRs. */
+	disable_cache();
+	for (i = 0; i < sol->num_used; i++) {
+		wrmsr(MTRRphysBase_MSR(i), sol->regs[i].base);
+		wrmsr(MTRRphysMask_MSR(i), sol->regs[i].mask);
+	}
+	/* Clear the ones that are unused. */
+	for (; i < total_mtrrs; i++)
 		clear_var_mtrr(i);
+	enable_cache();
+
 }
 
 void x86_setup_var_mtrrs(unsigned int address_bits, unsigned int above4gb)
 {
-	static int mtrr_default_type = -1;
+	static struct var_mtrr_solution *sol = NULL;
 	struct memranges *addr_space;
 
 	addr_space = get_physical_address_space();
 
-	if (mtrr_default_type == -1) {
+	if (sol == NULL) {
 		if (above4gb == 2)
 			detect_var_mtrrs();
-		mtrr_default_type =
+		sol = &mtrr_global_solution;
+		sol->mtrr_default_type =
 			calc_var_mtrrs(addr_space, !!above4gb, address_bits);
+		prepare_var_mtrrs(addr_space, sol->mtrr_default_type,
+				  !!above4gb, address_bits, sol);
 	}
 
-	disable_cache();
-	commit_var_mtrrs(addr_space, mtrr_default_type, !!above4gb,
-	                 address_bits);
-	enable_var_mtrr(mtrr_default_type);
-	enable_cache();
+	commit_var_mtrrs(sol);
+	enable_var_mtrr(sol->mtrr_default_type);
 }
 
 void x86_setup_mtrrs(void)