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

[Joe Jin <joe.jin@xxxxxxxxxx>]

On 2009-11-24 14:12, Jiang, Yunhong wrote:
> Joe, thanks for your patch very much. 
> A small question is, can we check if (!(dev->msi_enabled))  only once in 
> pci_disable_msi()? 

Check once is OK, twice check made codes easy to understand(compare
pci_enable_msi()), and will not impact program's flow :)

Thanks,
Joe

> 
> --jyh
> 
> Joe Jin wrote:
> > 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