All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Edworthy <phil.edworthy@renesas.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: <linux-pci@vger.kernel.org>, <linux-renesas-soc@vger.kernel.org>,
	Phil Edworthy <phil.edworthy@renesas.com>
Subject: [RFC] pci: Provide a domain limited version of pdev_fixup_irq
Date: Fri,  8 Jul 2016 12:49:55 +0100	[thread overview]
Message-ID: <1467978595-25386-1-git-send-email-phil.edworthy@renesas.com> (raw)

Hi Bjorn,

I've marked this as RFC as I guess there might be a better way to do
this, but I'm not sure how. I would appreciate your thoughts on this.

Thanks
Phil

---
pdev_fixup_irq() performs interrupt swizzling on all PCI devices,
no matter that the device may be on a completely unrelated PCI
Host controller.

When you have multiple PCI Host controllers, pdev_fixup_irq() can
clobber the dev->irq of devices it should not be changing.
This has been seen when performing suspend/resume on boards with
two R-Car PCIe Host controllers, resulting in a NULL ptr access
in __pci_restore_msi_state. This happens because dev->irq has been
overwritten with 0, and irq_get_msi_desc(dev->irq) returns NULL.

This patch introduces a new function, pci_fixup_irqs_local(), that
performs the same operation as pdev_fixup_irq(), but only changes
the dev->irq of device on the same domain.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/setup-irq.c | 25 ++++++++++++++++++++++---
 include/linux/pci.h     |  3 +++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
index 95c225b..90ea8fa 100644
--- a/drivers/pci/setup-irq.c
+++ b/drivers/pci/setup-irq.c
@@ -22,7 +22,8 @@ void __weak pcibios_update_irq(struct pci_dev *dev, int irq)
 	pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
 }
 
-static void pdev_fixup_irq(struct pci_dev *dev,
+static void pdev_fixup_irq(int domain_nr,
+			   struct pci_dev *dev,
 			   u8 (*swizzle)(struct pci_dev *, u8 *),
 			   int (*map_irq)(const struct pci_dev *, u8, u8))
 {
@@ -48,8 +49,15 @@ static void pdev_fixup_irq(struct pci_dev *dev,
 		if (irq == -1)
 			irq = 0;
 	}
-	dev->irq = irq;
+	/* Since pci_fixup_irqs() can be called more than once due to multiple
+	 * host controllers, and we scan all PCI devices, not just those
+	 * attached to this controller, make sure we don't clobber dev->irq
+	 * that has nothing to do with this domain.
+	 */
+	if (domain_nr >= 0 && dev->bus->domain_nr != domain_nr)
+		return;
 
+	dev->irq = irq;
 	dev_dbg(&dev->dev, "fixup irq: got %d\n", dev->irq);
 
 	/* Always tell the device, so the driver knows what is
@@ -63,6 +71,17 @@ void pci_fixup_irqs(u8 (*swizzle)(struct pci_dev *, u8 *),
 	struct pci_dev *dev = NULL;
 
 	for_each_pci_dev(dev)
-		pdev_fixup_irq(dev, swizzle, map_irq);
+		pdev_fixup_irq(-1, dev, swizzle, map_irq);
 }
 EXPORT_SYMBOL_GPL(pci_fixup_irqs);
+
+void pci_fixup_irqs_local(struct pci_bus *bus,
+		    u8 (*swizzle)(struct pci_dev *, u8 *),
+		    int (*map_irq)(const struct pci_dev *, u8, u8))
+{
+	struct pci_dev *dev = NULL;
+
+	for_each_pci_dev(dev)
+		pdev_fixup_irq(bus->domain_nr, dev, swizzle, map_irq);
+}
+EXPORT_SYMBOL_GPL(pci_fixup_irqs_local);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8badb66..37a97df 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1134,6 +1134,9 @@ void pdev_enable_device(struct pci_dev *);
 int pci_enable_resources(struct pci_dev *, int mask);
 void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
 		    int (*)(const struct pci_dev *, u8, u8));
+void pci_fixup_irqs_local(struct pci_bus *,
+		    u8 (*)(struct pci_dev *, u8 *),
+		    int (*)(const struct pci_dev *, u8, u8));
 #define HAVE_PCI_REQ_REGIONS	2
 int __must_check pci_request_regions(struct pci_dev *, const char *);
 int __must_check pci_request_regions_exclusive(struct pci_dev *, const char *);
-- 
2.5.0

             reply	other threads:[~2016-07-08 11:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-08 11:49 Phil Edworthy [this message]
2016-07-11  8:39 ` [RFC] pci: Provide a domain limited version of pdev_fixup_irq Phil Edworthy
2016-07-21  9:35 ` Lorenzo Pieralisi
2016-08-08  9:31   ` Phil Edworthy
2016-08-08 23:19     ` Bjorn Helgaas
2016-08-09  3:40       ` Matthew Minter
2016-08-09  9:14       ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1467978595-25386-1-git-send-email-phil.edworthy@renesas.com \
    --to=phil.edworthy@renesas.com \
    --cc=bhelgaas@google.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.