[PATCH] x86/ioapic: add IO-APIC irq_chip retrigger hook

[PATCH] x86/ioapic: add IO-APIC irq_chip retrigger hook

[Ruslan Ruslichenko]

After commit d32932d02e18  ("x86/irq: Convert IOAPIC to use hierarchical
irqdomain interfaces") IO-APIC is switched to hierarchical irqdomain
implementation. Also this patch removed irq_retrigger hook for IO-APIC
chip which seem to be wrong. Without this hook some edge-triggered irq's
might be lost, since kernel uses it to retrigger edge irq's triggered
while they are being handled or disabled by driver
(see "Delayed_interrupt_disable" section at "Generic-IRQ-Guide"
of kernel docbook).

Thus adding this hook same way as it was done for MSI domain.

Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
---
 arch/x86/kernel/apic/io_apic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 945e512..1e35dd0 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1875,6 +1875,7 @@ static int ioapic_set_affinity(struct irq_data *irq_data,
 	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= ioapic_ack_level,
 	.irq_set_affinity	= ioapic_set_affinity,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
@@ -1886,6 +1887,7 @@ static int ioapic_set_affinity(struct irq_data *irq_data,
 	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= ioapic_ir_ack_level,
 	.irq_set_affinity	= ioapic_set_affinity,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
-- 
1.9.1

Re: [PATCH] x86/ioapic: add IO-APIC irq_chip retrigger hook

[Thomas Gleixner]

On Tue, 17 Jan 2017, Ruslan Ruslichenko wrote:

> After commit d32932d02e18  ("x86/irq: Convert IOAPIC to use hierarchical
> irqdomain interfaces") IO-APIC is switched to hierarchical irqdomain
> implementation. Also this patch removed irq_retrigger hook for IO-APIC
> chip which seem to be wrong. Without this hook some edge-triggered irq's
> might be lost, since kernel uses it to retrigger edge irq's triggered
> while they are being handled or disabled by driver

Restoring the callbacks is the right thing to do, but there is no problem
because if the retrigger callback is empty then pending edge interrupts are
resent by software. It's just less efficient, but not a issue.

Thanks,

	tglx

[tip:x86/apic] x86/ioapic: Restore IO-APIC irq_chip retrigger callback

[tip-bot for Ruslan Ruslichenko]

Commit-ID:  a9b4f08770b415f30f2fb0f8329a370c8f554aa3
Gitweb:     http://git.kernel.org/tip/a9b4f08770b415f30f2fb0f8329a370c8f554aa3
Author:     Ruslan Ruslichenko <rruslich@cisco.com>
AuthorDate: Tue, 17 Jan 2017 16:13:52 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 18 Jan 2017 11:51:02 +0100

x86/ioapic: Restore IO-APIC irq_chip retrigger callback

commit d32932d02e18 removed the irq_retrigger callback from the IO-APIC
chip and did not add it to the new IO-APIC-IR irq chip.

There is no harm because the interrupts are resent in software when the
retrigger callback is NULL, but it's less efficient. So restore them.

[ tglx: Massaged changelog ]

Fixes: d32932d02e18  ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
Cc: xe-linux-external@cisco.com
Link: http://lkml.kernel.org/r/1484662432-13580-1-git-send-email-rruslich@cisco.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/apic/io_apic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 945e512..1e35dd0 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1875,6 +1875,7 @@ static struct irq_chip ioapic_chip __read_mostly = {
 	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= ioapic_ack_level,
 	.irq_set_affinity	= ioapic_set_affinity,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
@@ -1886,6 +1887,7 @@ static struct irq_chip ioapic_ir_chip __read_mostly = {
 	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= ioapic_ir_ack_level,
 	.irq_set_affinity	= ioapic_set_affinity,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 

Re: [PATCH] x86/ioapic: add IO-APIC irq_chip retrigger hook

[Ruslan Ruslichenko -X (rruslich - GLOBALLOGIC INC at Cisco)]

On 01/18/2017 12:46 PM, Thomas Gleixner wrote:
> On Tue, 17 Jan 2017, Ruslan Ruslichenko wrote:
>
>> After commit d32932d02e18  ("x86/irq: Convert IOAPIC to use hierarchical
>> irqdomain interfaces") IO-APIC is switched to hierarchical irqdomain
>> implementation. Also this patch removed irq_retrigger hook for IO-APIC
>> chip which seem to be wrong. Without this hook some edge-triggered irq's
>> might be lost, since kernel uses it to retrigger edge irq's triggered
>> while they are being handled or disabled by driver
> Restoring the callbacks is the right thing to do, but there is no problem
> because if the retrigger callback is empty then pending edge interrupts are
> resent by software. It's just less efficient, but not a issue.

Software resend will only work if CONFIG_HARDIRQS_SW_RESEND is enabled,
which is not selected for x86 architectures.

So it's good that patch now picked up.
> Thanks,
>
> 	tglx

Re: [PATCH] x86/ioapic: add IO-APIC irq_chip retrigger hook

[Thomas Gleixner]

On Wed, 18 Jan 2017, Ruslan Ruslichenko -X (rruslich - GLOBALLOGIC INC at Cisco) wrote:
> On 01/18/2017 12:46 PM, Thomas Gleixner wrote:
> > On Tue, 17 Jan 2017, Ruslan Ruslichenko wrote:
> > 
> > > After commit d32932d02e18  ("x86/irq: Convert IOAPIC to use hierarchical
> > > irqdomain interfaces") IO-APIC is switched to hierarchical irqdomain
> > > implementation. Also this patch removed irq_retrigger hook for IO-APIC
> > > chip which seem to be wrong. Without this hook some edge-triggered irq's
> > > might be lost, since kernel uses it to retrigger edge irq's triggered
> > > while they are being handled or disabled by driver
> > Restoring the callbacks is the right thing to do, but there is no problem
> > because if the retrigger callback is empty then pending edge interrupts are
> > resent by software. It's just less efficient, but not a issue.
> 
> Software resend will only work if CONFIG_HARDIRQS_SW_RESEND is enabled,
> which is not selected for x86 architectures.

Dammit, right you are. Good question why nobody noticed so far.

Lemme amend that commit, so it gets back into stable then.

Thanks,

	tglx

[tip:x86/urgent] x86/ioapic: Restore IO-APIC irq_chip retrigger callback

[tip-bot for Ruslan Ruslichenko]

Commit-ID:  020eb3daaba2857b32c4cf4c82f503d6a00a67de
Gitweb:     http://git.kernel.org/tip/020eb3daaba2857b32c4cf4c82f503d6a00a67de
Author:     Ruslan Ruslichenko <rruslich@cisco.com>
AuthorDate: Tue, 17 Jan 2017 16:13:52 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 18 Jan 2017 15:37:28 +0100

x86/ioapic: Restore IO-APIC irq_chip retrigger callback

commit d32932d02e18 removed the irq_retrigger callback from the IO-APIC
chip and did not add it to the new IO-APIC-IR irq chip.

Unfortunately the software resend fallback is not enabled on X86, so edge
interrupts which are received during the lazy disabled state of the
interrupt line are not retriggered and therefor lost.

Restore the callbacks.

[ tglx: Massaged changelog ]

Fixes: d32932d02e18  ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
Cc: xe-linux-external@cisco.com
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/1484662432-13580-1-git-send-email-rruslich@cisco.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/apic/io_apic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 945e512..1e35dd0 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1875,6 +1875,7 @@ static struct irq_chip ioapic_chip __read_mostly = {
 	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= ioapic_ack_level,
 	.irq_set_affinity	= ioapic_set_affinity,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };
 
@@ -1886,6 +1887,7 @@ static struct irq_chip ioapic_ir_chip __read_mostly = {
 	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= ioapic_ir_ack_level,
 	.irq_set_affinity	= ioapic_set_affinity,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.flags			= IRQCHIP_SKIP_SET_WAKE,
 };