On Thu, May 24, 2007 at 02:53:08PM -0600, Eric W. Biederman wrote:
> 
> 
> Could you please try the patch below.  Unless I have misread something
> this should fix your problem....
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 000c9ae..2e1d4af 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -404,7 +404,7 @@ static int msix_capability_init(struct pci_dev *dev,
>               entry->dev = dev;
>               entry->mask_base = base;
>  
> -             list_add(&entry->list, &dev->msi_list);
> +             list_add_tail(&entry->list, &dev->msi_list);
>       }

Yes, this is what we found. But we made 2 changes to list_add. Just sent the 
patch. 

>  
>       ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
> 
> > Michael or Eric, would you please review this patch and see if it's OK? 
> > Adding
> > an else
> > between the the if (list_is....) and the writel resolved the Oops. I'm not 
> > sure
> > how this
> > is supposed to work but using entry->mask_base after iounmap'ing seems 
> > wrong.
> 
> I think I would rather just swap those two lines of code.
> We are manually setting the mask bit to make certain the irq doesn't
> fire after we free it, and we clearly want to set the mask bit for
> all the irq entries.

OK, new patch on the way.

mikem
> 
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index d9cbd58..0e67723 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -560,10 +560,11 @@ static int msi_free_irqs(struct pci_dev* dev)
> >             if (entry->msi_attrib.type == PCI_CAP_ID_MSIX) {
> >                     if (list_is_last(&entry->list, &dev->msi_list))
> >                             iounmap(entry->mask_base);
> > -
> > -                   writel(1, entry->mask_base + entry->msi_attrib.entry_nr
> > -                             * PCI_MSIX_ENTRY_SIZE
> > -                             + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> > +                   else
> > +                           writel(1, entry->mask_base
> > +                                   + entry->msi_attrib.entry_nr
> > +                                   * PCI_MSIX_ENTRY_SIZE
> > +                                   + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> >             }
> >             list_del(&entry->list);
> >             kfree(entry);
> > -----------------------------------------------------------------------------------------
> >
> > I hope this clears up a little of the fog.
> 
> Yes it does thanks.
> 
> Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to