2023-10-24 15:04:15

by Herve Codina

[permalink] [raw]
Subject: [PATCH 1/1] genirq/generic_chip: Fix irq_remove_generic_chip() when an irq domain is used

irq_remove_generic_chip() can call (depending on the msk parameter
value) several operations on irqs based on gc->irq_base such as
irq_set_handler(irq, NULL) to remove an handler.

When the generic chip is present in an irq domain (created with a call
to irq_alloc_domain_generic_chips()), gc->irq_base is the base hardware
irq for this chip. It is set to 0 for the first chip in the domain,
0 + n for the next chip (with n the number of hardware irqs per chip)
and so on.
In that case, the operations done on irqs based on gc->irq_base touch
some irqs not related to the chip nor the domain breaking some unrelated
components in the system.

In order to avoid touching these "outside" irqs, take care of the domain
irq mapping and translate the chip hardware irq to an irq number
suitable for the several operations done.

Fixes: cfefd21e693d ("genirq: Add chip suspend and resume callbacks")
Cc: [email protected]
Signed-off-by: Herve Codina <[email protected]>
---
kernel/irq/generic-chip.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c653cd31548d..494584e25ef4 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -544,21 +544,28 @@ EXPORT_SYMBOL_GPL(irq_setup_alt_chip);
void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
unsigned int clr, unsigned int set)
{
- unsigned int i = gc->irq_base;
+ unsigned int irq;
+ unsigned int i;

raw_spin_lock(&gc_lock);
list_del(&gc->list);
raw_spin_unlock(&gc_lock);

- for (; msk; msk >>= 1, i++) {
+ for (i = 0; msk; msk >>= 1, i++) {
if (!(msk & 0x01))
continue;

+ irq = gc->domain ?
+ irq_find_mapping(gc->domain, gc->irq_base + i) :
+ gc->irq_base + i;
+ if (!irq)
+ continue;
+
/* Remove handler first. That will mask the irq line */
- irq_set_handler(i, NULL);
- irq_set_chip(i, &no_irq_chip);
- irq_set_chip_data(i, NULL);
- irq_modify_status(i, clr, set);
+ irq_set_handler(irq, NULL);
+ irq_set_chip(irq, &no_irq_chip);
+ irq_set_chip_data(irq, NULL);
+ irq_modify_status(irq, clr, set);
}
}
EXPORT_SYMBOL_GPL(irq_remove_generic_chip);
--
2.41.0


Subject: [tip: irq/core] genirq/generic_chip: Make irq_remove_generic_chip() irqdomain aware

The following commit has been merged into the irq/core branch of tip:

Commit-ID: 5e7afb2eb7b2a7c81e9f608cbdf74a07606fd1b5
Gitweb: https://git.kernel.org/tip/5e7afb2eb7b2a7c81e9f608cbdf74a07606fd1b5
Author: Herve Codina <[email protected]>
AuthorDate: Tue, 24 Oct 2023 17:03:35 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Fri, 27 Oct 2023 09:15:44 +02:00

genirq/generic_chip: Make irq_remove_generic_chip() irqdomain aware

irq_remove_generic_chip() calculates the Linux interrupt number for removing the
handler and interrupt chip based on gc::irq_base as a linear function of
the bit positions of set bits in the @msk argument.

When the generic chip is present in an irq domain, i.e. created with a call
to irq_alloc_domain_generic_chips(), gc::irq_base contains not the base
Linux interrupt number. It contains the base hardware interrupt for this
chip. It is set to 0 for the first chip in the domain, 0 + N for the next
chip, where $N is the number of hardware interrupts per chip.

That means the Linux interrupt number cannot be calculated based on
gc::irq_base for irqdomain based chips without a domain map lookup, which
is currently missing.

Rework the code to take the irqdomain case into account and calculate the
Linux interrupt number by a irqdomain lookup of the domain specific
hardware interrupt number.

[ tglx: Massage changelog. Reshuffle the logic and add a proper comment. ]

Fixes: cfefd21e693d ("genirq: Add chip suspend and resume callbacks")
Signed-off-by: Herve Codina <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]

---
kernel/irq/generic-chip.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 81ecca0..d39a40b 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -548,21 +548,34 @@ EXPORT_SYMBOL_GPL(irq_setup_alt_chip);
void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
unsigned int clr, unsigned int set)
{
- unsigned int i = gc->irq_base;
+ unsigned int i, virq;

raw_spin_lock(&gc_lock);
list_del(&gc->list);
raw_spin_unlock(&gc_lock);

- for (; msk; msk >>= 1, i++) {
+ for (i = 0; msk; msk >>= 1, i++) {
if (!(msk & 0x01))
continue;

+ /*
+ * Interrupt domain based chips store the base hardware
+ * interrupt number in gc::irq_base. Otherwise gc::irq_base
+ * contains the base Linux interrupt number.
+ */
+ if (gc->domain) {
+ virq = irq_find_mapping(gc->domain, gc->irq_base + i);
+ if (!virq)
+ continue;
+ } else {
+ virq = gc->irq_base + i;
+ }
+
/* Remove handler first. That will mask the irq line */
- irq_set_handler(i, NULL);
- irq_set_chip(i, &no_irq_chip);
- irq_set_chip_data(i, NULL);
- irq_modify_status(i, clr, set);
+ irq_set_handler(virq, NULL);
+ irq_set_chip(virq, &no_irq_chip);
+ irq_set_chip_data(virq, NULL);
+ irq_modify_status(virq, clr, set);
}
}
EXPORT_SYMBOL_GPL(irq_remove_generic_chip);