2014-11-12 13:43:09

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 09/16] genirq: Add generic msi irq domain support

From: Jiang Liu <[email protected]>

Implement the basic functions for MSI interrupt support with
hierarchical interrupt domains.

[ tglx: Extracted and combined from several patches ]

Signed-off-by: Jiang Liu <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Yingjoe Chen <[email protected]>
Cc: Yijing Wang <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/msi.h | 35 +++++++++++++++
kernel/irq/Kconfig | 10 ++++
kernel/irq/Makefile | 1
kernel/irq/msi.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 165 insertions(+)

Index: tip/include/linux/msi.h
===================================================================
--- tip.orig/include/linux/msi.h
+++ tip/include/linux/msi.h
@@ -77,4 +77,39 @@ struct msi_chip {
void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
};

+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+struct irq_domain;
+struct irq_chip;
+struct device_node;
+struct msi_domain_info;
+
+struct msi_domain_ops {
+ void (*calc_hwirq)(struct msi_domain_info *info, void *arg,
+ struct msi_desc *desc);
+ irq_hw_number_t (*get_hwirq)(struct msi_domain_info *info, void *arg);
+ int (*msi_init)(struct irq_domain *domain,
+ struct msi_domain_info *info,
+ unsigned int virq, irq_hw_number_t hwirq,
+ void *arg);
+ void (*msi_free)(struct irq_domain *domain,
+ struct msi_domain_info *info,
+ unsigned int virq);
+};
+
+struct msi_domain_info {
+ struct msi_domain_ops *ops;
+ struct irq_chip *chip;
+ void *data;
+};
+
+int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
+ bool force);
+
+struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
+ struct msi_domain_info *info,
+ struct irq_domain *parent);
+struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
+
+#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
+
#endif /* LINUX_MSI_H */
Index: tip/kernel/irq/Kconfig
===================================================================
--- tip.orig/kernel/irq/Kconfig
+++ tip/kernel/irq/Kconfig
@@ -60,6 +60,16 @@ config IRQ_DOMAIN_HIERARCHY
bool
select IRQ_DOMAIN

+# Generic MSI interrupt support
+config GENERIC_MSI_IRQ
+ bool
+
+# Generic MSI hierarchical interrupt domain support
+config GENERIC_MSI_IRQ_DOMAIN
+ bool
+ select IRQ_DOMAIN_HIERARCHY
+ select GENERIC_MSI_IRQ
+
config HANDLE_DOMAIN_IRQ
bool

Index: tip/kernel/irq/Makefile
===================================================================
--- tip.orig/kernel/irq/Makefile
+++ tip/kernel/irq/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_IRQ_DOMAIN) += irqdomain.o
obj-$(CONFIG_PROC_FS) += proc.o
obj-$(CONFIG_GENERIC_PENDING_IRQ) += migration.o
obj-$(CONFIG_PM_SLEEP) += pm.o
+obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
Index: tip/kernel/irq/msi.c
===================================================================
--- /dev/null
+++ tip/kernel/irq/msi.c
@@ -0,0 +1,119 @@
+/*
+ * linux/kernel/irq/msi.c
+ *
+ * Copyright (C) 2014 Intel Corp.
+ * Author: Jiang Liu <[email protected]>
+ *
+ * This file is licensed under GPLv2.
+ *
+ * This file contains common code to support Message Signalled Interrupt for
+ * PCI compatible and non PCI compatible devices.
+ */
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/msi.h>
+
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+int msi_domain_set_affinity(struct irq_data *irq_data,
+ const struct cpumask *mask, bool force)
+{
+ struct irq_data *parent = irq_data->parent_data;
+ struct msi_msg msg;
+ int ret;
+
+ ret = parent->chip->irq_set_affinity(parent, mask, force);
+ if (ret >= 0 && ret != IRQ_SET_MASK_OK_DONE) {
+ BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
+ irq_chip_write_msi_msg(irq_data, &msg);
+ }
+
+ return ret;
+}
+
+static void msi_domain_activate(struct irq_domain *domain,
+ struct irq_data *irq_data)
+{
+ struct msi_msg msg;
+
+ BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
+ irq_chip_write_msi_msg(irq_data, &msg);
+}
+
+static void msi_domain_deactivate(struct irq_domain *domain,
+ struct irq_data *irq_data)
+{
+ struct msi_msg msg;
+
+ memset(&msg, 0, sizeof(msg));
+ irq_chip_write_msi_msg(irq_data, &msg);
+}
+
+static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ struct msi_domain_info *info = domain->host_data;
+ struct msi_domain_ops *ops = info->ops;
+ irq_hw_number_t hwirq = ops->get_hwirq(info, arg);
+ int i, ret;
+
+ if (irq_find_mapping(domain, hwirq) > 0)
+ return -EEXIST;
+
+ ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++) {
+ ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
+ if (ret < 0) {
+ if (ops->msi_free) {
+ for (i--; i > 0; i--)
+ ops->msi_free(domain, info, virq + i);
+ }
+ irq_domain_free_irqs_top(domain, virq, nr_irqs);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static void msi_domain_free(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs)
+{
+ struct msi_domain_info *info = domain->host_data;
+ int i;
+
+ if (info->ops->msi_free) {
+ for (i = 0; i < nr_irqs; i++)
+ info->ops->msi_free(domain, info, virq + i);
+ }
+ irq_domain_free_irqs_top(domain, virq, nr_irqs);
+}
+
+static struct irq_domain_ops msi_domain_ops = {
+ .alloc = msi_domain_alloc,
+ .free = msi_domain_free,
+ .activate = msi_domain_activate,
+ .deactivate = msi_domain_deactivate,
+};
+
+struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
+ struct msi_domain_info *info,
+ struct irq_domain *parent)
+{
+ struct irq_domain *domain;
+
+ domain = irq_domain_add_tree(of_node, &msi_domain_ops, info);
+ if (domain)
+ domain->parent = parent;
+
+ return domain;
+}
+
+struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
+{
+ return (struct msi_domain_info *)domain->host_data;
+}
+
+#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */


2014-11-18 12:07:42

by Abel Wu

[permalink] [raw]
Subject: Re: [patch 09/16] genirq: Add generic msi irq domain support

On 2014/11/12 21:43, Thomas Gleixner wrote:

> From: Jiang Liu <[email protected]>
>
> Implement the basic functions for MSI interrupt support with
> hierarchical interrupt domains.
>
> [ tglx: Extracted and combined from several patches ]
>
> Signed-off-by: Jiang Liu <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Yingjoe Chen <[email protected]>
> Cc: Yijing Wang <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> include/linux/msi.h | 35 +++++++++++++++
> kernel/irq/Kconfig | 10 ++++
> kernel/irq/Makefile | 1
> kernel/irq/msi.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 165 insertions(+)
>
> Index: tip/include/linux/msi.h
> ===================================================================
> --- tip.orig/include/linux/msi.h
> +++ tip/include/linux/msi.h
> @@ -77,4 +77,39 @@ struct msi_chip {
> void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
> };
>
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +struct irq_domain;
> +struct irq_chip;
> +struct device_node;
> +struct msi_domain_info;
> +
> +struct msi_domain_ops {
> + void (*calc_hwirq)(struct msi_domain_info *info, void *arg,
> + struct msi_desc *desc);
> + irq_hw_number_t (*get_hwirq)(struct msi_domain_info *info, void *arg);
> + int (*msi_init)(struct irq_domain *domain,
> + struct msi_domain_info *info,
> + unsigned int virq, irq_hw_number_t hwirq,
> + void *arg);
> + void (*msi_free)(struct irq_domain *domain,
> + struct msi_domain_info *info,
> + unsigned int virq);
> +};
> +
> +struct msi_domain_info {
> + struct msi_domain_ops *ops;
> + struct irq_chip *chip;
> + void *data;
> +};
> +
> +int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
> + bool force);
> +
> +struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
> + struct msi_domain_info *info,
> + struct irq_domain *parent);
> +struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
> +
> +#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
> +
> #endif /* LINUX_MSI_H */
> Index: tip/kernel/irq/Kconfig
> ===================================================================
> --- tip.orig/kernel/irq/Kconfig
> +++ tip/kernel/irq/Kconfig
> @@ -60,6 +60,16 @@ config IRQ_DOMAIN_HIERARCHY
> bool
> select IRQ_DOMAIN
>
> +# Generic MSI interrupt support
> +config GENERIC_MSI_IRQ
> + bool
> +
> +# Generic MSI hierarchical interrupt domain support
> +config GENERIC_MSI_IRQ_DOMAIN
> + bool
> + select IRQ_DOMAIN_HIERARCHY
> + select GENERIC_MSI_IRQ
> +
> config HANDLE_DOMAIN_IRQ
> bool
>
> Index: tip/kernel/irq/Makefile
> ===================================================================
> --- tip.orig/kernel/irq/Makefile
> +++ tip/kernel/irq/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_IRQ_DOMAIN) += irqdomain.o
> obj-$(CONFIG_PROC_FS) += proc.o
> obj-$(CONFIG_GENERIC_PENDING_IRQ) += migration.o
> obj-$(CONFIG_PM_SLEEP) += pm.o
> +obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
> Index: tip/kernel/irq/msi.c
> ===================================================================
> --- /dev/null
> +++ tip/kernel/irq/msi.c
> @@ -0,0 +1,119 @@
> +/*
> + * linux/kernel/irq/msi.c
> + *
> + * Copyright (C) 2014 Intel Corp.
> + * Author: Jiang Liu <[email protected]>
> + *
> + * This file is licensed under GPLv2.
> + *
> + * This file contains common code to support Message Signalled Interrupt for
> + * PCI compatible and non PCI compatible devices.
> + */
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/msi.h>
> +
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +int msi_domain_set_affinity(struct irq_data *irq_data,
> + const struct cpumask *mask, bool force)
> +{
> + struct irq_data *parent = irq_data->parent_data;
> + struct msi_msg msg;
> + int ret;
> +
> + ret = parent->chip->irq_set_affinity(parent, mask, force);
> + if (ret >= 0 && ret != IRQ_SET_MASK_OK_DONE) {
> + BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
> + irq_chip_write_msi_msg(irq_data, &msg);
> + }
> +
> + return ret;
> +}
> +
> +static void msi_domain_activate(struct irq_domain *domain,
> + struct irq_data *irq_data)
> +{
> + struct msi_msg msg;
> +
> + BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
> + irq_chip_write_msi_msg(irq_data, &msg);
> +}
> +
> +static void msi_domain_deactivate(struct irq_domain *domain,
> + struct irq_data *irq_data)
> +{
> + struct msi_msg msg;
> +
> + memset(&msg, 0, sizeof(msg));
> + irq_chip_write_msi_msg(irq_data, &msg);
> +}
> +
> +static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + struct msi_domain_info *info = domain->host_data;
> + struct msi_domain_ops *ops = info->ops;
> + irq_hw_number_t hwirq = ops->get_hwirq(info, arg);
> + int i, ret;
> +
> + if (irq_find_mapping(domain, hwirq) > 0)
> + return -EEXIST;

I think we need to check range here.
I fixed this by applying a patch showed in the bottom.

> +
> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < nr_irqs; i++) {
> + ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
> + if (ret < 0) {
> + if (ops->msi_free) {
> + for (i--; i > 0; i--)

while (i--) ?

Regards,
Abel

> + ops->msi_free(domain, info, virq + i);
> + }
> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void msi_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct msi_domain_info *info = domain->host_data;
> + int i;
> +
> + if (info->ops->msi_free) {
> + for (i = 0; i < nr_irqs; i++)
> + info->ops->msi_free(domain, info, virq + i);
> + }
> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
> +}
> +
> +static struct irq_domain_ops msi_domain_ops = {
> + .alloc = msi_domain_alloc,
> + .free = msi_domain_free,
> + .activate = msi_domain_activate,
> + .deactivate = msi_domain_deactivate,
> +};
> +
> +struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
> + struct msi_domain_info *info,
> + struct irq_domain *parent)
> +{
> + struct irq_domain *domain;
> +
> + domain = irq_domain_add_tree(of_node, &msi_domain_ops, info);
> + if (domain)
> + domain->parent = parent;
> +
> + return domain;
> +}
> +
> +struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
> +{
> + return (struct msi_domain_info *)domain->host_data;
> +}
> +
> +#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>
>

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index cc18182..6d1be00 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -568,6 +568,34 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
}
EXPORT_SYMBOL_GPL(irq_find_mapping);

+/**
+ * irq_find_mapping_many() - Find a range of linux irqs from a range of hwirqs.
+ * @domain: domain owning these hardware interrupts
+ * @hwirq: start hardware irq number in @domain space
+ * @count: number of interrupts to find
+ *
+ * Returns the first linux irq number on success, or 0 when not found, or error.
+ */
+int irq_find_mapping_many(struct irq_domain *domain, irq_hw_number_t hwirq,
+ int count)
+{
+ unsigned int from;
+ int i;
+
+ for (i = from = 0; i < count; i++) {
+ if (!from) {
+ from = irq_find_mapping(domain, hwirq + i);
+ if (from && i)
+ return -1;
+ } else if ((from + i) != irq_find_mapping(domain, hwirq + i)) {
+ return -1;
+ }
+ }
+
+ return from;
+}
+EXPORT_SYMBOL_GPL(irq_find_mapping_many);
+
#ifdef CONFIG_IRQ_DOMAIN_DEBUG
static int virq_debug_show(struct seq_file *m, void *private)
{


2014-11-18 12:50:05

by Jiang Liu

[permalink] [raw]
Subject: Re: [patch 09/16] genirq: Add generic msi irq domain support


On 2014/11/18 20:07, Yun Wu (Abel) wrote:
> On 2014/11/12 21:43, Thomas Gleixner wrote:
>
>> From: Jiang Liu <[email protected]>
<snip>
>> +static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs, void *arg)
>> +{
>> + struct msi_domain_info *info = domain->host_data;
>> + struct msi_domain_ops *ops = info->ops;
>> + irq_hw_number_t hwirq = ops->get_hwirq(info, arg);
>> + int i, ret;
>> +
>> + if (irq_find_mapping(domain, hwirq) > 0)
>> + return -EEXIST;
>
> I think we need to check range here.
> I fixed this by applying a patch showed in the bottom.
Hi Yun,
Yes, we have some assumptions here about the way the interface
will be used for. Will improve it in future patches.
Regards!
Gerry
>
>> +
>> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>> + if (ret < 0)
>> + return ret;
>> +
>> + for (i = 0; i < nr_irqs; i++) {
>> + ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
>> + if (ret < 0) {
>> + if (ops->msi_free) {
>> + for (i--; i > 0; i--)
>
> while (i--) ?
>
> Regards,
> Abel
>
>> + ops->msi_free(domain, info, virq + i);
>> + }
>> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void msi_domain_free(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs)
>> +{
>> + struct msi_domain_info *info = domain->host_data;
>> + int i;
>> +
>> + if (info->ops->msi_free) {
>> + for (i = 0; i < nr_irqs; i++)
>> + info->ops->msi_free(domain, info, virq + i);
>> + }
>> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
>> +}
>> +
>> +static struct irq_domain_ops msi_domain_ops = {
>> + .alloc = msi_domain_alloc,
>> + .free = msi_domain_free,
>> + .activate = msi_domain_activate,
>> + .deactivate = msi_domain_deactivate,
>> +};
>> +
>> +struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>> + struct msi_domain_info *info,
>> + struct irq_domain *parent)
>> +{
>> + struct irq_domain *domain;
>> +
>> + domain = irq_domain_add_tree(of_node, &msi_domain_ops, info);
>> + if (domain)
>> + domain->parent = parent;
>> +
>> + return domain;
>> +}
>> +
>> +struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
>> +{
>> + return (struct msi_domain_info *)domain->host_data;
>> +}
>> +
>> +#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>>
>>
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index cc18182..6d1be00 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -568,6 +568,34 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
> }
> EXPORT_SYMBOL_GPL(irq_find_mapping);
>
> +/**
> + * irq_find_mapping_many() - Find a range of linux irqs from a range of hwirqs.
> + * @domain: domain owning these hardware interrupts
> + * @hwirq: start hardware irq number in @domain space
> + * @count: number of interrupts to find
> + *
> + * Returns the first linux irq number on success, or 0 when not found, or error.
> + */
> +int irq_find_mapping_many(struct irq_domain *domain, irq_hw_number_t hwirq,
> + int count)
> +{
> + unsigned int from;
> + int i;
> +
> + for (i = from = 0; i < count; i++) {
> + if (!from) {
> + from = irq_find_mapping(domain, hwirq + i);
> + if (from && i)
> + return -1;
> + } else if ((from + i) != irq_find_mapping(domain, hwirq + i)) {
> + return -1;
> + }
> + }
> +
> + return from;
> +}
> +EXPORT_SYMBOL_GPL(irq_find_mapping_many);
> +
> #ifdef CONFIG_IRQ_DOMAIN_DEBUG
> static int virq_debug_show(struct seq_file *m, void *private)
> {
>
>
>

2014-11-18 13:55:55

by Abel Wu

[permalink] [raw]
Subject: Re: [patch 09/16] genirq: Add generic msi irq domain support

On 2014/11/18 20:49, Jiang Liu wrote:

>
> On 2014/11/18 20:07, Yun Wu (Abel) wrote:
>> On 2014/11/12 21:43, Thomas Gleixner wrote:
>>
>>> From: Jiang Liu <[email protected]>
> <snip>
>>> +static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>>> + unsigned int nr_irqs, void *arg)
>>> +{
>>> + struct msi_domain_info *info = domain->host_data;
>>> + struct msi_domain_ops *ops = info->ops;
>>> + irq_hw_number_t hwirq = ops->get_hwirq(info, arg);
>>> + int i, ret;
>>> +
>>> + if (irq_find_mapping(domain, hwirq) > 0)
>>> + return -EEXIST;
>>
>> I think we need to check range here.
>> I fixed this by applying a patch showed in the bottom.
> Hi Yun,
> Yes, we have some assumptions here about the way the interface
> will be used for. Will improve it in future patches.
> Regards!
> Gerry
>>

OK. Just note that don't forget to check my second inline comment below. :)

>>> +
>>> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + for (i = 0; i < nr_irqs; i++) {
>>> + ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
>>> + if (ret < 0) {
>>> + if (ops->msi_free) {
>>> + for (i--; i > 0; i--)
>>
>> while (i--) ?
>>
Here.


Thanks,
Abel

>>
>>> + ops->msi_free(domain, info, virq + i);
>>> + }
>>> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void msi_domain_free(struct irq_domain *domain, unsigned int virq,
>>> + unsigned int nr_irqs)
>>> +{
>>> + struct msi_domain_info *info = domain->host_data;
>>> + int i;
>>> +
>>> + if (info->ops->msi_free) {
>>> + for (i = 0; i < nr_irqs; i++)
>>> + info->ops->msi_free(domain, info, virq + i);
>>> + }
>>> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
>>> +}
>>> +
>>> +static struct irq_domain_ops msi_domain_ops = {
>>> + .alloc = msi_domain_alloc,
>>> + .free = msi_domain_free,
>>> + .activate = msi_domain_activate,
>>> + .deactivate = msi_domain_deactivate,
>>> +};
>>> +
>>> +struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>>> + struct msi_domain_info *info,
>>> + struct irq_domain *parent)
>>> +{
>>> + struct irq_domain *domain;
>>> +
>>> + domain = irq_domain_add_tree(of_node, &msi_domain_ops, info);
>>> + if (domain)
>>> + domain->parent = parent;
>>> +
>>> + return domain;
>>> +}
>>> +
>>> +struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
>>> +{
>>> + return (struct msi_domain_info *)domain->host_data;
>>> +}
>>> +
>>> +#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>>>
>>>
>>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index cc18182..6d1be00 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -568,6 +568,34 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>> }
>> EXPORT_SYMBOL_GPL(irq_find_mapping);
>>
>> +/**
>> + * irq_find_mapping_many() - Find a range of linux irqs from a range of hwirqs.
>> + * @domain: domain owning these hardware interrupts
>> + * @hwirq: start hardware irq number in @domain space
>> + * @count: number of interrupts to find
>> + *
>> + * Returns the first linux irq number on success, or 0 when not found, or error.
>> + */
>> +int irq_find_mapping_many(struct irq_domain *domain, irq_hw_number_t hwirq,
>> + int count)
>> +{
>> + unsigned int from;
>> + int i;
>> +
>> + for (i = from = 0; i < count; i++) {
>> + if (!from) {
>> + from = irq_find_mapping(domain, hwirq + i);
>> + if (from && i)
>> + return -1;
>> + } else if ((from + i) != irq_find_mapping(domain, hwirq + i)) {
>> + return -1;
>> + }
>> + }
>> +
>> + return from;
>> +}
>> +EXPORT_SYMBOL_GPL(irq_find_mapping_many);
>> +
>> #ifdef CONFIG_IRQ_DOMAIN_DEBUG
>> static int virq_debug_show(struct seq_file *m, void *private)
>> {
>>
>>
>>
>
> .
>


2014-11-18 14:24:12

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 09/16] genirq: Add generic msi irq domain support

On Tue, 18 Nov 2014, Yun Wu (Abel) wrote:
> On 2014/11/18 20:49, Jiang Liu wrote:
> >>> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> >>> + if (ret < 0)
> >>> + return ret;
> >>> +
> >>> + for (i = 0; i < nr_irqs; i++) {
> >>> + ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
> >>> + if (ret < 0) {
> >>> + if (ops->msi_free) {
> >>> + for (i--; i > 0; i--)
> >>
> >> while (i--) ?

The allocation for 'i' failed. So we don't free i. You missed the real
bug here:

> >>> + for (i--; i > 0; i--)

Must be i >= 0.

Thanks,

tglx

2014-11-18 14:40:54

by Abel Wu

[permalink] [raw]
Subject: Re: [patch 09/16] genirq: Add generic msi irq domain support

On 2014/11/18 22:24, Thomas Gleixner wrote:

> On Tue, 18 Nov 2014, Yun Wu (Abel) wrote:
>> On 2014/11/18 20:49, Jiang Liu wrote:
>>>>> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + for (i = 0; i < nr_irqs; i++) {
>>>>> + ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
>>>>> + if (ret < 0) {
>>>>> + if (ops->msi_free) {
>>>>> + for (i--; i > 0; i--)
>>>>
>>>> while (i--) ?
>
> The allocation for 'i' failed. So we don't free i. You missed the real
> bug here:
>
>>>>> + for (i--; i > 0; i--)
>
> Must be i >= 0.
>

"for (i--; i >= 0; i--)" is just the same as "while (i--)".

Thanks,
Abel

2014-11-20 02:29:44

by Jiang Liu

[permalink] [raw]
Subject: Re: [patch 09/16] genirq: Add generic msi irq domain support

On 2014/11/18 22:24, Thomas Gleixner wrote:
> On Tue, 18 Nov 2014, Yun Wu (Abel) wrote:
>> On 2014/11/18 20:49, Jiang Liu wrote:
>>>>> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + for (i = 0; i < nr_irqs; i++) {
>>>>> + ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
>>>>> + if (ret < 0) {
>>>>> + if (ops->msi_free) {
>>>>> + for (i--; i > 0; i--)
>>>>
>>>> while (i--) ?
>
> The allocation for 'i' failed. So we don't free i. You missed the real
> bug here:
>
>>>>> + for (i--; i > 0; i--)
>
> Must be i >= 0.
Hi Thomas,
You will take care of this bug, right? Or should I send a patch
for it?
Regards!
Gerry