RE: [Xen-devel] [PATCH] Dont call msi_unmap_pirq() if did not enabled msi

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

Seems this list handling need a clean-up, sometimes it is protected, sometimes 
seems not. I will check it again because I didn't touch the this code for a 
long time.

--jyh

xen-devel-bounces@xxxxxxxxxxxxxxxxxxx wrote:
> I also noticed following code  have no lock protection in the
> list_for_each_entry_safe(pirq_entry, tmp,
> &msi_dev_entry->pirq_list_head, list) and needs a fix.
> 
> -- jyh
> 
>> void pci_disable_msix(struct pci_dev* dev)
>> {
>>     int pos;
>>     u16 control;
>> 
>>     if (!pci_msi_enable)
>>         return;
>>     if (!dev)
>>         return;
>> 
>> #ifdef CONFIG_XEN_PCIDEV_FRONTEND
>>     if (!is_initial_xendomain()) {
>>         struct msi_dev_list *msi_dev_entry;
>>         struct msi_pirq_entry *pirq_entry, *tmp;
>> 
>>         pci_frontend_disable_msix(dev);
>> 
>>         msi_dev_entry = get_msi_dev_pirq_list(dev);
>>         list_for_each_entry_safe(pirq_entry, tmp,
>>                                  &msi_dev_entry->pirq_list_head,
>>             list) { evtchn_map_pirq(pirq_entry->pirq, 0);
>>             list_del(&pirq_entry->list);
>>             kfree(pirq_entry);
>>         }
>> 
>>         dev->irq = msi_dev_entry->default_irq;
>>         return;
>>     }
>> #endif
> 
> 
> Jan Beulich wrote:
>> Wouldn't we need the same also for MSI-X?
>> 
>> Jan
>> 
>>>>> Joe Jin <joe.jin@xxxxxxxxxx> 24.11.09 03:02 >>>
>> Sorry I lost to set @dev->msi_enabled to false in pci_disable_msi,
>> here are the patch, please review and comment:
>> 
>> When device driver unload, it may call pci_disable_msi(), if
>> msi did not
>> enabled but do msi_unmap_pirq(), then later driver reload and without
>> msi, then will failed in request_irq() for irq_desc[irq]->chip valie
>> is no_irq_chip. So when did not enable msi during driver
>> initializing, then unloaded driver will not try to disable it.
>> 
>> How to reproduce it:
>>  At the server with QLogic 25xx, try to reload qla2xxx will hit it.
>> 
>> 
>> Signed-off-by: Joe Jin <joe.jin@xxxxxxxxxx>
>> ---
>> msi-xen.c |   13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>> 
>> 
>> diff -r c5c40e80bd7d drivers/pci/msi-xen.c
>> --- a/drivers/pci/msi-xen.c  Fri Nov 13 22:01:54 2009 +0000
>> +++ b/drivers/pci/msi-xen.c  Tue Nov 24 09:56:52 2009 +0800 @@ -618,6
>> +618,7 @@ return ret; 
>> 
>>              dev->irq = evtchn_map_pirq(-1, dev->irq);
>> +            dev->msi_enabled = 1;
>>              msi_dev_entry->default_irq = temp;
>> 
>>              return ret;
>> @@ -662,9 +663,15 @@
>> 
>> #ifdef CONFIG_XEN_PCIDEV_FRONTEND
>>      if (!is_initial_xendomain()) {
>> +            if (!(dev->msi_enabled)) {
>> +                    printk(KERN_INFO "PCI: %s: Device did
>> not enabled MSI.\n",
>> +                           pci_name(dev));
>> +                    return;
>> +            }
>>              evtchn_map_pirq(dev->irq, 0);
>>              pci_frontend_disable_msi(dev);
>>              dev->irq = msi_dev_entry->default_irq;
>> +            dev->msi_enabled = 0;
>>              return;
>>      }
>> #endif
>> @@ -673,6 +680,12 @@
>>      if (!pos)
>>              return;
>> 
>> +    if (!(dev->msi_enabled)) {
>> +            printk(KERN_INFO "PCI: %s: Device did not
>> enabled MSI.\n",
>> +                   pci_name(dev));
>> +            return;
>> +    }
>> +
>>      pirq = dev->irq;
>>      /* Restore dev->irq to its default pin-assertion vector */
>>      dev->irq = msi_dev_entry->default_irq;
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel