public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] irqchip: mips-gic: Cleanup chip & handler setup
@ 2016-08-19 17:07 Paul Burton
  2016-08-19 17:07 ` [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain Paul Burton
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paul Burton @ 2016-08-19 17:07 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-kernel
  Cc: linux-mips, stable, Paul Burton

gic_shared_irq_domain_map() is called from gic_irq_domain_alloc() where
the wrong chip has been set, and is then overwritten. Tidy this up by
setting the correct chip the first time, and setting the
handle_level_irq handler from gic_irq_domain_alloc() too.

gic_shared_irq_domain_map() is also called from gic_irq_domain_map(),
which now calls irq_set_chip_and_handler() to retain its previous
behaviour.

This patch also prepares for a follow-on which will call
gic_shared_irq_domain_map() from a callback where the lock on the struct
irq_desc is held, which without this change would cause the call to
irq_set_chip_and_handler() to lead to a deadlock.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Cc: stable@vger.kernel.org # v4.6+
---
Marked with the Fixes: tag as it's a prerequisite for the following bug fix
patch. Feel free to drop the tag if it's considered clearer without.
---
 drivers/irqchip/irq-mips-gic.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 70ed1d0..d7ec984 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -713,9 +713,6 @@ static int gic_shared_irq_domain_map(struct irq_domain *d, unsigned int virq,
 	unsigned long flags;
 	int i;
 
-	irq_set_chip_and_handler(virq, &gic_level_irq_controller,
-				 handle_level_irq);
-
 	spin_lock_irqsave(&gic_lock, flags);
 	gic_map_to_pin(intr, gic_cpu_pin);
 	gic_map_to_vpe(intr, mips_cm_vp_id(vpe));
@@ -732,6 +729,10 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
 {
 	if (GIC_HWIRQ_TO_LOCAL(hw) < GIC_NUM_LOCAL_INTRS)
 		return gic_local_irq_domain_map(d, virq, hw);
+
+	irq_set_chip_and_handler(virq, &gic_level_irq_controller,
+				 handle_level_irq);
+
 	return gic_shared_irq_domain_map(d, virq, hw, 0);
 }
 
@@ -771,11 +772,13 @@ static int gic_irq_domain_alloc(struct irq_domain *d, unsigned int virq,
 			hwirq = GIC_SHARED_TO_HWIRQ(base_hwirq + i);
 
 			ret = irq_domain_set_hwirq_and_chip(d, virq + i, hwirq,
-							    &gic_edge_irq_controller,
+							    &gic_level_irq_controller,
 							    NULL);
 			if (ret)
 				goto error;
 
+			irq_set_handler(virq + i, handle_level_irq);
+
 			ret = gic_shared_irq_domain_map(d, virq + i, hwirq, cpu);
 			if (ret)
 				goto error;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain
  2016-08-19 17:07 [PATCH 1/2] irqchip: mips-gic: Cleanup chip & handler setup Paul Burton
@ 2016-08-19 17:07 ` Paul Burton
  2016-08-22 15:14   ` [tip:x86/urgent] irqchip/mips-gic: " tip-bot for Paul Burton
  2016-08-22 16:43   ` [tip:irq/urgent] " tip-bot for Paul Burton
  2016-08-22 15:13 ` [tip:x86/urgent] irqchip/mips-gic: Cleanup chip and handler setup tip-bot for Paul Burton
  2016-08-22 16:43 ` [tip:irq/urgent] " tip-bot for Paul Burton
  2 siblings, 2 replies; 6+ messages in thread
From: Paul Burton @ 2016-08-19 17:07 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-kernel
  Cc: linux-mips, stable, Paul Burton

If an IRQ is setup using __setup_irq(), which is used by the
request_irq() family of functions, and we are using an SMP kernel then
the affinity of the IRQ will be set via setup_affinity() immediately
after the IRQ is enabled. This call to gic_set_affinity() will lead to
the interrupt being mapped to a VPE. However there are other ways to use
IRQs which don't cause affinity to be set, for example if it is used to
chain to another IRQ controller with irq_set_chained_handler_and_data().
The irq_set_chained_handler_and_data() code path will enable the IRQ,
but will not trigger a call to gic_set_affinity() and in this case
nothing will map the interrupt to a VPE, meaning that the interrupt is
never received.

Fix this by implementing the activate operation for the GIC device IRQ
domain, using gic_shared_irq_domain_map() to map the interrupt to the
correct pin of cpu 0.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Cc: stable@vger.kernel.org # v4.6+
---
This one would be great to get in ASAP, for v4.8 if possible, as it breaks
certain uses of GIC interrupts.
---
 drivers/irqchip/irq-mips-gic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index d7ec984..d3ef0fc 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -893,10 +893,17 @@ void gic_dev_domain_free(struct irq_domain *d, unsigned int virq,
 	return;
 }
 
+static void gic_dev_domain_activate(struct irq_domain *domain,
+				    struct irq_data *d)
+{
+	gic_shared_irq_domain_map(domain, d->irq, d->hwirq, 0);
+}
+
 static struct irq_domain_ops gic_dev_domain_ops = {
 	.xlate = gic_dev_domain_xlate,
 	.alloc = gic_dev_domain_alloc,
 	.free = gic_dev_domain_free,
+	.activate = gic_dev_domain_activate,
 };
 
 static int gic_ipi_domain_xlate(struct irq_domain *d, struct device_node *ctrlr,
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] irqchip/mips-gic: Cleanup chip and handler setup
  2016-08-19 17:07 [PATCH 1/2] irqchip: mips-gic: Cleanup chip & handler setup Paul Burton
  2016-08-19 17:07 ` [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain Paul Burton
@ 2016-08-22 15:13 ` tip-bot for Paul Burton
  2016-08-22 16:43 ` [tip:irq/urgent] " tip-bot for Paul Burton
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Paul Burton @ 2016-08-22 15:13 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: paul.burton, tglx, hpa, jason, marc.zyngier, linux-kernel, mingo

Commit-ID:  437844c1f96ceeb7153c09cc92d29b48b9beb337
Gitweb:     http://git.kernel.org/tip/437844c1f96ceeb7153c09cc92d29b48b9beb337
Author:     Paul Burton <paul.burton@imgtec.com>
AuthorDate: Fri, 19 Aug 2016 18:07:14 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 22 Aug 2016 17:07:34 +0200

irqchip/mips-gic: Cleanup chip and handler setup

gic_shared_irq_domain_map() is called from gic_irq_domain_alloc() where
the wrong chip has been set, and is then overwritten. Tidy this up by
setting the correct chip the first time, and setting the
handle_level_irq handler from gic_irq_domain_alloc() too.

gic_shared_irq_domain_map() is also called from gic_irq_domain_map(),
which now calls irq_set_chip_and_handler() to retain its previous
behaviour.

This patch prepares for a follow-on which will call
gic_shared_irq_domain_map() from a callback where the lock on the struct
irq_desc is held, which without this change would cause the call to
irq_set_chip_and_handler() to lead to a deadlock.

Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160819170715.27820-1-paul.burton@imgtec.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 drivers/irqchip/irq-mips-gic.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index c5f33c3..2e0f499 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -713,9 +713,6 @@ static int gic_shared_irq_domain_map(struct irq_domain *d, unsigned int virq,
 	unsigned long flags;
 	int i;
 
-	irq_set_chip_and_handler(virq, &gic_level_irq_controller,
-				 handle_level_irq);
-
 	spin_lock_irqsave(&gic_lock, flags);
 	gic_map_to_pin(intr, gic_cpu_pin);
 	gic_map_to_vpe(intr, mips_cm_vp_id(vpe));
@@ -732,6 +729,10 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
 {
 	if (GIC_HWIRQ_TO_LOCAL(hw) < GIC_NUM_LOCAL_INTRS)
 		return gic_local_irq_domain_map(d, virq, hw);
+
+	irq_set_chip_and_handler(virq, &gic_level_irq_controller,
+				 handle_level_irq);
+
 	return gic_shared_irq_domain_map(d, virq, hw, 0);
 }
 
@@ -771,11 +772,13 @@ static int gic_irq_domain_alloc(struct irq_domain *d, unsigned int virq,
 			hwirq = GIC_SHARED_TO_HWIRQ(base_hwirq + i);
 
 			ret = irq_domain_set_hwirq_and_chip(d, virq + i, hwirq,
-							    &gic_edge_irq_controller,
+							    &gic_level_irq_controller,
 							    NULL);
 			if (ret)
 				goto error;
 
+			irq_set_handler(virq + i, handle_level_irq);
+
 			ret = gic_shared_irq_domain_map(d, virq + i, hwirq, cpu);
 			if (ret)
 				goto error;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] irqchip/mips-gic: Implement activate op for device domain
  2016-08-19 17:07 ` [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain Paul Burton
@ 2016-08-22 15:14   ` tip-bot for Paul Burton
  2016-08-22 16:43   ` [tip:irq/urgent] " tip-bot for Paul Burton
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Paul Burton @ 2016-08-22 15:14 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: marc.zyngier, paul.burton, linux-kernel, tglx, hpa, jason, mingo

Commit-ID:  758f7981d114535e23e7c3dd98b4c11d5d02ff32
Gitweb:     http://git.kernel.org/tip/758f7981d114535e23e7c3dd98b4c11d5d02ff32
Author:     Paul Burton <paul.burton@imgtec.com>
AuthorDate: Fri, 19 Aug 2016 18:07:15 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 22 Aug 2016 17:07:34 +0200

irqchip/mips-gic: Implement activate op for device domain

If an IRQ is setup using __setup_irq(), which is used by the
request_irq() family of functions, and we are using an SMP kernel then
the affinity of the IRQ will be set via setup_affinity() immediately
after the IRQ is enabled. This call to gic_set_affinity() will lead to
the interrupt being mapped to a VPE. However there are other ways to use
IRQs which don't cause affinity to be set, for example if it is used to
chain to another IRQ controller with irq_set_chained_handler_and_data().
The irq_set_chained_handler_and_data() code path will enable the IRQ,
but will not trigger a call to gic_set_affinity() and in this case
nothing will map the interrupt to a VPE, meaning that the interrupt is
never received.

Fix this by implementing the activate operation for the GIC device IRQ
domain, using gic_shared_irq_domain_map() to map the interrupt to the
correct pin of cpu 0.

Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160819170715.27820-2-paul.burton@imgtec.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 drivers/irqchip/irq-mips-gic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 2e0f499..83f4983 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -893,10 +893,17 @@ void gic_dev_domain_free(struct irq_domain *d, unsigned int virq,
 	return;
 }
 
+static void gic_dev_domain_activate(struct irq_domain *domain,
+				    struct irq_data *d)
+{
+	gic_shared_irq_domain_map(domain, d->irq, d->hwirq, 0);
+}
+
 static struct irq_domain_ops gic_dev_domain_ops = {
 	.xlate = gic_dev_domain_xlate,
 	.alloc = gic_dev_domain_alloc,
 	.free = gic_dev_domain_free,
+	.activate = gic_dev_domain_activate,
 };
 
 static int gic_ipi_domain_xlate(struct irq_domain *d, struct device_node *ctrlr,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:irq/urgent] irqchip/mips-gic: Cleanup chip and handler setup
  2016-08-19 17:07 [PATCH 1/2] irqchip: mips-gic: Cleanup chip & handler setup Paul Burton
  2016-08-19 17:07 ` [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain Paul Burton
  2016-08-22 15:13 ` [tip:x86/urgent] irqchip/mips-gic: Cleanup chip and handler setup tip-bot for Paul Burton
@ 2016-08-22 16:43 ` tip-bot for Paul Burton
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Paul Burton @ 2016-08-22 16:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, marc.zyngier, tglx, jason, mingo, paul.burton, linux-kernel

Commit-ID:  6a33fa2b87513fee44cb8f0cd17b1acd6316bc6b
Gitweb:     http://git.kernel.org/tip/6a33fa2b87513fee44cb8f0cd17b1acd6316bc6b
Author:     Paul Burton <paul.burton@imgtec.com>
AuthorDate: Fri, 19 Aug 2016 18:07:14 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 22 Aug 2016 18:37:51 +0200

irqchip/mips-gic: Cleanup chip and handler setup

gic_shared_irq_domain_map() is called from gic_irq_domain_alloc() where
the wrong chip has been set, and is then overwritten. Tidy this up by
setting the correct chip the first time, and setting the
handle_level_irq handler from gic_irq_domain_alloc() too.

gic_shared_irq_domain_map() is also called from gic_irq_domain_map(),
which now calls irq_set_chip_and_handler() to retain its previous
behaviour.

This patch prepares for a follow-on which will call
gic_shared_irq_domain_map() from a callback where the lock on the struct
irq_desc is held, which without this change would cause the call to
irq_set_chip_and_handler() to lead to a deadlock.

Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160819170715.27820-1-paul.burton@imgtec.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 drivers/irqchip/irq-mips-gic.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index c5f33c3..2e0f499 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -713,9 +713,6 @@ static int gic_shared_irq_domain_map(struct irq_domain *d, unsigned int virq,
 	unsigned long flags;
 	int i;
 
-	irq_set_chip_and_handler(virq, &gic_level_irq_controller,
-				 handle_level_irq);
-
 	spin_lock_irqsave(&gic_lock, flags);
 	gic_map_to_pin(intr, gic_cpu_pin);
 	gic_map_to_vpe(intr, mips_cm_vp_id(vpe));
@@ -732,6 +729,10 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
 {
 	if (GIC_HWIRQ_TO_LOCAL(hw) < GIC_NUM_LOCAL_INTRS)
 		return gic_local_irq_domain_map(d, virq, hw);
+
+	irq_set_chip_and_handler(virq, &gic_level_irq_controller,
+				 handle_level_irq);
+
 	return gic_shared_irq_domain_map(d, virq, hw, 0);
 }
 
@@ -771,11 +772,13 @@ static int gic_irq_domain_alloc(struct irq_domain *d, unsigned int virq,
 			hwirq = GIC_SHARED_TO_HWIRQ(base_hwirq + i);
 
 			ret = irq_domain_set_hwirq_and_chip(d, virq + i, hwirq,
-							    &gic_edge_irq_controller,
+							    &gic_level_irq_controller,
 							    NULL);
 			if (ret)
 				goto error;
 
+			irq_set_handler(virq + i, handle_level_irq);
+
 			ret = gic_shared_irq_domain_map(d, virq + i, hwirq, cpu);
 			if (ret)
 				goto error;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:irq/urgent] irqchip/mips-gic: Implement activate op for device domain
  2016-08-19 17:07 ` [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain Paul Burton
  2016-08-22 15:14   ` [tip:x86/urgent] irqchip/mips-gic: " tip-bot for Paul Burton
@ 2016-08-22 16:43   ` tip-bot for Paul Burton
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Paul Burton @ 2016-08-22 16:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jason, linux-kernel, paul.burton, mingo, marc.zyngier, hpa, tglx

Commit-ID:  2564970a381651865364974ea414384b569cb9c0
Gitweb:     http://git.kernel.org/tip/2564970a381651865364974ea414384b569cb9c0
Author:     Paul Burton <paul.burton@imgtec.com>
AuthorDate: Fri, 19 Aug 2016 18:07:15 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 22 Aug 2016 18:37:51 +0200

irqchip/mips-gic: Implement activate op for device domain

If an IRQ is setup using __setup_irq(), which is used by the
request_irq() family of functions, and we are using an SMP kernel then
the affinity of the IRQ will be set via setup_affinity() immediately
after the IRQ is enabled. This call to gic_set_affinity() will lead to
the interrupt being mapped to a VPE. However there are other ways to use
IRQs which don't cause affinity to be set, for example if it is used to
chain to another IRQ controller with irq_set_chained_handler_and_data().
The irq_set_chained_handler_and_data() code path will enable the IRQ,
but will not trigger a call to gic_set_affinity() and in this case
nothing will map the interrupt to a VPE, meaning that the interrupt is
never received.

Fix this by implementing the activate operation for the GIC device IRQ
domain, using gic_shared_irq_domain_map() to map the interrupt to the
correct pin of cpu 0.

Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160819170715.27820-2-paul.burton@imgtec.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 drivers/irqchip/irq-mips-gic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 2e0f499..83f4983 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -893,10 +893,17 @@ void gic_dev_domain_free(struct irq_domain *d, unsigned int virq,
 	return;
 }
 
+static void gic_dev_domain_activate(struct irq_domain *domain,
+				    struct irq_data *d)
+{
+	gic_shared_irq_domain_map(domain, d->irq, d->hwirq, 0);
+}
+
 static struct irq_domain_ops gic_dev_domain_ops = {
 	.xlate = gic_dev_domain_xlate,
 	.alloc = gic_dev_domain_alloc,
 	.free = gic_dev_domain_free,
+	.activate = gic_dev_domain_activate,
 };
 
 static int gic_ipi_domain_xlate(struct irq_domain *d, struct device_node *ctrlr,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-22 16:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-19 17:07 [PATCH 1/2] irqchip: mips-gic: Cleanup chip & handler setup Paul Burton
2016-08-19 17:07 ` [PATCH 2/2] irqchip: mips-gic: Implement activate op for device domain Paul Burton
2016-08-22 15:14   ` [tip:x86/urgent] irqchip/mips-gic: " tip-bot for Paul Burton
2016-08-22 16:43   ` [tip:irq/urgent] " tip-bot for Paul Burton
2016-08-22 15:13 ` [tip:x86/urgent] irqchip/mips-gic: Cleanup chip and handler setup tip-bot for Paul Burton
2016-08-22 16:43 ` [tip:irq/urgent] " tip-bot for Paul Burton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox