Re: AMD 8131 and MSI quirk

["Michael S. Tsirkin" <mst@mellanox.co.il>]

Quoting r. Roland Dreier <rolandd@cisco.com>:
> Subject: AMD 8131 and MSI quirk
> 
> The current quirk_amd_8131_ioapic() function sets a global
> pci_msi_quirk flag, which disables MSI/MSI-X for all devices in the
> system.  This is safe but suboptimal, because there may be devices on
> other buses not related to the AMD 8131 bridge, for which MSI would
> work fine.  As an example, see the end of this email for a lspci -t
> from a real Opteron system that has PCI-X buses coming from an AMD
> 8131 and PCI Express buses coming from an Nforce4 bridge -- MSI works
> fine for the Mellanox InfiniBand adapter on the PCIe bus, if we allow
> it to be enabled.

The following should do this IMO. Roland, could you test this patch please?

---

It turns out AMD 8131 quirk only affects MSI for devices behind the 8131 bridge.
Handle this by adding a flags field in pci_bus, inherited from parent to child.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

Index: linux-2.6.16-rc2/drivers/pci/msi.c
===================================================================
--- linux-2.6.16-rc2.orig/drivers/pci/msi.c	2006-02-14 17:09:23.000000000 +0200
+++ linux-2.6.16-rc2/drivers/pci/msi.c	2006-02-14 18:14:00.000000000 +0200
@@ -699,6 +699,9 @@ int pci_enable_msi(struct pci_dev* dev)
 	if (dev->no_msi)
 		return status;
 
+	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+		return -EINVAL;
+
 	temp = dev->irq;
 
 	if ((status = msi_init()) < 0)
@@ -924,6 +927,9 @@ int pci_enable_msix(struct pci_dev* dev,
 	if (!pci_msi_enable || !dev || !entries)
  		return -EINVAL;
 
+	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+		return -EINVAL;
+
 	if ((status = msi_init()) < 0)
 		return status;
 
Index: linux-2.6.16-rc2/drivers/pci/probe.c
===================================================================
--- linux-2.6.16-rc2.orig/drivers/pci/probe.c	2006-02-14 17:09:23.000000000 +0200
+++ linux-2.6.16-rc2/drivers/pci/probe.c	2006-02-14 17:58:17.000000000 +0200
@@ -347,6 +347,7 @@ pci_alloc_child_bus(struct pci_bus *pare
 	child->parent = parent;
 	child->ops = parent->ops;
 	child->sysdata = parent->sysdata;
+	child->bus_flags = parent->bus_flags;
 	child->bridge = get_device(&bridge->dev);
 
 	child->class_dev.class = &pcibus_class;
Index: linux-2.6.16-rc2/drivers/pci/quirks.c
===================================================================
--- linux-2.6.16-rc2.orig/drivers/pci/quirks.c	2006-02-14 17:09:23.000000000 +0200
+++ linux-2.6.16-rc2/drivers/pci/quirks.c	2006-02-14 17:58:17.000000000 +0200
@@ -575,8 +575,11 @@ static void __init quirk_amd_8131_ioapic
 { 
         unsigned char revid, tmp;
         
-	pci_msi_quirk = 1;
-	printk(KERN_WARNING "PCI: MSI quirk detected. pci_msi_quirk set.\n");
+	if (dev->subordinate) {
+		printk(KERN_WARNING "PCI: MSI quirk detected. "
+		       "PCI_BUS_FLAGS_NO_MSI set for subordinate bus.\n");
+		dev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+	}
 
         if (nr_ioapics == 0) 
                 return;
Index: linux-2.6.16-rc2/include/linux/pci.h
===================================================================
--- linux-2.6.16-rc2.orig/include/linux/pci.h	2006-02-14 17:09:24.000000000 +0200
+++ linux-2.6.16-rc2/include/linux/pci.h	2006-02-14 17:58:17.000000000 +0200
@@ -95,6 +95,11 @@ enum pci_channel_state {
 	pci_channel_io_perm_failure = (__force pci_channel_state_t) 3,
 };
 
+typedef unsigned short __bitwise pci_bus_flags_t;
+enum pci_bus_flags {
+	PCI_BUS_FLAGS_NO_MSI = (pci_bus_flags_t) 1,
+};
+
 /*
  * The pci_dev structure is used to describe PCI devices.
  */
@@ -203,7 +208,7 @@ struct pci_bus {
 	char		name[48];
 
 	unsigned short  bridge_ctl;	/* manage NO_ISA/FBB/et al behaviors */
-	unsigned short  pad2;
+	pci_bus_flags_t bus_flags;	/* Inherited by child busses */
 	struct device		*bridge;
 	struct class_device	class_dev;
 	struct bin_attribute	*legacy_io; /* legacy I/O for this bus */



-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies