From: tip-bot for Thomas Gleixner <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: mingo@kernel.org, tglx@linutronix.de, hpa@zytor.com,
marc.zyngier@arm.com, Robert.Hodaszi@digi.com,
linux-kernel@vger.kernel.org
Subject: [tip:x86/apic] x86/irq: Handle spurious interrupt after shutdown gracefully
Date: Wed, 3 Jul 2019 01:18:55 -0700 [thread overview]
Message-ID: <tip-b7107a67f0d125459fe41f86e8079afd1a5e0b15@git.kernel.org> (raw)
In-Reply-To: <20190628111440.459647741@linutronix.de>
Commit-ID: b7107a67f0d125459fe41f86e8079afd1a5e0b15
Gitweb: https://git.kernel.org/tip/b7107a67f0d125459fe41f86e8079afd1a5e0b15
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 28 Jun 2019 13:11:53 +0200
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 3 Jul 2019 10:12:30 +0200
x86/irq: Handle spurious interrupt after shutdown gracefully
Since the rework of the vector management, warnings about spurious
interrupts have been reported. Robert provided some more information and
did an initial analysis. The following situation leads to these warnings:
CPU 0 CPU 1 IO_APIC
interrupt is raised
sent to CPU1
Unable to handle
immediately
(interrupts off,
deep idle delay)
mask()
...
free()
shutdown()
synchronize_irq()
clear_vector()
do_IRQ()
-> vector is clear
Before the rework the vector entries of legacy interrupts were statically
assigned and occupied precious vector space while most of them were
unused. Due to that the above situation was handled silently because the
vector was handled and the core handler of the assigned interrupt
descriptor noticed that it is shut down and returned.
While this has been usually observed with legacy interrupts, this situation
is not limited to them. Any other interrupt source, e.g. MSI, can cause the
same issue.
After adding proper synchronization for level triggered interrupts, this
can only happen for edge triggered interrupts where the IO-APIC obviously
cannot provide information about interrupts in flight.
While the spurious warning is actually harmless in this case it worries
users and driver developers.
Handle it gracefully by marking the vector entry as VECTOR_SHUTDOWN instead
of VECTOR_UNUSED when the vector is freed up.
If that above late handling happens the spurious detector will not complain
and switch the entry to VECTOR_UNUSED. Any subsequent spurious interrupt on
that line will trigger the spurious warning as before.
Fixes: 464d12309e1b ("x86/vector: Switch IOAPIC to global reservation mode")
Reported-by: Robert Hodaszi <Robert.Hodaszi@digi.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>-
Tested-by: Robert Hodaszi <Robert.Hodaszi@digi.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Link: https://lkml.kernel.org/r/20190628111440.459647741@linutronix.de
---
arch/x86/include/asm/hw_irq.h | 3 ++-
arch/x86/kernel/apic/vector.c | 4 ++--
arch/x86/kernel/irq.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index 32e666e1231e..626e1ac6516e 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -151,7 +151,8 @@ extern char irq_entries_start[];
#endif
#define VECTOR_UNUSED NULL
-#define VECTOR_RETRIGGERED ((void *)~0UL)
+#define VECTOR_SHUTDOWN ((void *)~0UL)
+#define VECTOR_RETRIGGERED ((void *)~1UL)
typedef struct irq_desc* vector_irq_t[NR_VECTORS];
DECLARE_PER_CPU(vector_irq_t, vector_irq);
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 3173e07d3791..1c6d1d5f28d3 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -343,7 +343,7 @@ static void clear_irq_vector(struct irq_data *irqd)
trace_vector_clear(irqd->irq, vector, apicd->cpu, apicd->prev_vector,
apicd->prev_cpu);
- per_cpu(vector_irq, apicd->cpu)[vector] = VECTOR_UNUSED;
+ per_cpu(vector_irq, apicd->cpu)[vector] = VECTOR_SHUTDOWN;
irq_matrix_free(vector_matrix, apicd->cpu, vector, managed);
apicd->vector = 0;
@@ -352,7 +352,7 @@ static void clear_irq_vector(struct irq_data *irqd)
if (!vector)
return;
- per_cpu(vector_irq, apicd->prev_cpu)[vector] = VECTOR_UNUSED;
+ per_cpu(vector_irq, apicd->prev_cpu)[vector] = VECTOR_SHUTDOWN;
irq_matrix_free(vector_matrix, apicd->prev_cpu, vector, managed);
apicd->prev_vector = 0;
apicd->move_in_progress = 0;
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 59b5f2ea7c2f..a975246074b5 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -246,7 +246,7 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
if (!handle_irq(desc, regs)) {
ack_APIC_irq();
- if (desc != VECTOR_RETRIGGERED) {
+ if (desc != VECTOR_RETRIGGERED && desc != VECTOR_SHUTDOWN) {
pr_emerg_ratelimited("%s: %d.%d No irq handler for vector\n",
__func__, smp_processor_id(),
vector);
next prev parent reply other threads:[~2019-07-03 8:19 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-28 11:11 [patch V2 0/6] x86/irq: Cure various interrupt issues Thomas Gleixner
2019-06-28 11:11 ` [patch V2 1/6] genirq: Delay deactivation in free_irq() Thomas Gleixner
2019-07-03 8:16 ` [tip:x86/apic] " tip-bot for Thomas Gleixner
2019-06-28 11:11 ` [patch V2 2/6] genirq: Fix misleading synchronize_irq() documentation Thomas Gleixner
2019-07-01 14:53 ` Peter Zijlstra
2019-07-01 18:01 ` Thomas Gleixner
2019-07-01 18:23 ` Peter Zijlstra
2019-07-03 8:16 ` [tip:x86/apic] " tip-bot for Thomas Gleixner
2019-06-28 11:11 ` [patch V2 3/6] genirq: Add optional hardware synchronization for shutdown Thomas Gleixner
2019-07-01 8:48 ` Marc Zyngier
2019-07-01 14:56 ` Peter Zijlstra
2019-07-01 18:02 ` Thomas Gleixner
2019-07-03 8:17 ` [tip:x86/apic] " tip-bot for Thomas Gleixner
2019-06-28 11:11 ` [patch V2 4/6] x86/ioapic: Implement irq_get_irqchip_state() callback Thomas Gleixner
2019-07-03 8:18 ` [tip:x86/apic] " tip-bot for Thomas Gleixner
2019-06-28 11:11 ` [patch V2 5/6] x86/irq: Handle spurious interrupt after shutdown gracefully Thomas Gleixner
2019-07-03 8:18 ` tip-bot for Thomas Gleixner [this message]
2019-06-28 11:11 ` [patch V2 6/6] x86/irq: Seperate unused system vectors from spurious entry again Thomas Gleixner
2019-07-03 8:19 ` [tip:x86/apic] " tip-bot for Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tip-b7107a67f0d125459fe41f86e8079afd1a5e0b15@git.kernel.org \
--to=tipbot@zytor.com \
--cc=Robert.Hodaszi@digi.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mingo@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.