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

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

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