Skip to contentSkip to conversation

harden MTRR access

PATCH In review Jan Kiszka 2 participants 2 messages Oct 28, 2007 – Nov 6, 2007

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 harden MTRR access

Running Xenomai's latency test while firing up X not just points out once again long delays due to MTRR fiddling and wbinvd invocation - it simply locks up the box! The reason is that MTRR fiddling needs hard IRQ protection, which this patch introduces. Note: I focused on standard MTRR, skipping the legacy CPUs (and dead MTRR code).

Jan

---
 arch/i386/kernel/cpu/mtrr/generic.c |    8 ++++----
 arch/i386/kernel/cpu/mtrr/main.c    |   12 ++++++------
 2 files changed, 10 insertions(+), 10 deletions(-)
arch/i386/kernel/cpu/mtrr/generic.c +4 -4
Index: linux-2.6.23.1-xeno/arch/i386/kernel/cpu/mtrr/generic.c
===================================================================
@@ -392,14 +392,14 @@ static void generic_set_all(void)
 	unsigned long mask, count;
 	unsigned long flags;
 
-	local_irq_save(flags);
+	local_irq_save_hw(flags);
 	prepare_set();
 
 	/* Actually set the state */
 	mask = set_mtrr_state();
 
 	post_set();
-	local_irq_restore(flags);
+	local_irq_restore_hw(flags);
 
 	/*  Use the atomic bitops to update the global mask  */
 	for (count = 0; count < sizeof mask * 8; ++count) {
@@ -427,7 +427,7 @@ static void generic_set_mtrr(unsigned in
 
 	vr = &mtrr_state.var_ranges[reg];
 
-	local_irq_save(flags);
+	local_irq_save_hw(flags);
 	prepare_set();
 
 	if (size == 0) {
@@ -446,7 +446,7 @@ static void generic_set_mtrr(unsigned in
 	}
 
 	post_set();
-	local_irq_restore(flags);
+	local_irq_restore_hw(flags);
 }
 
 int generic_validate_add_page(unsigned long base, unsigned long size, unsigned int type)
arch/i386/kernel/cpu/mtrr/main.c +6 -6
Index: linux-2.6.23.1-xeno/arch/i386/kernel/cpu/mtrr/main.c
===================================================================
@@ -149,7 +149,7 @@ static void ipi_handler(void *info)
 	struct set_mtrr_data *data = info;
 	unsigned long flags;
 
-	local_irq_save(flags);
+	local_irq_save_hw(flags);
 
 	atomic_dec(&data->count);
 	while(!atomic_read(&data->gate))
@@ -167,7 +167,7 @@ static void ipi_handler(void *info)
 		cpu_relax();
 
 	atomic_dec(&data->count);
-	local_irq_restore(flags);
+	local_irq_restore_hw(flags);
 }
 
 #endif
@@ -237,7 +237,7 @@ static void set_mtrr(unsigned int reg, u
 	if (smp_call_function(ipi_handler, &data, 1, 0) != 0)
 		panic("mtrr: timed out waiting for other CPUs\n");
 
-	local_irq_save(flags);
+	local_irq_save_hw(flags);
 
 	while(atomic_read(&data.count))
 		cpu_relax();
@@ -273,7 +273,7 @@ static void set_mtrr(unsigned int reg, u
 	while(atomic_read(&data.count))
 		cpu_relax();
 
-	local_irq_restore(flags);
+	local_irq_restore_hw(flags);
 }
 
 /**
@@ -726,11 +726,11 @@ void mtrr_ap_init(void)
 	 * 2.cpu hotadd time. We let mtrr_add/del_page hold cpuhotplug lock to
 	 * prevent mtrr entry changes
 	 */
-	local_irq_save(flags);
+	local_irq_save_hw(flags);
 
 	mtrr_if->set_all();
 
-	local_irq_restore(flags);
+	local_irq_restore_hw(flags);
 }
 
 /**
#1 lore raw

Hi Jan

Quoted 5 lines · Jan Kiszka

Running Xenomai's latency test while firing up X not just points out once again long delays due to MTRR fiddling and wbinvd invocation - it simply locks up the box! The reason is that MTRR fiddling needs hard IRQ protection, which this patch introduces. Note: I focused on standard MTRR, skipping the legacy CPUs (and dead MTRR code).

Preliminary indications are that this fixes an intermittant & random lockup on my x86_64 build. Fortuitous that the same code is used for both i386 and x86_64 kernels ;)

Regards, Paul.