[Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

This patch set is based on tip/irq/irqdomain and tries to refine
interfaces to support irqdomain for generic MSI and PCI MSI.

Patch 1 is just minor fixes for tip/irq/irqdomain.

Patch 2 introduces some helpers to hide struct msi_desc implementation
details, so later we could move msi_list from struct pci_dev into
struct device to enable generic MSI support.

Patch 3 introduces msi_domain_{alloc|free}_irqs() which generalize
pci_msi_domain_alloc_irqs() to support generic MSI.

Patch 4 introduces default data structures and callback implementations
to support msi_domain_alloc_irqs(), so reduce burden on generic MSI
users.

Patch 5 converts PCI MSI to use generic MSI interfaces, and also
implement default callbacks for PCI MSI.

Patch 6 introduces a mechanism to replace arch_setup_msi_irq()/
arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().

With this patch set applied, the generic MSI and PCI MSI interfaces
are much easier to use. For extreme case, you only need to define
a "struct msi_domain_info" and don't need to implement any callbacks,
just using the default callbacks is OK:)

This patch set is also a preparation for:
1) Kill all weak functions in drivers/pci/msi.c
2) Implement support for non-PCI-compliant MSI device

It has been tested on x86 platforms, comments are welcomed!

Jiang Liu (6):
  PCI, MSI: Fix errors caused by commit e5f1a59c4e12
  PCI, MSI: Introduce helpers to hide struct msi_desc implemenation
    details
  genirq: Introduce msi_domain_{alloc|free}_irqs()
  genirq: Provide default callbacks for msi_domain_ops
  PCI, MSI: Refine irqdomain interfaces to simplify its usage
  PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from
    irqdomain

 drivers/pci/msi.c   |  170 ++++++++++++++++++++++++++++++++++++++++-----------
 include/linux/msi.h |   90 ++++++++++++++++++++++++---
 kernel/irq/msi.c    |  150 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 366 insertions(+), 44 deletions(-)

-- 
1.7.10.4

[Patch V1 1/6] PCI, MSI: Fix errors caused by commit e5f1a59c4e12

[Jiang Liu]

Better to fold into commit e5f1a59c4e12 "PCI/MSI: Rename write_msi_msg()
to pci_write_msi_msg()".

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/pci/msi.c   |    2 +-
 include/linux/msi.h |    2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 564850b1316e..9c53b865cb1b 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1084,7 +1084,7 @@ void pci_msi_domain_write_msg(struct irq_data *irq_data, struct msi_msg *msg)
 	 * MSI message denotes a contiguous group of IRQs, written for 0th IRQ.
 	 */
 	if (desc->irq == irq_data->irq)
-		pci_write_msi_msg(desc, msg);
+		__pci_write_msi_msg(desc, msg);
 }
 
 /*
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8112a17cdca1..190c7abbec84 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -44,11 +44,9 @@ struct msi_desc {
 	struct msi_msg msg;
 };
 
-#ifdef CONFIG_PCI_MSI
 void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
 void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
 void pci_write_msi_msg(unsigned int irq, struct msi_msg *msg);
-#endif
 
 /*
  * The arch hooks to setup up msi irqs. Those functions are
-- 
1.7.10.4

[Patch V1 2/6] PCI, MSI: Introduce helpers to hide struct msi_desc implemenation details

[Jiang Liu]

Introduce helpers to hide struct msi_desc implementation details,
so we could easily support non-PCI-compliant MSI devices later by
moving msi_list into struct device.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 include/linux/msi.h |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 190c7abbec84..714716a3ffdd 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -44,6 +44,25 @@ struct msi_desc {
 	struct msi_msg msg;
 };
 
+/* Helpers to hide struct msi_desc implementation details */
+#define msi_desc_to_dev(desc)		(&(desc)->dev.dev)
+#define dev_to_msi_list(dev)		(&to_pci_dev((dev))->msi_list)
+#define first_msi_entry(dev)		\
+	list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
+#define for_each_msi_entry(desc, dev)	\
+	list_for_each_entry((desc), dev_to_msi_list((dev)), list)
+
+#ifdef CONFIG_PCI_MSI
+#define first_pci_msi_entry(pdev)	first_msi_entry(&(pdev)->dev)
+#define for_each_pci_msi_entry(desc, pdev)	\
+	for_each_msi_entry((desc), &(pdev)->dev)
+
+static inline struct pci_dev *msi_desc_to_pci_dev(struct msi_desc *desc)
+{
+	return desc->dev;
+}
+#endif /* CONFIG_PCI_MSI */
+
 void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
 void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
 void pci_write_msi_msg(unsigned int irq, struct msi_msg *msg);
-- 
1.7.10.4

[Patch V1 3/6] genirq: Introduce msi_domain_{alloc|free}_irqs()

[Jiang Liu]

Introduce msi_domain_{alloc|free}_irqs() to alloc/free interrupts
from generic MSI irqdomain.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
 kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 714716a3ffdd..6b356a1410b2 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -3,6 +3,7 @@
 
 #include <linux/kobject.h>
 #include <linux/list.h>
+#include <asm/hw_irq.h>		/* for msi_alloc_info_t */
 
 struct msi_msg {
 	u32	address_lo;	/* low 32 bits of msi message address */
@@ -100,7 +101,26 @@ struct irq_chip;
 struct device_node;
 struct msi_domain_info;
 
+#ifndef NUM_MSI_ALLOC_SCRATCHPAD_REGS
+#define NUM_MSI_ALLOC_SCRATCHPAD_REGS	2
+#endif
+
+/*
+ * Default structure for MSI interrupt allocation.
+ * Arch may overwrite it by define msi_alloc_info_t.
+ */
+struct msi_alloc_info {
+	struct msi_desc *desc;
+	irq_hw_number_t hwirq;
+	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
+};
+
+#ifndef msi_alloc_info_t
+typedef struct msi_alloc_info msi_alloc_info_t;
+#endif
+
 struct msi_domain_ops {
+	/* Callbacks for msi_create_irq_domain() */
 	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);
@@ -111,6 +131,19 @@ struct msi_domain_ops {
 	void		(*msi_free)(struct irq_domain *domain,
 				    struct msi_domain_info *info,
 				    unsigned int virq);
+
+	/* Callbacks for msi_irq_domain_alloc_irqs() based on msi_descs */
+	int		(*msi_check)(struct irq_domain *domain,
+				     struct msi_domain_info *info,
+				     struct device *dev);
+	int		(*msi_prepare)(struct irq_domain *domain,
+				       struct device *dev,
+				       msi_alloc_info_t *arg);
+	void		(*msi_finish)(msi_alloc_info_t *arg, int retval);
+	void		(*set_desc)(msi_alloc_info_t *arg,
+				    struct msi_desc *desc);
+	int		(*handle_error)(struct irq_domain *domain,
+					struct msi_desc *desc, int error);
 };
 
 struct msi_domain_info {
@@ -125,6 +158,8 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
 struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
 					 struct msi_domain_info *info,
 					 struct irq_domain *parent);
+int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev);
+void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
 struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
 
 #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 94d6d87ee23e..0b6665de3a60 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -13,6 +13,9 @@
 #include <linux/irqdomain.h>
 #include <linux/msi.h>
 
+/* Temparory solution for building, will be removed later */
+#include <linux/pci.h>
+
 void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 {
 	*msg = entry->msg;
@@ -124,6 +127,62 @@ struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
 	return domain;
 }
 
+int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev)
+{
+	int i, ret, virq = -1;
+	msi_alloc_info_t arg;
+	struct msi_desc *desc;
+	struct msi_domain_info *info = domain->host_data;
+	struct msi_domain_ops *ops = info->ops;
+
+	ret = ops->msi_check(domain, info, dev);
+	if (ret == 0)
+		ret = ops->msi_prepare(domain, dev, &arg);
+	if (ret)
+		return ret;
+
+	for_each_msi_entry(desc, dev) {
+		ops->set_desc(&arg, desc);
+
+		virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used,
+					       dev_to_node(dev), &arg, false);
+		if (virq < 0) {
+			ret = -ENOSPC;
+			if (ops->handle_error)
+				ret = ops->handle_error(domain, desc, ret);
+			if (ops->msi_finish)
+				ops->msi_finish(&arg, ret);
+			return ret;
+		}
+
+		for (i = 0; i < desc->nvec_used; i++)
+			irq_set_msi_desc_off(virq, i, desc);
+	}
+
+	if (ops->msi_finish)
+		ops->msi_finish(&arg, 0);
+
+	for_each_msi_entry(desc, dev) {
+		if (desc->nvec_used == 1)
+			dev_dbg(dev, "irq %d for MSI\n", virq);
+		else
+			dev_dbg(dev, "irq [%d-%d] for MSI\n",
+				virq, virq + desc->nvec_used - 1);
+	}
+
+	return 0;
+}
+
+void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
+{
+	struct msi_desc *desc;
+
+	for_each_msi_entry(desc, dev) {
+		irq_domain_free_irqs(desc->irq, desc->nvec_used);
+		desc->irq = 0;
+	}
+}
+
 struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
 {
 	return (struct msi_domain_info *)domain->host_data;
-- 
1.7.10.4

Re: [Patch V1 3/6] genirq: Introduce msi_domain_{alloc|free}_irqs()

[Marc Zyngier]

On 13/11/14 11:43, Jiang Liu wrote:
> Introduce msi_domain_{alloc|free}_irqs() to alloc/free interrupts
> from generic MSI irqdomain.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
>  kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
> 
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 714716a3ffdd..6b356a1410b2 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -3,6 +3,7 @@
>  
>  #include <linux/kobject.h>
>  #include <linux/list.h>
> +#include <asm/hw_irq.h>		/* for msi_alloc_info_t */
>  
>  struct msi_msg {
>  	u32	address_lo;	/* low 32 bits of msi message address */
> @@ -100,7 +101,26 @@ struct irq_chip;
>  struct device_node;
>  struct msi_domain_info;
>  
> +#ifndef NUM_MSI_ALLOC_SCRATCHPAD_REGS
> +#define NUM_MSI_ALLOC_SCRATCHPAD_REGS	2
> +#endif
> +
> +/*
> + * Default structure for MSI interrupt allocation.
> + * Arch may overwrite it by define msi_alloc_info_t.
> + */
> +struct msi_alloc_info {
> +	struct msi_desc *desc;
> +	irq_hw_number_t hwirq;
> +	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];

As much as I'm relieved to see a generic structure here, could
scratchpad be slightly less awkward to use? Something like:

struct msi_alloc_info {
	[...]
	union	{
		unsigned long ul;
		void *ptr;
	} scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
};

That would avoid some very ugly casting.

Thanks,

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

Re: [Patch V1 3/6] genirq: Introduce msi_domain_{alloc|free}_irqs()

[Jiang Liu]

On 2014/11/14 4:23, Marc Zyngier wrote:
> On 13/11/14 11:43, Jiang Liu wrote:
>> Introduce msi_domain_{alloc|free}_irqs() to alloc/free interrupts
>> from generic MSI irqdomain.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>>  include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
>>  kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 94 insertions(+)
>>
>> +/*
>> + * Default structure for MSI interrupt allocation.
>> + * Arch may overwrite it by define msi_alloc_info_t.
>> + */
>> +struct msi_alloc_info {
>> +	struct msi_desc *desc;
>> +	irq_hw_number_t hwirq;
>> +	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
> 
> As much as I'm relieved to see a generic structure here, could
> scratchpad be slightly less awkward to use? Something like:
> 
> struct msi_alloc_info {
> 	[...]
> 	union	{
> 		unsigned long ul;
> 		void *ptr;
> 	} scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
> };
> 
> That would avoid some very ugly casting.
Great, will changed to this way in next version:)

> 
> Thanks,
> 
> 	M.
> 

[Patch V1 3/6] genirq: Introduce msi_irq_domain_{alloc|free}_irqs()

[Jiang Liu]

Introduce msi_irq_domain_{alloc|free}_irqs() to alloc/free interrupts
from irqdomain for generic MSI interrupts.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
 kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 714716a3ffdd..6b356a1410b2 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -3,6 +3,7 @@
 
 #include <linux/kobject.h>
 #include <linux/list.h>
+#include <asm/hw_irq.h>		/* for msi_alloc_info_t */
 
 struct msi_msg {
 	u32	address_lo;	/* low 32 bits of msi message address */
@@ -100,7 +101,26 @@ struct irq_chip;
 struct device_node;
 struct msi_domain_info;
 
+#ifndef NUM_MSI_ALLOC_SCRATCHPAD_REGS
+#define NUM_MSI_ALLOC_SCRATCHPAD_REGS	2
+#endif
+
+/*
+ * Default structure for MSI interrupt allocation.
+ * Arch may overwrite it by define msi_alloc_info_t.
+ */
+struct msi_alloc_info {
+	struct msi_desc *desc;
+	irq_hw_number_t hwirq;
+	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
+};
+
+#ifndef msi_alloc_info_t
+typedef struct msi_alloc_info msi_alloc_info_t;
+#endif
+
 struct msi_domain_ops {
+	/* Callbacks for msi_create_irq_domain() */
 	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);
@@ -111,6 +131,19 @@ struct msi_domain_ops {
 	void		(*msi_free)(struct irq_domain *domain,
 				    struct msi_domain_info *info,
 				    unsigned int virq);
+
+	/* Callbacks for msi_irq_domain_alloc_irqs() based on msi_descs */
+	int		(*msi_check)(struct irq_domain *domain,
+				     struct msi_domain_info *info,
+				     struct device *dev);
+	int		(*msi_prepare)(struct irq_domain *domain,
+				       struct device *dev,
+				       msi_alloc_info_t *arg);
+	void		(*msi_finish)(msi_alloc_info_t *arg, int retval);
+	void		(*set_desc)(msi_alloc_info_t *arg,
+				    struct msi_desc *desc);
+	int		(*handle_error)(struct irq_domain *domain,
+					struct msi_desc *desc, int error);
 };
 
 struct msi_domain_info {
@@ -125,6 +158,8 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
 struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
 					 struct msi_domain_info *info,
 					 struct irq_domain *parent);
+int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev);
+void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
 struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
 
 #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 94d6d87ee23e..0b6665de3a60 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -13,6 +13,9 @@
 #include <linux/irqdomain.h>
 #include <linux/msi.h>
 
+/* Temparory solution for building, will be removed later */
+#include <linux/pci.h>
+
 void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 {
 	*msg = entry->msg;
@@ -124,6 +127,62 @@ struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
 	return domain;
 }
 
+int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev)
+{
+	int i, ret, virq = -1;
+	msi_alloc_info_t arg;
+	struct msi_desc *desc;
+	struct msi_domain_info *info = domain->host_data;
+	struct msi_domain_ops *ops = info->ops;
+
+	ret = ops->msi_check(domain, info, dev);
+	if (ret == 0)
+		ret = ops->msi_prepare(domain, dev, &arg);
+	if (ret)
+		return ret;
+
+	for_each_msi_entry(desc, dev) {
+		ops->set_desc(&arg, desc);
+
+		virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used,
+					       dev_to_node(dev), &arg, false);
+		if (virq < 0) {
+			ret = -ENOSPC;
+			if (ops->handle_error)
+				ret = ops->handle_error(domain, desc, ret);
+			if (ops->msi_finish)
+				ops->msi_finish(&arg, ret);
+			return ret;
+		}
+
+		for (i = 0; i < desc->nvec_used; i++)
+			irq_set_msi_desc_off(virq, i, desc);
+	}
+
+	if (ops->msi_finish)
+		ops->msi_finish(&arg, 0);
+
+	for_each_msi_entry(desc, dev) {
+		if (desc->nvec_used == 1)
+			dev_dbg(dev, "irq %d for MSI\n", virq);
+		else
+			dev_dbg(dev, "irq [%d-%d] for MSI\n",
+				virq, virq + desc->nvec_used - 1);
+	}
+
+	return 0;
+}
+
+void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
+{
+	struct msi_desc *desc;
+
+	for_each_msi_entry(desc, dev) {
+		irq_domain_free_irqs(desc->irq, desc->nvec_used);
+		desc->irq = 0;
+	}
+}
+
 struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
 {
 	return (struct msi_domain_info *)domain->host_data;
-- 
1.7.10.4

Re: [Patch V1 3/6] genirq: Introduce msi_irq_domain_{alloc|free}_irqs()

[Yijing Wang]

Gerry, this patch seems to be the same as last one.


On 2014/11/13 19:43, Jiang Liu wrote:
> Introduce msi_irq_domain_{alloc|free}_irqs() to alloc/free interrupts
> from irqdomain for generic MSI interrupts.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
>  kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
> 
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 714716a3ffdd..6b356a1410b2 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -3,6 +3,7 @@
>  
>  #include <linux/kobject.h>
>  #include <linux/list.h>
> +#include <asm/hw_irq.h>		/* for msi_alloc_info_t */
>  
>  struct msi_msg {
>  	u32	address_lo;	/* low 32 bits of msi message address */
> @@ -100,7 +101,26 @@ struct irq_chip;
>  struct device_node;
>  struct msi_domain_info;
>  
> +#ifndef NUM_MSI_ALLOC_SCRATCHPAD_REGS
> +#define NUM_MSI_ALLOC_SCRATCHPAD_REGS	2
> +#endif
> +
> +/*
> + * Default structure for MSI interrupt allocation.
> + * Arch may overwrite it by define msi_alloc_info_t.
> + */
> +struct msi_alloc_info {
> +	struct msi_desc *desc;
> +	irq_hw_number_t hwirq;
> +	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
> +};
> +
> +#ifndef msi_alloc_info_t
> +typedef struct msi_alloc_info msi_alloc_info_t;
> +#endif
> +
>  struct msi_domain_ops {
> +	/* Callbacks for msi_create_irq_domain() */
>  	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);
> @@ -111,6 +131,19 @@ struct msi_domain_ops {
>  	void		(*msi_free)(struct irq_domain *domain,
>  				    struct msi_domain_info *info,
>  				    unsigned int virq);
> +
> +	/* Callbacks for msi_irq_domain_alloc_irqs() based on msi_descs */
> +	int		(*msi_check)(struct irq_domain *domain,
> +				     struct msi_domain_info *info,
> +				     struct device *dev);
> +	int		(*msi_prepare)(struct irq_domain *domain,
> +				       struct device *dev,
> +				       msi_alloc_info_t *arg);
> +	void		(*msi_finish)(msi_alloc_info_t *arg, int retval);
> +	void		(*set_desc)(msi_alloc_info_t *arg,
> +				    struct msi_desc *desc);
> +	int		(*handle_error)(struct irq_domain *domain,
> +					struct msi_desc *desc, int error);
>  };
>  
>  struct msi_domain_info {
> @@ -125,6 +158,8 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
>  struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>  					 struct msi_domain_info *info,
>  					 struct irq_domain *parent);
> +int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev);
> +void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
>  struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
>  
>  #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 94d6d87ee23e..0b6665de3a60 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -13,6 +13,9 @@
>  #include <linux/irqdomain.h>
>  #include <linux/msi.h>
>  
> +/* Temparory solution for building, will be removed later */
> +#include <linux/pci.h>
> +
>  void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>  {
>  	*msg = entry->msg;
> @@ -124,6 +127,62 @@ struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>  	return domain;
>  }
>  
> +int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev)
> +{
> +	int i, ret, virq = -1;
> +	msi_alloc_info_t arg;
> +	struct msi_desc *desc;
> +	struct msi_domain_info *info = domain->host_data;
> +	struct msi_domain_ops *ops = info->ops;
> +
> +	ret = ops->msi_check(domain, info, dev);
> +	if (ret == 0)
> +		ret = ops->msi_prepare(domain, dev, &arg);
> +	if (ret)
> +		return ret;
> +
> +	for_each_msi_entry(desc, dev) {
> +		ops->set_desc(&arg, desc);
> +
> +		virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used,
> +					       dev_to_node(dev), &arg, false);
> +		if (virq < 0) {
> +			ret = -ENOSPC;
> +			if (ops->handle_error)
> +				ret = ops->handle_error(domain, desc, ret);
> +			if (ops->msi_finish)
> +				ops->msi_finish(&arg, ret);
> +			return ret;
> +		}
> +
> +		for (i = 0; i < desc->nvec_used; i++)
> +			irq_set_msi_desc_off(virq, i, desc);
> +	}
> +
> +	if (ops->msi_finish)
> +		ops->msi_finish(&arg, 0);
> +
> +	for_each_msi_entry(desc, dev) {
> +		if (desc->nvec_used == 1)
> +			dev_dbg(dev, "irq %d for MSI\n", virq);
> +		else
> +			dev_dbg(dev, "irq [%d-%d] for MSI\n",
> +				virq, virq + desc->nvec_used - 1);
> +	}
> +
> +	return 0;
> +}
> +
> +void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
> +{
> +	struct msi_desc *desc;
> +
> +	for_each_msi_entry(desc, dev) {
> +		irq_domain_free_irqs(desc->irq, desc->nvec_used);
> +		desc->irq = 0;
> +	}
> +}
> +
>  struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
>  {
>  	return (struct msi_domain_info *)domain->host_data;
> 


-- 
Thanks!
Yijing

Re: [Patch V1 3/6] genirq: Introduce msi_irq_domain_{alloc|free}_irqs()

[Jiang Liu]

On 2014/11/13 20:34, Yijing Wang wrote:
> Gerry, this patch seems to be the same as last one.
Do you means patch 6/6? They are different:)

> 
> 
> On 2014/11/13 19:43, Jiang Liu wrote:
>> Introduce msi_irq_domain_{alloc|free}_irqs() to alloc/free interrupts
>> from irqdomain for generic MSI interrupts.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>>  include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
>>  kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 94 insertions(+)
>>
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index 714716a3ffdd..6b356a1410b2 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -3,6 +3,7 @@
>>  
>>  #include <linux/kobject.h>
>>  #include <linux/list.h>
>> +#include <asm/hw_irq.h>		/* for msi_alloc_info_t */
>>  
>>  struct msi_msg {
>>  	u32	address_lo;	/* low 32 bits of msi message address */
>> @@ -100,7 +101,26 @@ struct irq_chip;
>>  struct device_node;
>>  struct msi_domain_info;
>>  
>> +#ifndef NUM_MSI_ALLOC_SCRATCHPAD_REGS
>> +#define NUM_MSI_ALLOC_SCRATCHPAD_REGS	2
>> +#endif
>> +
>> +/*
>> + * Default structure for MSI interrupt allocation.
>> + * Arch may overwrite it by define msi_alloc_info_t.
>> + */
>> +struct msi_alloc_info {
>> +	struct msi_desc *desc;
>> +	irq_hw_number_t hwirq;
>> +	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
>> +};
>> +
>> +#ifndef msi_alloc_info_t
>> +typedef struct msi_alloc_info msi_alloc_info_t;
>> +#endif
>> +
>>  struct msi_domain_ops {
>> +	/* Callbacks for msi_create_irq_domain() */
>>  	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);
>> @@ -111,6 +131,19 @@ struct msi_domain_ops {
>>  	void		(*msi_free)(struct irq_domain *domain,
>>  				    struct msi_domain_info *info,
>>  				    unsigned int virq);
>> +
>> +	/* Callbacks for msi_irq_domain_alloc_irqs() based on msi_descs */
>> +	int		(*msi_check)(struct irq_domain *domain,
>> +				     struct msi_domain_info *info,
>> +				     struct device *dev);
>> +	int		(*msi_prepare)(struct irq_domain *domain,
>> +				       struct device *dev,
>> +				       msi_alloc_info_t *arg);
>> +	void		(*msi_finish)(msi_alloc_info_t *arg, int retval);
>> +	void		(*set_desc)(msi_alloc_info_t *arg,
>> +				    struct msi_desc *desc);
>> +	int		(*handle_error)(struct irq_domain *domain,
>> +					struct msi_desc *desc, int error);
>>  };
>>  
>>  struct msi_domain_info {
>> @@ -125,6 +158,8 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
>>  struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>>  					 struct msi_domain_info *info,
>>  					 struct irq_domain *parent);
>> +int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev);
>> +void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
>>  struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
>>  
>>  #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
>> index 94d6d87ee23e..0b6665de3a60 100644
>> --- a/kernel/irq/msi.c
>> +++ b/kernel/irq/msi.c
>> @@ -13,6 +13,9 @@
>>  #include <linux/irqdomain.h>
>>  #include <linux/msi.h>
>>  
>> +/* Temparory solution for building, will be removed later */
>> +#include <linux/pci.h>
>> +
>>  void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>>  {
>>  	*msg = entry->msg;
>> @@ -124,6 +127,62 @@ struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>>  	return domain;
>>  }
>>  
>> +int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev)
>> +{
>> +	int i, ret, virq = -1;
>> +	msi_alloc_info_t arg;
>> +	struct msi_desc *desc;
>> +	struct msi_domain_info *info = domain->host_data;
>> +	struct msi_domain_ops *ops = info->ops;
>> +
>> +	ret = ops->msi_check(domain, info, dev);
>> +	if (ret == 0)
>> +		ret = ops->msi_prepare(domain, dev, &arg);
>> +	if (ret)
>> +		return ret;
>> +
>> +	for_each_msi_entry(desc, dev) {
>> +		ops->set_desc(&arg, desc);
>> +
>> +		virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used,
>> +					       dev_to_node(dev), &arg, false);
>> +		if (virq < 0) {
>> +			ret = -ENOSPC;
>> +			if (ops->handle_error)
>> +				ret = ops->handle_error(domain, desc, ret);
>> +			if (ops->msi_finish)
>> +				ops->msi_finish(&arg, ret);
>> +			return ret;
>> +		}
>> +
>> +		for (i = 0; i < desc->nvec_used; i++)
>> +			irq_set_msi_desc_off(virq, i, desc);
>> +	}
>> +
>> +	if (ops->msi_finish)
>> +		ops->msi_finish(&arg, 0);
>> +
>> +	for_each_msi_entry(desc, dev) {
>> +		if (desc->nvec_used == 1)
>> +			dev_dbg(dev, "irq %d for MSI\n", virq);
>> +		else
>> +			dev_dbg(dev, "irq [%d-%d] for MSI\n",
>> +				virq, virq + desc->nvec_used - 1);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
>> +{
>> +	struct msi_desc *desc;
>> +
>> +	for_each_msi_entry(desc, dev) {
>> +		irq_domain_free_irqs(desc->irq, desc->nvec_used);
>> +		desc->irq = 0;
>> +	}
>> +}
>> +
>>  struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
>>  {
>>  	return (struct msi_domain_info *)domain->host_data;
>>
> 
> 

Re: [Patch V1 3/6] genirq: Introduce msi_irq_domain_{alloc|free}_irqs()

[Yijing Wang]

On 2014/11/13 20:41, Jiang Liu wrote:
> On 2014/11/13 20:34, Yijing Wang wrote:
>> Gerry, this patch seems to be the same as last one.
> Do you means patch 6/6? They are different:)

Oh, it's not the patch problem, I received two double patch.

> 
>>
>>
>> On 2014/11/13 19:43, Jiang Liu wrote:
>>> Introduce msi_irq_domain_{alloc|free}_irqs() to alloc/free interrupts
>>> from irqdomain for generic MSI interrupts.
>>>
>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>> ---
>>>  include/linux/msi.h |   35 ++++++++++++++++++++++++++++++
>>>  kernel/irq/msi.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 94 insertions(+)
>>>
>>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>>> index 714716a3ffdd..6b356a1410b2 100644
>>> --- a/include/linux/msi.h
>>> +++ b/include/linux/msi.h
>>> @@ -3,6 +3,7 @@
>>>  
>>>  #include <linux/kobject.h>
>>>  #include <linux/list.h>
>>> +#include <asm/hw_irq.h>		/* for msi_alloc_info_t */
>>>  
>>>  struct msi_msg {
>>>  	u32	address_lo;	/* low 32 bits of msi message address */
>>> @@ -100,7 +101,26 @@ struct irq_chip;
>>>  struct device_node;
>>>  struct msi_domain_info;
>>>  
>>> +#ifndef NUM_MSI_ALLOC_SCRATCHPAD_REGS
>>> +#define NUM_MSI_ALLOC_SCRATCHPAD_REGS	2
>>> +#endif
>>> +
>>> +/*
>>> + * Default structure for MSI interrupt allocation.
>>> + * Arch may overwrite it by define msi_alloc_info_t.
>>> + */
>>> +struct msi_alloc_info {
>>> +	struct msi_desc *desc;
>>> +	irq_hw_number_t hwirq;
>>> +	long		scratchpad[NUM_MSI_ALLOC_SCRATCHPAD_REGS];
>>> +};
>>> +
>>> +#ifndef msi_alloc_info_t
>>> +typedef struct msi_alloc_info msi_alloc_info_t;
>>> +#endif
>>> +
>>>  struct msi_domain_ops {
>>> +	/* Callbacks for msi_create_irq_domain() */
>>>  	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);
>>> @@ -111,6 +131,19 @@ struct msi_domain_ops {
>>>  	void		(*msi_free)(struct irq_domain *domain,
>>>  				    struct msi_domain_info *info,
>>>  				    unsigned int virq);
>>> +
>>> +	/* Callbacks for msi_irq_domain_alloc_irqs() based on msi_descs */
>>> +	int		(*msi_check)(struct irq_domain *domain,
>>> +				     struct msi_domain_info *info,
>>> +				     struct device *dev);
>>> +	int		(*msi_prepare)(struct irq_domain *domain,
>>> +				       struct device *dev,
>>> +				       msi_alloc_info_t *arg);
>>> +	void		(*msi_finish)(msi_alloc_info_t *arg, int retval);
>>> +	void		(*set_desc)(msi_alloc_info_t *arg,
>>> +				    struct msi_desc *desc);
>>> +	int		(*handle_error)(struct irq_domain *domain,
>>> +					struct msi_desc *desc, int error);
>>>  };
>>>  
>>>  struct msi_domain_info {
>>> @@ -125,6 +158,8 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
>>>  struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>>>  					 struct msi_domain_info *info,
>>>  					 struct irq_domain *parent);
>>> +int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev);
>>> +void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
>>>  struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
>>>  
>>>  #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
>>> index 94d6d87ee23e..0b6665de3a60 100644
>>> --- a/kernel/irq/msi.c
>>> +++ b/kernel/irq/msi.c
>>> @@ -13,6 +13,9 @@
>>>  #include <linux/irqdomain.h>
>>>  #include <linux/msi.h>
>>>  
>>> +/* Temparory solution for building, will be removed later */
>>> +#include <linux/pci.h>
>>> +
>>>  void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>>>  {
>>>  	*msg = entry->msg;
>>> @@ -124,6 +127,62 @@ struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
>>>  	return domain;
>>>  }
>>>  
>>> +int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev)
>>> +{
>>> +	int i, ret, virq = -1;
>>> +	msi_alloc_info_t arg;
>>> +	struct msi_desc *desc;
>>> +	struct msi_domain_info *info = domain->host_data;
>>> +	struct msi_domain_ops *ops = info->ops;
>>> +
>>> +	ret = ops->msi_check(domain, info, dev);
>>> +	if (ret == 0)
>>> +		ret = ops->msi_prepare(domain, dev, &arg);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	for_each_msi_entry(desc, dev) {
>>> +		ops->set_desc(&arg, desc);
>>> +
>>> +		virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used,
>>> +					       dev_to_node(dev), &arg, false);
>>> +		if (virq < 0) {
>>> +			ret = -ENOSPC;
>>> +			if (ops->handle_error)
>>> +				ret = ops->handle_error(domain, desc, ret);
>>> +			if (ops->msi_finish)
>>> +				ops->msi_finish(&arg, ret);
>>> +			return ret;
>>> +		}
>>> +
>>> +		for (i = 0; i < desc->nvec_used; i++)
>>> +			irq_set_msi_desc_off(virq, i, desc);
>>> +	}
>>> +
>>> +	if (ops->msi_finish)
>>> +		ops->msi_finish(&arg, 0);
>>> +
>>> +	for_each_msi_entry(desc, dev) {
>>> +		if (desc->nvec_used == 1)
>>> +			dev_dbg(dev, "irq %d for MSI\n", virq);
>>> +		else
>>> +			dev_dbg(dev, "irq [%d-%d] for MSI\n",
>>> +				virq, virq + desc->nvec_used - 1);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
>>> +{
>>> +	struct msi_desc *desc;
>>> +
>>> +	for_each_msi_entry(desc, dev) {
>>> +		irq_domain_free_irqs(desc->irq, desc->nvec_used);
>>> +		desc->irq = 0;
>>> +	}
>>> +}
>>> +
>>>  struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
>>>  {
>>>  	return (struct msi_domain_info *)domain->host_data;
>>>
>>
>>
> 
> .
> 


-- 
Thanks!
Yijing

[Patch V1 4/6] genirq: Provide default callbacks for msi_domain_ops

[Jiang Liu]

Extend struct msi_domain_info and provide default callbacks for
msi_domain_ops.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 include/linux/msi.h |   26 +++++++++++---
 kernel/irq/msi.c    |   93 +++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 6b356a1410b2..c62a1ca71241 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -121,13 +121,15 @@ typedef struct msi_alloc_info msi_alloc_info_t;
 
 struct msi_domain_ops {
 	/* Callbacks for msi_create_irq_domain() */
-	void		(*calc_hwirq)(struct msi_domain_info *info, void *arg,
+	void		(*calc_hwirq)(struct msi_domain_info *info,
+				      msi_alloc_info_t *arg,
 				      struct msi_desc *desc);
-	irq_hw_number_t	(*get_hwirq)(struct msi_domain_info *info, void *arg);
+	irq_hw_number_t	(*get_hwirq)(struct msi_domain_info *info,
+				     msi_alloc_info_t *arg);
 	int		(*msi_init)(struct irq_domain *domain,
 				    struct msi_domain_info *info,
 				    unsigned int virq, irq_hw_number_t hwirq,
-				    void *arg);
+				    msi_alloc_info_t *arg);
 	void		(*msi_free)(struct irq_domain *domain,
 				    struct msi_domain_info *info,
 				    unsigned int virq);
@@ -147,15 +149,29 @@ struct msi_domain_ops {
 };
 
 struct msi_domain_info {
+	u32			flags;
 	struct msi_domain_ops	*ops;
 	struct irq_chip		*chip;
-	void			*data;
+	void			*chip_data;	/* optional chip data */
+	void			*handler;	/* optional flow handler */
+	void			*handler_data;	/* optional handler data */
+	const char		*handler_name;	/* optional handler name */
+	void			*data;		/* optional private data */
 };
 
+/* Use default MSI domain ops if possible */
+#define MSI_FLAG_USE_DEF_OPS		0x1
+/* Build identity map between hwirq and irq */
+#define MSI_FLAG_IDENTITY_MAP		0x2
+/* Support multiple PCI MSI interrupts */
+#define MSI_FLAG_MULTI_PCI_MSI		0x4
+/* Support PCI MSIX interrupts */
+#define MSI_FLAG_PCI_MSIX		0x8
+
 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 irq_domain *msi_create_irq_domain(struct device_node *node,
 					 struct msi_domain_info *info,
 					 struct irq_domain *parent);
 int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev);
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 0b6665de3a60..5512aa155a50 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -9,6 +9,8 @@
  * This file contains common code to support Message Signalled Interrupt for
  * PCI compatible and non PCI compatible devices.
  */
+#include <linux/types.h>
+#include <linux/device.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/msi.h>
@@ -114,13 +116,94 @@ static struct irq_domain_ops msi_domain_ops = {
 	.deactivate	= msi_domain_deactivate,
 };
 
-struct irq_domain *msi_create_irq_domain(struct device_node *of_node,
+#ifndef msi_alloc_info_t
+static irq_hw_number_t msi_domain_ops_get_hwirq(struct msi_domain_info *info,
+						struct msi_alloc_info *arg)
+{
+	return arg->hwirq;
+}
+
+static int msi_domain_ops_prepare(struct irq_domain *domain, struct device *dev,
+				  struct msi_alloc_info *arg)
+{
+	memset(arg, 0, sizeof(*arg));
+
+	return 0;
+}
+
+static void msi_domain_ops_set_desc(struct msi_alloc_info *arg,
+				    struct msi_desc *desc)
+{
+	arg->desc = desc;
+}
+#else
+#define msi_domain_ops_get_hwirq	NULL
+#define msi_domain_ops_prepare		NULL
+#define msi_domain_ops_set_desc		NULL
+#endif /* msi_alloc_info_t */
+
+static int msi_domain_ops_init(struct irq_domain *domain,
+			       struct msi_domain_info *info,
+			       unsigned int virq, irq_hw_number_t hwirq,
+			       msi_alloc_info_t *arg)
+{
+	irq_domain_set_hwirq_and_chip(domain, virq, hwirq, info->chip,
+				      info->chip_data);
+	if (info->handler && info->handler_name) {
+		__irq_set_handler(virq, info->handler, 0, info->handler_name);
+		if (info->handler_data)
+			irq_set_handler_data(virq, info->handler_data);
+	}
+
+	return 0;
+}
+
+static int msi_domain_ops_check(struct irq_domain *domain,
+				struct msi_domain_info *info,
+				struct device *dev)
+{
+	return 0;
+}
+
+static struct msi_domain_ops msi_domain_ops_default = {
+	.get_hwirq	= msi_domain_ops_get_hwirq,
+	.msi_init	= msi_domain_ops_init,
+	.msi_check	= msi_domain_ops_check,
+	.msi_prepare	= msi_domain_ops_prepare,
+	.set_desc	= msi_domain_ops_set_desc,
+};
+
+static void msi_domain_update_ops(struct msi_domain_info *info)
+{
+	struct msi_domain_ops *ops = info->ops;
+
+	if (ops == NULL) {
+		info->ops = &msi_domain_ops_default;
+		return;
+	}
+
+	if (ops->get_hwirq == NULL)
+		ops->get_hwirq = msi_domain_ops_default.get_hwirq;
+	if (ops->msi_init == NULL)
+		ops->msi_init = msi_domain_ops_default.msi_init;
+	if (ops->msi_check == NULL)
+		ops->msi_check = msi_domain_ops_default.msi_check;
+	if (ops->msi_prepare == NULL)
+		ops->msi_prepare = msi_domain_ops_default.msi_prepare;
+	if (ops->set_desc == NULL)
+		ops->set_desc = msi_domain_ops_default.set_desc;
+}
+
+struct irq_domain *msi_create_irq_domain(struct device_node *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 (info->flags & MSI_FLAG_USE_DEF_OPS)
+		msi_domain_update_ops(info);
+
+	domain = irq_domain_add_tree(node, &msi_domain_ops, info);
 	if (domain)
 		domain->parent = parent;
 
@@ -143,8 +226,12 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev)
 
 	for_each_msi_entry(desc, dev) {
 		ops->set_desc(&arg, desc);
+		if (info->flags & MSI_FLAG_IDENTITY_MAP)
+			virq = (int)ops->get_hwirq(info, &arg);
+		else
+			virq = -1;
 
-		virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used,
+		virq = __irq_domain_alloc_irqs(domain, virq, desc->nvec_used,
 					       dev_to_node(dev), &arg, false);
 		if (virq < 0) {
 			ret = -ENOSPC;
-- 
1.7.10.4

[Patch V1 5/6] PCI, MSI: Refine irqdomain interfaces to simplify its usage

[Jiang Liu]

Refine irqdomain interfaces to simplify its usage.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/pci/msi.c   |  108 +++++++++++++++++++++++++++++++++++++--------------
 include/linux/msi.h |   11 ++++--
 2 files changed, 86 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 9c53b865cb1b..0319c21dd208 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1099,38 +1099,88 @@ irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
 		(pci_domain_nr(dev->bus) & 0xFFFFFFFF) << 27;
 }
 
-int pci_msi_domain_alloc_irqs(struct irq_domain *domain, int type,
-			      struct pci_dev *dev, void *arg)
+static inline bool pci_msi_desc_is_multi_msi(struct msi_desc *desc)
 {
-	struct msi_domain_info *info = domain->host_data;
-	int node = dev_to_node(&dev->dev);
-	struct msi_desc *desc;
-	int i, virq;
-
-	list_for_each_entry(desc, &dev->msi_list, list) {
-		if (info->ops->calc_hwirq)
-			info->ops->calc_hwirq(info, arg, desc);
-
-		virq = irq_domain_alloc_irqs(domain, desc->nvec_used,
-					     node, arg);
-		if (virq < 0) {
-			/* Special handling for pci_enable_msi_range(). */
-			if (type == PCI_CAP_ID_MSI && desc->nvec_used > 1)
-				return 1;
-			else
-				return -ENOSPC;
-		}
-		for (i = 0; i < desc->nvec_used; i++)
-			irq_set_msi_desc_off(virq, i, desc);
-	}
+	return !desc->msi_attrib.is_msix && desc->nvec_used > 1;
+}
+
+int pci_msi_domain_check_cap(struct irq_domain *domain,
+			     struct msi_domain_info *info, struct device *dev)
+{
+	struct msi_desc *desc = first_pci_msi_entry(to_pci_dev(dev));
 
-	list_for_each_entry(desc, &dev->msi_list, list)
-		if (desc->nvec_used == 1)
-			dev_dbg(&dev->dev, "irq %d for MSI/MSI-X\n", virq);
-		else
-			dev_dbg(&dev->dev, "irq [%d-%d] for MSI/MSI-X\n",
-				virq, virq + desc->nvec_used - 1);
+	/* Special handling to support pci_enable_msi_range() */
+	if (pci_msi_desc_is_multi_msi(desc) &&
+	    !(info->flags & MSI_FLAG_MULTI_PCI_MSI))
+		return 1;
+	else if (desc->msi_attrib.is_msix && !(info->flags & MSI_FLAG_PCI_MSIX))
+		return -ENOTSUPP;
 
 	return 0;
 }
+
+static int pci_msi_domain_handle_error(struct irq_domain *domain,
+				       struct msi_desc *desc, int error)
+{
+	/* Special handling to support pci_enable_msi_range() */
+	if (pci_msi_desc_is_multi_msi(desc) && error == -ENOSPC)
+		return 1;
+
+	return error;
+}
+
+#ifndef msi_alloc_info_t
+static void pci_msi_domain_set_desc(struct msi_alloc_info *arg,
+				    struct msi_desc *desc)
+{
+	arg->desc = desc;
+	arg->hwirq = pci_msi_domain_calc_hwirq(msi_desc_to_pci_dev(desc),
+					       desc);
+}
+#else
+#define pci_msi_domain_set_desc		NULL
+#endif
+
+static struct msi_domain_ops pci_msi_domain_ops_default = {
+	.set_desc	= pci_msi_domain_set_desc,
+	.msi_check	= pci_msi_domain_check_cap,
+	.handle_error	= pci_msi_domain_handle_error,
+};
+
+static void pci_msi_domain_update_ops(struct msi_domain_info *info)
+{
+	struct msi_domain_ops *ops = info->ops;
+
+	if (ops == NULL) {
+		info->ops = &pci_msi_domain_ops_default;
+	} else {
+		if (ops->set_desc == NULL)
+			ops->set_desc = pci_msi_domain_set_desc;
+		if (ops->msi_check == NULL)
+			ops->msi_check = pci_msi_domain_check_cap;
+		if (ops->handle_error == NULL)
+			ops->handle_error = pci_msi_domain_handle_error;
+	}
+}
+
+struct irq_domain *pci_msi_create_irq_domain(struct device_node *node,
+					     struct msi_domain_info *info,
+					     struct irq_domain *parent)
+{
+	if (info->flags & MSI_FLAG_USE_DEF_OPS)
+		pci_msi_domain_update_ops(info);
+
+	return msi_create_irq_domain(node, info, parent);
+}
+
+int pci_msi_domain_alloc_irqs(struct irq_domain *domain, int type,
+			      struct pci_dev *dev)
+{
+	return msi_domain_alloc_irqs(domain, &dev->dev);
+}
+
+void pci_msi_domain_free_irqs(struct irq_domain *domain, struct pci_dev *dev)
+{
+	msi_domain_free_irqs(domain, &dev->dev);
+}
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
diff --git a/include/linux/msi.h b/include/linux/msi.h
index c62a1ca71241..9031791e9b4c 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -121,9 +121,6 @@ typedef struct msi_alloc_info msi_alloc_info_t;
 
 struct msi_domain_ops {
 	/* Callbacks for msi_create_irq_domain() */
-	void		(*calc_hwirq)(struct msi_domain_info *info,
-				      msi_alloc_info_t *arg,
-				      struct msi_desc *desc);
 	irq_hw_number_t	(*get_hwirq)(struct msi_domain_info *info,
 				     msi_alloc_info_t *arg);
 	int		(*msi_init)(struct irq_domain *domain,
@@ -182,10 +179,16 @@ struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
 
 #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
 void pci_msi_domain_write_msg(struct irq_data *irq_data, struct msi_msg *msg);
+struct irq_domain *pci_msi_create_irq_domain(struct device_node *node,
+					     struct msi_domain_info *info,
+					     struct irq_domain *parent);
 int pci_msi_domain_alloc_irqs(struct irq_domain *domain, int type,
-			      struct pci_dev *dev, void *arg);
+			      struct pci_dev *dev);
+void pci_msi_domain_free_irqs(struct irq_domain *domain, struct pci_dev *dev);
 irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
 					  struct msi_desc *desc);
+int pci_msi_domain_check_cap(struct irq_domain *domain,
+			     struct msi_domain_info *info, struct device *dev);
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
 
 #endif /* LINUX_MSI_H */
-- 
1.7.10.4

[Patch V1 6/6] PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from irqdomain

[Jiang Liu]

Provide mechanism to directly alloc/free MSI/MSIX interrupt from
irqdomain, which will be used to replace arch_setup_msi_irq()/
arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().

To kill weak functions, this patch introduce a new weak function
arch_get_pci_msi_domain(), which is to retrieve the MSI irqdomain
for a PCI device. This weak function could be killed once we get
a common way to associate MSI domain with PCI device.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/pci/msi.c   |   60 +++++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/msi.h |    3 +++
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 0319c21dd208..6f8a2f93187a 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -27,8 +27,41 @@ static int pci_msi_enable = 1;
 
 #define msix_table_size(flags)	((flags & PCI_MSIX_FLAGS_QSIZE) + 1)
 
-/* Arch hooks */
+#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
+static struct irq_domain *pci_msi_default_domain;
+
+struct irq_domain * __weak arch_get_pci_msi_domain(struct pci_dev *dev)
+{
+	return pci_msi_default_domain;
+}
+
+static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+	struct irq_domain *domain;
 
+	domain = arch_get_pci_msi_domain(dev);
+	if (domain)
+		return pci_msi_domain_alloc_irqs(domain, type, dev);
+
+	return arch_setup_msi_irqs(dev, nvec, type);
+}
+
+static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)
+{
+	struct irq_domain *domain;
+
+	domain = arch_get_pci_msi_domain(dev);
+	if (domain)
+		pci_msi_domain_free_irqs(domain, dev);
+	else
+		arch_teardown_msi_irqs(dev);
+}
+#else
+#define pci_msi_setup_msi_irqs		arch_setup_msi_irqs
+#define pci_msi_teardown_msi_irqs	arch_teardown_msi_irqs
+#endif
+
+/* Arch hooks */
 int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
 {
 	struct msi_chip *chip = dev->bus->msi;
@@ -329,7 +362,7 @@ static void free_msi_irqs(struct pci_dev *dev)
 			for (i = 0; i < entry->nvec_used; i++)
 				BUG_ON(irq_has_action(entry->irq + i));
 
-	arch_teardown_msi_irqs(dev);
+	pci_msi_teardown_msi_irqs(dev);
 
 	list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
 		if (entry->msi_attrib.is_msix) {
@@ -581,7 +614,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	list_add_tail(&entry->list, &dev->msi_list);
 
 	/* Configure MSI capability structure */
-	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI);
+	ret = pci_msi_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI);
 	if (ret) {
 		msi_mask_irq(entry, mask, ~mask);
 		free_msi_irqs(dev);
@@ -696,7 +729,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	if (ret)
 		return ret;
 
-	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
+	ret = pci_msi_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
 	if (ret)
 		goto out_avail;
 
@@ -1183,4 +1216,23 @@ void pci_msi_domain_free_irqs(struct irq_domain *domain, struct pci_dev *dev)
 {
 	msi_domain_free_irqs(domain, &dev->dev);
 }
+
+struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
+		 struct msi_domain_info *info, struct irq_domain *parent)
+{
+	struct irq_domain *domain;
+	static DEFINE_MUTEX(pci_msi_domain_lock);
+
+	mutex_lock(&pci_msi_domain_lock);
+	if (pci_msi_default_domain) {
+		pr_err("PCI: default irq domain for PCI MSI has already been created.\n");
+		domain = NULL;
+	} else {
+		domain = msi_create_irq_domain(node, info, parent);
+		pci_msi_default_domain = domain;
+	}
+	mutex_unlock(&pci_msi_domain_lock);
+
+	return domain;
+}
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 9031791e9b4c..e3ec97349535 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -185,6 +185,9 @@ struct irq_domain *pci_msi_create_irq_domain(struct device_node *node,
 int pci_msi_domain_alloc_irqs(struct irq_domain *domain, int type,
 			      struct pci_dev *dev);
 void pci_msi_domain_free_irqs(struct irq_domain *domain, struct pci_dev *dev);
+struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
+		 struct msi_domain_info *info, struct irq_domain *parent);
+
 irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
 					  struct msi_desc *desc);
 int pci_msi_domain_check_cap(struct irq_domain *domain,
-- 
1.7.10.4

Re: [Patch V1 6/6] PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from irqdomain

[Marc Zyngier]

Hi Jiang,

On 13/11/14 11:43, Jiang Liu wrote:
> Provide mechanism to directly alloc/free MSI/MSIX interrupt from
> irqdomain, which will be used to replace arch_setup_msi_irq()/
> arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().
> 
> To kill weak functions, this patch introduce a new weak function
> arch_get_pci_msi_domain(), which is to retrieve the MSI irqdomain
> for a PCI device. This weak function could be killed once we get
> a common way to associate MSI domain with PCI device.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  drivers/pci/msi.c   |   60 +++++++++++++++++++++++++++++++++++++++++++++++----
>  include/linux/msi.h |    3 +++
>  2 files changed, 59 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 0319c21dd208..6f8a2f93187a 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -27,8 +27,41 @@ static int pci_msi_enable = 1;
>  
>  #define msix_table_size(flags)	((flags & PCI_MSIX_FLAGS_QSIZE) + 1)
>  
> -/* Arch hooks */
> +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
> +static struct irq_domain *pci_msi_default_domain;
> +
> +struct irq_domain * __weak arch_get_pci_msi_domain(struct pci_dev *dev)
> +{
> +	return pci_msi_default_domain;
> +}
> +
> +static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> +{
> +	struct irq_domain *domain;
>  
> +	domain = arch_get_pci_msi_domain(dev);
> +	if (domain)
> +		return pci_msi_domain_alloc_irqs(domain, type, dev);
> +
> +	return arch_setup_msi_irqs(dev, nvec, type);
> +}
> +
> +static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)
> +{
> +	struct irq_domain *domain;
> +
> +	domain = arch_get_pci_msi_domain(dev);
> +	if (domain)
> +		pci_msi_domain_free_irqs(domain, dev);
> +	else
> +		arch_teardown_msi_irqs(dev);
> +}
> +#else
> +#define pci_msi_setup_msi_irqs		arch_setup_msi_irqs
> +#define pci_msi_teardown_msi_irqs	arch_teardown_msi_irqs
> +#endif
> +
> +/* Arch hooks */
>  int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
>  {
>  	struct msi_chip *chip = dev->bus->msi;
> @@ -329,7 +362,7 @@ static void free_msi_irqs(struct pci_dev *dev)
>  			for (i = 0; i < entry->nvec_used; i++)
>  				BUG_ON(irq_has_action(entry->irq + i));
>  
> -	arch_teardown_msi_irqs(dev);
> +	pci_msi_teardown_msi_irqs(dev);
>  
>  	list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
>  		if (entry->msi_attrib.is_msix) {
> @@ -581,7 +614,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
>  	list_add_tail(&entry->list, &dev->msi_list);
>  
>  	/* Configure MSI capability structure */
> -	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI);
> +	ret = pci_msi_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI);
>  	if (ret) {
>  		msi_mask_irq(entry, mask, ~mask);
>  		free_msi_irqs(dev);
> @@ -696,7 +729,7 @@ static int msix_capability_init(struct pci_dev *dev,
>  	if (ret)
>  		return ret;
>  
> -	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
> +	ret = pci_msi_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
>  	if (ret)
>  		goto out_avail;
>  
> @@ -1183,4 +1216,23 @@ void pci_msi_domain_free_irqs(struct irq_domain *domain, struct pci_dev *dev)
>  {
>  	msi_domain_free_irqs(domain, &dev->dev);
>  }
> +
> +struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
> +		 struct msi_domain_info *info, struct irq_domain *parent)
> +{
> +	struct irq_domain *domain;
> +	static DEFINE_MUTEX(pci_msi_domain_lock);
> +
> +	mutex_lock(&pci_msi_domain_lock);
> +	if (pci_msi_default_domain) {
> +		pr_err("PCI: default irq domain for PCI MSI has already been created.\n");
> +		domain = NULL;
> +	} else {
> +		domain = msi_create_irq_domain(node, info, parent);

Surely this needs to be a call to pci_msi_create_irq_domain().
Otherwise, not much is working on the PCI side...

Thanks,

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

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Yijing Wang]

On 2014/11/13 19:43, Jiang Liu wrote:
> This patch set is based on tip/irq/irqdomain and tries to refine
> interfaces to support irqdomain for generic MSI and PCI MSI.
> 
> Patch 1 is just minor fixes for tip/irq/irqdomain.
> 
> Patch 2 introduces some helpers to hide struct msi_desc implementation
> details, so later we could move msi_list from struct pci_dev into
> struct device to enable generic MSI support.

Hi Gerry,
   I tried to move msi info(eg. msi_list) into struct device, but I found
DMAR fault interrupt is initialized before the driver core init. And I don't
know whether there are other devices like ARM consolidator(introduced in GIC v3)
need to be initialized before driver core. What do you think about this ?

Thanks!
Yijing.

> 
> Patch 3 introduces msi_domain_{alloc|free}_irqs() which generalize
> pci_msi_domain_alloc_irqs() to support generic MSI.
> 
> Patch 4 introduces default data structures and callback implementations
> to support msi_domain_alloc_irqs(), so reduce burden on generic MSI
> users.
> 
> Patch 5 converts PCI MSI to use generic MSI interfaces, and also
> implement default callbacks for PCI MSI.
> 
> Patch 6 introduces a mechanism to replace arch_setup_msi_irq()/
> arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().
> 
> With this patch set applied, the generic MSI and PCI MSI interfaces
> are much easier to use. For extreme case, you only need to define
> a "struct msi_domain_info" and don't need to implement any callbacks,
> just using the default callbacks is OK:)
> 
> This patch set is also a preparation for:
> 1) Kill all weak functions in drivers/pci/msi.c
> 2) Implement support for non-PCI-compliant MSI device
> 
> It has been tested on x86 platforms, comments are welcomed!
> 
> Jiang Liu (6):
>   PCI, MSI: Fix errors caused by commit e5f1a59c4e12
>   PCI, MSI: Introduce helpers to hide struct msi_desc implemenation
>     details
>   genirq: Introduce msi_domain_{alloc|free}_irqs()
>   genirq: Provide default callbacks for msi_domain_ops
>   PCI, MSI: Refine irqdomain interfaces to simplify its usage
>   PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from
>     irqdomain
> 
>  drivers/pci/msi.c   |  170 ++++++++++++++++++++++++++++++++++++++++-----------
>  include/linux/msi.h |   90 ++++++++++++++++++++++++---
>  kernel/irq/msi.c    |  150 ++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 366 insertions(+), 44 deletions(-)
> 


-- 
Thanks!
Yijing

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/13 20:28, Yijing Wang wrote:
> On 2014/11/13 19:43, Jiang Liu wrote:
>> This patch set is based on tip/irq/irqdomain and tries to refine
>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>
>> Patch 1 is just minor fixes for tip/irq/irqdomain.
>>
>> Patch 2 introduces some helpers to hide struct msi_desc implementation
>> details, so later we could move msi_list from struct pci_dev into
>> struct device to enable generic MSI support.
> 
> Hi Gerry,
>    I tried to move msi info(eg. msi_list) into struct device, but I found
> DMAR fault interrupt is initialized before the driver core init. And I don't
> know whether there are other devices like ARM consolidator(introduced in GIC v3)
> need to be initialized before driver core. What do you think about this ?
Hi Yijing,
	DMAR interrupt doesn't make use of msi_desc, so has no
dependency on msi_list.
	Actually there are two levels of generic MSI sharing. The first
level is to share common irq_chip/irqdomain code, such as HPET, DMAR and
HT_IRQ. The second level is to share msi_desc, such as some device
on ARM side as you have mentioned.
	With this patch set applied, we achieve level one sharing. For
level two sharing, we need to move msi_list into struct device, refactor
struct msi_desc, and provide a generic pci_enable_msix_range alike
user interfaces. I'm still working on this part, so we could cooperate
with each other.
Thanks!
Gerry

> 
> Thanks!
> Yijing.
> 
>>
>> Patch 3 introduces msi_domain_{alloc|free}_irqs() which generalize
>> pci_msi_domain_alloc_irqs() to support generic MSI.
>>
>> Patch 4 introduces default data structures and callback implementations
>> to support msi_domain_alloc_irqs(), so reduce burden on generic MSI
>> users.
>>
>> Patch 5 converts PCI MSI to use generic MSI interfaces, and also
>> implement default callbacks for PCI MSI.
>>
>> Patch 6 introduces a mechanism to replace arch_setup_msi_irq()/
>> arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().
>>
>> With this patch set applied, the generic MSI and PCI MSI interfaces
>> are much easier to use. For extreme case, you only need to define
>> a "struct msi_domain_info" and don't need to implement any callbacks,
>> just using the default callbacks is OK:)
>>
>> This patch set is also a preparation for:
>> 1) Kill all weak functions in drivers/pci/msi.c
>> 2) Implement support for non-PCI-compliant MSI device
>>
>> It has been tested on x86 platforms, comments are welcomed!
>>
>> Jiang Liu (6):
>>   PCI, MSI: Fix errors caused by commit e5f1a59c4e12
>>   PCI, MSI: Introduce helpers to hide struct msi_desc implemenation
>>     details
>>   genirq: Introduce msi_domain_{alloc|free}_irqs()
>>   genirq: Provide default callbacks for msi_domain_ops
>>   PCI, MSI: Refine irqdomain interfaces to simplify its usage
>>   PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from
>>     irqdomain
>>
>>  drivers/pci/msi.c   |  170 ++++++++++++++++++++++++++++++++++++++++-----------
>>  include/linux/msi.h |   90 ++++++++++++++++++++++++---
>>  kernel/irq/msi.c    |  150 ++++++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 366 insertions(+), 44 deletions(-)
>>
> 
> 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Yijing Wang]

On 2014/11/13 20:39, Jiang Liu wrote:
> 
> 
> On 2014/11/13 20:28, Yijing Wang wrote:
>> On 2014/11/13 19:43, Jiang Liu wrote:
>>> This patch set is based on tip/irq/irqdomain and tries to refine
>>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>>
>>> Patch 1 is just minor fixes for tip/irq/irqdomain.
>>>
>>> Patch 2 introduces some helpers to hide struct msi_desc implementation
>>> details, so later we could move msi_list from struct pci_dev into
>>> struct device to enable generic MSI support.
>>
>> Hi Gerry,
>>    I tried to move msi info(eg. msi_list) into struct device, but I found
>> DMAR fault interrupt is initialized before the driver core init. And I don't
>> know whether there are other devices like ARM consolidator(introduced in GIC v3)
>> need to be initialized before driver core. What do you think about this ?
> Hi Yijing,
> 	DMAR interrupt doesn't make use of msi_desc, so has no
> dependency on msi_list.

OK, I thought we could use msi_desc to describe DMAR/HPET irq, so they could
share the mask/unmask, write_msg/read_etc.. But maybe it's not a right direction. :)

> 	Actually there are two levels of generic MSI sharing. The first
> level is to share common irq_chip/irqdomain code, such as HPET, DMAR and
> HT_IRQ. The second level is to share msi_desc, such as some device
> on ARM side as you have mentioned.
> 	With this patch set applied, we achieve level one sharing. For
> level two sharing, we need to move msi_list into struct device, refactor
> struct msi_desc, and provide a generic pci_enable_msix_range alike
> user interfaces. I'm still working on this part, so we could cooperate
> with each other.

That's good :)


> Thanks!
> Gerry
> 
>>
>> Thanks!
>> Yijing.
>>
>>>
>>> Patch 3 introduces msi_domain_{alloc|free}_irqs() which generalize
>>> pci_msi_domain_alloc_irqs() to support generic MSI.
>>>
>>> Patch 4 introduces default data structures and callback implementations
>>> to support msi_domain_alloc_irqs(), so reduce burden on generic MSI
>>> users.
>>>
>>> Patch 5 converts PCI MSI to use generic MSI interfaces, and also
>>> implement default callbacks for PCI MSI.
>>>
>>> Patch 6 introduces a mechanism to replace arch_setup_msi_irq()/
>>> arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().
>>>
>>> With this patch set applied, the generic MSI and PCI MSI interfaces
>>> are much easier to use. For extreme case, you only need to define
>>> a "struct msi_domain_info" and don't need to implement any callbacks,
>>> just using the default callbacks is OK:)
>>>
>>> This patch set is also a preparation for:
>>> 1) Kill all weak functions in drivers/pci/msi.c
>>> 2) Implement support for non-PCI-compliant MSI device
>>>
>>> It has been tested on x86 platforms, comments are welcomed!
>>>
>>> Jiang Liu (6):
>>>   PCI, MSI: Fix errors caused by commit e5f1a59c4e12
>>>   PCI, MSI: Introduce helpers to hide struct msi_desc implemenation
>>>     details
>>>   genirq: Introduce msi_domain_{alloc|free}_irqs()
>>>   genirq: Provide default callbacks for msi_domain_ops
>>>   PCI, MSI: Refine irqdomain interfaces to simplify its usage
>>>   PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from
>>>     irqdomain
>>>
>>>  drivers/pci/msi.c   |  170 ++++++++++++++++++++++++++++++++++++++++-----------
>>>  include/linux/msi.h |   90 ++++++++++++++++++++++++---
>>>  kernel/irq/msi.c    |  150 ++++++++++++++++++++++++++++++++++++++++++++-
>>>  3 files changed, 366 insertions(+), 44 deletions(-)
>>>
>>
>>
> 
> .
> 


-- 
Thanks!
Yijing

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/13 20:55, Yijing Wang wrote:
> On 2014/11/13 20:39, Jiang Liu wrote:
>>
>>
>> On 2014/11/13 20:28, Yijing Wang wrote:
>>> On 2014/11/13 19:43, Jiang Liu wrote:
>>>> This patch set is based on tip/irq/irqdomain and tries to refine
>>>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>>>
>>>> Patch 1 is just minor fixes for tip/irq/irqdomain.
>>>>
>>>> Patch 2 introduces some helpers to hide struct msi_desc implementation
>>>> details, so later we could move msi_list from struct pci_dev into
>>>> struct device to enable generic MSI support.
>>>
>>> Hi Gerry,
>>>    I tried to move msi info(eg. msi_list) into struct device, but I found
>>> DMAR fault interrupt is initialized before the driver core init. And I don't
>>> know whether there are other devices like ARM consolidator(introduced in GIC v3)
>>> need to be initialized before driver core. What do you think about this ?
>> Hi Yijing,
>> 	DMAR interrupt doesn't make use of msi_desc, so has no
>> dependency on msi_list.
> 
> OK, I thought we could use msi_desc to describe DMAR/HPET irq, so they could
> share the mask/unmask, write_msg/read_etc.. But maybe it's not a right direction. :)
It doesn't help us much because there's no common way to implement
mask/unmask operations. Read/write can't be shared too.

HT_IRQ may be switched to use msi_desc, but it's not so popular:)

> 
>> 	Actually there are two levels of generic MSI sharing. The first
>> level is to share common irq_chip/irqdomain code, such as HPET, DMAR and
>> HT_IRQ. The second level is to share msi_desc, such as some device
>> on ARM side as you have mentioned.
>> 	With this patch set applied, we achieve level one sharing. For
>> level two sharing, we need to move msi_list into struct device, refactor
>> struct msi_desc, and provide a generic pci_enable_msix_range alike
>> user interfaces. I'm still working on this part, so we could cooperate
>> with each other.
> 
> That's good :)
> 
> 
>> Thanks!
>> Gerry
>>
>>>
>>> Thanks!
>>> Yijing.
>>>
>>>>
>>>> Patch 3 introduces msi_domain_{alloc|free}_irqs() which generalize
>>>> pci_msi_domain_alloc_irqs() to support generic MSI.
>>>>
>>>> Patch 4 introduces default data structures and callback implementations
>>>> to support msi_domain_alloc_irqs(), so reduce burden on generic MSI
>>>> users.
>>>>
>>>> Patch 5 converts PCI MSI to use generic MSI interfaces, and also
>>>> implement default callbacks for PCI MSI.
>>>>
>>>> Patch 6 introduces a mechanism to replace arch_setup_msi_irq()/
>>>> arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().
>>>>
>>>> With this patch set applied, the generic MSI and PCI MSI interfaces
>>>> are much easier to use. For extreme case, you only need to define
>>>> a "struct msi_domain_info" and don't need to implement any callbacks,
>>>> just using the default callbacks is OK:)
>>>>
>>>> This patch set is also a preparation for:
>>>> 1) Kill all weak functions in drivers/pci/msi.c
>>>> 2) Implement support for non-PCI-compliant MSI device
>>>>
>>>> It has been tested on x86 platforms, comments are welcomed!
>>>>
>>>> Jiang Liu (6):
>>>>   PCI, MSI: Fix errors caused by commit e5f1a59c4e12
>>>>   PCI, MSI: Introduce helpers to hide struct msi_desc implemenation
>>>>     details
>>>>   genirq: Introduce msi_domain_{alloc|free}_irqs()
>>>>   genirq: Provide default callbacks for msi_domain_ops
>>>>   PCI, MSI: Refine irqdomain interfaces to simplify its usage
>>>>   PCI, MSI: Provide mechanism to alloc/free MSI/MSIX interrupt from
>>>>     irqdomain
>>>>
>>>>  drivers/pci/msi.c   |  170 ++++++++++++++++++++++++++++++++++++++++-----------
>>>>  include/linux/msi.h |   90 ++++++++++++++++++++++++---
>>>>  kernel/irq/msi.c    |  150 ++++++++++++++++++++++++++++++++++++++++++++-
>>>>  3 files changed, 366 insertions(+), 44 deletions(-)
>>>>
>>>
>>>
>>
>> .
>>
> 
> 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/13 20:55, Yijing Wang wrote:
> On 2014/11/13 20:39, Jiang Liu wrote:
>>
>>
>> On 2014/11/13 20:28, Yijing Wang wrote:
>>> On 2014/11/13 19:43, Jiang Liu wrote:
>>>> This patch set is based on tip/irq/irqdomain and tries to refine
>>>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>>>
>>>> Patch 1 is just minor fixes for tip/irq/irqdomain.
>>>>
>>>> Patch 2 introduces some helpers to hide struct msi_desc implementation
>>>> details, so later we could move msi_list from struct pci_dev into
>>>> struct device to enable generic MSI support.
>>>
>>> Hi Gerry,
>>>    I tried to move msi info(eg. msi_list) into struct device, but I found
>>> DMAR fault interrupt is initialized before the driver core init. And I don't
>>> know whether there are other devices like ARM consolidator(introduced in GIC v3)
>>> need to be initialized before driver core. What do you think about this ?
>> Hi Yijing,
>> 	DMAR interrupt doesn't make use of msi_desc, so has no
>> dependency on msi_list.
> 
> OK, I thought we could use msi_desc to describe DMAR/HPET irq, so they could
> share the mask/unmask, write_msg/read_etc.. But maybe it's not a right direction. :)
And DMAR/HPET interrupts are not associated with any struct device
object, no way to build msi_desc list at all:)

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Marc Zyngier]

On 13/11/14 11:43, Jiang Liu wrote:
> This patch set is based on tip/irq/irqdomain and tries to refine
> interfaces to support irqdomain for generic MSI and PCI MSI.
> 
> Patch 1 is just minor fixes for tip/irq/irqdomain.
> 
> Patch 2 introduces some helpers to hide struct msi_desc implementation
> details, so later we could move msi_list from struct pci_dev into
> struct device to enable generic MSI support.
> 
> Patch 3 introduces msi_domain_{alloc|free}_irqs() which generalize
> pci_msi_domain_alloc_irqs() to support generic MSI.
> 
> Patch 4 introduces default data structures and callback implementations
> to support msi_domain_alloc_irqs(), so reduce burden on generic MSI
> users.
> 
> Patch 5 converts PCI MSI to use generic MSI interfaces, and also
> implement default callbacks for PCI MSI.
> 
> Patch 6 introduces a mechanism to replace arch_setup_msi_irq()/
> arch_setup_msi_irqs()/arch_teardown_msi_irq()/arch_teardown_msi_irqs().
> 
> With this patch set applied, the generic MSI and PCI MSI interfaces
> are much easier to use. For extreme case, you only need to define
> a "struct msi_domain_info" and don't need to implement any callbacks,
> just using the default callbacks is OK:)
> 
> This patch set is also a preparation for:
> 1) Kill all weak functions in drivers/pci/msi.c
> 2) Implement support for non-PCI-compliant MSI device

I've rebased (once more!) the GICv3 ITS driver on top of this, and this
is definitely a major improvement. This is basically the first version
I can use without having to hack into the core code (apart from the
couple of nits I've mentioned earlier).

Now, Thomas' idea of putting the irq_domain close to the bus is very
appealing, and I've tweaked an earlier patch in order to do this:

>From f73c2df2f1e66fd13902b2c6bb5773df6538b7df Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Wed, 12 Nov 2014 10:32:46 +0000
Subject: [PATCH] PCI/MSI: Allow an msi_chip to be associated to an irq domain

With the new stacked irq domains, it becomes pretty tempting
to allocate an MSI domain per PCI bus, which would remove
the requirement of either relying on arch-specific code, or
a default PCI MSI domain.

By allowing the msi_chip structure to carry a pointer to
an irq_domain, we can easily use this in pci_msi_setup_msi_irqs.
The existing code can still be used as a fallback if the MSI driver
does not populate the domain field.

Tested on arm64 with the GICv3 ITS driver.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/msi.c   | 9 +++++++--
 include/linux/msi.h | 3 +++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 9947fb4..1752537 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -86,9 +86,14 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 
 static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 {
-	struct irq_domain *domain;
+	struct irq_domain *domain = NULL;
 
-	domain = arch_get_pci_msi_domain(dev);
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	if (dev->bus->msi)
+		domain = dev->bus->msi->domain;
+#endif
+	if (!domain)
+		domain = arch_get_pci_msi_domain(dev);
 	if (domain)
 		return pci_msi_domain_alloc_irqs(domain, type, dev);
 
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 9b2e73e..2325950 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -89,6 +89,9 @@ struct msi_chip {
 	struct device *dev;
 	struct device_node *of_node;
 	struct list_head list;
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	struct irq_domain *domain;
+#endif
 
 	int (*setup_irq)(struct msi_chip *chip, struct pci_dev *dev,
 			 struct msi_desc *desc);
-- 
2.0.4

Thoughts?

Thanks,

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

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Thomas Gleixner]

On Thu, 13 Nov 2014, Marc Zyngier wrote:
> With the new stacked irq domains, it becomes pretty tempting
> to allocate an MSI domain per PCI bus, which would remove
> the requirement of either relying on arch-specific code, or
> a default PCI MSI domain.

Right. That's what I roughly had in mind. And that would solve the
multi-iommu issue on x86 nicely as well. We establish the association
at the time where the bus gets populated. So the whole lookup magic
simply goes away.

Thanks,

	tglx

 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Marc Zyngier]

On 13/11/14 21:11, Thomas Gleixner wrote:
> On Thu, 13 Nov 2014, Marc Zyngier wrote:
>> With the new stacked irq domains, it becomes pretty tempting
>> to allocate an MSI domain per PCI bus, which would remove
>> the requirement of either relying on arch-specific code, or
>> a default PCI MSI domain.
> 
> Right. That's what I roughly had in mind. And that would solve the
> multi-iommu issue on x86 nicely as well. We establish the association
> at the time where the bus gets populated. So the whole lookup magic
> simply goes away.

Great. I've pushed the whole thing out with this patch, the couple
of fixes I mentioned earlier, as well as the whole ITS code:

git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/stacked-its-v2

Time to go home.

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

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/14 5:28, Marc Zyngier wrote:
> On 13/11/14 21:11, Thomas Gleixner wrote:
>> On Thu, 13 Nov 2014, Marc Zyngier wrote:
>>> With the new stacked irq domains, it becomes pretty tempting
>>> to allocate an MSI domain per PCI bus, which would remove
>>> the requirement of either relying on arch-specific code, or
>>> a default PCI MSI domain.
>>
>> Right. That's what I roughly had in mind. And that would solve the
>> multi-iommu issue on x86 nicely as well. We establish the association
>> at the time where the bus gets populated. So the whole lookup magic
>> simply goes away.
> 
> Great. I've pushed the whole thing out with this patch, the couple
> of fixes I mentioned earlier, as well as the whole ITS code:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/stacked-its-v2
Hi Marc,
	I have looked at the code, and found some issues.
1) With my next version, no need to implemeent its_pci_msi_free()
anymore. msi_domain_free_irqs() will reset desc->irq to zero
for all architectures.

2) This piece of code in its_msi_prepare() may run into trouble
for PCI device with both MSI and MSIX capability. I will change
msi_prepare() prototype to pass in the "nvec" parameter. And you
may access first_pci_msi_entry()->msi_attrib.is_msix to get
allocation type if needed.
	nvec = pci_msix_vec_count(pdev);
	if (nvec < 0)
		nvec = pci_msi_vec_count(pdev);
	if (nvec < 0)
		return nvec;

3) Do we need to increase the default of NUM_MSI_ALLOC_SCRATCHPAD_REGS
to 4? 2 is a little too limited.

Regards!
Gerry

> 
> Time to go home.
> 
> 	M.
> 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Marc Zyngier]

On 14/11/14 15:54, Jiang Liu wrote:
> On 2014/11/14 5:28, Marc Zyngier wrote:
>> On 13/11/14 21:11, Thomas Gleixner wrote:
>>> On Thu, 13 Nov 2014, Marc Zyngier wrote:
>>>> With the new stacked irq domains, it becomes pretty tempting
>>>> to allocate an MSI domain per PCI bus, which would remove
>>>> the requirement of either relying on arch-specific code, or
>>>> a default PCI MSI domain.
>>>
>>> Right. That's what I roughly had in mind. And that would solve the
>>> multi-iommu issue on x86 nicely as well. We establish the association
>>> at the time where the bus gets populated. So the whole lookup magic
>>> simply goes away.
>>
>> Great. I've pushed the whole thing out with this patch, the couple
>> of fixes I mentioned earlier, as well as the whole ITS code:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/stacked-its-v2
> Hi Marc,
> 	I have looked at the code, and found some issues.
> 1) With my next version, no need to implemeent its_pci_msi_free()
> anymore. msi_domain_free_irqs() will reset desc->irq to zero
> for all architectures.

Great. The less code, the better.

> 2) This piece of code in its_msi_prepare() may run into trouble
> for PCI device with both MSI and MSIX capability. I will change
> msi_prepare() prototype to pass in the "nvec" parameter. And you
> may access first_pci_msi_entry()->msi_attrib.is_msix to get
> allocation type if needed.
> 	nvec = pci_msix_vec_count(pdev);
> 	if (nvec < 0)
> 		nvec = pci_msi_vec_count(pdev);
> 	if (nvec < 0)
> 		return nvec;

Ah, good point. Having nvec will make things simpler indeed. I have the
feeling that the ITS driver may not be the only one to require such a thing.

> 3) Do we need to increase the default of NUM_MSI_ALLOC_SCRATCHPAD_REGS
> to 4? 2 is a little too limited.

I'd say that 2 is enough for now. The ITS only uses one (the second one
is for debug only). Should the need arise, we can bump it up later.

Thanks,

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

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/14 5:00, Marc Zyngier wrote:
> On 13/11/14 11:43, Jiang Liu wrote:
>> This patch set is based on tip/irq/irqdomain and tries to refine
>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>
>> With this patch set applied, the generic MSI and PCI MSI interfaces
>> are much easier to use. For extreme case, you only need to define
>> a "struct msi_domain_info" and don't need to implement any callbacks,
>> just using the default callbacks is OK:)
>>
>> This patch set is also a preparation for:
>> 1) Kill all weak functions in drivers/pci/msi.c
>> 2) Implement support for non-PCI-compliant MSI device
> 
> I've rebased (once more!) the GICv3 ITS driver on top of this, and this
> is definitely a major improvement. This is basically the first version
> I can use without having to hack into the core code (apart from the
> couple of nits I've mentioned earlier).
Sorry for the rebasing, but I hope it worthy rebasing:)

> 
> Now, Thomas' idea of putting the irq_domain close to the bus is very
> appealing, and I've tweaked an earlier patch in order to do this:
I feel that's the right direction. There are other threads discussing
associating an MSI controller structure with each PCI bus (at least
root bus).
http://www.spinics.net/lists/arm-kernel/msg376328.html
Regards!
Gerry

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Yijing Wang]

On 2014/11/14 8:25, Jiang Liu wrote:
> On 2014/11/14 5:00, Marc Zyngier wrote:
>> On 13/11/14 11:43, Jiang Liu wrote:
>>> This patch set is based on tip/irq/irqdomain and tries to refine
>>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>>
>>> With this patch set applied, the generic MSI and PCI MSI interfaces
>>> are much easier to use. For extreme case, you only need to define
>>> a "struct msi_domain_info" and don't need to implement any callbacks,
>>> just using the default callbacks is OK:)
>>>
>>> This patch set is also a preparation for:
>>> 1) Kill all weak functions in drivers/pci/msi.c
>>> 2) Implement support for non-PCI-compliant MSI device
>>
>> I've rebased (once more!) the GICv3 ITS driver on top of this, and this
>> is definitely a major improvement. This is basically the first version
>> I can use without having to hack into the core code (apart from the
>> couple of nits I've mentioned earlier).
> Sorry for the rebasing, but I hope it worthy rebasing:)
> 
>>
>> Now, Thomas' idea of putting the irq_domain close to the bus is very
>> appealing, and I've tweaked an earlier patch in order to do this:
> I feel that's the right direction. There are other threads discussing
> associating an MSI controller structure with each PCI bus (at least
> root bus).
> http://www.spinics.net/lists/arm-kernel/msg376328.html

Associate the irq domain and PCI bus is not necessary, because all PCI buses under same host bridge
always share same MSI chip/irq domain, we only need associate them and pci host bridge.
I'm refactoring the pci_host_bridge, make it be a generic one, rip out of the pci root bus
creation, so we could put the irq domain and pci domain etc.. in it. Finally, we could
eliminate lots platform arch functions. I will post it out within one week.

Thanks!
Yijing.

> Regards!
> Gerry
> 
> .
> 


-- 
Thanks!
Yijing

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/14 9:09, Yijing Wang wrote:
> On 2014/11/14 8:25, Jiang Liu wrote:
>> On 2014/11/14 5:00, Marc Zyngier wrote:
>>> On 13/11/14 11:43, Jiang Liu wrote:
>>>> This patch set is based on tip/irq/irqdomain and tries to refine
>>>> interfaces to support irqdomain for generic MSI and PCI MSI.
>>>>
>>>> With this patch set applied, the generic MSI and PCI MSI interfaces
>>>> are much easier to use. For extreme case, you only need to define
>>>> a "struct msi_domain_info" and don't need to implement any callbacks,
>>>> just using the default callbacks is OK:)
>>>>
>>>> This patch set is also a preparation for:
>>>> 1) Kill all weak functions in drivers/pci/msi.c
>>>> 2) Implement support for non-PCI-compliant MSI device
>>>
>>> I've rebased (once more!) the GICv3 ITS driver on top of this, and this
>>> is definitely a major improvement. This is basically the first version
>>> I can use without having to hack into the core code (apart from the
>>> couple of nits I've mentioned earlier).
>> Sorry for the rebasing, but I hope it worthy rebasing:)
>>
>>>
>>> Now, Thomas' idea of putting the irq_domain close to the bus is very
>>> appealing, and I've tweaked an earlier patch in order to do this:
>> I feel that's the right direction. There are other threads discussing
>> associating an MSI controller structure with each PCI bus (at least
>> root bus).
>> http://www.spinics.net/lists/arm-kernel/msg376328.html
> 
> Associate the irq domain and PCI bus is not necessary, because all PCI buses under same host bridge
> always share same MSI chip/irq domain, we only need associate them and pci host bridge.
> I'm refactoring the pci_host_bridge, make it be a generic one, rip out of the pci root bus
> creation, so we could put the irq domain and pci domain etc.. in it. Finally, we could
> eliminate lots platform arch functions. I will post it out within one week.
Hi Yijing,
	Theoretically, it's not true on x86 when interrupt remapping
is enabled. There may be multiple IOMMU(interrupt remapping) units
serving the same host bridge, so we need different MSI domains
to serve different PCI devices.
Regards!
Gerry

> 
> Thanks!
> Yijing.
> 
>> Regards!
>> Gerry
>>
>> .
>>
> 
> 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Thomas Gleixner]

On Fri, 14 Nov 2014, Yijing Wang wrote:

Could you please use a mail client which does proper line wraps or
configure yours to do so?

> Associate the irq domain and PCI bus is not necessary, because all
> PCI buses under same host bridge always share same MSI chip/irq
> domain, we only need associate them and pci host bridge.
>
> I'm refactoring the pci_host_bridge, make it be a generic one, rip
> out of the pci root bus creation, so we could put the irq domain and
> pci domain etc.. in it. Finally, we could eliminate lots platform
> arch functions. I will post it out within one week.

That's a completely orthogonal problem. From the MSI/interrupt
handling POV it does not matter at all where that information is
stored. All we care about is that it is retrievable via the (pci)
device which tries to setup MSI[X].

So we can store/retrieve it via generic functions into/from whatever
is available right now. If the irq side has generic interfaces to do
so then this wont conflict with your decisions to change the final
storage point because all it takes is to tweak the storage/retrieve
functions.

So all we need at the moment is an agreed on way to store/retrieve
that information which is based on the current shared infrastructure,
aka. Linus tree. If we can utilize that you are completely free to
change the association mechanism underneath.

Thanks,

	tglx

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/14 9:31, Thomas Gleixner wrote:
> On Fri, 14 Nov 2014, Yijing Wang wrote:
> 
> Could you please use a mail client which does proper line wraps or
> configure yours to do so?
> 
>> Associate the irq domain and PCI bus is not necessary, because all
>> PCI buses under same host bridge always share same MSI chip/irq
>> domain, we only need associate them and pci host bridge.
>>
>> I'm refactoring the pci_host_bridge, make it be a generic one, rip
>> out of the pci root bus creation, so we could put the irq domain and
>> pci domain etc.. in it. Finally, we could eliminate lots platform
>> arch functions. I will post it out within one week.
> 
> That's a completely orthogonal problem. From the MSI/interrupt
> handling POV it does not matter at all where that information is
> stored. All we care about is that it is retrievable via the (pci)
> device which tries to setup MSI[X].
> 
> So we can store/retrieve it via generic functions into/from whatever
> is available right now. If the irq side has generic interfaces to do
> so then this wont conflict with your decisions to change the final
> storage point because all it takes is to tweak the storage/retrieve
> functions.
> 
> So all we need at the moment is an agreed on way to store/retrieve
> that information which is based on the current shared infrastructure,
> aka. Linus tree. If we can utilize that you are completely free to
> change the association mechanism underneath.
Hi Thomas,
	So we need something like:
struct msi_chip *pci_get_msi_chip(struct pci_dev *);
or:
struct irq_domain *pci_get_msi_domain(struct pci_dev *);

BTW, there's a conflict when merging tip/irq/irqdomain into
tip/x86/apic. It's my first time to deal with merging conflicts,
what's the preferred way? Is it working like this?
1) I merge the two branch
2) I rebase my x86 irqdomain patch sets and send them to you
3) You merge the two branch and apply my patch set.
Regards!
Gerry
> 
> Thanks,
> 
> 	tglx
> 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Thomas Gleixner]

On Fri, 14 Nov 2014, Jiang Liu wrote:
> On 2014/11/14 9:31, Thomas Gleixner wrote:
> > On Fri, 14 Nov 2014, Yijing Wang wrote:
> > 
> > Could you please use a mail client which does proper line wraps or
> > configure yours to do so?
> > 
> >> Associate the irq domain and PCI bus is not necessary, because all
> >> PCI buses under same host bridge always share same MSI chip/irq
> >> domain, we only need associate them and pci host bridge.
> >>
> >> I'm refactoring the pci_host_bridge, make it be a generic one, rip
> >> out of the pci root bus creation, so we could put the irq domain and
> >> pci domain etc.. in it. Finally, we could eliminate lots platform
> >> arch functions. I will post it out within one week.
> > 
> > That's a completely orthogonal problem. From the MSI/interrupt
> > handling POV it does not matter at all where that information is
> > stored. All we care about is that it is retrievable via the (pci)
> > device which tries to setup MSI[X].
> > 
> > So we can store/retrieve it via generic functions into/from whatever
> > is available right now. If the irq side has generic interfaces to do
> > so then this wont conflict with your decisions to change the final
> > storage point because all it takes is to tweak the storage/retrieve
> > functions.
> > 
> > So all we need at the moment is an agreed on way to store/retrieve
> > that information which is based on the current shared infrastructure,
> > aka. Linus tree. If we can utilize that you are completely free to
> > change the association mechanism underneath.
> Hi Thomas,
> 	So we need something like:
> struct msi_chip *pci_get_msi_chip(struct pci_dev *);
> or:
> struct irq_domain *pci_get_msi_domain(struct pci_dev *);
> 
> BTW, there's a conflict when merging tip/irq/irqdomain into
> tip/x86/apic. It's my first time to deal with merging conflicts,
> what's the preferred way? Is it working like this?
> 1) I merge the two branch
> 2) I rebase my x86 irqdomain patch sets and send them to you
> 3) You merge the two branch and apply my patch set.

When we have the generic parts sorted out, i'll make the irq/irqdomain
branch official and immutable and then merge it into x86/apic fix the
conflicts and add the x86 specific stuff on top.

Thanks,

	tglx

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Yijing Wang]

在 2014/11/14 9:39, Jiang Liu 写道:
> On 2014/11/14 9:31, Thomas Gleixner wrote:
>> On Fri, 14 Nov 2014, Yijing Wang wrote:
>>
>> Could you please use a mail client which does proper line wraps or
>> configure yours to do so?
>>
>>> Associate the irq domain and PCI bus is not necessary, because all
>>> PCI buses under same host bridge always share same MSI chip/irq
>>> domain, we only need associate them and pci host bridge.
>>>
>>> I'm refactoring the pci_host_bridge, make it be a generic one, rip
>>> out of the pci root bus creation, so we could put the irq domain and
>>> pci domain etc.. in it. Finally, we could eliminate lots platform
>>> arch functions. I will post it out within one week.
>> That's a completely orthogonal problem. From the MSI/interrupt
>> handling POV it does not matter at all where that information is
>> stored. All we care about is that it is retrievable via the (pci)
>> device which tries to setup MSI[X].
>>
>> So we can store/retrieve it via generic functions into/from whatever
>> is available right now. If the irq side has generic interfaces to do
>> so then this wont conflict with your decisions to change the final
>> storage point because all it takes is to tweak the storage/retrieve
>> functions.
>>
>> So all we need at the moment is an agreed on way to store/retrieve
>> that information which is based on the current shared infrastructure,
>> aka. Linus tree. If we can utilize that you are completely free to
>> change the association mechanism underneath.
> Hi Thomas,
> 	So we need something like:
> struct msi_chip *pci_get_msi_chip(struct pci_dev *);
> or:
> struct irq_domain *pci_get_msi_domain(struct pci_dev *);

Hi Gerry,
what about associate the platform specific  struct msi_chip 
*pci_get_msi_chip(struct pci_dev *)
with struct pci_host_bridge. we could provide the private 
"pci_get_msi_chip()" in the PCI
host drivers.

Thanks!
Yijing.

>
> BTW, there's a conflict when merging tip/irq/irqdomain into
> tip/x86/apic. It's my first time to deal with merging conflicts,
> what's the preferred way? Is it working like this?
> 1) I merge the two branch
> 2) I rebase my x86 irqdomain patch sets and send them to you
> 3) You merge the two branch and apply my patch set.
> Regards!
> Gerry
>> Thanks,
>>
>> 	tglx
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/14 22:11, Yijing Wang wrote:
> 
> 在 2014/11/14 9:39, Jiang Liu 写道:
>> On 2014/11/14 9:31, Thomas Gleixner wrote:
>>> On Fri, 14 Nov 2014, Yijing Wang wrote:
>>>
>> Hi Thomas,
>>     So we need something like:
>> struct msi_chip *pci_get_msi_chip(struct pci_dev *);
>> or:
>> struct irq_domain *pci_get_msi_domain(struct pci_dev *);
> 
> Hi Gerry,
> what about associate the platform specific  struct msi_chip
> *pci_get_msi_chip(struct pci_dev *)
> with struct pci_host_bridge. we could provide the private
> "pci_get_msi_chip()" in the PCI
> host drivers.
Hi Yijing,
	Still need some time to dig into msi_chip related code.
When refining the PCI MSI code, I feel the best way is:
1) Every PCI device is associated with an PCI MSI irqdomain.
2) PCI MSI core directly talks to irqdomain to allocate/free
   interrupts.
3) Kill all weak functions in pci/drivers/msi.c.
4) Kill struct msi_chip.

We have achieved 1 and 2. And seems we could also achieve 3 by
converting all arch specific PCI MSI code to use hierarchy
irqdomain. But not sure whether we could achieve 4, not familiar
with ARM world:)

On x86, we could kill all PCI MSI weak function after converting
Xen to irqdomain. I think we may have a prototype for x86 in next
week.
Thanks!
Gerry

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Marc Zyngier]

On 14/11/14 14:26, Jiang Liu wrote:
> On 2014/11/14 22:11, Yijing Wang wrote:
>>
>> 在 2014/11/14 9:39, Jiang Liu 写道:
>>> On 2014/11/14 9:31, Thomas Gleixner wrote:
>>>> On Fri, 14 Nov 2014, Yijing Wang wrote:
>>>>
>>> Hi Thomas,
>>>     So we need something like:
>>> struct msi_chip *pci_get_msi_chip(struct pci_dev *);
>>> or:
>>> struct irq_domain *pci_get_msi_domain(struct pci_dev *);
>>
>> Hi Gerry,
>> what about associate the platform specific  struct msi_chip
>> *pci_get_msi_chip(struct pci_dev *)
>> with struct pci_host_bridge. we could provide the private
>> "pci_get_msi_chip()" in the PCI
>> host drivers.
> Hi Yijing,
> 	Still need some time to dig into msi_chip related code.
> When refining the PCI MSI code, I feel the best way is:
> 1) Every PCI device is associated with an PCI MSI irqdomain.
> 2) PCI MSI core directly talks to irqdomain to allocate/free
>    interrupts.
> 3) Kill all weak functions in pci/drivers/msi.c.
> 4) Kill struct msi_chip.
> 
> We have achieved 1 and 2. And seems we could also achieve 3 by
> converting all arch specific PCI MSI code to use hierarchy
> irqdomain. But not sure whether we could achieve 4, not familiar
> with ARM world:)

Killing all the weak functions shouldn't be a problem for ARM, we're
trying very hard not to rely on them.

Killing msi_chip is a different story, as this is what we use to match a
PCI host controller with its MSI controller (that's what the of_node
field in msi_chip is for). See drivers/of/of_pci.c for details.

Also, we use msi_chip directly in the MSI drivers as a way to go from a
pci_dev to the MSI controller specific structure:

http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143

If we're going to kill msi_chip, we must make sure we have mechanisms
that allow the conversion of the existing code.

Thanks,

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

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Jiang Liu]

On 2014/11/14 23:16, Marc Zyngier wrote:
> On 14/11/14 14:26, Jiang Liu wrote:
>> On 2014/11/14 22:11, Yijing Wang wrote:
>>>
>> We have achieved 1 and 2. And seems we could also achieve 3 by
>> converting all arch specific PCI MSI code to use hierarchy
>> irqdomain. But not sure whether we could achieve 4, not familiar
>> with ARM world:)
> 
> Killing all the weak functions shouldn't be a problem for ARM, we're
> trying very hard not to rely on them.
> 
> Killing msi_chip is a different story, as this is what we use to match a
> PCI host controller with its MSI controller (that's what the of_node
> field in msi_chip is for). See drivers/of/of_pci.c for details.
> 
> Also, we use msi_chip directly in the MSI drivers as a way to go from a
> pci_dev to the MSI controller specific structure:
> 
> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143
> 
> If we're going to kill msi_chip, we must make sure we have mechanisms
> that allow the conversion of the existing code.
Hi Marc,
	Seem it doesn't worth the effort to remove irq_chip,
so I will focus on killing all weak functions first:)
After that, we may kill setup_irq and teardown_irq in irq_chip,
but seems irq_chip still has other usages, so I won't try to
kill irq_chip.
Regards!
Gerry
> 
> Thanks,
> 
> 	M.
> 

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Marc Zyngier]

On 14/11/14 15:25, Jiang Liu wrote:
> On 2014/11/14 23:16, Marc Zyngier wrote:
>> On 14/11/14 14:26, Jiang Liu wrote:
>>> On 2014/11/14 22:11, Yijing Wang wrote:
>>>>
>>> We have achieved 1 and 2. And seems we could also achieve 3 by
>>> converting all arch specific PCI MSI code to use hierarchy
>>> irqdomain. But not sure whether we could achieve 4, not familiar
>>> with ARM world:)
>>
>> Killing all the weak functions shouldn't be a problem for ARM, we're
>> trying very hard not to rely on them.
>>
>> Killing msi_chip is a different story, as this is what we use to match a
>> PCI host controller with its MSI controller (that's what the of_node
>> field in msi_chip is for). See drivers/of/of_pci.c for details.
>>
>> Also, we use msi_chip directly in the MSI drivers as a way to go from a
>> pci_dev to the MSI controller specific structure:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143
>>
>> If we're going to kill msi_chip, we must make sure we have mechanisms
>> that allow the conversion of the existing code.
> Hi Marc,
> 	Seem it doesn't worth the effort to remove irq_chip,
> so I will focus on killing all weak functions first:)
> After that, we may kill setup_irq and teardown_irq in irq_chip,
> but seems irq_chip still has other usages, so I won't try to
> kill irq_chip.

I assume you mean msi_chip! ;-)

Killing setup/teardown_irq is going to require some work. There are a
few users in the tree (I can see three in drivers/pci/host and one in
drivers/irqchip).

Hopefully, we can get the maintainers to convert them.

Thanks,

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

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Lucas Stach]

Am Freitag, den 14.11.2014, 16:03 +0000 schrieb Marc Zyngier:
> On 14/11/14 15:25, Jiang Liu wrote:
> > On 2014/11/14 23:16, Marc Zyngier wrote:
> >> On 14/11/14 14:26, Jiang Liu wrote:
> >>> On 2014/11/14 22:11, Yijing Wang wrote:
> >>>>
> >>> We have achieved 1 and 2. And seems we could also achieve 3 by
> >>> converting all arch specific PCI MSI code to use hierarchy
> >>> irqdomain. But not sure whether we could achieve 4, not familiar
> >>> with ARM world:)
> >>
> >> Killing all the weak functions shouldn't be a problem for ARM, we're
> >> trying very hard not to rely on them.
> >>
> >> Killing msi_chip is a different story, as this is what we use to match a
> >> PCI host controller with its MSI controller (that's what the of_node
> >> field in msi_chip is for). See drivers/of/of_pci.c for details.
> >>
> >> Also, we use msi_chip directly in the MSI drivers as a way to go from a
> >> pci_dev to the MSI controller specific structure:
> >>
> >> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/tree/drivers/irqchip/irq-gic-v3-its.c?h=irq/stacked-its-v2#n1143
> >>
> >> If we're going to kill msi_chip, we must make sure we have mechanisms
> >> that allow the conversion of the existing code.
> > Hi Marc,
> > 	Seem it doesn't worth the effort to remove irq_chip,
> > so I will focus on killing all weak functions first:)
> > After that, we may kill setup_irq and teardown_irq in irq_chip,
> > but seems irq_chip still has other usages, so I won't try to
> > kill irq_chip.
> 
> I assume you mean msi_chip! ;-)
> 
> Killing setup/teardown_irq is going to require some work. There are a
> few users in the tree (I can see three in drivers/pci/host and one in
> drivers/irqchip).
> 
> Hopefully, we can get the maintainers to convert them.
> 

I can help with both the Designware and Tegra host bridge drivers.

But that requires:
1. CCing patches to me, so I know what is going on
2. Providing good descriptions on what is done in the patchset and how
the msi_chip users should adjust

With all the MSI changes by various people going on in parallel it's
hard to follow for someone not looking at this stuff every day.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [Patch V1 0/6] Refine generic/PCI MSI irqodmian interfaces

[Yijing Wang]

On 2014/11/14 9:31, Thomas Gleixner wrote:
> On Fri, 14 Nov 2014, Yijing Wang wrote:
> 
> Could you please use a mail client which does proper line wraps or
> configure yours to do so?
> 
>> Associate the irq domain and PCI bus is not necessary, because all
>> PCI buses under same host bridge always share same MSI chip/irq
>> domain, we only need associate them and pci host bridge.
>>
>> I'm refactoring the pci_host_bridge, make it be a generic one, rip
>> out of the pci root bus creation, so we could put the irq domain and
>> pci domain etc.. in it. Finally, we could eliminate lots platform
>> arch functions. I will post it out within one week.
> 
> That's a completely orthogonal problem. From the MSI/interrupt
> handling POV it does not matter at all where that information is
> stored. All we care about is that it is retrievable via the (pci)
> device which tries to setup MSI[X].
> 
> So we can store/retrieve it via generic functions into/from whatever
> is available right now. If the irq side has generic interfaces to do
> so then this wont conflict with your decisions to change the final
> storage point because all it takes is to tweak the storage/retrieve
> functions.
> 
> So all we need at the moment is an agreed on way to store/retrieve
> that information which is based on the current shared infrastructure,
> aka. Linus tree. If we can utilize that you are completely free to
> change the association mechanism underneath.

Hi Thomas, thanks for your explanation. Now I got it,
I will consider more about it.

Thanks!
Yijing.


> 
> Thanks,
> 
> 	tglx
> 
> .
> 


-- 
Thanks!
Yijing