x86, MSI, AHCI: Support multiple MSIs

The MSI specification has several constraints in comparison with MSI-X, most notable of them is the inability to configure MSIs independently. As a result, it is impossible to dispatch interrupts from different queues to different CPUs. This is largely devalues the support of multiple MSIs in SMP systems.

Also, a necessity to allocate a contiguous block of vector numbers for devices capable of multiple MSIs might cause a considerable pressure on x86 interrupt vector allocator and could lead to fragmentation of the interrupt vectors space.

This patch overcomes both drawbacks in presense of IRQ remapping and lets devices take advantage of multiple queues and per-IRQ affinity assignments.

Signed-off-by Alexander Gordeev <[email protected]>

---
 arch/x86/kernel/apic/io_apic.c |  174 +++++++++++++++++++++++++++++++++------
 include/linux/irq.h            |    6 ++
 kernel/irq/chip.c              |   30 +++++--
 kernel/irq/irqdesc.c           |   31 +++++++
 4 files changed, 206 insertions(+), 35 deletions(-)

arch/x86/kernel/apic/io_apic.c +44 -18

index c265593..d5cb13c 100644
@@ -305,6 +305,11 @@ static int alloc_irq_from(unsigned int from, int node)
 	return irq_alloc_desc_from(from, node);
 }
 
+static int alloc_irqs_from(unsigned int from, unsigned int count, int node)
+{
+	return irq_alloc_descs_from(from, count, node);
+}
+
 static void free_irq_at(unsigned int at, struct irq_cfg *cfg)
 {
 	free_irq_cfg(at, cfg);

@@ -2991,37 +2996,58 @@ device_initcall(ioapic_init_ops);
 /*
  * Dynamic irq allocate and deallocation
  */
-unsigned int create_irq_nr(unsigned int from, int node)
+unsigned int __create_irqs(unsigned int from, unsigned int count, int node)
 {
-	struct irq_cfg *cfg;
+	struct irq_cfg **cfg;
 	unsigned long flags;
-	unsigned int ret = 0;
-	int irq;
+	int irq, i;
 
 	if (from < nr_irqs_gsi)
 		from = nr_irqs_gsi;
 
-	irq = alloc_irq_from(from, node);
-	if (irq < 0)
-		return 0;
-	cfg = alloc_irq_cfg(irq, node);
-	if (!cfg) {
-		free_irq_at(irq, NULL);
+	cfg = kzalloc_node(count * sizeof(cfg[0]), GFP_KERNEL, node);
+	if (!cfg)
 		return 0;
+
+	irq = alloc_irqs_from(from, count, node);
+	if (irq < 0)
+		goto out_cfgs;
+
+	for (i = 0; i < count; i++) {
+		cfg[i] = alloc_irq_cfg(irq + i, node);
+		if (!cfg[i])
+			goto out_irqs;
 	}
 
 	raw_spin_lock_irqsave(&vector_lock, flags);
-	if (!__assign_irq_vector(irq, cfg, apic->target_cpus()))
-		ret = irq;
+	for (i = 0; i < count; i++)
+		if (__assign_irq_vector(irq + i, cfg[i], apic->target_cpus()))
+			goto out_vecs;
 	raw_spin_unlock_irqrestore(&vector_lock, flags);
 
-	if (ret) {
-		irq_set_chip_data(irq, cfg);
-		irq_clear_status_flags(irq, IRQ_NOREQUEST);
-	} else {
-		free_irq_at(irq, cfg);
+	for (i = 0; i < count; i++) {
+		irq_set_chip_data(irq + i, cfg[i]);
+		irq_clear_status_flags(irq + i, IRQ_NOREQUEST);
 	}
-	return ret;
+
+	kfree(cfg);
+	return irq;
+
+out_vecs:
+	for (; i; i--)
+		__clear_irq_vector(irq + i - 1, cfg[i - 1]);
+	raw_spin_unlock_irqrestore(&vector_lock, flags);
+out_irqs:
+	for (i = 0; i < count; i++)
+		free_irq_at(irq + i, cfg[i]);
+out_cfgs:
+	kfree(cfg);
+	return 0;
+}
+
+unsigned int create_irq_nr(unsigned int from, int node)
+{
+	return __create_irqs(from, 1, node);
 }
 
 int create_irq(void)

arch/x86/kernel/apic/io_apic.c +20 -1

+		destroy_irq(irq + i);
+}
+
+static inline int
+can_create_pow_of_two_irqs(unsigned int from, unsigned int count)
+{
+	if ((count > 1) && (count % 2))
+		return -EINVAL;
+
+	for (; count; count = count / 2) {
+		if (!irq_can_alloc_irqs(from, count))
+			return count;
+	}
+
+	return -ENOSPC;
+}
+
 /*
  * MSI message composition
  */

@@ -3145,18 +3192,25 @@ static struct irq_chip msi_chip = {
 	.irq_retrigger		= ioapic_retrigger_irq,
 };
 
-static int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int irq)
+static int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
+			 unsigned int irq_base, unsigned int irq_offset)
 {
 	struct irq_chip *chip = &msi_chip;
 	struct msi_msg msg;
+	unsigned int irq = irq_base + irq_offset;
 	int ret;
 
 	ret = msi_compose_msg(dev, irq, &msg, -1);

arch/x86/kernel/apic/io_apic.c +51 -6

+	if (!irq_offset)
+		write_msi_msg(irq, &msg);
 
 	if (irq_remapped(irq_get_chip_data(irq))) {
 		irq_set_status_flags(irq, IRQ_MOVE_PCNTXT);

@@ -3170,16 +3224,12 @@ static int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int irq)
 	return 0;
 }
 
-int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+int setup_msix_irqs(struct pci_dev *dev, int nvec)
 {
 	int node, ret, sub_handle, index = 0;
 	unsigned int irq, irq_want;
 	struct msi_desc *msidesc;
 
-	/* x86 doesn't support multiple MSI yet */
-	if (type == PCI_CAP_ID_MSI && nvec > 1)
-		return 1;
-
 	node = dev_to_node(&dev->dev);
 	irq_want = nr_irqs_gsi;
 	sub_handle = 0;

@@ -3208,7 +3258,7 @@ int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 				goto error;
 		}
 no_ir:
-		ret = setup_msi_irq(dev, msidesc, irq);
+		ret = setup_msi_irq(dev, msidesc, irq, 0);
 		if (ret < 0)
 			goto error;
 		sub_handle++;

@@ -3220,6 +3270,76 @@ error:
 	return ret;
 }
 
+int setup_msi_irqs(struct pci_dev *dev, int nvec)
+{
+	int node, ret, sub_handle, index = 0;
+	unsigned int irq;
+	struct msi_desc *msidesc;
+
+	if (nvec > 1 && !irq_remapping_enabled)
+		return 1;
+
+	nvec = __roundup_pow_of_two(nvec);
+	ret = can_create_pow_of_two_irqs(nr_irqs_gsi, nvec);
+	if (ret != nvec)
+		return ret;
+
+	WARN_ON(!list_is_singular(&dev->msi_list));
+	msidesc = list_entry(dev->msi_list.next, struct msi_desc, list);
+	WARN_ON(msidesc->irq);
+	WARN_ON(msidesc->msi_attrib.multiple);
+
+	node = dev_to_node(&dev->dev);
+	irq = __create_irqs(nr_irqs_gsi, nvec, node);
+	if (irq == 0)
+		return -ENOSPC;
+
+	if (!irq_remapping_enabled) {
+		ret = setup_msi_irq(dev, msidesc, irq, 0);
+		if (ret < 0)
+			goto error;
+		return 0;
+	}
+
+	msidesc->msi_attrib.multiple = ilog2(nvec);
+	for (sub_handle = 0; sub_handle < nvec; sub_handle++) {
+		if (!sub_handle) {
+			index = msi_alloc_remapped_irq(dev, irq, nvec);
+			if (index < 0) {
+				ret = index;
+				goto error;
+			}
+		} else {
+			ret = msi_setup_remapped_irq(dev, irq + sub_handle,
+						     index, sub_handle);
+			if (ret < 0)
+				goto error;
+		}
+		ret = setup_msi_irq(dev, msidesc, irq, sub_handle);
+		if (ret < 0)

arch/x86/kernel/apic/io_apic.c +13 -0

+	msidesc->irq = 0;
+	msidesc->msi_attrib.multiple = 0;
+
+	return ret;
+}
+
+int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+	if (type == PCI_CAP_ID_MSI)
+		return setup_msi_irqs(dev, nvec);
+	return setup_msix_irqs(dev, nvec);
+}
+
 void native_teardown_msi_irq(unsigned int irq)
 {
 	destroy_irq(irq);

include/linux/irq.h +2 -0

index 216b0ba..c3ba39f 100644
@@ -522,6 +522,8 @@ extern int irq_set_handler_data(unsigned int irq, void *data);
 extern int irq_set_chip_data(unsigned int irq, void *data);
 extern int irq_set_irq_type(unsigned int irq, unsigned int type);
 extern int irq_set_msi_desc(unsigned int irq, struct msi_desc *entry);
+extern int irq_set_msi_desc_off(unsigned int irq_base, unsigned int irq_offset,
+				struct msi_desc *entry);
 extern struct irq_data *irq_get_irq_data(unsigned int irq);
 
 static inline struct irq_chip *irq_get_chip(unsigned int irq)

include/linux/irq.h +1 -0

 void irq_free_descs(unsigned int irq, unsigned int cnt);
 int irq_reserve_irqs(unsigned int from, unsigned int cnt);
+int irq_can_alloc_irqs(unsigned int from, unsigned int cnt);
 
 static inline void irq_free_desc(unsigned int irq)
 {

kernel/irq/chip.c +22 -8

index 57d86d0..2230389 100644
@@ -90,27 +90,41 @@ int irq_set_handler_data(unsigned int irq, void *data)
 EXPORT_SYMBOL(irq_set_handler_data);
 
 /**
- *	irq_set_msi_desc - set MSI descriptor data for an irq
- *	@irq:	Interrupt number
- *	@entry:	Pointer to MSI descriptor data
+ *	irq_set_msi_desc_off - set MSI descriptor data for an irq at offset
+ *	@irq_base:	Interrupt number base
+ *	@irq_offset:	Interrupt number offset
+ *	@entry:		Pointer to MSI descriptor data
  *
- *	Set the MSI descriptor entry for an irq
+ *	Set the MSI descriptor entry for an irq at offset
  */
-int irq_set_msi_desc(unsigned int irq, struct msi_desc *entry)
+int irq_set_msi_desc_off(unsigned int irq_base, unsigned int irq_offset,
+			 struct msi_desc *entry)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_lock(irq_base + irq_offset, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
 
 	if (!desc)
 		return -EINVAL;
 	desc->irq_data.msi_desc = entry;
-	if (entry)
-		entry->irq = irq;
+	if (entry && !irq_offset)
+		entry->irq = irq_base;
 	irq_put_desc_unlock(desc, flags);
 	return 0;
 }
 
 /**
+ *	irq_set_msi_desc - set MSI descriptor data for an irq
+ *	@irq:	Interrupt number
+ *	@entry:	Pointer to MSI descriptor data
+ *
+ *	Set the MSI descriptor entry for an irq
+ */
+int irq_set_msi_desc(unsigned int irq, struct msi_desc *entry)
+{
+	return irq_set_msi_desc_off(irq, 0, entry);
+}
+
+/**
  *	irq_set_chip_data - set irq chip data for an irq
  *	@irq:	Interrupt number
  *	@data:	Pointer to chip specific data

kernel/irq/irqdesc.c +18 -0

index 192a302..8287b78 100644
@@ -210,6 +210,13 @@ static int irq_expand_nr_irqs(unsigned int nr)
 	return 0;
 }
 
+static int irq_can_expand_nr_irqs(unsigned int nr)
+{
+	if (nr > IRQ_BITMAP_BITS)
+		return -ENOMEM;
+	return 0;
+}
+
 int __init early_irq_init(void)
 {
 	int i, initcnt, node = first_online_node;

@@ -414,6 +421,30 @@ int irq_reserve_irqs(unsigned int from, unsigned int cnt)
 }
 
 /**
+ * irq_can_alloc_irqs - checks if a range of irqs could be allocated
+ * @from:	check from irq number
+ * @cnt:	number of irqs to check
+ *
+ * Returns 0 on success or an appropriate error code
+ */
+int irq_can_alloc_irqs(unsigned int from, unsigned int cnt)
+{
+	unsigned int start;
+	int ret = 0;
+

include/linux/irq.h int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, · line 591

Quoted 7 lines

include/linux/irq.h +3 -0

@@ -584,8 +586,12 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 #define irq_alloc_desc_from(from, node)		\
 	irq_alloc_descs(-1, from, 1, node)
 
+#define irq_alloc_descs_from(from, cnt, node)	\
+	irq_alloc_descs(-1, from, cnt, node)
+

Please use inlines instead of macros. Might transform the one above it as well in the process.

Quoted 9 lines

kernel/irq/irqdesc.c +9 -0

+int irq_can_alloc_irqs(unsigned int from, unsigned int cnt)
+{
+	unsigned int start;
+	int ret = 0;
+
+	if (!cnt)
+		return -EINVAL;
+
+	mutex_lock(&sparse_irq_lock);

kernel/irq/irqdesc.c int irq_reserve_irqs(unsigned int from, unsigned int cnt) · line 444

Quoted 6 lines

kernel/irq/irqdesc.c +6 -0

+	start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,
+					   from, cnt, 0);
+	mutex_unlock(&sparse_irq_lock);
+	if (start + cnt > nr_irqs)
+		ret = irq_can_expand_nr_irqs(start + cnt);
+	return ret;

How is this supposed to work wrt. races?

On Tue, Oct 02, 2012 at 06:55:18AM +0200, Ingo Molnar wrote:

Thanks for the review, Ingo.

Quoted 10 lines

include/linux/irq.h int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, · line 591

Quoted 7 lines

include/linux/irq.h +3 -0

@@ -584,8 +586,12 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 #define irq_alloc_desc_from(from, node)		\
 	irq_alloc_descs(-1, from, 1, node)
 
+#define irq_alloc_descs_from(from, cnt, node)	\
+	irq_alloc_descs(-1, from, cnt, node)
+

Please use inlines instead of macros. Might transform the one above it as well in the process.

You mean here do not introduce irq_alloc_descs_from, but rather use irq_alloc_descs() directly?

Quoted 9 lines

Quoted 9 lines

kernel/irq/irqdesc.c +9 -0

+int irq_can_alloc_irqs(unsigned int from, unsigned int cnt)
+{
+	unsigned int start;
+	int ret = 0;
+
+	if (!cnt)
+		return -EINVAL;
+
+	mutex_lock(&sparse_irq_lock);

Quoted 8 lines

kernel/irq/irqdesc.c int irq_reserve_irqs(unsigned int from, unsigned int cnt) · line 444

Quoted 6 lines

kernel/irq/irqdesc.c +6 -0

+	start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,
+					   from, cnt, 0);
+	mutex_unlock(&sparse_irq_lock);
+	if (start + cnt > nr_irqs)
+		ret = irq_can_expand_nr_irqs(start + cnt);
+	return ret;

How is this supposed to work wrt. races?

It is not supposed. Just a quick check if there are enough bits before an attempt to allocate memory in __create_irqs(). Otherwise __create_irqs() might allocate irq_cfg's, then realize there are no bits, then deallocate and fail.

But strictly speaking, irq_can_alloc_irqs() is unnecessary.

Quoted 11 lines

Quoted 10 lines

include/linux/irq.h int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, · line 591

Quoted 7 lines

include/linux/irq.h +3 -0

@@ -584,8 +586,12 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 #define irq_alloc_desc_from(from, node)		\
 	irq_alloc_descs(-1, from, 1, node)
 
+#define irq_alloc_descs_from(from, cnt, node)	\
+	irq_alloc_descs(-1, from, cnt, node)
+

Please use inlines instead of macros. Might transform the one above it as well in the process.

Quoted 2 lines

You mean here do not introduce irq_alloc_descs_from, but rather use irq_alloc_descs() directly?

My suggestion is to add irq_alloc_descs_from() as a (very simple) inline function and change irq_alloc_desc_from() to be an inline function as well.

Quoted 14 lines · Ingo Molnar

Quoted 13 lines

Quoted 10 lines

include/linux/irq.h int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, · line 591

Quoted 7 lines

include/linux/irq.h +3 -0

@@ -584,8 +586,12 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 #define irq_alloc_desc_from(from, node)		\
 	irq_alloc_descs(-1, from, 1, node)
 
+#define irq_alloc_descs_from(from, cnt, node)	\
+	irq_alloc_descs(-1, from, cnt, node)
+

Please use inlines instead of macros. Might transform the one above it as well in the process.

You mean here do not introduce irq_alloc_descs_from, but rather use irq_alloc_descs() directly?

Quoted 3 lines

My suggestion is to add irq_alloc_descs_from() as a (very simple) inline function and change irq_alloc_desc_from() to be an inline function as well.

These defines were added on purpose with commit ec53cf2 ("irq:
don't put module.h into irq.h for tracking irqgen modules.") - the relevant hunk is below. I suppose we do not want to revert it?

When multiple MSIs are enabled with pci_enable_msi_block() the number of allocated IRQs 'nvec' is rounded up to the nearest value of power of two. That could lead to a condition when number of requested and used IRQs is less than number of actually allocated IRQs.

This fix introduces 'msi_desc::nvec' field to address the above issue - when non-zero, it holds the number of allocated IRQs. Otherwise, the old method is used.

Signed-off-by Alexander Gordeev <[email protected]>

---
 arch/x86/kernel/apic/io_apic.c |   16 +++++++---------
 drivers/pci/msi.c              |   10 ++++++++--
 include/linux/msi.h            |    1 +
 3 files changed, 16 insertions(+), 11 deletions(-)

Signed-off-by Alexander Gordeev <[email protected]>

---
 arch/x86/kernel/apic/io_apic.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

arch/x86/kernel/apic/io_apic.c +2 -2

index 84d632b..0489699 100644
@@ -3123,7 +3123,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
 
 	if (irq_remapped(cfg)) {
 		compose_remapped_msi_msg(pdev, irq, dest, msg, hpet_id);
-		return err;
+		return 0;
 	}
 
 	if (x2apic_enabled())

@@ -3150,7 +3150,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
 			MSI_DATA_DELIVERY_LOWPRI) |
 		MSI_DATA_VECTOR(cfg->vector);
 
-	return err;
+	return 0;
 }
 
 static int

Take advantage of multiple MSIs implementation on x86 - on systems with IRQ remapping AHCI ports not only get assigned separate MSI vectors - but also separate IRQs. As result, interrupts generated by different ports could be serviced on different CPUs rather than on a single one.

In cases when number of allocated MSIs is less than requested the Sharing Last MSI mode does not get used, no matter implemented in hardware or not. Instead, the driver assumes the advantage of multiple MSIs is negated and falls back to the single MSI mode as if MRSM bit was set (some Intel chips implement this strategy anyway - MRSM bit gets set even if the number of allocated MSIs exceeds the number of implemented ports).

Signed-off-by Alexander Gordeev <[email protected]>

---
  drivers/ata/ahci.c    |   91 ++++++++++++++++++++++++++++++++++++--
  drivers/ata/ahci.h    |    6 +++
  drivers/ata/libahci.c |  118 ++++++++++++++++++++++++++++++++++++++++++++++---
  3 files changed, 205 insertions(+), 10 deletions(-)

Quoted 13 lines · Alexander Gordeev

Take advantage of multiple MSIs implementation on x86 - on systems with IRQ remapping AHCI ports not only get assigned separate MSI vectors - but also separate IRQs. As result, interrupts generated by different ports could be serviced on different CPUs rather than on a single one.

In cases when number of allocated MSIs is less than requested the Sharing Last MSI mode does not get used, no matter implemented in hardware or not. Instead, the driver assumes the advantage of multiple MSIs is negated and falls back to the single MSI mode as if MRSM bit was set (some Intel chips implement this strategy anyway - MRSM bit gets set even if the number of allocated MSIs exceeds the number of implemented ports).

Quoted 7 lines

Signed-off-by Alexander Gordeev <[email protected]>

---
  drivers/ata/ahci.c    |   91 ++++++++++++++++++++++++++++++++++++--
  drivers/ata/ahci.h    |    6 +++
  drivers/ata/libahci.c |  118
++++++++++++++++++++++++++++++++++++++++++++++---
  3 files changed, 205 insertions(+), 10 deletions(-)

Acked-by Jeff Garzik <[email protected]>

Normally, this amount of changes would -really- need to go through the libata tree. However, given the amount of dependencies, it either needs a merge tree or to go through the PCI tree...?

Any maintainer comments on disposition?

Quoted 29 lines · Jeff Garzik

Quoted 13 lines · Alexander Gordeev

Take advantage of multiple MSIs implementation on x86 - on systems with IRQ remapping AHCI ports not only get assigned separate MSI vectors - but also separate IRQs. As result, interrupts generated by different ports could be serviced on different CPUs rather than on a single one.

In cases when number of allocated MSIs is less than requested the Sharing Last MSI mode does not get used, no matter implemented in hardware or not. Instead, the driver assumes the advantage of multiple MSIs is negated and falls back to the single MSI mode as if MRSM bit was set (some Intel chips implement this strategy anyway - MRSM bit gets set even if the number of allocated MSIs exceeds the number of implemented ports).

Quoted 7 lines

Signed-off-by Alexander Gordeev <[email protected]>

---
   drivers/ata/ahci.c    |   91 ++++++++++++++++++++++++++++++++++++--
   drivers/ata/ahci.h    |    6 +++
   drivers/ata/libahci.c |  118
++++++++++++++++++++++++++++++++++++++++++++++---
   3 files changed, 205 insertions(+), 10 deletions(-)

Acked-by Jeff Garzik <[email protected]>

Normally, this amount of changes would -really- need to go through the libata tree. However, given the amount of dependencies, it either needs a merge tree or to go through the PCI tree...?

Quoted 1 line

Any maintainer comments on disposition?

For what it's worth, the bulk of this change is outside PCI, so it doesn't seem to me like it should go through the PCI tree. I think I did ack the part that touched PCI, and there's not much activity in the PCI MSI area right now, so I'm fine with it going through libata or whatever people think makes sense.

Take advantage of multiple MSIs implementation on x86 - on systems with IRQ remapping AHCI ports not only get assigned separate MSI vectors - but also separate IRQs. As result, interrupts generated by different ports could be serviced on different CPUs rather than on a single one.

In cases when number of allocated MSIs is less than requested the Sharing Last MSI mode does not get used, no matter implemented in hardware or not. Instead, the driver assumes the advantage of multiple MSIs is negated and falls back to the single MSI mode as if MRSM bit was set (some Intel chips implement this strategy anyway - MRSM bit gets set even if the number of allocated MSIs exceeds the number of implemented ports).

Signed-off-by Alexander Gordeev <[email protected]>

---
 drivers/ata/ahci.c    |   91 ++++++++++++++++++++++++++++++++++++--
 drivers/ata/ahci.h    |    6 +++
 drivers/ata/libahci.c |  118 ++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 205 insertions(+), 10 deletions(-)

drivers/ata/ahci.c +2 -0

index 7862d17..5463fcea 100644
@@ -1057,6 +1057,84 @@ static inline void ahci_gtf_filter_workaround(struct ata_host *host)
 {}
 #endif
 
+int ahci_init_interrupts(struct pci_dev *pdev, struct ahci_host_priv *hpriv)
+{

drivers/ata/ahci.c +52 -0

+			pci_disable_msi(pdev);
+			if (!pci_enable_msi(pdev))
+				return 1;
+		}
+	}
+
+	pci_intx(pdev, 1);
+	return 0;
+}
+
+/**
+ *	ahci_host_activate - start AHCI host, request IRQs and register it
+ *	@host: target ATA host
+ *	@irq: base IRQ number to request
+ *	@n_msis: number of MSIs allocated for this host
+ *	@irq_handler: irq_handler used when requesting IRQs
+ *	@irq_flags: irq_flags used when requesting IRQs
+ *
+ *	Similar to ata_host_activate, but requests IRQs according to AHCI-1.1
+ *	when multiple MSIs were allocated. That is one MSI per port, starting
+ *	from @irq.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+int ahci_host_activate(struct ata_host *host, int irq, unsigned int n_msis)
+{
+	int i, rc;
+
+	/* Sharing Last Message among several ports is not supported */
+	if (n_msis < host->n_ports)
+		return -EINVAL;
+
+	rc = ata_host_start(host);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < host->n_ports; i++) {
+		rc = devm_request_threaded_irq(host->dev,
+			irq + i, ahci_hw_interrupt, ahci_thread_fn, IRQF_SHARED,
+			dev_driver_string(host->dev), host->ports[i]);
+		if (rc)
+			goto out_free_irqs;
+	}
+
+	for (i = 0; i < host->n_ports; i++)
+		ata_port_desc(host->ports[i], "irq %d", irq + i);
+
+	rc = ata_host_register(host, &ahci_sht);

drivers/ata/ahci.c +13 -4

+
+	return rc;
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	unsigned int board_id = ent->driver_data;

@@ -1065,7 +1143,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
 	struct ata_host *host;
-	int n_ports, i, rc;
+	int n_ports, n_msis, i, rc;
 	int ahci_pci_bar = AHCI_PCI_BAR_STANDARD;
 
 	VPRINTK("ENTER\n");

@@ -1150,11 +1228,12 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ahci_sb600_enable_64bit(pdev))
 		hpriv->flags &= ~AHCI_HFLAG_32BIT_ONLY;
 
-	if ((hpriv->flags & AHCI_HFLAG_NO_MSI) || pci_enable_msi(pdev))
-		pci_intx(pdev, 1);
-
 	hpriv->mmio = pcim_iomap_table(pdev)[ahci_pci_bar];
 
+	n_msis = ahci_init_interrupts(pdev, hpriv);
+	if (n_msis > 1)
+		hpriv->flags |= AHCI_HFLAG_MULTI_MSI;
+
 	/* save initial config */
 	ahci_pci_save_initial_config(pdev, hpriv);
 

@@ -1250,6 +1329,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	ahci_pci_print_info(host);
 
 	pci_set_master(pdev);
+
+	if (hpriv->flags & AHCI_HFLAG_MULTI_MSI)
+		return ahci_host_activate(host, pdev->irq, n_msis);
+
 	return ata_host_activate(host, pdev->irq, ahci_interrupt, IRQF_SHARED,
 				 &ahci_sht);
 }

drivers/ata/ahci.h +1 -0

index 57eb1c2..24251e8 100644
@@ -216,6 +216,7 @@ enum {
 	AHCI_HFLAG_DELAY_ENGINE		= (1 << 15), /* do not start engine on
 						        port start (wait until
 						        error-handling stage) */
+	AHCI_HFLAG_MULTI_MSI		= (1 << 16), /* multiple PCI MSIs */
 
 	/* ap->flags bits */
 

drivers/ata/ahci.h +3 -0

 	u32 			intr_mask;	/* interrupts to enable */
 	bool			fbs_supported;	/* set iff FBS is supported */
 	bool			fbs_enabled;	/* set iff FBS is enabled */

@@ -343,7 +346,10 @@ void ahci_set_em_messages(struct ahci_host_priv *hpriv,
 			  struct ata_port_info *pi);
 int ahci_reset_em(struct ata_host *host);
 irqreturn_t ahci_interrupt(int irq, void *dev_instance);
+irqreturn_t ahci_hw_interrupt(int irq, void *dev_instance);
+irqreturn_t ahci_thread_fn(int irq, void *dev_instance);
 void ahci_print_info(struct ata_host *host, const char *scc_s);
+int ahci_host_activate(struct ata_host *host, int irq, unsigned int n_msis);
 
 static inline void __iomem *__ahci_port_base(struct ata_host *host,
 					     unsigned int port_no)

drivers/ata/libahci.c +49 -6

index 555c07a..3b54e84 100644
@@ -1639,19 +1639,16 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
 		ata_port_abort(ap);
 }
 
-static void ahci_port_intr(struct ata_port *ap)
+static void ahci_handle_port_interrupt(struct ata_port *ap,
+				       void __iomem *port_mmio, u32 status)
 {
-	void __iomem *port_mmio = ahci_port_base(ap);
 	struct ata_eh_info *ehi = &ap->link.eh_info;
 	struct ahci_port_priv *pp = ap->private_data;
 	struct ahci_host_priv *hpriv = ap->host->private_data;
 	int resetting = !!(ap->pflags & ATA_PFLAG_RESETTING);
-	u32 status, qc_active = 0;
+	u32 qc_active = 0;
 	int rc;
 
-	status = readl(port_mmio + PORT_IRQ_STAT);
-	writel(status, port_mmio + PORT_IRQ_STAT);
-
 	/* ignore BAD_PMP while resetting */
 	if (unlikely(resetting))
 		status &= ~PORT_IRQ_BAD_PMP;

@@ -1727,6 +1724,107 @@ static void ahci_port_intr(struct ata_port *ap)
 	}
 }
 
+void ahci_port_intr(struct ata_port *ap)
+{
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u32 status;
+
+	status = readl(port_mmio + PORT_IRQ_STAT);
+	writel(status, port_mmio + PORT_IRQ_STAT);
+
+	ahci_handle_port_interrupt(ap, port_mmio, status);
+}
+
+irqreturn_t ahci_thread_fn(int irq, void *dev_instance)
+{
+	struct ata_port *ap = dev_instance;
+	struct ahci_port_priv *pp = ap->private_data;
+	void __iomem *port_mmio = ahci_port_base(ap);
+	unsigned long flags;
+	u32 status;
+
+	spin_lock_irqsave(&ap->host->lock, flags);
+	status = pp->intr_status;
+	if (status)
+		pp->intr_status = 0;
+	spin_unlock_irqrestore(&ap->host->lock, flags);
+
+	spin_lock_bh(ap->lock);
+	ahci_handle_port_interrupt(ap, port_mmio, status);
+	spin_unlock_bh(ap->lock);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL_GPL(ahci_thread_fn);
+
+void ahci_hw_port_interrupt(struct ata_port *ap)
+{
+	void __iomem *port_mmio = ahci_port_base(ap);
+	struct ahci_port_priv *pp = ap->private_data;
+	u32 status;
+
+	status = readl(port_mmio + PORT_IRQ_STAT);
+	writel(status, port_mmio + PORT_IRQ_STAT);
+
+	pp->intr_status |= status;
+}
+
+irqreturn_t ahci_hw_interrupt(int irq, void *dev_instance)

Quoted 6 lines

drivers/ata/libahci.c +6 -0

+irqreturn_t ahci_hw_interrupt(int irq, void *dev_instance)
+{
+	struct ata_port *ap_this = dev_instance;
+	struct ahci_port_priv *pp = ap_this->private_data;
+	struct ata_host *host = ap_this->host;
+	struct ahci_host_priv *hpriv = host->private_data;

drivers/ata/libahci.c static void ahci_port_intr(struct ata_port *ap) · line 1782

Quoted 5 lines

drivers/ata/libahci.c +5 -0

+	void __iomem *mmio = hpriv->mmio;
+	unsigned int i;
+	u32 irq_stat, irq_masked;
+
+	VPRINTK("ENTER\n");

Is this per IRQ handler execution debugging code still needed? Same for the other VPRINTK() lines in this patch.

Quoted 6 lines · Ingo Molnar

Quoted 6 lines

drivers/ata/libahci.c +6 -0

+irqreturn_t ahci_hw_interrupt(int irq, void *dev_instance)
+{
+	struct ata_port *ap_this = dev_instance;
+	struct ahci_port_priv *pp = ap_this->private_data;
+	struct ata_host *host = ap_this->host;
+	struct ahci_host_priv *hpriv = host->private_data;

Quoted 8 lines

drivers/ata/libahci.c static void ahci_port_intr(struct ata_port *ap) · line 1782

Quoted 5 lines

drivers/ata/libahci.c +5 -0

+	void __iomem *mmio = hpriv->mmio;
+	unsigned int i;
+	u32 irq_stat, irq_masked;
+
+	VPRINTK("ENTER\n");

Is this per IRQ handler execution debugging code still needed? Same for the other VPRINTK() lines in this patch.

These VPRINKs are only to make new handlers look like ahci_interrupt() which did not change. I believe, if they need to go it is better to remove them altogether, with a separate followup patch.

Quoted 8 lines

Quoted 4 lines

drivers/ata/libahci.c static void ahci_port_intr(struct ata_port *ap) · line 1782

Quoted 1 line

drivers/ata/libahci.c +1 -0

+	VPRINTK("ENTER\n");

Is this per IRQ handler execution debugging code still needed? Same for the other VPRINTK() lines in this patch.

These VPRINKs are only to make new handlers look like ahci_interrupt() which did not change. I believe, if they need to go it is better to remove them altogether, with a separate followup patch.

Definitely followup patch material + discussion :)