genirq: Only call irq_ack if implemented by chip
PATCH 1/1 genirq: Only call irq_ack if implemented by chip
Restore the check if an IRQ chip implements the irq_ack function prior to its invocation. Commit 22a49163e90d ("genirq: Provide compat handling for chip->ack()") removed the check from handle_edge_irq while keeping the check in other call paths.
--- kernel/irq/chip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/irq/chip.c +2 -1
@@ -586,7 +586,8 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)kstat_incr_irqs_this_cpu (irq, desc);/* Start handling the irq */ - desc->irq_data.chip->irq_ack (&desc->irq_data); +if (desc->irq_data.chip->irq_ack) + desc->irq_data.chip->irq_ack (&desc->irq_data); do { if (unlikely(!desc->action)) {
Signature · 1 line
1.7.10.4
Quoted 4 lines · Adrian-Ken Rueegsegger
Restore the check if an IRQ chip implements the
irq_ackfunction prior to its invocation. Commit22a49163e90d("genirq: Provide compat handling forchip->ack()") removed the check fromhandle_edge_irqwhile keeping the check in other call paths.
How's an edge triggered interrupt without ack supposed to work?
You are missing to describe which problem you solve.
Thanks,
tglx
Quoted 23 lines
Signed-off-by Adrian-Ken Rueegsegger <[email protected]>--- kernel/irq/chip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)kernel/irq/chip.c +2
-1@@ -586,7 +586,8 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)kstat_incr_irqs_this_cpu (irq, desc);/* Start handling the irq */ - desc->irq_data.chip->irq_ack (&desc->irq_data); +if (desc->irq_data.chip->irq_ack) + desc->irq_data.chip->irq_ack (&desc->irq_data); do { if (unlikely(!desc->action)) {Signature · 1 line
1.7.10.4
Hi,
Quoted 9 lines · Thomas Gleixner
Quoted 4 lines · Adrian-Ken Rueegsegger
Restore the check if an IRQ chip implements the
irq_ackfunction prior to its invocation. Commit22a49163e90d("genirq: Provide compat handling forchip->ack()") removed the check fromhandle_edge_irqwhile keeping the check in other call paths.How's an edge triggered interrupt without ack supposed to work?
You are missing to describe which problem you solve.
I am running Linux as a VM on top of the Muen Separation Kernel (SK) [1], where we have implemented PCI device passthrough using VT-d. In this case the hardware interrupt is handled by the SK/hypervisor and injected to Linux.
To support PCI MSI(-X), we register our own platform-specific MSI operations (x86_msi.setup_msi_irqs, etc) and implement an irq_chip that simply provides the irq_mask and irq_unmask operations (by reusing the "regular" mask_msi_irq/unmask_msi_irq).
After encountering a null pointer dereference due to the irq_chip not providing an irq_ack operation, I examined the commit that changed the source of the irq_ack call, noticing that only the check in handle_edge_irq was dropped while the other call sites were kept. As the commit message did not provide me with additional information for that particular fragment of the change, I (prematurely) concluded that it was not intentional.
If it is a prerequisite for IRQ chips to provide the irq_ack operation when used in conjunction with handle_edge_irq, then please ignore my patch. In that case, I will adjust our chip implementation accordingly.
Thanks for your time,
Adrian
[1] - http://muen.codelabs.ch/
Quoted 4 lines · Adrian-Ken Rueegsegger
To support PCI MSI(-X), we register our own platform-specific MSI operations (x86_msi.setup_msi_irqs, etc) and implement an
irq_chipthat simply provides theirq_maskandirq_unmaskoperations (by reusing the "regular" mask_msi_irq/unmask_msi_irq).
But why are you using the edge flow handler then?
Quoted 8 lines
After encountering a null pointer dereference due to the
irq_chipnot providing anirq_ackoperation, I examined the commit that changed the source of theirq_ackcall, noticing that only the check inhandle_edge_irqwas dropped while the other call sites were kept. As the commit message did not provide me with additional information for that particular fragment of the change, I (prematurely) concluded that it was not intentional.
Quoted 4 lines
If it is a prerequisite for IRQ chips to provide the
irq_ackoperation when used in conjunction withhandle_edge_irq, then please ignore my patch. In that case, I will adjust our chip implementation accordingly.
Yes, it is more or less. And I really want to avoid the extra conditional in the handler hotpath.
Thanks,