From ebiederm@xmission.com Wed Jan 31 21:59:15 2007 From: ebiederm@xmission.com (Eric W. Biederman) Date: Sun, 28 Jan 2007 12:42:52 -0700 Subject: msi: Kill msi_lookup_irq To: Greg Kroah-Hartman Cc: "David S. Miller" , Kyle McMartin , , Brice Goglin , , Michael Ellerman , Grant Grundler , Tony Luck , Ingo Molnar Message-ID: The function msi_lookup_irq was horrible. As a side effect of running it changed dev->irq, and then the callers would need to change it back. In addition it does a global scan through all of the irqs, which seems to be the sole justification of the msi_lock. To remove the neede for msi_lookup_irq I added first_msi_irq to struct pci_dev. Then depending on the context I replaced msi_lookup_irq with dev->first_msi_irq, dev->msi_enabled, or dev->msix_enabled. msi_enabled and msix_enabled were already present in pci_dev for other reasons. Signed-off-by: Eric W. Biederman Acked-by: Ingo Molnar Signed-off-by: Greg Kroah-Hartman --- drivers/pci/msi.c | 149 ++++++++++++++++++++-------------------------------- include/linux/pci.h | 3 + 2 files changed, 62 insertions(+), 90 deletions(-) --- gregkh-2.6.orig/drivers/pci/msi.c +++ gregkh-2.6/drivers/pci/msi.c @@ -272,28 +272,6 @@ void disable_msi_mode(struct pci_dev *de pci_intx(dev, 1); /* enable intx */ } -static int msi_lookup_irq(struct pci_dev *dev, int type) -{ - int irq; - unsigned long flags; - - spin_lock_irqsave(&msi_lock, flags); - for (irq = 0; irq < NR_IRQS; irq++) { - if (!msi_desc[irq] || msi_desc[irq]->dev != dev || - msi_desc[irq]->msi_attrib.type != type || - msi_desc[irq]->msi_attrib.default_irq != dev->irq) - continue; - spin_unlock_irqrestore(&msi_lock, flags); - /* This pre-assigned MSI irq for this device - already exists. Override dev->irq with this irq */ - dev->irq = irq; - return 0; - } - spin_unlock_irqrestore(&msi_lock, flags); - - return -EACCES; -} - #ifdef CONFIG_PM static int __pci_save_msi_state(struct pci_dev *dev) { @@ -364,11 +342,13 @@ static void __pci_restore_msi_state(stru static int __pci_save_msix_state(struct pci_dev *dev) { int pos; - int temp; int irq, head, tail = 0; u16 control; struct pci_cap_saved_state *save_state; + if (!dev->msix_enabled) + return 0; + pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); if (pos <= 0 || dev->no_msi) return 0; @@ -386,13 +366,7 @@ static int __pci_save_msix_state(struct *((u16 *)&save_state->data[0]) = control; /* save the table */ - temp = dev->irq; - if (msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) { - kfree(save_state); - return -EINVAL; - } - - irq = head = dev->irq; + irq = head = dev->first_msi_irq; while (head != tail) { struct msi_desc *entry; @@ -402,7 +376,6 @@ static int __pci_save_msix_state(struct tail = msi_desc[irq]->link.tail; irq = tail; } - dev->irq = temp; save_state->cap_nr = PCI_CAP_ID_MSIX; pci_add_saved_cap(dev, save_state); @@ -428,9 +401,11 @@ static void __pci_restore_msix_state(str int pos; int irq, head, tail = 0; struct msi_desc *entry; - int temp; struct pci_cap_saved_state *save_state; + if (!dev->msix_enabled) + return; + save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSIX); if (!save_state) return; @@ -443,10 +418,7 @@ static void __pci_restore_msix_state(str return; /* route the table */ - temp = dev->irq; - if (msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) - return; - irq = head = dev->irq; + irq = head = dev->first_msi_irq; while (head != tail) { entry = msi_desc[irq]; write_msi_msg(irq, &entry->msg_save); @@ -454,7 +426,6 @@ static void __pci_restore_msix_state(str tail = msi_desc[irq]->link.tail; irq = tail; } - dev->irq = temp; pci_write_config_word(dev, msi_control_reg(pos), save); enable_msi_mode(dev, pos, PCI_CAP_ID_MSIX); @@ -524,6 +495,7 @@ static int msi_capability_init(struct pc return status; } + dev->first_msi_irq = irq; attach_msi_entry(entry, irq); /* Set MSI enabled bits */ enable_msi_mode(dev, pos, PCI_CAP_ID_MSI); @@ -620,6 +592,7 @@ static int msix_capability_init(struct p avail = -EBUSY; return avail; } + dev->first_msi_irq = entries[0].vector; /* Set MSI-X enabled bits */ enable_msi_mode(dev, pos, PCI_CAP_ID_MSIX); @@ -667,13 +640,11 @@ int pci_msi_supported(struct pci_dev * d **/ int pci_enable_msi(struct pci_dev* dev) { - int pos, temp, status; + int pos, status; if (pci_msi_supported(dev) < 0) return -EINVAL; - temp = dev->irq; - status = msi_init(); if (status < 0) return status; @@ -682,15 +653,14 @@ int pci_enable_msi(struct pci_dev* dev) if (!pos) return -EINVAL; - WARN_ON(!msi_lookup_irq(dev, PCI_CAP_ID_MSI)); + WARN_ON(!!dev->msi_enabled); /* Check whether driver already requested for MSI-X irqs */ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); - if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) { + if (pos > 0 && dev->msix_enabled) { printk(KERN_INFO "PCI: %s: Can't enable MSI. " - "Device already has MSI-X irq assigned\n", + "Device already has MSI-X enabled\n", pci_name(dev)); - dev->irq = temp; return -EINVAL; } status = msi_capability_init(dev); @@ -709,6 +679,9 @@ void pci_disable_msi(struct pci_dev* dev if (!dev) return; + if (!dev->msi_enabled) + return; + pos = pci_find_capability(dev, PCI_CAP_ID_MSI); if (!pos) return; @@ -717,28 +690,30 @@ void pci_disable_msi(struct pci_dev* dev if (!(control & PCI_MSI_FLAGS_ENABLE)) return; + disable_msi_mode(dev, pos, PCI_CAP_ID_MSI); spin_lock_irqsave(&msi_lock, flags); - entry = msi_desc[dev->irq]; + entry = msi_desc[dev->first_msi_irq]; if (!entry || !entry->dev || entry->msi_attrib.type != PCI_CAP_ID_MSI) { spin_unlock_irqrestore(&msi_lock, flags); return; } - if (irq_has_action(dev->irq)) { + if (irq_has_action(dev->first_msi_irq)) { spin_unlock_irqrestore(&msi_lock, flags); printk(KERN_WARNING "PCI: %s: pci_disable_msi() called without " "free_irq() on MSI irq %d\n", - pci_name(dev), dev->irq); - BUG_ON(irq_has_action(dev->irq)); + pci_name(dev), dev->first_msi_irq); + BUG_ON(irq_has_action(dev->first_msi_irq)); } else { default_irq = entry->msi_attrib.default_irq; spin_unlock_irqrestore(&msi_lock, flags); - msi_free_irq(dev, dev->irq); + msi_free_irq(dev, dev->first_msi_irq); /* Restore dev->irq to its default pin-assertion irq */ dev->irq = default_irq; } + dev->first_msi_irq = 0; } static int msi_free_irq(struct pci_dev* dev, int irq) @@ -797,7 +772,7 @@ static int msi_free_irq(struct pci_dev* int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec) { int status, pos, nr_entries; - int i, j, temp; + int i, j; u16 control; if (!entries || pci_msi_supported(dev) < 0) @@ -825,16 +800,14 @@ int pci_enable_msix(struct pci_dev* dev, return -EINVAL; /* duplicate entry */ } } - temp = dev->irq; - WARN_ON(!msi_lookup_irq(dev, PCI_CAP_ID_MSIX)); + WARN_ON(!!dev->msix_enabled); /* Check whether driver already requested for MSI irq */ if (pci_find_capability(dev, PCI_CAP_ID_MSI) > 0 && - !msi_lookup_irq(dev, PCI_CAP_ID_MSI)) { + dev->msi_enabled) { printk(KERN_INFO "PCI: %s: Can't enable MSI-X. " "Device already has an MSI irq assigned\n", pci_name(dev)); - dev->irq = temp; return -EINVAL; } status = msix_capability_init(dev, entries, nvec); @@ -843,7 +816,9 @@ int pci_enable_msix(struct pci_dev* dev, void pci_disable_msix(struct pci_dev* dev) { - int pos, temp; + int irq, head, tail = 0, warning = 0; + unsigned long flags; + int pos; u16 control; if (!pci_msi_enable) @@ -851,6 +826,9 @@ void pci_disable_msix(struct pci_dev* de if (!dev) return; + if (!dev->msix_enabled) + return; + pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); if (!pos) return; @@ -861,31 +839,25 @@ void pci_disable_msix(struct pci_dev* de disable_msi_mode(dev, pos, PCI_CAP_ID_MSIX); - temp = dev->irq; - if (!msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) { - int irq, head, tail = 0, warning = 0; - unsigned long flags; - - irq = head = dev->irq; - dev->irq = temp; /* Restore pin IRQ */ - while (head != tail) { - spin_lock_irqsave(&msi_lock, flags); - tail = msi_desc[irq]->link.tail; - spin_unlock_irqrestore(&msi_lock, flags); - if (irq_has_action(irq)) - warning = 1; - else if (irq != head) /* Release MSI-X irq */ - msi_free_irq(dev, irq); - irq = tail; - } - msi_free_irq(dev, irq); - if (warning) { - printk(KERN_WARNING "PCI: %s: pci_disable_msix() called without " - "free_irq() on all MSI-X irqs\n", - pci_name(dev)); - BUG_ON(warning > 0); - } + irq = head = dev->first_msi_irq; + while (head != tail) { + spin_lock_irqsave(&msi_lock, flags); + tail = msi_desc[irq]->link.tail; + spin_unlock_irqrestore(&msi_lock, flags); + if (irq_has_action(irq)) + warning = 1; + else if (irq != head) /* Release MSI-X irq */ + msi_free_irq(dev, irq); + irq = tail; + } + msi_free_irq(dev, irq); + if (warning) { + printk(KERN_WARNING "PCI: %s: pci_disable_msix() called without " + "free_irq() on all MSI-X irqs\n", + pci_name(dev)); + BUG_ON(warning > 0); } + dev->first_msi_irq = 0; } /** @@ -899,30 +871,28 @@ void pci_disable_msix(struct pci_dev* de **/ void msi_remove_pci_irq_vectors(struct pci_dev* dev) { - int pos, temp; + int pos; unsigned long flags; if (!pci_msi_enable || !dev) return; - temp = dev->irq; /* Save IOAPIC IRQ */ pos = pci_find_capability(dev, PCI_CAP_ID_MSI); - if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSI)) { - if (irq_has_action(dev->irq)) { + if (pos > 0 && dev->msi_enabled) { + if (irq_has_action(dev->first_msi_irq)) { printk(KERN_WARNING "PCI: %s: msi_remove_pci_irq_vectors() " "called without free_irq() on MSI irq %d\n", - pci_name(dev), dev->irq); - BUG_ON(irq_has_action(dev->irq)); + pci_name(dev), dev->first_msi_irq); + BUG_ON(irq_has_action(dev->first_msi_irq)); } else /* Release MSI irq assigned to this device */ - msi_free_irq(dev, dev->irq); - dev->irq = temp; /* Restore IOAPIC IRQ */ + msi_free_irq(dev, dev->first_msi_irq); } pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); - if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) { + if (pos > 0 && dev->msix_enabled) { int irq, head, tail = 0, warning = 0; void __iomem *base = NULL; - irq = head = dev->irq; + irq = head = dev->first_msi_irq; while (head != tail) { spin_lock_irqsave(&msi_lock, flags); tail = msi_desc[irq]->link.tail; @@ -942,7 +912,6 @@ void msi_remove_pci_irq_vectors(struct p pci_name(dev)); BUG_ON(warning > 0); } - dev->irq = temp; /* Restore IOAPIC IRQ */ } } --- gregkh-2.6.orig/include/linux/pci.h +++ gregkh-2.6/include/linux/pci.h @@ -174,6 +174,9 @@ struct pci_dev { struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */ int rom_attr_enabled; /* has display of the rom attribute been enabled? */ struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */ +#ifdef CONFIG_PCI_MSI + unsigned int first_msi_irq; +#endif }; #define pci_dev_g(n) list_entry(n, struct pci_dev, global_list)