lkml.org 
[lkml]   [2007]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: msi_free_irqs #2
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);
> }
>
> 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.
>
> > 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);

Here's a patch that just reverses the 2 lines of code as Eric suggests. Please
consider this for inclusion.

Signed-off-by: Mike Miller <mike.miller@hp.com>
Signed-off-by: Chase Maupin <chase.maupin@hp.com>

Thanks,
mikem
----------------------------------------------------------------------------------------
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index e01380b..6632150 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -558,12 +558,12 @@ static int msi_free_irqs(struct pci_dev* dev)

list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
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);
+
+ if (list_is_last(&entry->list, &dev->msi_list))
+ iounmap(entry->mask_base);
}
list_del(&entry->list);
kfree(entry);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2007-05-24 23:13    [W:0.200 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site