[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <11284577.ZthGdGvihU@jclayton-pc>
Date: Mon, 19 Jan 2015 08:21:54 -0800
From: Joshua Clayton <joshua.clayton@...west.com>
To: linux-arm-kernel@...ts.infradead.org
Cc: Daniel Thompson <daniel.thompson@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Jason Cooper <jason@...edaemon.net>,
Russell King <linux@....linux.org.uk>,
linaro-kernel@...ts.linaro.org, patches@...aro.org,
Stephen Boyd <sboyd@...eaurora.org>,
linux-kernel@...r.kernel.org, Daniel Drake <drake@...lessm.com>,
Dmitry Pervushin <dpervushin@...il.com>,
Dirk Behme <dirk.behme@...bosch.com>,
John Stultz <john.stultz@...aro.org>,
Tim Sander <tim@...eglstein.org>,
Sumit Semwal <sumit.semwal@...aro.org>
Subject: Re: [RFC PATCH 2/5] irq: Allow interrupts to routed to NMI (or similar)
Daniel, feel free to ignore my comments.
I know less about this than anybody :)
I am especially unfamiliar with NMI, but I will risk exposing my ignorance
On Tuesday, January 13, 2015 04:35:28 PM Daniel Thompson wrote:
> Some combinations of architectures and interrupt controllers make it
> possible for abitrary interrupt signals to be selectively made immune to
> masking by local_irq_disable(). For example, on ARM platforms, many
> interrupt controllers allow interrupts to be routed to FIQ rather than IRQ.
>
> These features could be exploited to implement debug and tracing features
> that can be implemented using NMI on x86 platforms (perf, hard lockup,
> kgdb).
>
> This patch assumes that the management of the NMI handler itself will be
> architecture specific (maybe notifier list like x86, hard coded like ARM,
> or something else entirely). The generic layer can still manage the irq
> as normal (affinity, enable/disable, free) but is not responsible for
> dispatching.
>
> Signed-off-by: Daniel Thompson <daniel.thompson@...aro.org>
> ---
> include/linux/interrupt.h | 20 ++++++++++++++++++++
> include/linux/irq.h | 2 ++
> kernel/irq/manage.c | 29 +++++++++++++++++++++++++++--
> 3 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index d9b05b5bf8c7..b95dc28f4cc3 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -57,6 +57,9 @@
> * IRQF_NO_THREAD - Interrupt cannot be threaded
> * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
> * resume time.
> + * __IRQF_NMI - Route the interrupt to an NMI or some similar signal that
> is not + * masked by local_irq_disable(). Used internally by +
> * request_nmi_irq().
> */
> #define IRQF_DISABLED 0x00000020
> #define IRQF_SHARED 0x00000080
> @@ -70,6 +73,7 @@
> #define IRQF_FORCE_RESUME 0x00008000
> #define IRQF_NO_THREAD 0x00010000
> #define IRQF_EARLY_RESUME 0x00020000
> +#define __IRQF_NMI 0x00040000
>
> #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
>
> @@ -139,6 +143,22 @@ extern int __must_check
> request_percpu_irq(unsigned int irq, irq_handler_t handler,
> const char *devname, void __percpu *percpu_dev_id);
>
> +static inline int __must_check request_nmi_irq(unsigned int irq,
> + unsigned long flags,
> + const char *name, void *dev_id)
Why not pass your handler in here, instead of adding explicitly it to the
default FIQ handler?
request_nmi_irq need not exist at all. Your __IRQF_NMI flag is sufficient with
request_irq.
> +{
> + /*
> + * no_action unconditionally returns IRQ_NONE which is exactly
> + * what we need. The handler might be expected to be unreachable
> + * but some controllers may spuriously ack the NMI from the IRQ
> + * handling code. When this happens it occurs very rarely, thus
> + * by returning IRQ_NONE we can rely on the spurious interrupt
> + * logic to do the right thing.
> + */
> + return request_irq(irq, no_action, flags | IRQF_NO_THREAD | __IRQF_NMI,
> + name, dev_id);
no_action here looks kind of evil to me.
I'd prefer to see the nonsecure irq handler check for __IRQF_NMI and call
no_action.
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6f1c7a5..f810cff 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -711,7 +711,10 @@ void handle_percpu_devid_irq(unsigned int irq, struct
irq_desc *desc)
chip->irq_ack(&desc->irq_data);
trace_irq_handler_entry(irq, action);
- res = action->handler(irq, dev_id);
+ if (action->flags & __IRQF_NMI)
+ res = no_action(irq, dev_id);
+ else
+ res = action->handler(irq, dev_id);
trace_irq_handler_exit(irq, action, res);
if (chip->irq_eoi)
> +}
> +
> extern void free_irq(unsigned int, void *);
> extern void free_percpu_irq(unsigned int, void __percpu *);
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index d09ec7a1243e..695eb37f04ae 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -307,6 +307,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct
> irq_data *d) * @irq_eoi: end of interrupt
> * @irq_set_affinity: set the CPU affinity on SMP machines
> * @irq_retrigger: resend an IRQ to the CPU
> + * @irq_set_nmi_routing:set whether interrupt can act like NMI
> * @irq_set_type: set the flow type (IRQ_TYPE_LEVEL/etc.) of an IRQ
> * @irq_set_wake: enable/disable power-management wake-on of an IRQ
> * @irq_bus_lock: function to lock access to slow bus (i2c) chips
> @@ -341,6 +342,7 @@ struct irq_chip {
>
> int (*irq_set_affinity)(struct irq_data *data, const struct cpumask
> *dest, bool force); int (*irq_retrigger)(struct irq_data *data);
> + int (*irq_set_nmi_routing)(struct irq_data *data, unsigned int nmi);
> int (*irq_set_type)(struct irq_data *data, unsigned int flow_type);
> int (*irq_set_wake)(struct irq_data *data, unsigned int on);
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 80692373abd6..8e669051759d 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -571,6 +571,17 @@ int can_request_irq(unsigned int irq, unsigned long
> irqflags) return canrequest;
> }
>
> +int __irq_set_nmi_routing(struct irq_desc *desc, unsigned int irq,
> + unsigned int nmi)
> +{
> + struct irq_chip *chip = desc->irq_data.chip;
> +
> + if (!chip || !chip->irq_set_nmi_routing)
> + return -EINVAL;
> +
> + return chip->irq_set_nmi_routing(&desc->irq_data, nmi);
> +}
> +
> int __irq_set_trigger(struct irq_desc *desc, unsigned int irq,
> unsigned long flags)
> {
> @@ -1058,11 +1069,12 @@ __setup_irq(unsigned int irq, struct irq_desc *desc,
> struct irqaction *new) * the same type (level, edge, polarity). So both
> flag
> * fields must have IRQF_SHARED set and the bits which
> * set the trigger type must match. Also all must
> - * agree on ONESHOT.
> + * agree on ONESHOT and NMI
> */
> if (!((old->flags & new->flags) & IRQF_SHARED) ||
> ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
> - ((old->flags ^ new->flags) & IRQF_ONESHOT))
> + ((old->flags ^ new->flags) & IRQF_ONESHOT) ||
> + ((old->flags ^ new->flags) & __IRQF_NMI))
> goto mismatch;
>
> /* All handlers must agree on per-cpuness */
> @@ -1153,6 +1165,19 @@ __setup_irq(unsigned int irq, struct irq_desc *desc,
> struct irqaction *new)
>
> init_waitqueue_head(&desc->wait_for_threads);
>
> + if (new->flags & __IRQF_NMI) {
> + ret = __irq_set_nmi_routing(desc, irq, true);
> + if (ret != 1)
> + goto out_mask;
> + } else {
> + ret = __irq_set_nmi_routing(desc, irq, false);
> + if (ret == 1) {
> + pr_err("Failed to disable NMI routing for irq %d\n",
> + irq);
> + goto out_mask;
> + }
> + }
> +
> /* Setup the type (level, edge polarity) if configured: */
> if (new->flags & IRQF_TRIGGER_MASK) {
> ret = __irq_set_trigger(desc, irq,
> --
> 1.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Joshua Clayton
Software Engineer
UniWest
122 S. 4th Avenue
Pasco, WA 99301
Ph: (509) 544-0720
Fx: (509) 544-0868
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists