[PATCH v3 0/2] apic: Fix error interrupt report at all APs when boot

[PATCH v3 0/2] apic: Fix error interrupt report at all APs when boot

[Youquan Song]

Recently, customer report that once machine boot, there are many error interrupt
reported with exact number of all APs. 

The root cause is Local APIC will generate error interrupt when it detect
the illegal vector (one in 0 ~ 15) in an interrupt message received or
interrupt generate from local vector table or self IPI. SDM3A.chapter 10.

AP LAPIC thermal sensor register will be reset to 0x10000, if thermal throttling
interrupt take over by BIOS, it need restore AP with the thermal sensor register
value of geting from BSP, otherwise cause system issue. If BIOS does not take
over the thermal interrupt, The restore value will be CPU rest value of 0x10000,
which means the interrupt vector is zero. After writing 0x10000 to thermal
sensor LVT, the processor will recieve the error interrupt report if the APIC
error interrupt is also set.

This patch add check the BIOS whether take over the thermal interrupt by look
at interrupt delivery mode not fixed mode(BIOS handle will be SMI mode) before
restore AP's thermal LVT. So the agony noise of error interrupt will dismiss
when boot on machine that BIOS does not handle thermal interrupt..  

Also add print out error interrupt reason debug information.

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

[PATCH v3 1/2] apic: Fix error interrupt report at all APs

[Youquan Song]

Recently, customer report that once machine boot, there are many error interrupt
reported with exact number of all APs. 

The root cause is Local APIC will generate error interrupt when it detect
the illegal vector (one in 0 ~ 15) in an interrupt message received or
interrupt generate from local vector table or self IPI. SDM3A.chapter 10.

AP LAPIC thermal sensor register will be reset to 0x10000, if thermal throttling
interrupt take over by BIOS, it need restore AP with the thermal sensor register
value of geting from BSP, otherwise cause system issue. If BIOS does not take
over the thermal interrupt, The restore value will be CPU rest value of 0x10000,
which means the interrupt vector is zero. After writing 0x10000 to thermal
sensor LVT, the processor will recieve the error interrupt report if the APIC
error interrupt is also set.

This patch add check the BIOS whether take over the thermal interrupt by look
at interrupt delivery mode not fixed mode(BIOS handle will be SMI mode) before
restore AP's thermal LVT. So the agony noise of error interrupt will dismiss
when boot on machine that BIOS does not handle thermal interrupt..  


Signed-off-by: Youquan Song <youquan.song@intel.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Acked-by: Yong Wang <yong.y.wang@intel.com>
---
 arch/x86/include/asm/apicdef.h           |    1 +
 arch/x86/kernel/cpu/mcheck/therm_throt.c |   12 +++++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index a859ca4..1495098 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -85,6 +85,7 @@
 #define		APIC_DM_INIT		0x00500
 #define		APIC_DM_STARTUP		0x00600
 #define		APIC_DM_EXTINT		0x00700
+#define		APIC_DM_MASK		0x00700
 #define		APIC_VECTOR_MASK	0x000FF
 #define	APIC_ICR2	0x310
 #define		GET_APIC_DEST_FIELD(x)	(((x) >> 24) & 0xFF)
diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index 4b68326..22a3b4f 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -405,18 +405,20 @@ 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
 	 * 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
+	 * If BIOS take over the thermal interrupt and set its interrupt
+	 * delivery mode to SMI not fixed, it 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);
+	if (h & APIC_DM_MASK)
+		apic_write(APIC_LVTTHMR, lvtthmr_init);
 
-	h = lvtthmr_init;
 
 	if ((l & MSR_IA32_MISC_ENABLE_TM1) && (h & APIC_DM_SMI)) {
 		printk(KERN_DEBUG
-- 
1.6.4.2

[PATCH v3 2/2] apic: Add print error interrupt reason

[Youquan Song]

End user worry about the error interrupt information and intend to know what
kind of error interrupts are generated, so this patch add printing out the
detail debug information of error interrupt. 
dynamic debug is not initiated when LAPIC initiation, so the pr_debug will not
output the error interrupt debug information when boot.
In this patch, we use apic_printk(APIC_DEBUG,), so if add kernel option
apic=debug will output the error interrupt during boot.

Signed-off-by: Youquan Song <youquan.song@intel.com>
---
 arch/x86/kernel/apic/apic.c |   29 ++++++++++++++++++++++++++++-
 1 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 3f838d5..f0f2809 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1794,6 +1794,32 @@ void smp_spurious_interrupt(struct pt_regs *regs)
 	irq_exit();
 }
 
+static const char * const error_interrupt_reason[] = {
+	"Send CS error",
+	"Receive CS error",
+	"Send accept error",
+	"Receive accept error",
+	"Redirectable IPI",
+	"Send illegal vector",
+	"Received illegal vector",
+	"Illegal register address",
+};
+
+static void print_error_interrupt_reason(u32 reason)
+{
+	u32 i = 0;
+	reason = reason & 0xff;
+	do {
+		if (reason & 0x1)
+			apic_printk(APIC_DEBUG, " : %s",
+					error_interrupt_reason[i]);
+		i++;
+		reason >>= 1;
+	} while (reason > 0);
+
+	apic_printk(APIC_DEBUG, "\n");
+}
+
 /*
  * This interrupt should never happen with our APIC/SMP architecture
  */
@@ -1821,8 +1847,9 @@ void smp_error_interrupt(struct pt_regs *regs)
 	 * 6: Received illegal vector
 	 * 7: Illegal register address
 	 */
-	pr_debug("APIC error on CPU%d: %02x(%02x)\n",
+	apic_printk(APIC_DEBUG, "APIC error on CPU%d: %02x(%02x)",
 		smp_processor_id(), v , v1);
+	print_error_interrupt_reason(v1);
 	irq_exit();
 }
 
-- 
1.6.4.2

Re: [PATCH v3 2/2] apic: Add print error interrupt reason

[Joe Perches]

Hey Youquan.

Your patch removes KERN_DEBUG and prints at KERN_DEFAULT.
Is that what you want?

This one reduces a few lines and adds KERN_DEBUG and KERN_CONT.
Maybe this one's what you want?

 arch/x86/kernel/apic/apic.c |   38 +++++++++++++++++++++++++-------------
 1 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7821813..5a70e63 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1808,6 +1808,17 @@ void smp_spurious_interrupt(struct pt_regs *regs)
 void smp_error_interrupt(struct pt_regs *regs)
 {
 	u32 v, v1;
+	u32 i = 0;
+	static const char * const error_interrupt_reason[] = {
+		"Send CS error",		/* APIC Error Bit 0 */
+		"Receive CS error",		/* APIC Error Bit 1 */
+		"Send accept error",		/* APIC Error Bit 2 */
+		"Receive accept error",		/* APIC Error Bit 3 */
+		"Redirectable IPI",		/* APIC Error Bit 4 */
+		"Send illegal vector",		/* APIC Error Bit 5 */
+		"Received illegal vector",	/* APIC Error Bit 6 */
+		"Illegal register address",	/* APIC Error Bit 7 */
+	};
 
 	exit_idle();
 	irq_enter();
@@ -1818,19 +1829,20 @@ void smp_error_interrupt(struct pt_regs *regs)
 	ack_APIC_irq();
 	atomic_inc(&irq_err_count);
 
-	/*
-	 * Here is what the APIC error bits mean:
-	 * 0: Send CS error
-	 * 1: Receive CS error
-	 * 2: Send accept error
-	 * 3: Receive accept error
-	 * 4: Reserved
-	 * 5: Send illegal vector
-	 * 6: Received illegal vector
-	 * 7: Illegal register address
-	 */
-	pr_debug("APIC error on CPU%d: %02x(%02x)\n",
-		smp_processor_id(), v , v1);
+	apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x(%02x)",
+		    smp_processor_id(), v , v1);
+
+	v1 = v1 & 0xff;
+	while (v1) {
+		if (v1 & 0x1)
+			apic_printk(APIC_DEBUG, KERN_CONT " : %s",
+				    error_interrupt_reason[i]);
+		i++;
+		v1 >>= 1;
+	};
+
+	apic_printk(APIC_DEBUG, KERN_CONT "\n");
+
 	irq_exit();
 }
 

RE: [PATCH v3 1/2] apic: Fix error interrupt report at all APs

[Guo, Chaohong]

> -----Original Message-----
> From: Song, Youquan
> Sent: Saturday, January 08, 2011 10:20 AM
> To: linux-kernel@vger.kernel.org; hpa@linux.intel.com; Siddha, Suresh B
> Cc: yong.y.wang@linux.intel.com; joe@perches.com; jbaron@redhat.com;
> arjan@linux.intel.com; trenn@suse.de; Liu, Kent; Guo, Chaohong; Youquan
> Song; Song, Youquan
> Subject: [PATCH v3 1/2] apic: Fix error interrupt report at all APs
> 
> Recently, customer report that once machine boot, there are many error
> interrupt reported with exact number of all APs.
> 
> The root cause is Local APIC will generate error interrupt when it detect the
> illegal vector (one in 0 ~ 15) in an interrupt message received or interrupt
> generate from local vector table or self IPI. SDM3A.chapter 10.
> 
> AP LAPIC thermal sensor register will be reset to 0x10000, if thermal throttling
> interrupt take over by BIOS, it need restore AP with the thermal sensor register
> value of geting from BSP, otherwise cause system issue. If BIOS does not take
> over the thermal interrupt, The restore value will be CPU rest value of 0x10000,
> which means the interrupt vector is zero. After writing 0x10000 to thermal
> sensor LVT, the processor will recieve the error interrupt report if the APIC
> error interrupt is also set.
> 
> This patch add check the BIOS whether take over the thermal interrupt by look
> at interrupt delivery mode not fixed mode(BIOS handle will be SMI mode)
> before restore AP's thermal LVT. So the agony noise of error interrupt will
> dismiss when boot on machine that BIOS does not handle thermal interrupt..
> 
> 
> Signed-off-by: Youquan Song <youquan.song@intel.com>
> Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Acked-by: Yong Wang <yong.y.wang@intel.com>
> ---
>  arch/x86/include/asm/apicdef.h           |    1 +
>  arch/x86/kernel/cpu/mcheck/therm_throt.c |   12 +++++++-----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
> index a859ca4..1495098 100644
> --- a/arch/x86/include/asm/apicdef.h
> +++ b/arch/x86/include/asm/apicdef.h
> @@ -85,6 +85,7 @@
>  #define		APIC_DM_INIT		0x00500
>  #define		APIC_DM_STARTUP		0x00600
>  #define		APIC_DM_EXTINT		0x00700
> +#define		APIC_DM_MASK		0x00700
>  #define		APIC_VECTOR_MASK	0x000FF
>  #define	APIC_ICR2	0x310
>  #define		GET_APIC_DEST_FIELD(x)	(((x) >> 24) & 0xFF)
> diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> index 4b68326..22a3b4f 100644
> --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> @@ -405,18 +405,20 @@ 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
>  	 * 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
> +	 * If BIOS take over the thermal interrupt and set its interrupt
> +	 * delivery mode to SMI not fixed, it 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);
> +	if (h & APIC_DM_MASK)

How about changing it to :

     If ( (h & APIC_DM_MASK) != APIC_DM_FIXED)

-minskey












> +		apic_write(APIC_LVTTHMR, lvtthmr_init);
> 
> -	h = lvtthmr_init;
> 
>  	if ((l & MSR_IA32_MISC_ENABLE_TM1) && (h & APIC_DM_SMI)) {
>  		printk(KERN_DEBUG
> --
> 1.6.4.2