linux-kernel - Re: [PATCH 1/2] apic: Fix error interrupt report at all APs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 06 Jan 2011 18:01:57 -0800
From:	Suresh Siddha <suresh.b.siddha@...el.com>
To:	Youquan Song <youquan.song@...ux.intel.com>
Cc:	Yong Wang <yong.y.wang@...ux.intel.com>,
	"Song, Youquan" <youquan.song@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"hpa@...ux.intel.com" <hpa@...ux.intel.com>,
	"arjan@...ux.intel.com" <arjan@...ux.intel.com>,
	"trenn@...e.de" <trenn@...e.de>, "Liu, Kent" <kent.liu@...el.com>,
	"Guo, Chaohong" <chaohong.guo@...el.com>
Subject: Re: [PATCH 1/2] apic: Fix error interrupt report at all APs

On Thu, 2011-01-06 at 18:04 -0800, Youquan Song wrote:
> > If the thermal interrupt vector of all CPUs is 0 and you are seeing
> > LAPIC error interrupts, the correct way to fix it is to disable digital
> > thermal sensors from generating interrupts.
> Thanks for your feedback Yong!
> 
> No. Write LVT with vector(0~15) will always generate error interrupt if error
> interrupt is set. it has nothing with digital thermal device.
> Since you find some platform will cause issue because BIOS possible take
> over the thermal interrupt.
> 
> So my prefered patch is like following, move the APIC_LVTTHMR restore to
> vecotor check domain, which following your origin logic.
> 
> Can I consider BIOS under control thermal interrupt will not be illegal vector?

In that case, bios will specify the delivery mode as SMI and the vector
field will be (most likely) set to zero. So what you need to do is,
check if the delivery mode is !fixed and then restore the LVTTHMR to
that value.

> 
> diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> index 4b68326..a2cb9bd 100644
> --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> @@ -405,30 +405,28 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
>  	 */
>  	rdmsr(MSR_IA32_MISC_ENABLE, l, h);
>  
> -	/*
> -	 * The initial value of thermal LVT entries on all APs always reads
> -	 * 0x10000 because APs are woken up by BSP issuing INIT-SIPI-SIPI
> -	 * sequence to them and LVT registers are reset to 0s except for
> -	 * the mask bits which are set to 1s when APs receive INIT IPI.
> -	 * Always restore the value that BIOS has programmed on AP based on
> -	 * BSP's info we saved since BIOS is always setting the same value
> -	 * for all threads/cores
> -	 */
> -	apic_write(APIC_LVTTHMR, lvtthmr_init);
> -
>  	h = lvtthmr_init;
> -
> -	if ((l & MSR_IA32_MISC_ENABLE_TM1) && (h & APIC_DM_SMI)) {
> -		printk(KERN_DEBUG
> -		       "CPU%d: Thermal monitoring handled by SMI\n", cpu);
> -		return;
> -	}
> -
>  	/* Check whether a vector already exists */
>  	if (h & APIC_VECTOR_MASK) {
>  		printk(KERN_DEBUG
>  		       "CPU%d: Thermal LVT vector (%#x) already installed\n",
>  		       cpu, (h & APIC_VECTOR_MASK));
> +		/*
> +	 	 * The initial value of thermal LVT entries on all APs always reads
> +	 	 * 0x10000 because APs are woken up by BSP issuing INIT-SIPI-SIPI
> +	 	 * sequence to them and LVT registers are reset to 0s except for
> +	 	 * the mask bits which are set to 1s when APs receive INIT IPI.
> +	 	 * Always restore the value that BIOS has programmed on AP based on
> +	 	 * BSP's info we saved since BIOS is always setting the same value
> +	 	 * for all threads/cores
> +	 	*/
> +		apic_write(APIC_LVTTHMR, h);
> +		return;
> +	}
> +
> +	if ((l & MSR_IA32_MISC_ENABLE_TM1) && (h & APIC_DM_SMI)) {
> +		printk(KERN_DEBUG
> +		       "CPU%d: Thermal monitoring handled by SMI\n", cpu);
>  		return;
>  	}


So it should be more like?

diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index e12246f..f89a649 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -445,6 +445,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
 	 */
 	rdmsr(MSR_IA32_MISC_ENABLE, l, h);
 
+	h = lvtthmr_init;
 	/*
 	 * The initial value of thermal LVT entries on all APs always reads
 	 * 0x10000 because APs are woken up by BSP issuing INIT-SIPI-SIPI
@@ -454,9 +455,8 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
 	 * BSP's info we saved since BIOS is always setting the same value
 	 * for all threads/cores
 	 */
-	apic_write(APIC_LVTTHMR, lvtthmr_init);
-
-	h = lvtthmr_init;
+	if (!(h & APIC_DM_FIXED))
+		apic_write(APIC_LVTTHMR, h);
 
 	if ((l & MSR_IA32_MISC_ENABLE_TM1) && (h & APIC_DM_SMI)) {
 		printk(KERN_DEBUG

Can you check if this works and send an updated patch with the correct description etc.

thanks,
suresh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/