2009-12-13 02:09:58

by Cyrill Gorcunov

[permalink] [raw]
Subject: [PATCH resend -tip] x86,mcheck: Thermal monitoring depends on APIC being enabled

Add check if APIC is not disabled since thermal
monitoring depends on it. As only apic gets disabled
we should not try to install "thermal monitor" vector,
print out that thermal monitoring is enabled and etc...

Note that "Intel Correct Machine Check Interrupts" already
has such a check.

Also I decided to not add cpu_has_apic check into mcheck_intel_therm_init
since even if it'll call apic_read on disabled apic -- it's safe
here and allow us to save a few code bytes.

Reported-by: Thomas Gleixner <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
---

Please review. This should close a number of WARNs triggered
by intel_init_thermal when apic gets disabled state.

Ie the WARNs like

http://kerneloops.org/oops.php?number=974702

arch/x86/kernel/cpu/mcheck/therm_throt.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/cpu/mcheck/therm_throt.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ linux-2.6.git/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -274,8 +274,9 @@ void intel_init_thermal(struct cpuinfo_x
int tm2 = 0;
u32 l, h;

- /* Thermal monitoring depends on ACPI and clock modulation*/
- if (!cpu_has(c, X86_FEATURE_ACPI) || !cpu_has(c, X86_FEATURE_ACC))
+ /* Thermal monitoring depends on APIC, ACPI and clock modulation */
+ if (!cpu_has_apic || !cpu_has(c, X86_FEATURE_ACPI) ||
+ !cpu_has(c, X86_FEATURE_ACC))
return;

/*


2009-12-14 01:23:58

by Hidetoshi Seto

[permalink] [raw]
Subject: Re: [PATCH resend -tip] x86,mcheck: Thermal monitoring depends on APIC being enabled

(2009/12/12 23:31), Cyrill Gorcunov wrote:
> Add check if APIC is not disabled since thermal
> monitoring depends on it. As only apic gets disabled
> we should not try to install "thermal monitor" vector,
> print out that thermal monitoring is enabled and etc...
>
> Note that "Intel Correct Machine Check Interrupts" already
> has such a check.
>
> Also I decided to not add cpu_has_apic check into mcheck_intel_therm_init
> since even if it'll call apic_read on disabled apic -- it's safe
> here and allow us to save a few code bytes.
>
> Reported-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> ---
>
> Please review. This should close a number of WARNs triggered
> by intel_init_thermal when apic gets disabled state.
>
> Ie the WARNs like
>
> http://kerneloops.org/oops.php?number=974702
>
> arch/x86/kernel/cpu/mcheck/therm_throt.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)

Looks good.
Reviewed-by: Hidetoshi Seto <[email protected]>


And I think that the following incremental patch is reasonable
to apply on it.


Thanks,
H.Seto

===
Subject: [PATCH] x86, mce: introduce intel_thermal_supported

Have a common function.

Signed-off-by: Hidetoshi Seto <[email protected]>
---
arch/x86/kernel/cpu/mcheck/therm_throt.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index 687638e..4f4c8bb 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -256,6 +256,16 @@ asmlinkage void smp_thermal_interrupt(struct pt_regs *regs)
ack_APIC_irq();
}

+/* Thermal monitoring depends on APIC, ACPI and clock modulation */
+static int intel_thermal_supported(struct cpuinfo_x86 *c)
+{
+ if (!cpu_has_apic)
+ return 0;
+ if (!cpu_has(c, X86_FEATURE_ACPI) || !cpu_has(c, X86_FEATURE_ACC))
+ return 0;
+ return 1;
+}
+
void __init mcheck_intel_therm_init(void)
{
/*
@@ -263,8 +273,7 @@ void __init mcheck_intel_therm_init(void)
* LVT value on BSP and use that value to restore APs' thermal LVT
* entry BIOS programmed later
*/
- if (cpu_has(&boot_cpu_data, X86_FEATURE_ACPI) &&
- cpu_has(&boot_cpu_data, X86_FEATURE_ACC))
+ if (intel_thermal_supported(&boot_cpu_data))
lvtthmr_init = apic_read(APIC_LVTTHMR);
}

@@ -274,9 +283,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
int tm2 = 0;
u32 l, h;

- /* Thermal monitoring depends on APIC, ACPI and clock modulation */
- if (!cpu_has_apic || !cpu_has(c, X86_FEATURE_ACPI) ||
- !cpu_has(c, X86_FEATURE_ACC))
+ if (!intel_thermal_supported(c))
return;

/*
--
1.6.5.5