Messages in this thread Patch in this message | | | Date | Thu, 9 Apr 2009 22:18:02 +0400 | From | Cyrill Gorcunov <> | Subject | [RFC -tip] x86: do_IRQ - send APIC EOI for x86-32 on irq without handler v3 |
| |
Impact: bugfix, cleanup
We should send APIC EOI if it's enabled only. Since the same is needed for ack_bad_irq we just introduce ack_APIC_irq_safe which in turn check if we have APIC properly installed and initialized and do EOI then.
Also a tiny cleanup: use pr_... macros and add printk_ratelimit for ack_bad_irq to eliminate possible storm on screwed hardware.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org> ---
Ingo, I've checked the sources and as far as I see we could NOP'ify apic->write indeed but I have an internal feeling that this will bring us more problem in future (for example it could be the following scenario: some screwed APIC would require cleaning of LVT's or IRR after resume regardless if it was initialized or not at all). Mostly I mean that the idea of making apic->write NOP'ified is quite elegant indeed but cut off the subset of apic operations (we need apic->read anyway) somehow bothering me from inside :)
CC'ed a number of people I know were involved in this area.
On the other hand I could make a testing patch for nop'fied ->write operation so we could check if it bring any problems in real tests. Thoughts?
arch/x86/include/asm/apic.h | 10 +++++++++- arch/x86/kernel/irq.c | 18 ++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-)
Index: linux-2.6.git/arch/x86/include/asm/apic.h ===================================================================== --- linux-2.6.git.orig/arch/x86/include/asm/apic.h +++ linux-2.6.git/arch/x86/include/asm/apic.h @@ -392,7 +392,6 @@ static inline u32 safe_apic_wait_icr_idl return apic->safe_wait_icr_idle(); } - static inline void ack_APIC_irq(void) { #ifdef CONFIG_X86_LOCAL_APIC @@ -406,6 +405,15 @@ static inline void ack_APIC_irq(void) #endif } +/* Ack APIC irq if it's enabled only */ +static inline void ack_APIC_irq_safe(void) +{ +#ifdef CONFIG_X86_LOCAL_APIC + if (cpu_has_apic) + ack_APIC_irq(); +#endif +} + static inline unsigned default_get_apic_id(unsigned long x) { unsigned int ver = GET_APIC_VERSION(apic_read(APIC_LVR)); Index: linux-2.6.git/arch/x86/kernel/irq.c ===================================================================== --- linux-2.6.git.orig/arch/x86/kernel/irq.c +++ linux-2.6.git/arch/x86/kernel/irq.c @@ -24,9 +24,9 @@ void (*generic_interrupt_extension)(void */ void ack_bad_irq(unsigned int irq) { - printk(KERN_ERR "unexpected IRQ trap at vector %02x\n", irq); + if (printk_ratelimit()) + pr_err("unexpected IRQ trap at vector %02x\n", irq); -#ifdef CONFIG_X86_LOCAL_APIC /* * Currently unexpected vectors happen only on SMP and APIC. * We _must_ ack these because every local APIC has only N @@ -36,9 +36,7 @@ void ack_bad_irq(unsigned int irq) * completely. * But only ack when the APIC is enabled -AK */ - if (cpu_has_apic) - ack_APIC_irq(); -#endif + ack_APIC_irq_safe(); } #define irq_stats(x) (&per_cpu(irq_stat, x)) @@ -223,14 +221,10 @@ unsigned int __irq_entry do_IRQ(struct p irq = __get_cpu_var(vector_irq)[vector]; if (!handle_irq(irq, regs)) { -#ifdef CONFIG_X86_64 - if (!disable_apic) - ack_APIC_irq(); -#endif - + ack_APIC_irq_safe(); if (printk_ratelimit()) - printk(KERN_EMERG "%s: %d.%d No irq handler for vector (irq %d)\n", - __func__, smp_processor_id(), vector, irq); + pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n", + __func__, smp_processor_id(), vector, irq); } irq_exit();
| |