Linux EDAC development
 help / color / mirror / Atom feed
From: Maurice Hieronymus <mhi@mailbox.org>
To: "Edward Cree" <ecree.xilinx@gmail.com>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Justin Tee" <justin.tee@broadcom.com>,
	"Paul Ely" <paul.ely@broadcom.com>,
	"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	"Juergen Gross" <jgross@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Borislav Petkov" <bp@alien8.de>,
	"Tony Luck" <tony.luck@intel.com>
Cc: Danilo Krummrich <dakr@kernel.org>,
	rust-for-linux@vger.kernel.org,  netdev@vger.kernel.org,
	linux-net-drivers@amd.com,  linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org,  linux-scsi@vger.kernel.org,
	xen-devel@lists.xenproject.org,  linux-edac@vger.kernel.org,
	Lukas Wunner <lukas@wunner.de>,
	 Maurice Hieronymus <mhi@mailbox.org>
Subject: [PATCH v2] PCI: Move pci_dev->is_busmaster into priv_flags
Date: Tue, 14 Jul 2026 21:37:07 +0200	[thread overview]
Message-ID: <20260714-pci-dev-flags-v2-1-a1d7dc441cf3@mailbox.org> (raw)

`is_busmaster` is one bit of a ~60-bit C bitfield in `struct pci_dev`.
Bits sharing a bitfield word must not be modified concurrently, but its
writers take no common lock: `pci_set_master()` can run without the
device lock (e.g. from runtime PM resume paths), `pci_disable_device()`
clears the bit, and other bits in the same word are written from
entirely different contexts, e.g. `broken_parity_status` from sysfs.
Concurrent read-modify-write cycles of the shared word can then lose
updates.

Move `is_busmaster` into the existing `priv_flags` bitmap, which is
modified with atomic bitops. The bit definition stays private to
drivers/pci; xen-pciback, lpfc and sfc access the flag through the new
exported accessors `pci_dev_is_busmaster()` and
`pci_dev_assign_busmaster()`.

This also unblocks the Rust device enabling API rework [1], where a
guard object calls `pci_disable_device()` from contexts that may run
concurrently with `pci_set_master()`.

Link: https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org/ [1]
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Suggested-by: Lukas Wunner <lukas@wunner.de>
Cc: rust-for-linux@vger.kernel.org
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
`struct pci_dev` keeps ~60 flags in one C bitfield. Bits sharing a
word must not be modified concurrently, but several writers take no
common lock: `pci_set_master()` writes `is_busmaster` and can run
without the device lock (e.g. runtime PM resume paths),
`pci_disable_device()` clears it, and `broken_parity_status_store()`
writes the same word from sysfs at any time without any lock.

Move `is_busmaster` into the existing `priv_flags` bitmap, which is
modified with atomic bitops. The bit definition stays private to
drivers/pci; the drivers that use the flag (xen-pciback, lpfc, sfc)
access it through new exported accessor functions.

This is a prerequisite for the Rust device enabling API rework [1]:
the guard object planned there calls `pci_disable_device()` from
contexts that may run concurrently with `pci_set_master()`.
---
Changes in v2:
- Move the bit into the existing priv_flags bitmap instead of adding a
  new public flags bitmap (Lukas). The bit definition stays private to
  drivers/pci; outside users go through new exported accessors.
- Drop the broken_parity_status conversion (Lukas).
- Link to v1: https://lore.kernel.org/r/20260711-pci-dev-flags-v1-0-2fcf2811138c@mailbox.org
---
 drivers/net/ethernet/sfc/falcon/farch.c     |  2 +-
 drivers/net/ethernet/sfc/siena/farch.c      |  2 +-
 drivers/pci/pci-driver.c                    |  2 +-
 drivers/pci/pci.c                           | 33 ++++++++++++++++++++++++++---
 drivers/pci/pci.h                           |  1 +
 drivers/scsi/lpfc/lpfc_init.c               |  4 ++--
 drivers/xen/xen-pciback/conf_space_header.c |  4 ++--
 drivers/xen/xen-pciback/pciback_ops.c       |  4 ++--
 include/linux/pci.h                         |  3 ++-
 9 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon/farch.c b/drivers/net/ethernet/sfc/falcon/farch.c
index 23d507a3820d..c73db6081190 100644
--- a/drivers/net/ethernet/sfc/falcon/farch.c
+++ b/drivers/net/ethernet/sfc/falcon/farch.c
@@ -724,7 +724,7 @@ int ef4_farch_fini_dmaq(struct ef4_nic *efx)
 	/* Do not attempt to write to the NIC during EEH recovery */
 	if (efx->state != STATE_RECOVERY) {
 		/* Only perform flush if DMA is enabled */
-		if (efx->pci_dev->is_busmaster) {
+		if (pci_dev_is_busmaster(efx->pci_dev)) {
 			efx->type->prepare_flush(efx);
 			rc = ef4_farch_do_flush(efx);
 			efx->type->finish_flush(efx);
diff --git a/drivers/net/ethernet/sfc/siena/farch.c b/drivers/net/ethernet/sfc/siena/farch.c
index 7613d7988894..815487b1b227 100644
--- a/drivers/net/ethernet/sfc/siena/farch.c
+++ b/drivers/net/ethernet/sfc/siena/farch.c
@@ -723,7 +723,7 @@ int efx_farch_fini_dmaq(struct efx_nic *efx)
 	/* Do not attempt to write to the NIC during EEH recovery */
 	if (efx->state != STATE_RECOVERY) {
 		/* Only perform flush if DMA is enabled */
-		if (efx->pci_dev->is_busmaster) {
+		if (pci_dev_is_busmaster(efx->pci_dev)) {
 			efx->type->prepare_flush(efx);
 			rc = efx_farch_do_flush(efx);
 			efx->type->finish_flush(efx);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f36778e62ac1..f3b2255258df 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -649,7 +649,7 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
 	 * if the device was busmaster before the suspend, make it busmaster
 	 * again
 	 */
-	if (pci_dev->is_busmaster)
+	if (pci_dev_is_busmaster(pci_dev))
 		pci_set_master(pci_dev);
 
 	return retval;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..80c962811737 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2045,7 +2045,7 @@ static void pci_enable_bridge(struct pci_dev *dev)
 		pci_enable_bridge(bridge);
 
 	if (pci_is_enabled(dev)) {
-		if (!dev->is_busmaster)
+		if (!pci_dev_is_busmaster(dev))
 			pci_set_master(dev);
 		return;
 	}
@@ -2205,7 +2205,7 @@ void pci_disable_device(struct pci_dev *dev)
 
 	do_pci_disable_device(dev);
 
-	dev->is_busmaster = 0;
+	pci_dev_assign_busmaster(dev, false);
 }
 EXPORT_SYMBOL(pci_disable_device);
 
@@ -4106,6 +4106,33 @@ void pci_unmap_iospace(struct resource *res)
 }
 EXPORT_SYMBOL(pci_unmap_iospace);
 
+/**
+ * pci_dev_is_busmaster - Query the bus mastering bookkeeping flag
+ * @pdev: the PCI device to query
+ *
+ * Returns: the current value of the bookkeeping flag; the PCI_COMMAND
+ * register is not consulted.
+ */
+bool pci_dev_is_busmaster(const struct pci_dev *pdev)
+{
+	return test_bit(PCI_DEV_BUSMASTER, &pdev->priv_flags);
+}
+EXPORT_SYMBOL(pci_dev_is_busmaster);
+
+/**
+ * pci_dev_assign_busmaster - Set the bus mastering bookkeeping flag
+ * @pdev: the PCI device
+ * @busmaster: new flag value
+ *
+ * Only updates the bookkeeping flag; the PCI_COMMAND register is left
+ * untouched.
+ */
+void pci_dev_assign_busmaster(struct pci_dev *pdev, bool busmaster)
+{
+	assign_bit(PCI_DEV_BUSMASTER, &pdev->priv_flags, busmaster);
+}
+EXPORT_SYMBOL(pci_dev_assign_busmaster);
+
 static void __pci_set_master(struct pci_dev *dev, bool enable)
 {
 	u16 old_cmd, cmd;
@@ -4120,7 +4147,7 @@ static void __pci_set_master(struct pci_dev *dev, bool enable)
 			enable ? "enabling" : "disabling");
 		pci_write_config_word(dev, PCI_COMMAND, cmd);
 	}
-	dev->is_busmaster = enable;
+	pci_dev_assign_busmaster(dev, enable);
 }
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..faa4a676e62c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -773,6 +773,7 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
 #define PCI_LINK_CHANGING 5
 #define PCI_LINK_LBMS_SEEN	6
 #define PCI_DEV_ALLOW_BINDING 7
+#define PCI_DEV_BUSMASTER 8
 
 static inline void pci_dev_assign_added(struct pci_dev *dev)
 {
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 82af59c913e9..657453e6d7d2 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -14398,7 +14398,7 @@ lpfc_io_slot_reset_s3(struct pci_dev *pdev)
 
 	pci_restore_state(pdev);
 
-	if (pdev->is_busmaster)
+	if (pci_dev_is_busmaster(pdev))
 		pci_set_master(pdev);
 
 	spin_lock_irq(&phba->hbalock);
@@ -15251,7 +15251,7 @@ lpfc_io_slot_reset_s4(struct pci_dev *pdev)
 	 */
 	pci_save_state(pdev);
 
-	if (pdev->is_busmaster)
+	if (pci_dev_is_busmaster(pdev))
 		pci_set_master(pdev);
 
 	spin_lock_irq(&phba->hbalock);
diff --git a/drivers/xen/xen-pciback/conf_space_header.c b/drivers/xen/xen-pciback/conf_space_header.c
index 8b50cbcbdfe1..ecc9adba369c 100644
--- a/drivers/xen/xen-pciback/conf_space_header.c
+++ b/drivers/xen/xen-pciback/conf_space_header.c
@@ -81,10 +81,10 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
 			dev_data->enable_intx = 0;
 	}
 
-	if (!dev->is_busmaster && is_master_cmd(value)) {
+	if (!pci_dev_is_busmaster(dev) && is_master_cmd(value)) {
 		dev_dbg(&dev->dev, "set bus master\n");
 		pci_set_master(dev);
-	} else if (dev->is_busmaster && !is_master_cmd(value)) {
+	} else if (pci_dev_is_busmaster(dev) && !is_master_cmd(value)) {
 		dev_dbg(&dev->dev, "clear bus master\n");
 		pci_clear_master(dev);
 	}
diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
index bfc186bf05bc..01f4705421c9 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -125,14 +125,14 @@ void xen_pcibk_reset_device(struct pci_dev *dev)
 		if (pci_is_enabled(dev))
 			pci_disable_device(dev);
 
-		dev->is_busmaster = 0;
+		pci_dev_assign_busmaster(dev, false);
 	} else {
 		pci_read_config_word(dev, PCI_COMMAND, &cmd);
 		if (cmd & (PCI_COMMAND_INVALIDATE)) {
 			cmd &= ~(PCI_COMMAND_INVALIDATE);
 			pci_write_config_word(dev, PCI_COMMAND, cmd);
 
-			dev->is_busmaster = 0;
+			pci_dev_assign_busmaster(dev, false);
 		}
 	}
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..0542221ad996 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -461,7 +461,6 @@ struct pci_dev {
 	unsigned int	pref_64_window:1;	/* Pref mem window is 64-bit */
 	unsigned int	multifunction:1;	/* Multi-function device */
 
-	unsigned int	is_busmaster:1;		/* Is busmaster */
 	unsigned int	no_msi:1;		/* May not use MSI */
 	unsigned int	block_cfg_access:1;	/* Config space access blocked */
 	unsigned int	broken_parity_status:1;	/* Generates false positive parity */
@@ -1446,6 +1445,8 @@ void pci_disable_device(struct pci_dev *dev);
 extern unsigned int pcibios_max_latency;
 void pci_set_master(struct pci_dev *dev);
 void pci_clear_master(struct pci_dev *dev);
+bool pci_dev_is_busmaster(const struct pci_dev *pdev);
+void pci_dev_assign_busmaster(struct pci_dev *pdev, bool busmaster);
 
 int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
 int pci_set_cacheline_size(struct pci_dev *dev);

---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260711-pci-dev-flags-fbbcf4ff9031

Best regards,
-- 
Maurice Hieronymus <mhi@mailbox.org>


             reply	other threads:[~2026-07-14 19:37 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14 19:37 Maurice Hieronymus [this message]
2026-07-15  4:54 ` [PATCH v2] PCI: Move pci_dev->is_busmaster into priv_flags Lukas Wunner

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=20260714-pci-dev-flags-v2-1-a1d7dc441cf3@mailbox.org \
    --to=mhi@mailbox.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=bp@alien8.de \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=edumazet@google.com \
    --cc=gary@garyguo.net \
    --cc=jgross@suse.com \
    --cc=justin.tee@broadcom.com \
    --cc=kuba@kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-net-drivers@amd.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lukas@wunner.de \
    --cc=martin.petersen@oracle.com \
    --cc=netdev@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=pabeni@redhat.com \
    --cc=paul.ely@broadcom.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sstabellini@kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=tony.luck@intel.com \
    --cc=work@onurozkan.dev \
    --cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox