Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1532274

Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage!

Show all headers | View raw

On Mon, Nov 21, 2016 at 05:06:54PM +0100, Borislav Petkov wrote:
> IOW, what's the worst thing that can happen if we did this below?
> 
> We basically get rid of the detection and switch the timer to broadcast
> mode immediately on the halting CPU.
> 
> amd_e400_idle() is behind an "if (cpu_has_bug(c, X86_BUG_AMD_APIC_C1E))"
> check so it will run on the affected CPUs only...
> 
> Thoughts?

Actually, here's a better version. The E400 detection works only after
ACPI has been enabled so we piggyback the end of acpi_init().

We don't need the MSR read now - we do

	if (static_cpu_has_bug(X86_BUG_AMD_APIC_C1E))

on the idle path which is as fast as it gets.

Any complaints about this before I go and test it everywhere?

It builds and boots in my guest here ok, not that it means a whole lot.

The good news is, I have collected a fleet of boxes which all have that
erratum so testing should be pretty reliable. Something that doesn't
happen everytime!

:-)

---
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 5391b0ae7cc3..aa8c879e814f 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -94,7 +94,8 @@ static inline unsigned int acpi_processor_cstate_check(unsigned int max_cstate)
 	    boot_cpu_data.x86_model <= 0x05 &&
 	    boot_cpu_data.x86_mask < 0x0A)
 		return 1;
-	else if (amd_e400_c1e_detected)
+
+	else if (boot_cpu_has(X86_BUG_AMD_APIC_C1E))
 		return 1;
 	else
 		return max_cstate;
@@ -131,6 +132,7 @@ static inline bool acpi_has_cpu_in_madt(void)
 	return !!acpi_lapic;
 }
 
+extern void __init __weak arch_post_acpi_init(void);
 #else /* !CONFIG_ACPI */
 
 #define acpi_lapic 0
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 1d2b69fc0ceb..b3918609f6f1 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -204,6 +204,7 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
 
 #define static_cpu_has_bug(bit)		static_cpu_has((bit))
 #define boot_cpu_has_bug(bit)		cpu_has_bug(&boot_cpu_data, (bit))
+#define setup_clear_cpu_bug(bit)	setup_clear_cpu_cap((bit))
 
 #define MAX_CPU_FEATURES		(NCAPINTS * 32)
 #define cpu_have_feature		boot_cpu_has
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 1f6a92903b09..d98c5c432209 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -636,7 +636,6 @@ extern void select_idle_routine(const struct cpuinfo_x86 *c);
 extern void init_amd_e400_c1e_mask(void);
 
 extern unsigned long		boot_option_idle_override;
-extern bool			amd_e400_c1e_detected;
 
 enum idle_boot_override {IDLE_NO_OVERRIDE=0, IDLE_HALT, IDLE_NOMWAIT,
 			 IDLE_POLL};
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index ee023919e476..56653b7322f9 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -289,9 +289,6 @@ void stop_this_cpu(void *dummy)
 		halt();
 }
 
-bool amd_e400_c1e_detected;
-EXPORT_SYMBOL(amd_e400_c1e_detected);
-
 static cpumask_var_t amd_e400_c1e_mask;
 
 void amd_e400_remove_cpu(int cpu)
@@ -301,26 +298,14 @@ void amd_e400_remove_cpu(int cpu)
 }
 
 /*
- * AMD Erratum 400 aware idle routine. We check for C1E active in the interrupt
- * pending message MSR. If we detect C1E, then we handle it the same
- * way as C3 power states (local apic timer and TSC stop)
+ * AMD Erratum 400 aware idle routine.
+ *
+ *
+ * We handle it the same way as C3 power states (local apic timer and TSC stop)
  */
 static void amd_e400_idle(void)
 {
-	if (!amd_e400_c1e_detected) {
-		u32 lo, hi;
-
-		rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
-
-		if (lo & K8_INTP_C1E_ACTIVE_MASK) {
-			amd_e400_c1e_detected = true;
-			if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
-				mark_tsc_unstable("TSC halt in AMD C1E");
-			pr_info("System has AMD C1E enabled\n");
-		}
-	}
-
-	if (amd_e400_c1e_detected) {
+	if (static_cpu_has_bug(X86_BUG_AMD_APIC_C1E)) {
 		int cpu = smp_processor_id();
 
 		if (!cpumask_test_cpu(cpu, amd_e400_c1e_mask)) {
@@ -419,6 +404,30 @@ void __init init_amd_e400_c1e_mask(void)
 		zalloc_cpumask_var(&amd_e400_c1e_mask, GFP_KERNEL);
 }
 
+void __init arch_post_acpi_init(void)
+{
+	u32 lo, hi;
+
+	if (!boot_cpu_has_bug(X86_BUG_AMD_APIC_C1E))
+		return;
+
+	/*
+	 * AMD E400 detection needs to happen *after* ACPI has been enabled. We
+	 * check for C1E active in the interrupt pending message MSR. If we detect
+	 * C1E enabled, machine is affected.
+	 */
+	rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
+
+	if (!(lo & K8_INTP_C1E_ACTIVE_MASK)) {
+		setup_clear_cpu_bug(X86_BUG_AMD_APIC_C1E);
+		return;
+	}
+
+	if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+		mark_tsc_unstable("TSC halt in AMD C1E");
+	pr_info("System has AMD C1E enabled\n");
+}
+
 static int __init idle_setup(char *str)
 {
 	if (!str)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 56190d00fd87..da42fe354ffc 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -34,6 +34,7 @@
 #include <linux/reboot.h>
 #include <linux/delay.h>
 #ifdef CONFIG_X86
+#include <asm/acpi.h>
 #include <asm/mpspec.h>
 #endif
 #include <linux/acpi_iort.h>
@@ -1166,6 +1167,13 @@ static int __init acpi_bus_init(void)
 struct kobject *acpi_kobj;
 EXPORT_SYMBOL_GPL(acpi_kobj);
 
+/*
+ * Arch-specific one to override this.
+ */
+void __init __weak arch_post_acpi_init(void)
+{
+}
+
 static int __init acpi_init(void)
 {
 	int result;
@@ -1198,6 +1206,9 @@ static int __init acpi_init(void)
 	acpi_debugger_init();
 	acpi_setup_sb_notify_handler();
 	acpi_set_processor_mapping();
+
+	arch_post_acpi_init();
+
 	return 0;
 }
 
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 2237d3f24f0e..8eec3255d6f2 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -141,7 +141,7 @@ static void lapic_timer_check_state(int state, struct acpi_processor *pr,
 	if (cpu_has(&cpu_data(pr->id), X86_FEATURE_ARAT))
 		return;
 
-	if (amd_e400_c1e_detected)
+	if (boot_cpu_has(X86_BUG_AMD_APIC_C1E))
 		type = ACPI_STATE_C1;
 
 	/*

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread

Thread

Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Borislav Petkov <bp@alien8.de> - 2016-11-29 14:30 +0100
  Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Jiri Olsa <jolsa@redhat.com> - 2016-11-29 15:10 +0100
  Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Thomas Gleixner <tglx@linutronix.de> - 2016-11-29 15:10 +0100
    Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Borislav Petkov <bp@alien8.de> - 2016-11-30 09:50 +0100
      Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Thomas Gleixner <tglx@linutronix.de> - 2016-11-30 10:00 +0100
        Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Borislav Petkov <bp@alien8.de> - 2016-11-30 10:10 +0100
          Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Thomas Gleixner <tglx@linutronix.de> - 2016-11-30 10:20 +0100

csiph-web