All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
To: <linux-pci@vger.kernel.org>
Cc: "Bjorn Helgaas" <helgaas@kernel.org>,
	"Lukas Wunner" <lukas@wunner.de>, "Stefan Roese" <sr@denx.de>,
	"Andy Lavr" <andy.lavr@gmail.com>,
	"Christian König" <christian.koenig@amd.com>,
	"David Laight" <David.Laight@ACULAB.COM>,
	"Rajat Jain" <rajatja@google.com>,
	linux@yadro.com,
	"Sergei Miroshnichenko" <s.miroshnichenko@yadro.com>,
	"Srinath Mannam" <srinath.mannam@broadcom.com>,
	"Marta Rybczynska" <mrybczyn@kalray.eu>
Subject: [PATCH v9 01/26] PCI: Fix race condition in pci_enable/disable_device()
Date: Fri, 18 Dec 2020 20:39:46 +0300	[thread overview]
Message-ID: <20201218174011.340514-2-s.miroshnichenko@yadro.com> (raw)
In-Reply-To: <20201218174011.340514-1-s.miroshnichenko@yadro.com>

This is a yet another approach to fix an old [1-2] concurrency issue, when:
 - two or more devices are being hot-added into a bridge which was
   initially empty;
 - a bridge with two or more devices is being hot-added;
 - the BIOS/bootloader/firmware doesn't pre-enable bridges during boot;

The problem is that a bridge is reported as enabled before the MEM/IO bits
are actually written to the PCI_COMMAND register, so another driver thread
starts memory requests through the not-yet-enabled bridge:

 CPU0                                        CPU1

 pci_enable_device_mem()                     pci_enable_device_mem()
   pci_enable_bridge()                         pci_enable_bridge()
     pci_is_enabled()
       return false;
     atomic_inc_return(enable_cnt)
     Start actual enabling the bridge
     ...                                         pci_is_enabled()
     ...                                           return true;
     ...                                     Start memory requests <-- FAIL
     ...
     Set the PCI_COMMAND_MEMORY bit <-- Must wait for this

Protect the pci_enable_bridge(), similarly to the previous solution from
commit 40f11adc7cd9 ("PCI: Avoid race while enabling upstream bridges"),
but adding per-device mutexes.

To prevent false positives from the lockdep, use a lock_nested() with a
"depth" of a bridge within the PCI topology.

CC: Srinath Mannam <srinath.mannam@broadcom.com>
CC: Marta Rybczynska <mrybczyn@kalray.eu>
Signed-off-by: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>

[1] https://lore.kernel.org/linux-pci/1501858648-22228-1-git-send-email-srinath.mannam@broadcom.com/T/#u
    [RFC PATCH v3] pci: Concurrency issue during pci enable bridge

[2] https://lore.kernel.org/linux-pci/744877924.5841545.1521630049567.JavaMail.zimbra@kalray.eu/T/#u
    [RFC PATCH] nvme: avoid race-conditions when enabling devices
---
 drivers/pci/pci.c   | 16 ++++++++++++++++
 drivers/pci/probe.c |  1 +
 include/linux/pci.h |  1 +
 3 files changed, 18 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b9fecc25d213..076b908127fe 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1844,11 +1844,25 @@ int pci_reenable_device(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pci_reenable_device);
 
+#ifdef CONFIG_PROVE_LOCKING
+static int pci_bridge_depth(struct pci_dev *dev)
+{
+	struct pci_dev *bridge = pci_upstream_bridge(dev);
+
+	if (!bridge)
+		return 0;
+
+	return 1 + pci_bridge_depth(bridge);
+}
+#endif /* CONFIG_PROVE_LOCKING */
+
 static void pci_enable_bridge(struct pci_dev *dev)
 {
 	struct pci_dev *bridge;
 	int retval;
 
+	mutex_lock_nested(&dev->enable_mutex, pci_bridge_depth(dev));
+
 	bridge = pci_upstream_bridge(dev);
 	if (bridge)
 		pci_enable_bridge(bridge);
@@ -1856,6 +1870,7 @@ static void pci_enable_bridge(struct pci_dev *dev)
 	if (pci_is_enabled(dev)) {
 		if (!dev->is_busmaster)
 			pci_set_master(dev);
+		mutex_unlock(&dev->enable_mutex);
 		return;
 	}
 
@@ -1864,6 +1879,7 @@ static void pci_enable_bridge(struct pci_dev *dev)
 		pci_err(dev, "Error enabling bridge (%d), continuing\n",
 			retval);
 	pci_set_master(dev);
+	mutex_unlock(&dev->enable_mutex);
 }
 
 static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 953f15abc850..2f9631287719 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2240,6 +2240,7 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 	INIT_LIST_HEAD(&dev->bus_list);
 	dev->dev.type = &pci_dev_type;
 	dev->bus = pci_bus_get(bus);
+	mutex_init(&dev->enable_mutex);
 
 	return dev;
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b32126d26997..81d54889bd51 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -455,6 +455,7 @@ struct pci_dev {
 	unsigned int	no_command_memory:1;	/* No PCI_COMMAND_MEMORY */
 	pci_dev_flags_t dev_flags;
 	atomic_t	enable_cnt;	/* pci_enable_device has been called */
+	struct mutex	enable_mutex;
 
 	u32		saved_config_space[16]; /* Config space saved at suspend time */
 	struct hlist_head saved_cap_space;
-- 
2.24.1


  reply	other threads:[~2020-12-18 17:42 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-18 17:39 [PATCH v9 00/26] PCI: Allow BAR movement during boot and hotplug Sergei Miroshnichenko
2020-12-18 17:39 ` Sergei Miroshnichenko [this message]
2020-12-19 19:01   ` [PATCH v9 01/26] PCI: Fix race condition in pci_enable/disable_device() kernel test robot
2020-12-28 15:37   ` Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 02/26] PCI: Ensure a bridge has I/O and MEM access for hot-added devices Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 03/26] PCI: hotplug: Initial support of the movable BARs feature Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 04/26] PCI: Add version of release_child_resources() aware of fixed BARs Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 05/26] PCI: hotplug: Fix reassigning the released BARs Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 06/26] PCI: hotplug: Recalculate every bridge window during rescan Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 07/26] PCI: hotplug: Don't allow hot-added devices to steal resources Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 08/26] PCI: Reassign BARs if BIOS/bootloader had assigned not all of them Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 09/26] PCI: Movable BARs: Make just a single attempt to assign bridge resources Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 10/26] PCI: hotplug: Calculate fixed parts of bridge windows Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 11/26] PCI: Include fixed BARs into the bus size calculating Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 12/26] PCI: Make sure bridge windows include their fixed BARs Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 13/26] PCI: hotplug: Add support of fixed BARs to pci_assign_resource() Sergei Miroshnichenko
2020-12-18 17:39 ` [PATCH v9 14/26] PCI: hotplug: Sort fixed BARs before assignment Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 15/26] x86/PCI/ACPI: Fix up PCIBIOS_MIN_MEM if value computed from e820 is invalid Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 16/26] PCI: hotplug: Configure MPS after manual bus rescan Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 17/26] PCI: hotplug: Don't disable the released bridge windows immediately Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 18/26] PCI: pciehp: Trigger a domain rescan on hp events when enabled movable BARs Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 19/26] PCI: Don't claim fixed BARs Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 20/26] PCI: hotplug: Retry bus assignment without reserved space Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 21/26] PCI: Rescan the bus to resize a BAR Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 22/26] PCI: hotplug: Enable the movable BARs feature by default Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 23/26] PCI/portdrv: Declare support of movable BARs Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 24/26] nvme-pci: Handle " Sergei Miroshnichenko
2020-12-18 17:40   ` Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 25/26] PCI: Add a message for updating BARs Sergei Miroshnichenko
2020-12-18 17:40 ` [PATCH v9 26/26] resource: increase max nesting level for /proc/iomem Sergei Miroshnichenko
2021-01-28 14:53 ` [PATCH v9 00/26] PCI: Allow BAR movement during boot and hotplug Bjorn Helgaas
2021-01-28 20:39   ` Lukas Wunner
2021-02-01 12:55     ` Mika Westerberg
2021-02-03 20:17       ` Sergei Miroshnichenko
2021-02-04 10:49         ` mika.westerberg
2021-02-10 19:40           ` Sergei Miroshnichenko
2021-02-10 21:46             ` Lukas Wunner
2021-02-11 11:39             ` mika.westerberg
2021-02-11 17:45               ` Sergei Miroshnichenko
2021-02-12 12:52                 ` mika.westerberg
2021-02-12 20:54                   ` Sergei Miroshnichenko
2021-02-15 14:56                     ` mika.westerberg
2021-02-03 20:01     ` Sergei Miroshnichenko
2021-02-04  9:34       ` David Laight
2021-02-03 19:59   ` Sergei Miroshnichenko
2021-02-04  8:26     ` Hinko Kocevar

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=20201218174011.340514-2-s.miroshnichenko@yadro.com \
    --to=s.miroshnichenko@yadro.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=andy.lavr@gmail.com \
    --cc=christian.koenig@amd.com \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@yadro.com \
    --cc=lukas@wunner.de \
    --cc=mrybczyn@kalray.eu \
    --cc=rajatja@google.com \
    --cc=sr@denx.de \
    --cc=srinath.mannam@broadcom.com \
    /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.