linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/
@ 2021-02-01 14:27 Borislav Petkov
  2021-02-01 14:27 ` [PATCH v3 1/2] x86/mce: Get rid of mcheck_intel_therm_init() Borislav Petkov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Borislav Petkov @ 2021-02-01 14:27 UTC (permalink / raw)
  To: X86 ML
  Cc: LKML, Amit Kucheria, Daniel Lezcano, Srinivas Pandruvada,
	Tony Luck, Zhang Rui, linux-pm

From: Borislav Petkov <bp@suse.de>

Hi,

I know I already committed those but

https://lkml.kernel.org/r/20210201032427.GB12524@xsang-OptiPlex-9020

made me look at the IRQ handler registration. And it does happen per CPU
which is particularly daft and unneeded. And it used to do that before
that change too, for some unknown reason.

So I decided to not have a handler registration but simply call the
thermal interrupt handler if all is setup and before that issue the
message about the unexpected IRQ.

I did test it by sending bogus thermal interrupts before and after
registration - see hunk below - and it looks good:

[    0.136608] 0: Sending thermal IRQ
[    0.136760] CPU0: Unexpected LVT thermal interrupt!
[    0.136917] CPU0: Thermal monitoring enabled (TM1)
[    0.137071] 1: Sending thermal IRQ
[    0.043243] 0: Sending thermal IRQ
[    0.043243] 1: Sending thermal IRQ
[    0.043243] 0: Sending thermal IRQ
[    0.043243] 1: Sending thermal IRQ
[    0.043243] 0: Sending thermal IRQ
[    0.043243] 1: Sending thermal IRQ
[    0.043243] 0: Sending thermal IRQ
[    0.043243] 1: Sending thermal IRQ
[    0.043243] 0: Sending thermal IRQ
[    0.043243] 1: Sending thermal IRQ
[    0.149436] thermal_sys: Registered thermal governor 'fair_share'
[    0.149437] thermal_sys: Registered thermal governor 'bang_bang'
[    0.149595] thermal_sys: Registered thermal governor 'step_wise'
[    0.149753] thermal_sys: Registered thermal governor 'user_space'
[    0.445717] ACPI: \_SB_.PR00: _OSC native thermal LVT Acked
[    0.707539] thermal LNXTHERM:00: registered as thermal_zone0

Logic in patch 1 got a bit simplified too.

Thx.

---
diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c
index 6221f0f418f7..e544f04eac8c 100644
--- a/drivers/thermal/intel/therm_throt.c
+++ b/drivers/thermal/intel/therm_throt.c
@@ -705,6 +705,11 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
                                | PACKAGE_THERM_INT_HIGH_ENABLE), h);
        }
 
+       pr_info("0: Sending thermal IRQ\n");
+
+       apic->send_IPI(0, THERMAL_APIC_VECTOR);
+       apic->send_IPI(3, THERMAL_APIC_VECTOR);
+
        rdmsr(MSR_IA32_MISC_ENABLE, l, h);
        wrmsr(MSR_IA32_MISC_ENABLE, l | MSR_IA32_MISC_ENABLE_TM1, h);
 
@@ -713,4 +718,8 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
 
        /* enable thermal throttle processing */
        atomic_set(&therm_throt_en, 1);
+
+       pr_info("1: Sending thermal IRQ\n");
+       apic->send_IPI(0, THERMAL_APIC_VECTOR);
+       apic->send_IPI(3, THERMAL_APIC_VECTOR);
 }

Changelog:
==========

here's v2 which addresses peterz's comments to patch 2.

@thermal folks, lemme know if you have any objections otherwise I'll
route this through the tip tree.

v1:

so this has come up a bunch of times in the past and PeterZ is right
- that thing doesn't have anything to do with the MCE glue so move it
where it belongs.

Thx.

Borislav Petkov (2):
  x86/mce: Get rid of mcheck_intel_therm_init()
  thermal: Move therm_throt there from x86/mce

 arch/x86/Kconfig                              |  4 --
 arch/x86/include/asm/mce.h                    | 22 ----------
 arch/x86/include/asm/thermal.h                | 24 +++++++++++
 arch/x86/kernel/cpu/intel.c                   |  3 ++
 arch/x86/kernel/cpu/mce/Makefile              |  2 -
 arch/x86/kernel/cpu/mce/core.c                |  1 -
 arch/x86/kernel/cpu/mce/intel.c               |  1 -
 arch/x86/kernel/irq.c                         | 21 ++++++++++
 drivers/thermal/intel/Kconfig                 |  4 ++
 drivers/thermal/intel/Makefile                |  1 +
 .../thermal/intel}/therm_throt.c              | 41 ++++---------------
 drivers/thermal/intel/x86_pkg_temp_thermal.c  |  3 +-
 12 files changed, 64 insertions(+), 63 deletions(-)
 create mode 100644 arch/x86/include/asm/thermal.h
 rename {arch/x86/kernel/cpu/mce => drivers/thermal/intel}/therm_throt.c (96%)

2nd:rc6-therm_throt

-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] x86/mce: Get rid of mcheck_intel_therm_init()
  2021-02-01 14:27 [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Borislav Petkov
@ 2021-02-01 14:27 ` Borislav Petkov
  2021-02-01 14:27 ` [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
  2021-02-01 19:10 ` [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Srinivas Pandruvada
  2 siblings, 0 replies; 7+ messages in thread
From: Borislav Petkov @ 2021-02-01 14:27 UTC (permalink / raw)
  To: X86 ML
  Cc: LKML, Amit Kucheria, Daniel Lezcano, Srinivas Pandruvada,
	Tony Luck, Zhang Rui, linux-pm

From: Borislav Petkov <bp@suse.de>

Move the APIC_LVTTHMR read which needs to happen on the BSP, to
intel_init_thermal(). One less boot dependency.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/mce.h            |  6 ------
 arch/x86/kernel/cpu/mce/core.c        |  1 -
 arch/x86/kernel/cpu/mce/therm_throt.c | 15 ++++-----------
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 56cdeaac76a0..def9aa5e1fa4 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -304,12 +304,6 @@ extern int (*platform_thermal_package_notify)(__u64 msr_val);
  * callback has rate control */
 extern bool (*platform_thermal_package_rate_control)(void);
 
-#ifdef CONFIG_X86_THERMAL_VECTOR
-extern void mcheck_intel_therm_init(void);
-#else
-static inline void mcheck_intel_therm_init(void) { }
-#endif
-
 /*
  * Used by APEI to report memory error via /dev/mcelog
  */
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index e133ce1e562b..c68e21b4ea0d 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2178,7 +2178,6 @@ __setup("mce", mcheck_enable);
 
 int __init mcheck_init(void)
 {
-	mcheck_intel_therm_init();
 	mce_register_decode_chain(&early_nb);
 	mce_register_decode_chain(&mce_uc_nb);
 	mce_register_decode_chain(&mce_default_nb);
diff --git a/arch/x86/kernel/cpu/mce/therm_throt.c b/arch/x86/kernel/cpu/mce/therm_throt.c
index a7cd2d203ced..5b15d7cef1d1 100644
--- a/arch/x86/kernel/cpu/mce/therm_throt.c
+++ b/arch/x86/kernel/cpu/mce/therm_throt.c
@@ -633,17 +633,6 @@ static int intel_thermal_supported(struct cpuinfo_x86 *c)
 	return 1;
 }
 
-void __init mcheck_intel_therm_init(void)
-{
-	/*
-	 * This function is only called on boot CPU. Save the init thermal
-	 * LVT value on BSP and use that value to restore APs' thermal LVT
-	 * entry BIOS programmed later
-	 */
-	if (intel_thermal_supported(&boot_cpu_data))
-		lvtthmr_init = apic_read(APIC_LVTTHMR);
-}
-
 void intel_init_thermal(struct cpuinfo_x86 *c)
 {
 	unsigned int cpu = smp_processor_id();
@@ -653,6 +642,10 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
 	if (!intel_thermal_supported(c))
 		return;
 
+	/* On the BSP? */
+	if (c == &boot_cpu_data)
+		lvtthmr_init = apic_read(APIC_LVTTHMR);
+
 	/*
 	 * First check if its enabled already, in which case there might
 	 * be some SMM goo which handles it, so we can't even put a handler
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce
  2021-02-01 14:27 [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Borislav Petkov
  2021-02-01 14:27 ` [PATCH v3 1/2] x86/mce: Get rid of mcheck_intel_therm_init() Borislav Petkov
@ 2021-02-01 14:27 ` Borislav Petkov
  2021-02-01 19:10   ` Srinivas Pandruvada
  2021-02-01 19:10 ` [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Srinivas Pandruvada
  2 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2021-02-01 14:27 UTC (permalink / raw)
  To: X86 ML
  Cc: LKML, Peter Zijlstra, Amit Kucheria, Daniel Lezcano,
	Srinivas Pandruvada, Tony Luck, Zhang Rui, linux-pm

From: Borislav Petkov <bp@suse.de>

This functionality has nothing to do with MCE, move it to the thermal
framework and untangle it from MCE.

Requested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/Kconfig                              |  4 ---
 arch/x86/include/asm/mce.h                    | 16 ----------
 arch/x86/include/asm/thermal.h                | 24 ++++++++++++++
 arch/x86/kernel/cpu/intel.c                   |  3 ++
 arch/x86/kernel/cpu/mce/Makefile              |  2 --
 arch/x86/kernel/cpu/mce/intel.c               |  1 -
 arch/x86/kernel/irq.c                         | 21 ++++++++++++
 drivers/thermal/intel/Kconfig                 |  4 +++
 drivers/thermal/intel/Makefile                |  1 +
 .../thermal/intel}/therm_throt.c              | 32 +++++--------------
 drivers/thermal/intel/x86_pkg_temp_thermal.c  |  3 +-
 11 files changed, 63 insertions(+), 48 deletions(-)
 create mode 100644 arch/x86/include/asm/thermal.h
 rename {arch/x86/kernel/cpu/mce => drivers/thermal/intel}/therm_throt.c (97%)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 21f851179ff0..9989db3a9bf5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1158,10 +1158,6 @@ config X86_MCE_INJECT
 	  If you don't know what a machine check is and you don't do kernel
 	  QA it is safe to say n.
 
-config X86_THERMAL_VECTOR
-	def_bool y
-	depends on X86_MCE_INTEL
-
 source "arch/x86/events/Kconfig"
 
 config X86_LEGACY_VM86
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index def9aa5e1fa4..ddfb3cad8dff 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -288,22 +288,6 @@ extern void (*mce_threshold_vector)(void);
 /* Deferred error interrupt handler */
 extern void (*deferred_error_int_vector)(void);
 
-/*
- * Thermal handler
- */
-
-void intel_init_thermal(struct cpuinfo_x86 *c);
-
-/* Interrupt Handler for core thermal thresholds */
-extern int (*platform_thermal_notify)(__u64 msr_val);
-
-/* Interrupt Handler for package thermal thresholds */
-extern int (*platform_thermal_package_notify)(__u64 msr_val);
-
-/* Callback support of rate control, return true, if
- * callback has rate control */
-extern bool (*platform_thermal_package_rate_control)(void);
-
 /*
  * Used by APEI to report memory error via /dev/mcelog
  */
diff --git a/arch/x86/include/asm/thermal.h b/arch/x86/include/asm/thermal.h
new file mode 100644
index 000000000000..36f97bff62e1
--- /dev/null
+++ b/arch/x86/include/asm/thermal.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_THERMAL_H
+#define _ASM_X86_THERMAL_H
+
+/* Interrupt Handler for package thermal thresholds */
+extern int (*platform_thermal_package_notify)(__u64 msr_val);
+
+/* Interrupt Handler for core thermal thresholds */
+extern int (*platform_thermal_notify)(__u64 msr_val);
+
+/* Callback support of rate control, return true, if
+ * callback has rate control */
+extern bool (*platform_thermal_package_rate_control)(void);
+
+#ifdef CONFIG_X86_THERMAL_VECTOR
+void intel_init_thermal(struct cpuinfo_x86 *c);
+bool x86_thermal_enabled(void);
+void intel_thermal_interrupt(void);
+#else
+static inline void intel_init_thermal(struct cpuinfo_x86 *c) { }
+#endif
+
+
+#endif /* _ASM_X86_THERMAL_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 59a1e3ce3f14..71221af87cb1 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -24,6 +24,7 @@
 #include <asm/traps.h>
 #include <asm/resctrl.h>
 #include <asm/numa.h>
+#include <asm/thermal.h>
 
 #ifdef CONFIG_X86_64
 #include <linux/topology.h>
@@ -719,6 +720,8 @@ static void init_intel(struct cpuinfo_x86 *c)
 		tsx_disable();
 
 	split_lock_init();
+
+	intel_init_thermal(c);
 }
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/cpu/mce/Makefile b/arch/x86/kernel/cpu/mce/Makefile
index 9f020c994154..015856abdbb1 100644
--- a/arch/x86/kernel/cpu/mce/Makefile
+++ b/arch/x86/kernel/cpu/mce/Makefile
@@ -9,8 +9,6 @@ obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
 mce-inject-y			:= inject.o
 obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o
 
-obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
-
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
 
 obj-$(CONFIG_X86_MCELOG_LEGACY)	+= dev-mcelog.o
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index c2476fe0682e..e309476743b7 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -531,7 +531,6 @@ static void intel_imc_init(struct cpuinfo_x86 *c)
 
 void mce_intel_feature_init(struct cpuinfo_x86 *c)
 {
-	intel_init_thermal(c);
 	intel_init_cmci();
 	intel_init_lmce();
 	intel_ppin_init(c);
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index c5dd50369e2f..d4ad344e80bf 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -21,6 +21,7 @@
 #include <asm/hw_irq.h>
 #include <asm/desc.h>
 #include <asm/traps.h>
+#include <asm/thermal.h>
 
 #define CREATE_TRACE_POINTS
 #include <asm/trace/irq_vectors.h>
@@ -374,3 +375,23 @@ void fixup_irqs(void)
 	}
 }
 #endif
+
+#ifdef CONFIG_X86_THERMAL_VECTOR
+static void smp_thermal_vector(void)
+{
+	if (x86_thermal_enabled())
+		intel_thermal_interrupt();
+	else
+		pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
+		       smp_processor_id());
+}
+
+DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
+{
+	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
+	inc_irq_stat(irq_thermal_count);
+	smp_thermal_vector();
+	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
+	ack_APIC_irq();
+}
+#endif
diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
index 8025b21f43fa..ce4f59213c7a 100644
--- a/drivers/thermal/intel/Kconfig
+++ b/drivers/thermal/intel/Kconfig
@@ -8,6 +8,10 @@ config INTEL_POWERCLAMP
 	  enforce idle time which results in more package C-state residency. The
 	  user interface is exposed via generic thermal framework.
 
+config X86_THERMAL_VECTOR
+	def_bool y
+	depends on X86 && CPU_SUP_INTEL && X86_LOCAL_APIC
+
 config X86_PKG_TEMP_THERMAL
 	tristate "X86 package temperature thermal driver"
 	depends on X86_THERMAL_VECTOR
diff --git a/drivers/thermal/intel/Makefile b/drivers/thermal/intel/Makefile
index 0d9736ced5d4..ff2ad30ef397 100644
--- a/drivers/thermal/intel/Makefile
+++ b/drivers/thermal/intel/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_INTEL_QUARK_DTS_THERMAL)	+= intel_quark_dts_thermal.o
 obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
 obj-$(CONFIG_INTEL_BXT_PMIC_THERMAL) += intel_bxt_pmic_thermal.o
 obj-$(CONFIG_INTEL_PCH_THERMAL)	+= intel_pch_thermal.o
+obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
diff --git a/arch/x86/kernel/cpu/mce/therm_throt.c b/drivers/thermal/intel/therm_throt.c
similarity index 97%
rename from arch/x86/kernel/cpu/mce/therm_throt.c
rename to drivers/thermal/intel/therm_throt.c
index 5b15d7cef1d1..6221f0f418f7 100644
--- a/arch/x86/kernel/cpu/mce/therm_throt.c
+++ b/drivers/thermal/intel/therm_throt.c
@@ -26,13 +26,11 @@
 #include <linux/cpu.h>
 
 #include <asm/processor.h>
+#include <asm/thermal.h>
 #include <asm/traps.h>
 #include <asm/apic.h>
-#include <asm/mce.h>
+#include <asm/irq.h>
 #include <asm/msr.h>
-#include <asm/trace/irq_vectors.h>
-
-#include "internal.h"
 
 /* How long to wait between reporting thermal events */
 #define CHECK_INTERVAL		(300 * HZ)
@@ -570,7 +568,7 @@ static void notify_thresholds(__u64 msr_val)
 }
 
 /* Thermal transition interrupt handler */
-static void intel_thermal_interrupt(void)
+void intel_thermal_interrupt(void)
 {
 	__u64 msr_val;
 
@@ -606,23 +604,6 @@ static void intel_thermal_interrupt(void)
 	}
 }
 
-static void unexpected_thermal_interrupt(void)
-{
-	pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
-		smp_processor_id());
-}
-
-static void (*smp_thermal_vector)(void) = unexpected_thermal_interrupt;
-
-DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
-{
-	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
-	inc_irq_stat(irq_thermal_count);
-	smp_thermal_vector();
-	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
-	ack_APIC_irq();
-}
-
 /* Thermal monitoring depends on APIC, ACPI and clock modulation */
 static int intel_thermal_supported(struct cpuinfo_x86 *c)
 {
@@ -633,6 +614,11 @@ static int intel_thermal_supported(struct cpuinfo_x86 *c)
 	return 1;
 }
 
+bool x86_thermal_enabled(void)
+{
+	return atomic_read(&therm_throt_en);
+}
+
 void intel_init_thermal(struct cpuinfo_x86 *c)
 {
 	unsigned int cpu = smp_processor_id();
@@ -719,8 +705,6 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
 				| PACKAGE_THERM_INT_HIGH_ENABLE), h);
 	}
 
-	smp_thermal_vector = intel_thermal_interrupt;
-
 	rdmsr(MSR_IA32_MISC_ENABLE, l, h);
 	wrmsr(MSR_IA32_MISC_ENABLE, l | MSR_IA32_MISC_ENABLE_TM1, h);
 
diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
index b81c33202f41..090f9176ba62 100644
--- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -17,8 +17,9 @@
 #include <linux/pm.h>
 #include <linux/thermal.h>
 #include <linux/debugfs.h>
+
 #include <asm/cpu_device_id.h>
-#include <asm/mce.h>
+#include <asm/thermal.h>
 
 /*
 * Rate control delay: Idea is to introduce denounce effect
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/
  2021-02-01 14:27 [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Borislav Petkov
  2021-02-01 14:27 ` [PATCH v3 1/2] x86/mce: Get rid of mcheck_intel_therm_init() Borislav Petkov
  2021-02-01 14:27 ` [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
@ 2021-02-01 19:10 ` Srinivas Pandruvada
  2 siblings, 0 replies; 7+ messages in thread
From: Srinivas Pandruvada @ 2021-02-01 19:10 UTC (permalink / raw)
  To: Borislav Petkov, X86 ML
  Cc: LKML, Amit Kucheria, Daniel Lezcano, Tony Luck, Zhang Rui, linux-pm

On Mon, 2021-02-01 at 15:27 +0100, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Hi,
> 
> I know I already committed those but
> 
> https://lkml.kernel.org/r/20210201032427.GB12524@xsang-OptiPlex-9020
> 
> made me look at the IRQ handler registration. And it does happen per
> CPU
> which is particularly daft and unneeded. And it used to do that
> before
> that change too, for some unknown reason.
> 
> So I decided to not have a handler registration but simply call the
> thermal interrupt handler if all is setup and before that issue the
> message about the unexpected IRQ.
> 
> I did test it by sending bogus thermal interrupts before and after
> registration - see hunk below - and it looks good:
> 
> [    0.136608] 0: Sending thermal IRQ
> [    0.136760] CPU0: Unexpected LVT thermal interrupt!
> [    0.136917] CPU0: Thermal monitoring enabled (TM1)
> [    0.137071] 1: Sending thermal IRQ
> [    0.043243] 0: Sending thermal IRQ
> [    0.043243] 1: Sending thermal IRQ
> [    0.043243] 0: Sending thermal IRQ
> [    0.043243] 1: Sending thermal IRQ
> [    0.043243] 0: Sending thermal IRQ
> [    0.043243] 1: Sending thermal IRQ
> [    0.043243] 0: Sending thermal IRQ
> [    0.043243] 1: Sending thermal IRQ
> [    0.043243] 0: Sending thermal IRQ
> [    0.043243] 1: Sending thermal IRQ
> [    0.149436] thermal_sys: Registered thermal governor 'fair_share'
> [    0.149437] thermal_sys: Registered thermal governor 'bang_bang'
> [    0.149595] thermal_sys: Registered thermal governor 'step_wise'
> [    0.149753] thermal_sys: Registered thermal governor 'user_space'
> [    0.445717] ACPI: \_SB_.PR00: _OSC native thermal LVT Acked
> [    0.707539] thermal LNXTHERM:00: registered as thermal_zone0
> 
> Logic in patch 1 got a bit simplified too.
> 
[...]

> 
> Borislav Petkov (2):
>   x86/mce: Get rid of mcheck_intel_therm_init()
>   thermal: Move therm_throt there from x86/mce
> 

I have applied this series and tested. Didn't find any functional
issues. But I have one comment on patch 2/2.

Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Thanks,
Srinivas

>  arch/x86/Kconfig                              |  4 --
>  arch/x86/include/asm/mce.h                    | 22 ----------
>  arch/x86/include/asm/thermal.h                | 24 +++++++++++
>  arch/x86/kernel/cpu/intel.c                   |  3 ++
>  arch/x86/kernel/cpu/mce/Makefile              |  2 -
>  arch/x86/kernel/cpu/mce/core.c                |  1 -
>  arch/x86/kernel/cpu/mce/intel.c               |  1 -
>  arch/x86/kernel/irq.c                         | 21 ++++++++++
>  drivers/thermal/intel/Kconfig                 |  4 ++
>  drivers/thermal/intel/Makefile                |  1 +
>  .../thermal/intel}/therm_throt.c              | 41 ++++-------------
> --
>  drivers/thermal/intel/x86_pkg_temp_thermal.c  |  3 +-
>  12 files changed, 64 insertions(+), 63 deletions(-)
>  create mode 100644 arch/x86/include/asm/thermal.h
>  rename {arch/x86/kernel/cpu/mce =>
> drivers/thermal/intel}/therm_throt.c (96%)
> 
> 2nd:rc6-therm_throt
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce
  2021-02-01 14:27 ` [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
@ 2021-02-01 19:10   ` Srinivas Pandruvada
  2021-02-02 12:10     ` Borislav Petkov
  0 siblings, 1 reply; 7+ messages in thread
From: Srinivas Pandruvada @ 2021-02-01 19:10 UTC (permalink / raw)
  To: Borislav Petkov, X86 ML
  Cc: LKML, Peter Zijlstra, Amit Kucheria, Daniel Lezcano, Tony Luck,
	Zhang Rui, linux-pm

On Mon, 2021-02-01 at 15:27 +0100, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> This functionality has nothing to do with MCE, move it to the thermal
> framework and untangle it from MCE.
> 
> 

[...]

>  /*
>   * Used by APEI to report memory error via /dev/mcelog
>   */
> diff --git a/arch/x86/include/asm/thermal.h
> b/arch/x86/include/asm/thermal.h
> new file mode 100644
> index 000000000000..36f97bff62e1
> --- /dev/null
> +++ b/arch/x86/include/asm/thermal.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_THERMAL_H
> +#define _ASM_X86_THERMAL_H
> +
> +/* Interrupt Handler for package thermal thresholds */
> +extern int (*platform_thermal_package_notify)(__u64 msr_val);
> +
> +/* Interrupt Handler for core thermal thresholds */
> +extern int (*platform_thermal_notify)(__u64 msr_val);
> +
> +/* Callback support of rate control, return true, if
> + * callback has rate control */
> +extern bool (*platform_thermal_package_rate_control)(void);
> +

Only user of the above interface is in drivers/thermal/intel.
So why can't we move these to drivers/thermal/intel/thermal_interrupt.h
or similar?

Thanks,
Srinivas


> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +void intel_init_thermal(struct cpuinfo_x86 *c);
> +bool x86_thermal_enabled(void);
> +void intel_thermal_interrupt(void);
> +#else
> +static inline void intel_init_thermal(struct cpuinfo_x86 *c) { }
> +#endif
> +
> +
> +#endif /* _ASM_X86_THERMAL_H */
> diff --git a/arch/x86/kernel/cpu/intel.c
> b/arch/x86/kernel/cpu/intel.c
> index 59a1e3ce3f14..71221af87cb1 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -24,6 +24,7 @@
>  #include <asm/traps.h>
>  #include <asm/resctrl.h>
>  #include <asm/numa.h>
> +#include <asm/thermal.h>
>  
>  #ifdef CONFIG_X86_64
>  #include <linux/topology.h>
> @@ -719,6 +720,8 @@ static void init_intel(struct cpuinfo_x86 *c)
>  		tsx_disable();
>  
>  	split_lock_init();
> +
> +	intel_init_thermal(c);
>  }
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/kernel/cpu/mce/Makefile
> b/arch/x86/kernel/cpu/mce/Makefile
> index 9f020c994154..015856abdbb1 100644
> --- a/arch/x86/kernel/cpu/mce/Makefile
> +++ b/arch/x86/kernel/cpu/mce/Makefile
> @@ -9,8 +9,6 @@ obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
>  mce-inject-y			:= inject.o
>  obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o
>  
> -obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
> -
>  obj-$(CONFIG_ACPI_APEI)		+= apei.o
>  
>  obj-$(CONFIG_X86_MCELOG_LEGACY)	+= dev-mcelog.o
> diff --git a/arch/x86/kernel/cpu/mce/intel.c
> b/arch/x86/kernel/cpu/mce/intel.c
> index c2476fe0682e..e309476743b7 100644
> --- a/arch/x86/kernel/cpu/mce/intel.c
> +++ b/arch/x86/kernel/cpu/mce/intel.c
> @@ -531,7 +531,6 @@ static void intel_imc_init(struct cpuinfo_x86 *c)
>  
>  void mce_intel_feature_init(struct cpuinfo_x86 *c)
>  {
> -	intel_init_thermal(c);
>  	intel_init_cmci();
>  	intel_init_lmce();
>  	intel_ppin_init(c);
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index c5dd50369e2f..d4ad344e80bf 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -21,6 +21,7 @@
>  #include <asm/hw_irq.h>
>  #include <asm/desc.h>
>  #include <asm/traps.h>
> +#include <asm/thermal.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <asm/trace/irq_vectors.h>
> @@ -374,3 +375,23 @@ void fixup_irqs(void)
>  	}
>  }
>  #endif
> +
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +static void smp_thermal_vector(void)
> +{
> +	if (x86_thermal_enabled())
> +		intel_thermal_interrupt();
> +	else
> +		pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
> +		       smp_processor_id());
> +}
> +
> +DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
> +{
> +	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
> +	inc_irq_stat(irq_thermal_count);
> +	smp_thermal_vector();
> +	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
> +	ack_APIC_irq();
> +}
> +#endif
> diff --git a/drivers/thermal/intel/Kconfig
> b/drivers/thermal/intel/Kconfig
> index 8025b21f43fa..ce4f59213c7a 100644
> --- a/drivers/thermal/intel/Kconfig
> +++ b/drivers/thermal/intel/Kconfig
> @@ -8,6 +8,10 @@ config INTEL_POWERCLAMP
>  	  enforce idle time which results in more package C-state
> residency. The
>  	  user interface is exposed via generic thermal framework.
>  
> +config X86_THERMAL_VECTOR
> +	def_bool y
> +	depends on X86 && CPU_SUP_INTEL && X86_LOCAL_APIC
> +
>  config X86_PKG_TEMP_THERMAL
>  	tristate "X86 package temperature thermal driver"
>  	depends on X86_THERMAL_VECTOR
> diff --git a/drivers/thermal/intel/Makefile
> b/drivers/thermal/intel/Makefile
> index 0d9736ced5d4..ff2ad30ef397 100644
> --- a/drivers/thermal/intel/Makefile
> +++ b/drivers/thermal/intel/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_INTEL_QUARK_DTS_THERMAL)	+=
> intel_quark_dts_thermal.o
>  obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
>  obj-$(CONFIG_INTEL_BXT_PMIC_THERMAL) += intel_bxt_pmic_thermal.o
>  obj-$(CONFIG_INTEL_PCH_THERMAL)	+= intel_pch_thermal.o
> +obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
> diff --git a/arch/x86/kernel/cpu/mce/therm_throt.c
> b/drivers/thermal/intel/therm_throt.c
> similarity index 97%
> rename from arch/x86/kernel/cpu/mce/therm_throt.c
> rename to drivers/thermal/intel/therm_throt.c
> index 5b15d7cef1d1..6221f0f418f7 100644
> --- a/arch/x86/kernel/cpu/mce/therm_throt.c
> +++ b/drivers/thermal/intel/therm_throt.c
> @@ -26,13 +26,11 @@
>  #include <linux/cpu.h>
>  
>  #include <asm/processor.h>
> +#include <asm/thermal.h>
>  #include <asm/traps.h>
>  #include <asm/apic.h>
> -#include <asm/mce.h>
> +#include <asm/irq.h>
>  #include <asm/msr.h>
> -#include <asm/trace/irq_vectors.h>
> -
> -#include "internal.h"
>  
>  /* How long to wait between reporting thermal events */
>  #define CHECK_INTERVAL		(300 * HZ)
> @@ -570,7 +568,7 @@ static void notify_thresholds(__u64 msr_val)
>  }
>  
>  /* Thermal transition interrupt handler */
> -static void intel_thermal_interrupt(void)
> +void intel_thermal_interrupt(void)
>  {
>  	__u64 msr_val;
>  
> @@ -606,23 +604,6 @@ static void intel_thermal_interrupt(void)
>  	}
>  }
>  
> -static void unexpected_thermal_interrupt(void)
> -{
> -	pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
> -		smp_processor_id());
> -}
> -
> -static void (*smp_thermal_vector)(void) =
> unexpected_thermal_interrupt;
> -
> -DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
> -{
> -	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
> -	inc_irq_stat(irq_thermal_count);
> -	smp_thermal_vector();
> -	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
> -	ack_APIC_irq();
> -}
> -
>  /* Thermal monitoring depends on APIC, ACPI and clock modulation */
>  static int intel_thermal_supported(struct cpuinfo_x86 *c)
>  {
> @@ -633,6 +614,11 @@ static int intel_thermal_supported(struct
> cpuinfo_x86 *c)
>  	return 1;
>  }
>  
> +bool x86_thermal_enabled(void)
> +{
> +	return atomic_read(&therm_throt_en);
> +}
> +
>  void intel_init_thermal(struct cpuinfo_x86 *c)
>  {
>  	unsigned int cpu = smp_processor_id();
> @@ -719,8 +705,6 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
>  				| PACKAGE_THERM_INT_HIGH_ENABLE), h);
>  	}
>  
> -	smp_thermal_vector = intel_thermal_interrupt;
> -
>  	rdmsr(MSR_IA32_MISC_ENABLE, l, h);
>  	wrmsr(MSR_IA32_MISC_ENABLE, l | MSR_IA32_MISC_ENABLE_TM1, h);
>  
> diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> index b81c33202f41..090f9176ba62 100644
> --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -17,8 +17,9 @@
>  #include <linux/pm.h>
>  #include <linux/thermal.h>
>  #include <linux/debugfs.h>
> +
>  #include <asm/cpu_device_id.h>
> -#include <asm/mce.h>
> +#include <asm/thermal.h>
>  
>  /*
>  * Rate control delay: Idea is to introduce denounce effect


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce
  2021-02-01 19:10   ` Srinivas Pandruvada
@ 2021-02-02 12:10     ` Borislav Petkov
  2021-02-02 18:13       ` Srinivas Pandruvada
  0 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2021-02-02 12:10 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: X86 ML, LKML, Peter Zijlstra, Amit Kucheria, Daniel Lezcano,
	Tony Luck, Zhang Rui, linux-pm

On Mon, Feb 01, 2021 at 11:10:29AM -0800, Srinivas Pandruvada wrote:
> Only user of the above interface is in drivers/thermal/intel.
> So why can't we move these to drivers/thermal/intel/thermal_interrupt.h
> or similar?

Sure, see below.

---
From: Borislav Petkov <bp@suse.de>
Date: Thu, 7 Jan 2021 13:29:05 +0100
Subject: [PATCH] thermal: Move therm_throt there from x86/mce

This functionality has nothing to do with MCE, move it to the thermal
framework and untangle it from MCE.

Requested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/Kconfig                              |  4 ---
 arch/x86/include/asm/mce.h                    | 16 ----------
 arch/x86/include/asm/thermal.h                | 13 ++++++++
 arch/x86/kernel/cpu/intel.c                   |  3 ++
 arch/x86/kernel/cpu/mce/Makefile              |  2 --
 arch/x86/kernel/cpu/mce/intel.c               |  1 -
 arch/x86/kernel/irq.c                         | 21 ++++++++++++
 drivers/thermal/intel/Kconfig                 |  4 +++
 drivers/thermal/intel/Makefile                |  1 +
 .../thermal/intel}/therm_throt.c              | 32 ++++++-------------
 drivers/thermal/intel/thermal_interrupt.h     | 15 +++++++++
 drivers/thermal/intel/x86_pkg_temp_thermal.c  |  4 ++-
 12 files changed, 69 insertions(+), 47 deletions(-)
 create mode 100644 arch/x86/include/asm/thermal.h
 rename {arch/x86/kernel/cpu/mce => drivers/thermal/intel}/therm_throt.c (97%)
 create mode 100644 drivers/thermal/intel/thermal_interrupt.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 21f851179ff0..9989db3a9bf5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1158,10 +1158,6 @@ config X86_MCE_INJECT
 	  If you don't know what a machine check is and you don't do kernel
 	  QA it is safe to say n.
 
-config X86_THERMAL_VECTOR
-	def_bool y
-	depends on X86_MCE_INTEL
-
 source "arch/x86/events/Kconfig"
 
 config X86_LEGACY_VM86
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index def9aa5e1fa4..ddfb3cad8dff 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -288,22 +288,6 @@ extern void (*mce_threshold_vector)(void);
 /* Deferred error interrupt handler */
 extern void (*deferred_error_int_vector)(void);
 
-/*
- * Thermal handler
- */
-
-void intel_init_thermal(struct cpuinfo_x86 *c);
-
-/* Interrupt Handler for core thermal thresholds */
-extern int (*platform_thermal_notify)(__u64 msr_val);
-
-/* Interrupt Handler for package thermal thresholds */
-extern int (*platform_thermal_package_notify)(__u64 msr_val);
-
-/* Callback support of rate control, return true, if
- * callback has rate control */
-extern bool (*platform_thermal_package_rate_control)(void);
-
 /*
  * Used by APEI to report memory error via /dev/mcelog
  */
diff --git a/arch/x86/include/asm/thermal.h b/arch/x86/include/asm/thermal.h
new file mode 100644
index 000000000000..ddbdefd5b94f
--- /dev/null
+++ b/arch/x86/include/asm/thermal.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_THERMAL_H
+#define _ASM_X86_THERMAL_H
+
+#ifdef CONFIG_X86_THERMAL_VECTOR
+void intel_init_thermal(struct cpuinfo_x86 *c);
+bool x86_thermal_enabled(void);
+void intel_thermal_interrupt(void);
+#else
+static inline void intel_init_thermal(struct cpuinfo_x86 *c) { }
+#endif
+
+#endif /* _ASM_X86_THERMAL_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 59a1e3ce3f14..71221af87cb1 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -24,6 +24,7 @@
 #include <asm/traps.h>
 #include <asm/resctrl.h>
 #include <asm/numa.h>
+#include <asm/thermal.h>
 
 #ifdef CONFIG_X86_64
 #include <linux/topology.h>
@@ -719,6 +720,8 @@ static void init_intel(struct cpuinfo_x86 *c)
 		tsx_disable();
 
 	split_lock_init();
+
+	intel_init_thermal(c);
 }
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/cpu/mce/Makefile b/arch/x86/kernel/cpu/mce/Makefile
index 9f020c994154..015856abdbb1 100644
--- a/arch/x86/kernel/cpu/mce/Makefile
+++ b/arch/x86/kernel/cpu/mce/Makefile
@@ -9,8 +9,6 @@ obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
 mce-inject-y			:= inject.o
 obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o
 
-obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
-
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
 
 obj-$(CONFIG_X86_MCELOG_LEGACY)	+= dev-mcelog.o
diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index c2476fe0682e..e309476743b7 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -531,7 +531,6 @@ static void intel_imc_init(struct cpuinfo_x86 *c)
 
 void mce_intel_feature_init(struct cpuinfo_x86 *c)
 {
-	intel_init_thermal(c);
 	intel_init_cmci();
 	intel_init_lmce();
 	intel_ppin_init(c);
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index c5dd50369e2f..d4ad344e80bf 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -21,6 +21,7 @@
 #include <asm/hw_irq.h>
 #include <asm/desc.h>
 #include <asm/traps.h>
+#include <asm/thermal.h>
 
 #define CREATE_TRACE_POINTS
 #include <asm/trace/irq_vectors.h>
@@ -374,3 +375,23 @@ void fixup_irqs(void)
 	}
 }
 #endif
+
+#ifdef CONFIG_X86_THERMAL_VECTOR
+static void smp_thermal_vector(void)
+{
+	if (x86_thermal_enabled())
+		intel_thermal_interrupt();
+	else
+		pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
+		       smp_processor_id());
+}
+
+DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
+{
+	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
+	inc_irq_stat(irq_thermal_count);
+	smp_thermal_vector();
+	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
+	ack_APIC_irq();
+}
+#endif
diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
index 8025b21f43fa..ce4f59213c7a 100644
--- a/drivers/thermal/intel/Kconfig
+++ b/drivers/thermal/intel/Kconfig
@@ -8,6 +8,10 @@ config INTEL_POWERCLAMP
 	  enforce idle time which results in more package C-state residency. The
 	  user interface is exposed via generic thermal framework.
 
+config X86_THERMAL_VECTOR
+	def_bool y
+	depends on X86 && CPU_SUP_INTEL && X86_LOCAL_APIC
+
 config X86_PKG_TEMP_THERMAL
 	tristate "X86 package temperature thermal driver"
 	depends on X86_THERMAL_VECTOR
diff --git a/drivers/thermal/intel/Makefile b/drivers/thermal/intel/Makefile
index 0d9736ced5d4..ff2ad30ef397 100644
--- a/drivers/thermal/intel/Makefile
+++ b/drivers/thermal/intel/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_INTEL_QUARK_DTS_THERMAL)	+= intel_quark_dts_thermal.o
 obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
 obj-$(CONFIG_INTEL_BXT_PMIC_THERMAL) += intel_bxt_pmic_thermal.o
 obj-$(CONFIG_INTEL_PCH_THERMAL)	+= intel_pch_thermal.o
+obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
diff --git a/arch/x86/kernel/cpu/mce/therm_throt.c b/drivers/thermal/intel/therm_throt.c
similarity index 97%
rename from arch/x86/kernel/cpu/mce/therm_throt.c
rename to drivers/thermal/intel/therm_throt.c
index 5b15d7cef1d1..f8e882592ba5 100644
--- a/arch/x86/kernel/cpu/mce/therm_throt.c
+++ b/drivers/thermal/intel/therm_throt.c
@@ -26,13 +26,13 @@
 #include <linux/cpu.h>
 
 #include <asm/processor.h>
+#include <asm/thermal.h>
 #include <asm/traps.h>
 #include <asm/apic.h>
-#include <asm/mce.h>
+#include <asm/irq.h>
 #include <asm/msr.h>
-#include <asm/trace/irq_vectors.h>
 
-#include "internal.h"
+#include "thermal_interrupt.h"
 
 /* How long to wait between reporting thermal events */
 #define CHECK_INTERVAL		(300 * HZ)
@@ -570,7 +570,7 @@ static void notify_thresholds(__u64 msr_val)
 }
 
 /* Thermal transition interrupt handler */
-static void intel_thermal_interrupt(void)
+void intel_thermal_interrupt(void)
 {
 	__u64 msr_val;
 
@@ -606,23 +606,6 @@ static void intel_thermal_interrupt(void)
 	}
 }
 
-static void unexpected_thermal_interrupt(void)
-{
-	pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
-		smp_processor_id());
-}
-
-static void (*smp_thermal_vector)(void) = unexpected_thermal_interrupt;
-
-DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
-{
-	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
-	inc_irq_stat(irq_thermal_count);
-	smp_thermal_vector();
-	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
-	ack_APIC_irq();
-}
-
 /* Thermal monitoring depends on APIC, ACPI and clock modulation */
 static int intel_thermal_supported(struct cpuinfo_x86 *c)
 {
@@ -633,6 +616,11 @@ static int intel_thermal_supported(struct cpuinfo_x86 *c)
 	return 1;
 }
 
+bool x86_thermal_enabled(void)
+{
+	return atomic_read(&therm_throt_en);
+}
+
 void intel_init_thermal(struct cpuinfo_x86 *c)
 {
 	unsigned int cpu = smp_processor_id();
@@ -719,8 +707,6 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
 				| PACKAGE_THERM_INT_HIGH_ENABLE), h);
 	}
 
-	smp_thermal_vector = intel_thermal_interrupt;
-
 	rdmsr(MSR_IA32_MISC_ENABLE, l, h);
 	wrmsr(MSR_IA32_MISC_ENABLE, l | MSR_IA32_MISC_ENABLE_TM1, h);
 
diff --git a/drivers/thermal/intel/thermal_interrupt.h b/drivers/thermal/intel/thermal_interrupt.h
new file mode 100644
index 000000000000..53f427bb58dc
--- /dev/null
+++ b/drivers/thermal/intel/thermal_interrupt.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _INTEL_THERMAL_INTERRUPT_H
+#define _INTEL_THERMAL_INTERRUPT_H
+
+/* Interrupt Handler for package thermal thresholds */
+extern int (*platform_thermal_package_notify)(__u64 msr_val);
+
+/* Interrupt Handler for core thermal thresholds */
+extern int (*platform_thermal_notify)(__u64 msr_val);
+
+/* Callback support of rate control, return true, if
+ * callback has rate control */
+extern bool (*platform_thermal_package_rate_control)(void);
+
+#endif /* _INTEL_THERMAL_INTERRUPT_H */
diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
index b81c33202f41..295742e83960 100644
--- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -17,8 +17,10 @@
 #include <linux/pm.h>
 #include <linux/thermal.h>
 #include <linux/debugfs.h>
+
 #include <asm/cpu_device_id.h>
-#include <asm/mce.h>
+
+#include "thermal_interrupt.h"
 
 /*
 * Rate control delay: Idea is to introduce denounce effect
-- 
2.29.2

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce
  2021-02-02 12:10     ` Borislav Petkov
@ 2021-02-02 18:13       ` Srinivas Pandruvada
  0 siblings, 0 replies; 7+ messages in thread
From: Srinivas Pandruvada @ 2021-02-02 18:13 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: X86 ML, LKML, Peter Zijlstra, Amit Kucheria, Daniel Lezcano,
	Tony Luck, Zhang Rui, linux-pm

On Tue, 2021-02-02 at 13:10 +0100, Borislav Petkov wrote:
> On Mon, Feb 01, 2021 at 11:10:29AM -0800, Srinivas Pandruvada wrote:
> > Only user of the above interface is in drivers/thermal/intel.
> > So why can't we move these to
> > drivers/thermal/intel/thermal_interrupt.h
> > or similar?
> 
> Sure, see below.
> 
Thanks.

Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Thu, 7 Jan 2021 13:29:05 +0100
> Subject: [PATCH] thermal: Move therm_throt there from x86/mce
> 
> This functionality has nothing to do with MCE, move it to the thermal
> framework and untangle it from MCE.
> 
> Requested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/Kconfig                              |  4 ---
>  arch/x86/include/asm/mce.h                    | 16 ----------
>  arch/x86/include/asm/thermal.h                | 13 ++++++++
>  arch/x86/kernel/cpu/intel.c                   |  3 ++
>  arch/x86/kernel/cpu/mce/Makefile              |  2 --
>  arch/x86/kernel/cpu/mce/intel.c               |  1 -
>  arch/x86/kernel/irq.c                         | 21 ++++++++++++
>  drivers/thermal/intel/Kconfig                 |  4 +++
>  drivers/thermal/intel/Makefile                |  1 +
>  .../thermal/intel}/therm_throt.c              | 32 ++++++-----------
> --
>  drivers/thermal/intel/thermal_interrupt.h     | 15 +++++++++
>  drivers/thermal/intel/x86_pkg_temp_thermal.c  |  4 ++-
>  12 files changed, 69 insertions(+), 47 deletions(-)
>  create mode 100644 arch/x86/include/asm/thermal.h
>  rename {arch/x86/kernel/cpu/mce =>
> drivers/thermal/intel}/therm_throt.c (97%)
>  create mode 100644 drivers/thermal/intel/thermal_interrupt.h
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 21f851179ff0..9989db3a9bf5 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1158,10 +1158,6 @@ config X86_MCE_INJECT
>  	  If you don't know what a machine check is and you don't do
> kernel
>  	  QA it is safe to say n.
>  
> -config X86_THERMAL_VECTOR
> -	def_bool y
> -	depends on X86_MCE_INTEL
> -
>  source "arch/x86/events/Kconfig"
>  
>  config X86_LEGACY_VM86
> diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> index def9aa5e1fa4..ddfb3cad8dff 100644
> --- a/arch/x86/include/asm/mce.h
> +++ b/arch/x86/include/asm/mce.h
> @@ -288,22 +288,6 @@ extern void (*mce_threshold_vector)(void);
>  /* Deferred error interrupt handler */
>  extern void (*deferred_error_int_vector)(void);
>  
> -/*
> - * Thermal handler
> - */
> -
> -void intel_init_thermal(struct cpuinfo_x86 *c);
> -
> -/* Interrupt Handler for core thermal thresholds */
> -extern int (*platform_thermal_notify)(__u64 msr_val);
> -
> -/* Interrupt Handler for package thermal thresholds */
> -extern int (*platform_thermal_package_notify)(__u64 msr_val);
> -
> -/* Callback support of rate control, return true, if
> - * callback has rate control */
> -extern bool (*platform_thermal_package_rate_control)(void);
> -
>  /*
>   * Used by APEI to report memory error via /dev/mcelog
>   */
> diff --git a/arch/x86/include/asm/thermal.h
> b/arch/x86/include/asm/thermal.h
> new file mode 100644
> index 000000000000..ddbdefd5b94f
> --- /dev/null
> +++ b/arch/x86/include/asm/thermal.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_THERMAL_H
> +#define _ASM_X86_THERMAL_H
> +
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +void intel_init_thermal(struct cpuinfo_x86 *c);
> +bool x86_thermal_enabled(void);
> +void intel_thermal_interrupt(void);
> +#else
> +static inline void intel_init_thermal(struct cpuinfo_x86 *c) { }
> +#endif
> +
> +#endif /* _ASM_X86_THERMAL_H */
> diff --git a/arch/x86/kernel/cpu/intel.c
> b/arch/x86/kernel/cpu/intel.c
> index 59a1e3ce3f14..71221af87cb1 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -24,6 +24,7 @@
>  #include <asm/traps.h>
>  #include <asm/resctrl.h>
>  #include <asm/numa.h>
> +#include <asm/thermal.h>
>  
>  #ifdef CONFIG_X86_64
>  #include <linux/topology.h>
> @@ -719,6 +720,8 @@ static void init_intel(struct cpuinfo_x86 *c)
>  		tsx_disable();
>  
>  	split_lock_init();
> +
> +	intel_init_thermal(c);
>  }
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/kernel/cpu/mce/Makefile
> b/arch/x86/kernel/cpu/mce/Makefile
> index 9f020c994154..015856abdbb1 100644
> --- a/arch/x86/kernel/cpu/mce/Makefile
> +++ b/arch/x86/kernel/cpu/mce/Makefile
> @@ -9,8 +9,6 @@ obj-$(CONFIG_X86_MCE_THRESHOLD) += threshold.o
>  mce-inject-y			:= inject.o
>  obj-$(CONFIG_X86_MCE_INJECT)	+= mce-inject.o
>  
> -obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
> -
>  obj-$(CONFIG_ACPI_APEI)		+= apei.o
>  
>  obj-$(CONFIG_X86_MCELOG_LEGACY)	+= dev-mcelog.o
> diff --git a/arch/x86/kernel/cpu/mce/intel.c
> b/arch/x86/kernel/cpu/mce/intel.c
> index c2476fe0682e..e309476743b7 100644
> --- a/arch/x86/kernel/cpu/mce/intel.c
> +++ b/arch/x86/kernel/cpu/mce/intel.c
> @@ -531,7 +531,6 @@ static void intel_imc_init(struct cpuinfo_x86 *c)
>  
>  void mce_intel_feature_init(struct cpuinfo_x86 *c)
>  {
> -	intel_init_thermal(c);
>  	intel_init_cmci();
>  	intel_init_lmce();
>  	intel_ppin_init(c);
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index c5dd50369e2f..d4ad344e80bf 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -21,6 +21,7 @@
>  #include <asm/hw_irq.h>
>  #include <asm/desc.h>
>  #include <asm/traps.h>
> +#include <asm/thermal.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <asm/trace/irq_vectors.h>
> @@ -374,3 +375,23 @@ void fixup_irqs(void)
>  	}
>  }
>  #endif
> +
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +static void smp_thermal_vector(void)
> +{
> +	if (x86_thermal_enabled())
> +		intel_thermal_interrupt();
> +	else
> +		pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
> +		       smp_processor_id());
> +}
> +
> +DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
> +{
> +	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
> +	inc_irq_stat(irq_thermal_count);
> +	smp_thermal_vector();
> +	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
> +	ack_APIC_irq();
> +}
> +#endif
> diff --git a/drivers/thermal/intel/Kconfig
> b/drivers/thermal/intel/Kconfig
> index 8025b21f43fa..ce4f59213c7a 100644
> --- a/drivers/thermal/intel/Kconfig
> +++ b/drivers/thermal/intel/Kconfig
> @@ -8,6 +8,10 @@ config INTEL_POWERCLAMP
>  	  enforce idle time which results in more package C-state
> residency. The
>  	  user interface is exposed via generic thermal framework.
>  
> +config X86_THERMAL_VECTOR
> +	def_bool y
> +	depends on X86 && CPU_SUP_INTEL && X86_LOCAL_APIC
> +
>  config X86_PKG_TEMP_THERMAL
>  	tristate "X86 package temperature thermal driver"
>  	depends on X86_THERMAL_VECTOR
> diff --git a/drivers/thermal/intel/Makefile
> b/drivers/thermal/intel/Makefile
> index 0d9736ced5d4..ff2ad30ef397 100644
> --- a/drivers/thermal/intel/Makefile
> +++ b/drivers/thermal/intel/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_INTEL_QUARK_DTS_THERMAL)	+=
> intel_quark_dts_thermal.o
>  obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
>  obj-$(CONFIG_INTEL_BXT_PMIC_THERMAL) += intel_bxt_pmic_thermal.o
>  obj-$(CONFIG_INTEL_PCH_THERMAL)	+= intel_pch_thermal.o
> +obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
> diff --git a/arch/x86/kernel/cpu/mce/therm_throt.c
> b/drivers/thermal/intel/therm_throt.c
> similarity index 97%
> rename from arch/x86/kernel/cpu/mce/therm_throt.c
> rename to drivers/thermal/intel/therm_throt.c
> index 5b15d7cef1d1..f8e882592ba5 100644
> --- a/arch/x86/kernel/cpu/mce/therm_throt.c
> +++ b/drivers/thermal/intel/therm_throt.c
> @@ -26,13 +26,13 @@
>  #include <linux/cpu.h>
>  
>  #include <asm/processor.h>
> +#include <asm/thermal.h>
>  #include <asm/traps.h>
>  #include <asm/apic.h>
> -#include <asm/mce.h>
> +#include <asm/irq.h>
>  #include <asm/msr.h>
> -#include <asm/trace/irq_vectors.h>
>  
> -#include "internal.h"
> +#include "thermal_interrupt.h"
>  
>  /* How long to wait between reporting thermal events */
>  #define CHECK_INTERVAL		(300 * HZ)
> @@ -570,7 +570,7 @@ static void notify_thresholds(__u64 msr_val)
>  }
>  
>  /* Thermal transition interrupt handler */
> -static void intel_thermal_interrupt(void)
> +void intel_thermal_interrupt(void)
>  {
>  	__u64 msr_val;
>  
> @@ -606,23 +606,6 @@ static void intel_thermal_interrupt(void)
>  	}
>  }
>  
> -static void unexpected_thermal_interrupt(void)
> -{
> -	pr_err("CPU%d: Unexpected LVT thermal interrupt!\n",
> -		smp_processor_id());
> -}
> -
> -static void (*smp_thermal_vector)(void) =
> unexpected_thermal_interrupt;
> -
> -DEFINE_IDTENTRY_SYSVEC(sysvec_thermal)
> -{
> -	trace_thermal_apic_entry(THERMAL_APIC_VECTOR);
> -	inc_irq_stat(irq_thermal_count);
> -	smp_thermal_vector();
> -	trace_thermal_apic_exit(THERMAL_APIC_VECTOR);
> -	ack_APIC_irq();
> -}
> -
>  /* Thermal monitoring depends on APIC, ACPI and clock modulation */
>  static int intel_thermal_supported(struct cpuinfo_x86 *c)
>  {
> @@ -633,6 +616,11 @@ static int intel_thermal_supported(struct
> cpuinfo_x86 *c)
>  	return 1;
>  }
>  
> +bool x86_thermal_enabled(void)
> +{
> +	return atomic_read(&therm_throt_en);
> +}
> +
>  void intel_init_thermal(struct cpuinfo_x86 *c)
>  {
>  	unsigned int cpu = smp_processor_id();
> @@ -719,8 +707,6 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
>  				| PACKAGE_THERM_INT_HIGH_ENABLE), h);
>  	}
>  
> -	smp_thermal_vector = intel_thermal_interrupt;
> -
>  	rdmsr(MSR_IA32_MISC_ENABLE, l, h);
>  	wrmsr(MSR_IA32_MISC_ENABLE, l | MSR_IA32_MISC_ENABLE_TM1, h);
>  
> diff --git a/drivers/thermal/intel/thermal_interrupt.h
> b/drivers/thermal/intel/thermal_interrupt.h
> new file mode 100644
> index 000000000000..53f427bb58dc
> --- /dev/null
> +++ b/drivers/thermal/intel/thermal_interrupt.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _INTEL_THERMAL_INTERRUPT_H
> +#define _INTEL_THERMAL_INTERRUPT_H
> +
> +/* Interrupt Handler for package thermal thresholds */
> +extern int (*platform_thermal_package_notify)(__u64 msr_val);
> +
> +/* Interrupt Handler for core thermal thresholds */
> +extern int (*platform_thermal_notify)(__u64 msr_val);
> +
> +/* Callback support of rate control, return true, if
> + * callback has rate control */
> +extern bool (*platform_thermal_package_rate_control)(void);
> +
> +#endif /* _INTEL_THERMAL_INTERRUPT_H */
> diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> index b81c33202f41..295742e83960 100644
> --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -17,8 +17,10 @@
>  #include <linux/pm.h>
>  #include <linux/thermal.h>
>  #include <linux/debugfs.h>
> +
>  #include <asm/cpu_device_id.h>
> -#include <asm/mce.h>
> +
> +#include "thermal_interrupt.h"
>  
>  /*
>  * Rate control delay: Idea is to introduce denounce effect
> -- 
> 2.29.2
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-02-02 18:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 14:27 [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Borislav Petkov
2021-02-01 14:27 ` [PATCH v3 1/2] x86/mce: Get rid of mcheck_intel_therm_init() Borislav Petkov
2021-02-01 14:27 ` [PATCH v3 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
2021-02-01 19:10   ` Srinivas Pandruvada
2021-02-02 12:10     ` Borislav Petkov
2021-02-02 18:13       ` Srinivas Pandruvada
2021-02-01 19:10 ` [PATCH v3 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Srinivas Pandruvada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).