* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
@ 2021-02-09 2:27 Sergey Senozhatsky
2021-02-09 10:16 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: Sergey Senozhatsky @ 2021-02-09 2:27 UTC (permalink / raw)
To: Borislav Petkov
Cc: Peter Zijlstra, Zhang Rui, linux-pm, LKML, Amit Kucheria,
Daniel Lezcano, X86 ML
Hi,
Seems that the patch triggers some WARNs on my laptop.
For every CPU:
[ 0.003751] WARNING: CPU: 4 PID: 0 at arch/x86/kernel/irq.c:390 thermal_set_handler+0x12/0x25
[ 0.003751] Modules linked in:
[ 0.003751] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G W 5.11.0-rc6-next-20210208-00003-g3ba4c4f662ad-dirty #1928
[ 0.003751] RIP: 0010:thermal_set_handler+0x12/0x25
[ 0.003751] RSP: 0000:ffffb5f0c00c7ed8 EFLAGS: 00010097
[ 0.003751] RAX: 0000000000000003 RBX: 0000000000000000 RCX: 00000000000001b2
[ 0.003751] RDX: 0000000000000000 RSI: 0000000000000003 RDI: ffffffff98410d00
[ 0.003751] RBP: ffff9e3c5fb11460 R08: 0000000000000000 R09: 000000000003007f
[ 0.003751] R10: ffff9e3c5fb11480 R11: 0000000000000000 R12: 0000000000000428
[ 0.003751] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 0.003751] FS: 0000000000000000(0000) GS:ffff9e3c5fb00000(0000) knlGS:0000000000000000
[ 0.003751] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.003751] CR2: 0000000000000000 CR3: 000000039ee0a001 CR4: 00000000001706a0
[ 0.003751] Call Trace:
[ 0.003751] intel_init_thermal+0x16d/0x1c7
[ 0.003751] identify_cpu+0x249/0x329
[ 0.003751] identify_secondary_cpu+0x15/0x8c
[ 0.003751] smp_store_cpu_info+0x3f/0x48
[ 0.003751] start_secondary+0x42/0xfd
[ 0.003751] secondary_startup_64_no_verify+0xb0/0xbb
-ss
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-02-09 2:27 [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce Sergey Senozhatsky
@ 2021-02-09 10:16 ` Borislav Petkov
0 siblings, 0 replies; 9+ messages in thread
From: Borislav Petkov @ 2021-02-09 10:16 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Peter Zijlstra, Zhang Rui, linux-pm, LKML, Amit Kucheria,
Daniel Lezcano, X86 ML
Hi,
On Tue, Feb 09, 2021 at 11:27:57AM +0900, Sergey Senozhatsky wrote:
> Seems that the patch triggers some WARNs on my laptop.
yeah, that one is replaced with a better variant now and it should land
in the next linux-next, the one after next-20210208 :)
Thx.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 13:05 ` [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
2021-01-25 15:42 ` Zhang Rui
@ 2021-01-27 11:07 ` Zhang Rui
1 sibling, 0 replies; 9+ messages in thread
From: Zhang Rui @ 2021-01-27 11:07 UTC (permalink / raw)
To: Borislav Petkov, X86 ML
Cc: Daniel Lezcano, Amit Kucheria, linux-pm, LKML, Peter Zijlstra
On Mon, 2021-01-25 at 14:05 +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.
>
> Have thermal_set_handler() check the build-time assigned default
> handler
> stub was the one used before therm_throt assigns a new one.
>
> Requested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Zhang Rui <rui.zhang@intel.com>
thanks,
rui
> ---
> arch/x86/Kconfig | 4 ---
> arch/x86/include/asm/irq.h | 4 +++
> arch/x86/include/asm/mce.h | 16 ----------
> arch/x86/include/asm/thermal.h | 21 ++++++++++++++
> 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 | 29
> +++++++++++++++++++
> drivers/thermal/intel/Kconfig | 4 +++
> drivers/thermal/intel/Makefile | 1 +
> .../thermal/intel}/therm_throt.c | 25 ++--------------
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 3 +-
> 12 files changed, 67 insertions(+), 46 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/irq.h b/arch/x86/include/asm/irq.h
> index 528c8a71fe7f..ad65fe7dceb1 100644
> --- a/arch/x86/include/asm/irq.h
> +++ b/arch/x86/include/asm/irq.h
> @@ -53,4 +53,8 @@ void arch_trigger_cpumask_backtrace(const struct
> cpumask *mask,
> #define arch_trigger_cpumask_backtrace
> arch_trigger_cpumask_backtrace
> #endif
>
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +void thermal_set_handler(void (*handler)(void));
> +#endif
> +
> #endif /* _ASM_X86_IRQ_H */
> 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..58b0e0a4af6e
> --- /dev/null
> +++ b/arch/x86/include/asm/thermal.h
> @@ -0,0 +1,21 @@
> +/* 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);
> +#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..523fa5266d3e 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -374,3 +374,32 @@ void fixup_irqs(void)
> }
> }
> #endif
> +
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +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;
> +
> +void thermal_set_handler(void (*handler)(void))
> +{
> + if (handler) {
> + WARN_ON(smp_thermal_vector !=
> unexpected_thermal_interrupt);
> + smp_thermal_vector = handler;
> + } else {
> + smp_thermal_vector = 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();
> +}
> +#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 5b1aa0f30655..4f12fcd0e40a 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)
> @@ -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)
> {
> @@ -718,7 +699,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> | PACKAGE_THERM_INT_HIGH_ENABLE), h);
> }
>
> - smp_thermal_vector = intel_thermal_interrupt;
> + thermal_set_handler(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] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 17:18 ` Borislav Petkov
@ 2021-01-25 17:25 ` Srinivas Pandruvada
0 siblings, 0 replies; 9+ messages in thread
From: Srinivas Pandruvada @ 2021-01-25 17:25 UTC (permalink / raw)
To: Borislav Petkov
Cc: Zhang Rui, X86 ML, Daniel Lezcano, Amit Kucheria, linux-pm, LKML,
Peter Zijlstra
On Mon, 2021-01-25 at 18:18 +0100, Borislav Petkov wrote:
> On Mon, Jan 25, 2021 at 09:14:35AM -0800, Srinivas Pandruvada wrote:
> > Can the handler, processing architectural features via thermal
> > interrupt, reside in arch/x86 folder or need to be
> > drivers/thermal/intel?
>
> Look at...
>
> > > > @@ -718,7 +699,7 @@ void intel_init_thermal(struct cpuinfo_x86
> > > > *c)
> > > > |
> > > > PACKAGE_THERM_INT_HIGH_ENABLE), h);
> > > > }
> > > >
> > > > - smp_thermal_vector = intel_thermal_interrupt;
> > > > + thermal_set_handler(intel_thermal_interrupt);
>
> ... here. ^
>
> This should answer your question.
Thanks for the answer.
-Srinivas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 17:14 ` Srinivas Pandruvada
@ 2021-01-25 17:18 ` Borislav Petkov
2021-01-25 17:25 ` Srinivas Pandruvada
0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2021-01-25 17:18 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Zhang Rui, X86 ML, Daniel Lezcano, Amit Kucheria, linux-pm, LKML,
Peter Zijlstra
On Mon, Jan 25, 2021 at 09:14:35AM -0800, Srinivas Pandruvada wrote:
> Can the handler, processing architectural features via thermal
> interrupt, reside in arch/x86 folder or need to be
> drivers/thermal/intel?
Look at...
> > > @@ -718,7 +699,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> > > | PACKAGE_THERM_INT_HIGH_ENABLE), h);
> > > }
> > >
> > > - smp_thermal_vector = intel_thermal_interrupt;
> > > + thermal_set_handler(intel_thermal_interrupt);
... here. ^
This should answer your question.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 15:42 ` Zhang Rui
2021-01-25 16:47 ` Borislav Petkov
@ 2021-01-25 17:14 ` Srinivas Pandruvada
2021-01-25 17:18 ` Borislav Petkov
1 sibling, 1 reply; 9+ messages in thread
From: Srinivas Pandruvada @ 2021-01-25 17:14 UTC (permalink / raw)
To: Zhang Rui, Borislav Petkov, X86 ML
Cc: Daniel Lezcano, Amit Kucheria, linux-pm, LKML, Peter Zijlstra
Hi All,
On Mon, 2021-01-25 at 23:42 +0800, Zhang Rui wrote:
> Hi, Borislav,
>
> Thanks for the patch. CC Srinivas.
>
Thanks for adding me.
> On Mon, 2021-01-25 at 14:05 +0100, Borislav Petkov wrote:
> > From: Borislav Petkov <bp@suse.de>
> >
> > This functionality has nothing to do with MCE, move it to the
> > thermal(unfortunately)
> > framework and untangle it from MCE.
> >
Agreed.
But I have a question on how we should handle some new features.
Although interrupt handled in this code is called "Thermal interrupt",
this interrupt is also used for notification for some other power
/energy efficiency optimization events via IA32_THERM_STATUS and
IA32_PACKAGE_THERM_STATUS.
For example (From SDM):
IA32_PACKAGE_THERM_STATUS
Bit 26: Hardware Feedback Interface Structure Change Status
Can the handler, processing architectural features via thermal
interrupt, reside in arch/x86 folder or need to be
drivers/thermal/intel?
> Agreed.
>
> just one question,
> there are many overlaps between this kernel thermal throttling code
> and
> the x86_pkg_temp_thermal driver, is it possible to combine these two
> pieces of code altogether?
May not be a good idea as the events are not just for temperature
notifications.
Thanks,
Srinivas
>
> thanks,
> rui
>
>
> > Have thermal_set_handler() check the build-time assigned default
> > handler
> > stub was the one used before therm_throt assigns a new one.
> >
> > Requested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Borislav Petkov <bp@suse.de>
> > ---
> > arch/x86/Kconfig | 4 ---
> > arch/x86/include/asm/irq.h | 4 +++
> > arch/x86/include/asm/mce.h | 16 ----------
> > arch/x86/include/asm/thermal.h | 21 ++++++++++++++
> > 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 | 29
> > +++++++++++++++++++
> > drivers/thermal/intel/Kconfig | 4 +++
> > drivers/thermal/intel/Makefile | 1 +
> > .../thermal/intel}/therm_throt.c | 25 ++-------------
> > -
> > drivers/thermal/intel/x86_pkg_temp_thermal.c | 3 +-
> > 12 files changed, 67 insertions(+), 46 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/irq.h
> > b/arch/x86/include/asm/irq.h
> > index 528c8a71fe7f..ad65fe7dceb1 100644
> > --- a/arch/x86/include/asm/irq.h
> > +++ b/arch/x86/include/asm/irq.h
> > @@ -53,4 +53,8 @@ void arch_trigger_cpumask_backtrace(const struct
> > cpumask *mask,
> > #define arch_trigger_cpumask_backtrace
> > arch_trigger_cpumask_backtrace
> > #endif
> >
> > +#ifdef CONFIG_X86_THERMAL_VECTOR
> > +void thermal_set_handler(void (*handler)(void));
> > +#endif
> > +
> > #endif /* _ASM_X86_IRQ_H */
> > 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..58b0e0a4af6e
> > --- /dev/null
> > +++ b/arch/x86/include/asm/thermal.h
> > @@ -0,0 +1,21 @@
> > +/* 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);
> > +#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..523fa5266d3e 100644
> > --- a/arch/x86/kernel/irq.c
> > +++ b/arch/x86/kernel/irq.c
> > @@ -374,3 +374,32 @@ void fixup_irqs(void)
> > }
> > }
> > #endif
> > +
> > +#ifdef CONFIG_X86_THERMAL_VECTOR
> > +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;
> > +
> > +void thermal_set_handler(void (*handler)(void))
> > +{
> > + if (handler) {
> > + WARN_ON(smp_thermal_vector !=
> > unexpected_thermal_interrupt);
> > + smp_thermal_vector = handler;
> > + } else {
> > + smp_thermal_vector = 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();
> > +}
> > +#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 5b1aa0f30655..4f12fcd0e40a 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)
> > @@ -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)
> > {
> > @@ -718,7 +699,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> > | PACKAGE_THERM_INT_HIGH_ENABLE), h);
> > }
> >
> > - smp_thermal_vector = intel_thermal_interrupt;
> > + thermal_set_handler(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] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 15:42 ` Zhang Rui
@ 2021-01-25 16:47 ` Borislav Petkov
2021-01-25 17:14 ` Srinivas Pandruvada
1 sibling, 0 replies; 9+ messages in thread
From: Borislav Petkov @ 2021-01-25 16:47 UTC (permalink / raw)
To: Zhang Rui
Cc: X86 ML, Daniel Lezcano, Amit Kucheria, linux-pm, LKML,
Peter Zijlstra, Srinivas Pandruvada
On Mon, Jan 25, 2021 at 11:42:14PM +0800, Zhang Rui wrote:
> Agreed.
Ok I'll queue the stuff in tip if there are no objections. It is a lot
easier this way...
> just one question,
> there are many overlaps between this kernel thermal throttling code and
> the x86_pkg_temp_thermal driver, is it possible to combine these two
> pieces of code altogether?
You've CCed the right guy - I think Srinivas should be able to answer
that question.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 13:05 ` [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
@ 2021-01-25 15:42 ` Zhang Rui
2021-01-25 16:47 ` Borislav Petkov
2021-01-25 17:14 ` Srinivas Pandruvada
2021-01-27 11:07 ` Zhang Rui
1 sibling, 2 replies; 9+ messages in thread
From: Zhang Rui @ 2021-01-25 15:42 UTC (permalink / raw)
To: Borislav Petkov, X86 ML
Cc: Daniel Lezcano, Amit Kucheria, linux-pm, LKML, Peter Zijlstra,
Srinivas Pandruvada
Hi, Borislav,
Thanks for the patch. CC Srinivas.
On Mon, 2021-01-25 at 14:05 +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.
>
Agreed.
just one question,
there are many overlaps between this kernel thermal throttling code and
the x86_pkg_temp_thermal driver, is it possible to combine these two
pieces of code altogether?
thanks,
rui
> Have thermal_set_handler() check the build-time assigned default
> handler
> stub was the one used before therm_throt assigns a new one.
>
> Requested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> arch/x86/Kconfig | 4 ---
> arch/x86/include/asm/irq.h | 4 +++
> arch/x86/include/asm/mce.h | 16 ----------
> arch/x86/include/asm/thermal.h | 21 ++++++++++++++
> 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 | 29
> +++++++++++++++++++
> drivers/thermal/intel/Kconfig | 4 +++
> drivers/thermal/intel/Makefile | 1 +
> .../thermal/intel}/therm_throt.c | 25 ++--------------
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 3 +-
> 12 files changed, 67 insertions(+), 46 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/irq.h b/arch/x86/include/asm/irq.h
> index 528c8a71fe7f..ad65fe7dceb1 100644
> --- a/arch/x86/include/asm/irq.h
> +++ b/arch/x86/include/asm/irq.h
> @@ -53,4 +53,8 @@ void arch_trigger_cpumask_backtrace(const struct
> cpumask *mask,
> #define arch_trigger_cpumask_backtrace
> arch_trigger_cpumask_backtrace
> #endif
>
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +void thermal_set_handler(void (*handler)(void));
> +#endif
> +
> #endif /* _ASM_X86_IRQ_H */
> 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..58b0e0a4af6e
> --- /dev/null
> +++ b/arch/x86/include/asm/thermal.h
> @@ -0,0 +1,21 @@
> +/* 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);
> +#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..523fa5266d3e 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -374,3 +374,32 @@ void fixup_irqs(void)
> }
> }
> #endif
> +
> +#ifdef CONFIG_X86_THERMAL_VECTOR
> +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;
> +
> +void thermal_set_handler(void (*handler)(void))
> +{
> + if (handler) {
> + WARN_ON(smp_thermal_vector !=
> unexpected_thermal_interrupt);
> + smp_thermal_vector = handler;
> + } else {
> + smp_thermal_vector = 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();
> +}
> +#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 5b1aa0f30655..4f12fcd0e40a 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)
> @@ -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)
> {
> @@ -718,7 +699,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> | PACKAGE_THERM_INT_HIGH_ENABLE), h);
> }
>
> - smp_thermal_vector = intel_thermal_interrupt;
> + thermal_set_handler(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] 9+ messages in thread
* [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce
2021-01-25 13:05 [PATCH v2 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Borislav Petkov
@ 2021-01-25 13:05 ` Borislav Petkov
2021-01-25 15:42 ` Zhang Rui
2021-01-27 11:07 ` Zhang Rui
0 siblings, 2 replies; 9+ messages in thread
From: Borislav Petkov @ 2021-01-25 13:05 UTC (permalink / raw)
To: X86 ML
Cc: Zhang Rui, Daniel Lezcano, Amit Kucheria, linux-pm, LKML, Peter Zijlstra
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.
Have thermal_set_handler() check the build-time assigned default handler
stub was the one used before therm_throt assigns a new one.
Requested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/Kconfig | 4 ---
arch/x86/include/asm/irq.h | 4 +++
arch/x86/include/asm/mce.h | 16 ----------
arch/x86/include/asm/thermal.h | 21 ++++++++++++++
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 | 29 +++++++++++++++++++
drivers/thermal/intel/Kconfig | 4 +++
drivers/thermal/intel/Makefile | 1 +
.../thermal/intel}/therm_throt.c | 25 ++--------------
drivers/thermal/intel/x86_pkg_temp_thermal.c | 3 +-
12 files changed, 67 insertions(+), 46 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/irq.h b/arch/x86/include/asm/irq.h
index 528c8a71fe7f..ad65fe7dceb1 100644
--- a/arch/x86/include/asm/irq.h
+++ b/arch/x86/include/asm/irq.h
@@ -53,4 +53,8 @@ void arch_trigger_cpumask_backtrace(const struct cpumask *mask,
#define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
#endif
+#ifdef CONFIG_X86_THERMAL_VECTOR
+void thermal_set_handler(void (*handler)(void));
+#endif
+
#endif /* _ASM_X86_IRQ_H */
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..58b0e0a4af6e
--- /dev/null
+++ b/arch/x86/include/asm/thermal.h
@@ -0,0 +1,21 @@
+/* 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);
+#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..523fa5266d3e 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -374,3 +374,32 @@ void fixup_irqs(void)
}
}
#endif
+
+#ifdef CONFIG_X86_THERMAL_VECTOR
+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;
+
+void thermal_set_handler(void (*handler)(void))
+{
+ if (handler) {
+ WARN_ON(smp_thermal_vector != unexpected_thermal_interrupt);
+ smp_thermal_vector = handler;
+ } else {
+ smp_thermal_vector = 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();
+}
+#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 5b1aa0f30655..4f12fcd0e40a 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)
@@ -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)
{
@@ -718,7 +699,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
| PACKAGE_THERM_INT_HIGH_ENABLE), h);
}
- smp_thermal_vector = intel_thermal_interrupt;
+ thermal_set_handler(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] 9+ messages in thread
end of thread, other threads:[~2021-02-09 10:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 2:27 [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce Sergey Senozhatsky
2021-02-09 10:16 ` Borislav Petkov
-- strict thread matches above, loose matches on Subject: below --
2021-01-25 13:05 [PATCH v2 0/2] Move ...mce/therm_throt.c to drivers/thermal/ Borislav Petkov
2021-01-25 13:05 ` [PATCH v2 2/2] thermal: Move therm_throt there from x86/mce Borislav Petkov
2021-01-25 15:42 ` Zhang Rui
2021-01-25 16:47 ` Borislav Petkov
2021-01-25 17:14 ` Srinivas Pandruvada
2021-01-25 17:18 ` Borislav Petkov
2021-01-25 17:25 ` Srinivas Pandruvada
2021-01-27 11:07 ` Zhang Rui
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).