[PATCH] I/O APIC: AEOI timer acknowledgement clean-ups

[PATCH] I/O APIC: AEOI timer acknowledgement clean-ups

[Maciej W. Rozycki]

 The code that used to be in do_slow_gettimeoffset() that relied on the
IRR bit of the master 8259A PIC for IRQ0 to check the state of the output
timer 0 of the PIT is no longer there.  As a result, there is no need to
use the POLL command to acknowledge the timer interrupt in the "8259A
Virtual Wire", except for the NMI watchdog when the i82489DX APIC is used
(this is because this particular APIC treats NMIs as level-triggered and
keeping the input asserted would keep motherboard NMI sources held off for
too long).  Remove the unneeded bits and adjust comments accordingly.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 It seems to work for me with the NMI watchdog routed through the I/O 
APIC.  The next step should move the bits from timer_interrupt() to 
nmi_watchdog_tick() where they now belong, but let's check nothing breaks 
for people first.

  Maciej

patch-2.6.26-rc1-20080505-timer_ack-1
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/io_apic_32.c linux-2.6.26-rc1-20080505/arch/x86/kernel/io_apic_32.c
--- linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/io_apic_32.c	2008-05-05 02:55:24.000000000 +0000
+++ linux-2.6.26-rc1-20080505/arch/x86/kernel/io_apic_32.c	2008-05-16 20:08:32.000000000 +0000
@@ -2146,19 +2146,17 @@ static inline void __init check_timer(vo
 	set_intr_gate(vector, interrupt[0]);
 
 	/*
-	 * Subtle, code in do_timer_interrupt() expects an AEOI
-	 * mode for the 8259A whenever interrupts are routed
-	 * through I/O APICs.  Also IRQ0 has to be enabled in
-	 * the 8259A which implies the virtual wire has to be
-	 * disabled in the local APIC.  Finally timer interrupts
-	 * need to be acknowledged manually in the 8259A for
-	 * timer_interrupt() and for the i82489DX when using
-	 * the NMI watchdog.
+	 * As IRQ0 is to be enabled in the 8259A, the virtual
+	 * wire has to be disabled in the local APIC.  Also
+	 * timer interrupts need to be acknowledged manually in
+	 * the 8259A for the i82489DX when using the NMI
+	 * watchdog as that APIC treats NMIs as level-triggered.
+	 * The AEOI mode will finish them in the 8259A
+	 * automatically.
 	 */
 	apic_write_around(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	init_8259A(1);
-	timer_ack = !cpu_has_tsc;
-	timer_ack |= (nmi_watchdog == NMI_IO_APIC && !APIC_INTEGRATED(ver));
+	timer_ack = (nmi_watchdog == NMI_IO_APIC && !APIC_INTEGRATED(ver));
 	if (timer_over_8254 > 0)
 		enable_8259A_irq(0);
 
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/io_apic_64.c linux-2.6.26-rc1-20080505/arch/x86/kernel/io_apic_64.c
--- linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/io_apic_64.c	2008-05-05 02:55:24.000000000 +0000
+++ linux-2.6.26-rc1-20080505/arch/x86/kernel/io_apic_64.c	2008-05-17 03:43:40.000000000 +0000
@@ -1669,11 +1669,8 @@ static inline void __init check_timer(vo
 	assign_irq_vector(0, TARGET_CPUS);
 
 	/*
-	 * Subtle, code in do_timer_interrupt() expects an AEOI
-	 * mode for the 8259A whenever interrupts are routed
-	 * through I/O APICs.  Also IRQ0 has to be enabled in
-	 * the 8259A which implies the virtual wire has to be
-	 * disabled in the local APIC.
+	 * As IRQ0 is to be enabled in the 8259A, the virtual
+	 * wire has to be disabled in the local APIC.
 	 */
 	apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	init_8259A(1);
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/nmi_32.c linux-2.6.26-rc1-20080505/arch/x86/kernel/nmi_32.c
--- linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/nmi_32.c	2008-05-05 02:55:24.000000000 +0000
+++ linux-2.6.26-rc1-20080505/arch/x86/kernel/nmi_32.c	2008-05-16 20:11:41.000000000 +0000
@@ -131,7 +131,7 @@ int __init check_nmi_watchdog(void)
 	kfree(prev_nmi_count);
 	return 0;
 error:
-	timer_ack = !cpu_has_tsc;
+	timer_ack = 0;
 
 	return -1;
 }
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/time_32.c linux-2.6.26-rc1-20080505/arch/x86/kernel/time_32.c
--- linux-2.6.26-rc1-20080505.macro/arch/x86/kernel/time_32.c	2008-05-05 02:55:24.000000000 +0000
+++ linux-2.6.26-rc1-20080505/arch/x86/kernel/time_32.c	2008-05-16 20:03:36.000000000 +0000
@@ -84,8 +84,7 @@ irqreturn_t timer_interrupt(int irq, voi
 	if (timer_ack) {
 		/*
 		 * Subtle, when I/O APICs are used we have to ack timer IRQ
-		 * manually to reset the IRR bit for do_slow_gettimeoffset().
-		 * This will also deassert NMI lines for the watchdog if run
+		 * manually to deassert NMI lines for the watchdog if run
 		 * on an 82489DX-based system.
 		 */
 		spin_lock(&i8259A_lock);

Re: [PATCH] I/O APIC: AEOI timer acknowledgement clean-ups

[Ingo Molnar]

* Maciej W. Rozycki <macro@linux-mips.org> wrote:

> The code that used to be in do_slow_gettimeoffset() that relied on the 
> IRR bit of the master 8259A PIC for IRQ0 to check the state of the 
> output timer 0 of the PIT is no longer there.  As a result, there is 
> no need to use the POLL command to acknowledge the timer interrupt in 
> the "8259A Virtual Wire", except for the NMI watchdog when the 
> i82489DX APIC is used (this is because this particular APIC treats 
> NMIs as level-triggered and keeping the input asserted would keep 
> motherboard NMI sources held off for too long).  Remove the unneeded 
> bits and adjust comments accordingly.

applied to latest -tip, thanks Maciej!

>  It seems to work for me with the NMI watchdog routed through the I/O 
> APIC.  The next step should move the bits from timer_interrupt() to 
> nmi_watchdog_tick() where they now belong, but let's check nothing 
> breaks for people first.

ok.

	Ingo