Skip to contentSkip to conversation

Last minute x86 bug fixes for 2.6.23

PATCH New Andi Kleen 1 participant 4 messages Sep 6, 2007

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

cover Last minute x86 bug fixes for 2.6.23

- Don't use CLFLUSH for text patching because it breaks on some VIA CPUs.
- Fix NMI watchdog testing hang on i386
- Fix perfctr NMI watchdog on Yonah

-Andi

author
#1 lore raw

PATCH 1/3 i386: Fix perfctr watchdog on core duo

From: Stephane Eranian <[email protected]>

Fix the NMI watchdog on Intel CoreDuo processor where the kernel would get stuck during boot. The issue is related to errata AE49, where the PERFEVTSEL1 counter does not have a working enable bit. Thus it is not possible to use it for NMI. The patch creates a dedicated wd_ops for CoreDuo which falls back to using PERFEVTSEL0. The other Intel processors supporting the architectural PMU will keep on using PERFEVTSEL1 as this allows other subsystems, such as perfmon, to use PERFEVTSEL0 for PEBS monitoring in particular.
Bug initially reported by Daniel Walker.

AK: Added comments

Signed-off-by Stephane Eranian <[email protected]>
Signed-off-by Andi Kleen <[email protected]>
---
 arch/i386/kernel/cpu/perfctr-watchdog.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)
arch/i386/kernel/cpu/perfctr-watchdog.c +22 -6
Index: linux/arch/i386/kernel/cpu/perfctr-watchdog.c
===================================================================
@@ -263,8 +263,8 @@ static int setup_k7_watchdog(unsigned nm
 	unsigned int evntsel;
 	struct nmi_watchdog_ctlblk *wd = &__get_cpu_var(nmi_watchdog_ctlblk);
 
-	perfctr_msr = MSR_K7_PERFCTR0;
-	evntsel_msr = MSR_K7_EVNTSEL0;
+	perfctr_msr = wd_ops->perfctr;
+	evntsel_msr = wd_ops->evntsel;
 
 	wrmsrl(perfctr_msr, 0UL);
 

@@ -343,8 +343,8 @@ static int setup_p6_watchdog(unsigned nm
 	unsigned int evntsel;
 	struct nmi_watchdog_ctlblk *wd = &__get_cpu_var(nmi_watchdog_ctlblk);
 
-	perfctr_msr = MSR_P6_PERFCTR0;
-	evntsel_msr = MSR_P6_EVNTSEL0;
+	perfctr_msr = wd_ops->perfctr;
+	evntsel_msr = wd_ops->evntsel;
 
 	/* KVM doesn't implement this MSR */
 	if (wrmsr_safe(perfctr_msr, 0, 0) < 0)

@@ -569,8 +569,8 @@ static int setup_intel_arch_watchdog(uns
 	    (ebx & ARCH_PERFMON_UNHALTED_CORE_CYCLES_PRESENT))
 		return 0;
 
-	perfctr_msr = MSR_ARCH_PERFMON_PERFCTR1;
-	evntsel_msr = MSR_ARCH_PERFMON_EVENTSEL1;
+	perfctr_msr = wd_ops->perfctr;
+	evntsel_msr = wd_ops->evntsel;
 
 	wrmsrl(perfctr_msr, 0UL);
 

@@ -605,6 +605,16 @@ static struct wd_ops intel_arch_wd_ops =
 	.evntsel = MSR_ARCH_PERFMON_EVENTSEL1,
 };
 
+static struct wd_ops coreduo_wd_ops = {
+	.reserve = single_msr_reserve,
+	.unreserve = single_msr_unreserve,
+	.setup = setup_intel_arch_watchdog,
+	.rearm = p6_rearm,
+	.stop = single_msr_stop_watchdog,
+	.perfctr = MSR_ARCH_PERFMON_PERFCTR0,
+	.evntsel = MSR_ARCH_PERFMON_EVENTSEL0,
+};
+
 static void probe_nmi_watchdog(void)
 {
 	switch (boot_cpu_data.x86_vendor) {

@@ -615,6 +625,12 @@ static void probe_nmi_watchdog(void)
 		wd_ops = &k7_wd_ops;
 		break;
 	case X86_VENDOR_INTEL:
+		/* Work around Core Duo (Yonah) errata AE49 where perfctr1
+		   doesn't have a working enable bit. */
+		if (boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 14) {
+			wd_ops = &coreduo_wd_ops;
+			break;
+		}
 		if (cpu_has(&boot_cpu_data, X86_FEATURE_ARCH_PERFMON)) {
 			wd_ops = &intel_arch_wd_ops;
 			break;
author
#2 lore raw

PATCH 2/3 x86_64: Remove CLFLUSH in text_poke()

In reply to Andi Kleen

The CLFLUSH for the modified code line in text_poke was supposed to speed up CPU recovery. Unfortunately it seems to cause hangs on some VIA C3s (at least on VIA Esther Model 10 Stepping 9) Remove it.

Thanks to Stefan Becker for reporting/testing.

Signed-off-by Andi Kleen <[email protected]>
---
 arch/i386/kernel/alternative.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
arch/i386/kernel/alternative.c +2 -4
Index: linux/arch/i386/kernel/alternative.c
===================================================================
@@ -445,8 +445,6 @@ void __kprobes text_poke(void *addr, uns
 {
 	memcpy(addr, opcode, len);
 	sync_core();
-	/* Not strictly needed, but can speed CPU recovery up. Ignore cross cacheline
-	   case. */
-	if (cpu_has_clflush)
-		asm("clflush (%0) " :: "r" (addr) : "memory");
+	/* Could also do a CLFLUSH here to speed up CPU recovery; but
+	   that causes hangs on some VIA CPUs. */
 }
author
#3 lore raw

PATCH 3/3 i386: fix a hang on stuck nmi watchdog

In reply to Andi Kleen

From: Daniel Walker <[email protected]>
In the case when an nmi gets stucks the endflag stays equal to zero. This causes the busy looping on other cpus to continue, even tho the nmi test is done.

On my machine with out the change below the system would hang right after check_nmi_watchdog(). The change below just sets endflag prior to checking if the test was successful or not.

Signed-off-by Daniel Walker <[email protected]>
Signed-off-by Andi Kleen <[email protected]>
---
 arch/i386/kernel/nmi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/i386/kernel/nmi.c +1 -1
Index: linux/arch/i386/kernel/nmi.c
===================================================================
@@ -115,12 +115,12 @@ static int __init check_nmi_watchdog(voi
 			atomic_dec(&nmi_active);
 		}
 	}
+	endflag = 1;
 	if (!atomic_read(&nmi_active)) {
 		kfree(prev_nmi_count);
 		atomic_set(&nmi_active, -1);
 		return -1;
 	}
-	endflag = 1;
 	printk("OK.\n");
 
 	/* now that we know it works we can reduce NMI frequency to