linux-kernel - Re: [patch 07/16] genirq: Introduce helper irq_domain_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54662062.8030807@arm.com>
Date:	Fri, 14 Nov 2014 15:31:46 +0000
From:	Marc Zyngier <marc.zyngier@....com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	LKML <linux-kernel@...r.kernel.org>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	Yingjoe Chen <yingjoe.chen@...iatek.com>,
	Yijing Wang <wangyijing@...wei.com>
Subject: Re: [patch 07/16] genirq: Introduce helper irq_domain_set_info()
 to reduce duplicated code

On 12/11/14 13:43, Thomas Gleixner wrote:
> From: Jiang Liu <jiang.liu@...ux.intel.com>
> 
> Signed-off-by: Jiang Liu <jiang.liu@...ux.intel.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Grant Likely <grant.likely@...aro.org>
> Cc: Marc Zyngier <marc.zyngier@....com>
> Cc: Yingjoe Chen <yingjoe.chen@...iatek.com>
> Cc: Yijing Wang <wangyijing@...wei.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>  include/linux/irqdomain.h |    5 +++++
>  kernel/irq/irqdomain.c    |   10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> Index: tip/include/linux/irqdomain.h
> ===================================================================
> --- tip.orig/include/linux/irqdomain.h
> +++ tip/include/linux/irqdomain.h
> @@ -33,6 +33,7 @@
>  #define _LINUX_IRQDOMAIN_H
>  
>  #include <linux/types.h>
> +#include <linux/irqhandler.h>
>  #include <linux/radix-tree.h>
>  
>  struct device_node;
> @@ -263,6 +264,10 @@ extern int irq_domain_set_hwirq_and_chip
>  					 irq_hw_number_t hwirq,
>  					 struct irq_chip *chip,
>  					 void *chip_data);
> +extern void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
> +				irq_hw_number_t hwirq, struct irq_chip *chip,
> +				void *chip_data, irq_flow_handler_t handler,
> +				void *handler_data, const char *handler_name);
>  extern void irq_domain_reset_irq_data(struct irq_data *irq_data);
>  extern void irq_domain_free_irqs_common(struct irq_domain *domain,
>  					int virq, int nr_irqs);
> Index: tip/kernel/irq/irqdomain.c
> ===================================================================
> --- tip.orig/kernel/irq/irqdomain.c
> +++ tip/kernel/irq/irqdomain.c
> @@ -882,6 +882,16 @@ int irq_domain_set_hwirq_and_chip(struct
>  	return 0;
>  }
>  
> +void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
> +			 irq_hw_number_t hwirq, struct irq_chip *chip,
> +			 void *chip_data, irq_flow_handler_t handler,
> +			 void *handler_data, const char *handler_name)
> +{
> +	irq_domain_set_hwirq_and_chip(domain, virq, hwirq, chip, chip_data);
> +	__irq_set_handler(virq, handler, 0, handler_name);
> +	irq_set_handler_data(virq, handler_data);
> +}
> +

We still have the issue that, depending on where in the stack this is
 called, this will succeed or fail: If this is called from the inner
irqchip, __irq_set_handler() will fail, as it will look at the outer
domain as the (desc->irq_data.chip == &no_irq_chip) test fails (we
haven't set the top level yet).

I have this very imperfect workaround in my tree:

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index d028b34..91e6515 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -731,7 +731,16 @@ __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
 	if (!handle) {
 		handle = handle_bad_irq;
 	} else {
-		if (WARN_ON(desc->irq_data.chip == &no_irq_chip))
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+		struct irq_data *irq_data = &desc->irq_data;
+		while (irq_data) {
+			if (irq_data->chip != &no_irq_chip)
+				break;
+			irq_data = irq_data->parent_data;
+		}
+#endif
+
+		if (WARN_ON(!irq_data || irq_data->chip == &no_irq_chip))
 			goto out;
 	}
 
Which translate into: If there is at least one irqchip in the domain,
it will probably sort itself out. Not ideal. Any real solution to
this problem?

GICv2 faces this exact problem, as some of its interrupts are used
directly, and some others are used through the MSI domain. In the
GIC driver, it is almost impossible to find out...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

--
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/