[PATCH 1/3] genirq: Add support for priority-drop/deactivate interrupt controllers
Marc Zyngier
marc.zyngier at arm.com
Thu Oct 30 07:15:32 PDT 2014
On 29/10/14 10:26, Thomas Gleixner wrote:
> On Wed, 29 Oct 2014, Marc Zyngier wrote:
>> On 28/10/14 20:14, Thomas Gleixner wrote:
>>> irq_enable() calls chip->irq_unmask(), i.e. DIR. So that clears the
>>> ACTIVE bit and then the IRQ either gets resent by hardware (in case of
>>> level as the device interrupt is still active) or retriggered by the
>>> irq_retrigger() callback.
>>
>> The problem I see here is for an interrupt that has been flagged as
>> disabled with irq_disabled(), but that hasn't fired. We'd end up doing a
>> DIR on something that hasn't had an EOI first. I think that's the only
>> wrinkle in this scheme.
>
> Right. So the untested patch below should do the trick and prevent
> irq_enable() to invoke irq_unmask() if the interrupt is not flagged
> masked. And it only can be flagged masked if it was masked in the
> handler. The startup callback will make sure that irq_enable() is not
> invoked at startup time.
>
> Thanks,
>
> tglx
>
> ------------------
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index e5202f00cabc..9c0f73e1994a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -182,7 +182,7 @@ int irq_startup(struct irq_desc *desc, bool resend)
> ret = desc->irq_data.chip->irq_startup(&desc->irq_data);
> irq_state_clr_masked(desc);
> } else {
> - irq_enable(desc);
> + irq_enable(desc, false);
> }
> if (resend)
> check_irq_resend(desc, desc->irq_data.irq);
> @@ -202,13 +202,14 @@ void irq_shutdown(struct irq_desc *desc)
> irq_state_set_masked(desc);
> }
>
> -void irq_enable(struct irq_desc *desc)
> +void irq_enable(struct irq_desc *desc, bool ifmasked)
> {
> irq_state_clr_disabled(desc);
> - if (desc->irq_data.chip->irq_enable)
> + if (desc->irq_data.chip->irq_enable) {
> desc->irq_data.chip->irq_enable(&desc->irq_data);
> - else
> + } else if (!ifmasked || irqd_irq_masked(&desc->irq_data)) {
> desc->irq_data.chip->irq_unmask(&desc->irq_data);
> + }
> irq_state_clr_masked(desc);
> }
>
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index 4332d766619d..6eff2678cf6d 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -68,7 +68,7 @@ extern void __enable_irq(struct irq_desc *desc, unsigned int irq);
>
> extern int irq_startup(struct irq_desc *desc, bool resend);
> extern void irq_shutdown(struct irq_desc *desc);
> -extern void irq_enable(struct irq_desc *desc);
> +extern void irq_enable(struct irq_desc *desc, bool ifmasked);
> extern void irq_disable(struct irq_desc *desc);
> extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
> extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 0a9104b4608b..d8c474608c2d 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -448,7 +448,7 @@ void __enable_irq(struct irq_desc *desc, unsigned int irq)
> goto err_out;
> /* Prevent probing on this irq: */
> irq_settings_set_noprobe(desc);
> - irq_enable(desc);
> + irq_enable(desc, true);
> check_irq_resend(desc, irq);
> /* fall-through */
> }
>
So I actually implemented this, and did hit another snag: per cpu interrupts.
They don't use the startup/shutdown methods, and reproducing the above logic
on a per-cpu basis is not very pretty.
In order to make some progress, I went on a slightly different path, which
is to use enable/disable instead of startup/shutdown. As far as I can see,
the only thing we loose by doing so is the lazy disable, but the code
becomes very straightforward (see below). I gave it a go on an ARMv7 box,
and it even survived.
Thoughts?
M.
>From fee4719e3bd82536bf72a62aab619b873ed1b6f0 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier at arm.com>
Date: Thu, 23 Oct 2014 09:25:47 +0100
Subject: [PATCH] genirq: Add support for priority-drop/deactivate interrupt
controllers
Moderately recent ARM interrupt controllers can use a "split mode" EOI,
where instead of just using a single write to notify the controller of
the end of interrupt, uses the following:
- priority-drop: the interrupt is still active, but other interrupts can
now be taken (basically the equivalent of a mask)
- deactivate: the interrupt is not active anymore, and can be taken again
(equivalent to unmask+eoi).
This makes it very useful for threaded interrupts, as it avoids the usual
mask/unmask dance (and has the potential of being more efficient on ARM,
as it is using the CPU interface instead of the global distributor).
This patch implements a couple of new interrupt flows:
- handle_spliteoi_irq: this is the direct equivalent of handle_fasteoi_irq,
- handle_spliteoi_percpu_devid_irq: equivalent to handle_percpu_devid_irq.
It is expected that irqchip using these flows will implement something like:
.irq_enable = irqchip_unmask_irq,
.irq_disable = irqchip_mask_irq,
.irq_mask = irqchip_priority_drop_irq,
.irq_unmask = irqchip_deactivate_irq,
Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
---
include/linux/irq.h | 2 ++
kernel/irq/chip.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 84 insertions(+), 8 deletions(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 165fac0..0887634 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -444,11 +444,13 @@ static inline int irq_set_parent(int irq, int parent_irq)
*/
extern void handle_level_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc);
+extern void handle_spliteoi_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_edge_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_edge_eoi_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_simple_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_percpu_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc);
+extern void handle_spliteoi_percpu_devid_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_bad_irq(unsigned int irq, struct irq_desc *desc);
extern void handle_nested_irq(unsigned int irq);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index e5202f0..84d2162 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -542,6 +542,56 @@ out:
EXPORT_SYMBOL_GPL(handle_fasteoi_irq);
/**
+ * handle_spliteoi_irq - irq handler for 2-phase-eoi controllers
+ * @irq: the interrupt number
+ * @desc: the interrupt description structure for this irq
+ *
+ * This relies on mask being a very cheap operation, and on
+ * unmask performing both unmask+EOI. This avoids additional
+ * operations for threaded interrupts (typically ARM's GICv2/v3).
+ */
+void
+handle_spliteoi_irq(unsigned int irq, struct irq_desc *desc)
+{
+ raw_spin_lock(&desc->lock);
+
+ if (!irq_may_run(desc))
+ goto out;
+
+ desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+ kstat_incr_irqs_this_cpu(irq, desc);
+
+ /* Mark the IRQ as in progress */
+ mask_irq(desc);
+
+ /*
+ * If it's disabled or no action available
+ * then just get out of here:
+ */
+ if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+ desc->istate |= IRQS_PENDING;
+ goto out_unmask;
+ }
+
+ handle_irq_event(desc);
+
+ /* In case the handler itself disabled it */
+ if (irqd_irq_disabled(&desc->irq_data))
+ goto out_unmask;
+
+ /* If the thread is running, leave the IRQ in progress */
+ if (atomic_read(&desc->threads_active))
+ goto out;
+
+out_unmask:
+ /* Terminate the handling */
+ unmask_irq(desc);
+out:
+ raw_spin_unlock(&desc->lock);
+}
+EXPORT_SYMBOL_GPL(handle_spliteoi_irq);
+
+/**
* handle_edge_irq - edge type IRQ handler
* @irq: the interrupt number
* @desc: the interrupt description structure for this irq
@@ -683,6 +733,19 @@ handle_percpu_irq(unsigned int irq, struct irq_desc *desc)
chip->irq_eoi(&desc->irq_data);
}
+static void __handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
+{
+ struct irqaction *action = desc->action;
+ void *dev_id = raw_cpu_ptr(action->percpu_dev_id);
+ irqreturn_t res;
+
+ kstat_incr_irqs_this_cpu(irq, desc);
+
+ trace_irq_handler_entry(irq, action);
+ res = action->handler(irq, dev_id);
+ trace_irq_handler_exit(irq, action, res);
+}
+
/**
* handle_percpu_devid_irq - Per CPU local irq handler with per cpu dev ids
* @irq: the interrupt number
@@ -698,23 +761,34 @@ handle_percpu_irq(unsigned int irq, struct irq_desc *desc)
void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
- struct irqaction *action = desc->action;
- void *dev_id = raw_cpu_ptr(action->percpu_dev_id);
- irqreturn_t res;
-
- kstat_incr_irqs_this_cpu(irq, desc);
if (chip->irq_ack)
chip->irq_ack(&desc->irq_data);
- trace_irq_handler_entry(irq, action);
- res = action->handler(irq, dev_id);
- trace_irq_handler_exit(irq, action, res);
+ __handle_percpu_devid_irq(irq, desc);
if (chip->irq_eoi)
chip->irq_eoi(&desc->irq_data);
}
+/**
+ * handle_spliteoi_percpu_devid_irq - Per CPU local irq handler with per cpu dev ids
+ * @irq: the interrupt number
+ * @desc: the interrupt description structure for this irq
+ *
+ * Per CPU interrupts on SMP machines without locking requirements.
+ * Same as handle_percpu_devid_irq() above, but using the 2-phase-eoi
+ * model for the handling.
+ */
+void handle_spliteoi_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
+{
+ mask_irq(desc);
+
+ __handle_percpu_devid_irq(irq, desc);
+
+ unmask_irq(desc);
+}
+
void
__irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
const char *name)
--
2.0.4
--
Jazz is not dead. It just smells funny...
More information about the linux-arm-kernel
mailing list