Re: AMD 8131 MSI quirk called too late, bus_flags not inherited ?

[Brice Goglin <brice@myri.com>]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Michael S. Tsirkin wrote:
>> @@ -925,8 +926,9 @@
>>  	if (dev->no_msi)
>>  		return status;
>>  
>> -	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
>> -		return -EINVAL;
>> +	for (bus = dev->bus; bus; bus = bus->parent)
>> +		if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
>> +			return -EINVAL;
>>  
>>  	temp = dev->irq;
>>     
>
> It seems we must add this loop to pci_enable_msix as well.
>   

Right, thanks. Greg, what do you think of putting the attached patch in
2.6.17 ?

By the way, do we need to check dev->no_msi in pci_enable_msix() too ?

For 2.6.18, I don't know what's the best. We could drop the fact that
bus flags should be inherited and keep looking at parent busses. It
might be good to add a pci_check_flag_in_parent_busses(dev, flag) to
provide a generic way to do my for loop.

thanks,
Brice


Signed-off-by: Brice Goglin <brice@myri.com>


[-- Attachment #2: look_at_parent_busses_flags.patch --]
[-- Type: text/x-patch, Size: 1124 bytes --]

Index: linux-mm/drivers/pci/msi.c
===================================================================
--- linux-mm.orig/drivers/pci/msi.c	2006-05-21 15:12:04.000000000 +0200
+++ linux-mm/drivers/pci/msi.c	2006-05-21 15:15:34.000000000 +0200
@@ -916,6 +916,7 @@
  **/
 int pci_enable_msi(struct pci_dev* dev)
 {
+	struct pci_bus *bus;
 	int pos, temp, status = -EINVAL;
 	u16 control;
 
@@ -925,8 +926,9 @@
 	if (dev->no_msi)
 		return status;
 
-	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
-		return -EINVAL;
+	for (bus = dev->bus; bus; bus = bus->parent)
+		if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+			return -EINVAL;
 
 	temp = dev->irq;
 
@@ -1162,6 +1164,7 @@
  **/
 int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec)
 {
+	struct pci_bus *bus;
 	int status, pos, nr_entries, free_vectors;
 	int i, j, temp;
 	u16 control;
@@ -1170,6 +1173,10 @@
 	if (!pci_msi_enable || !dev || !entries)
  		return -EINVAL;
 
+	for (bus = dev->bus; bus; bus = bus->parent)
+		if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+			return -EINVAL;
+
 	status = msi_init();
 	if (status < 0)
 		return status;