On 17/03/16 14:19, Jon Hunter wrote:
> When mapping an IRQ, if a mapping already exists, then we simply return
> the virtual IRQ number. However, we do not check that the type settings
> for the existing mapping match those for the mapping that is about to be
> created. It may be unlikely that the type settings would not match, but
> check for this and don't return a valid IRQ mapping if the type settings
> do not match.
>
> WARN if the type return by irq_domain_translate() has bits outside the
> sense mask set and then clear these bits. If these bits are not cleared
> then this will cause the comparision of the type settings for an
> existing mapping to fail with that of the new mapping even if the sense
> bit themselves match. The reason being is that the existing type
> settings are read by calling irq_get_trigger_type() which will clear
> any bits outside the sense mask. This will allow us to detect irqchips
> that are not correctly clearing these bits and fix them.
>
> Signed-off-by: Jon Hunter <jonath...@nvidia.com>
> ---
> kernel/irq/irqdomain.c | 59
> ++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 3a519a01118b..0ea285baa619 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -549,6 +549,13 @@ static int irq_domain_translate(struct irq_domain *d,
> fwspec->param, fwspec->param_count,
> hwirq, type);
>
> + /*
> + * WARN if the irqchip returns a type with bits
> + * outside the sense mask set and clear these bits.
> + */
> + if (WARN_ON(*type & ~IRQ_TYPE_SENSE_MASK))
> + *type &= IRQ_TYPE_SENSE_MASK;
> +
> /* If domain has no translation, then we assume interrupt line */
> *hwirq = fwspec->param[0];
> return 0;
> @@ -570,6 +577,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec
> *fwspec)
> {
> struct irq_domain *domain;
> irq_hw_number_t hwirq;
> + unsigned int cur_type = IRQ_TYPE_NONE;
> unsigned int type = IRQ_TYPE_NONE;
> int virq;
>
> @@ -592,23 +600,42 @@ unsigned int irq_create_fwspec_mapping(struct
> irq_fwspec *fwspec)
> if (irq_domain_translate(domain, fwspec, &hwirq, &type))
> return 0;
>
> - if (irq_domain_is_hierarchy(domain)) {
> - /*
> - * If we've already configured this interrupt,
> - * don't do it again, or hell will break loose.
> - */
> - virq = irq_find_mapping(domain, hwirq);
> - if (virq)
> - return virq;
> -
> - virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec);
> - if (virq <= 0)
> - return 0;
> + /*
> + * If we've already configured this interrupt,
> + * don't do it again, or hell will break loose.
> + */
> + virq = irq_find_mapping(domain, hwirq);
> + if (!virq) {
> + if (irq_domain_is_hierarchy(domain)) {
> + virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE,
> + fwspec);
> + if (virq <= 0)
> + return 0;
> + } else {
> + /* Create mapping */
> + virq = irq_create_mapping(domain, hwirq);
> + if (!virq)
> + return virq;
> + }
> } else {
> - /* Create mapping */
> - virq = irq_create_mapping(domain, hwirq);
> - if (!virq)
> - return virq;
> + cur_type = irq_get_trigger_type(virq);
> + }
> +
> + /*
> + * If the trigger type is not specified or matches the current
> + * trigger type then we are done so return the interrupt number.
> + */
> + if (type == IRQ_TYPE_NONE || type == cur_type)
> + return virq;
> +
> + /*
> + * If the trigger type is already set and does
> + * not match this interrupt, then return 0.
> + */
> + if (cur_type != IRQ_TYPE_NONE) {
> + pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n",
> + hwirq, of_node_full_name(to_of_node(fwspec->fwnode)));
> + return 0;
If we created a new mapping and return here, then this will leak the
mapping. I will fix this. Sorry should have caught this before!
Jon
