* [PATCH v2] PCI: Move pci_dev->is_busmaster into priv_flags
@ 2026-07-14 19:37 Maurice Hieronymus
2026-07-15 4:54 ` Lukas Wunner
0 siblings, 1 reply; 2+ messages in thread
From: Maurice Hieronymus @ 2026-07-14 19:37 UTC (permalink / raw)
To: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Justin Tee, Paul Ely,
James E.J. Bottomley, Martin K. Petersen, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
Borislav Petkov, Tony Luck
Cc: Danilo Krummrich, rust-for-linux, netdev, linux-net-drivers,
linux-kernel, linux-pci, linux-scsi, xen-devel, linux-edac,
Lukas Wunner, Maurice Hieronymus
`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 ++--
| 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);
--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>
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] PCI: Move pci_dev->is_busmaster into priv_flags
2026-07-14 19:37 [PATCH v2] PCI: Move pci_dev->is_busmaster into priv_flags Maurice Hieronymus
@ 2026-07-15 4:54 ` Lukas Wunner
0 siblings, 0 replies; 2+ messages in thread
From: Lukas Wunner @ 2026-07-15 4:54 UTC (permalink / raw)
To: Maurice Hieronymus
Cc: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Bjorn Helgaas, Justin Tee, Paul Ely,
James E.J. Bottomley, Martin K. Petersen, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
Borislav Petkov, Tony Luck, Danilo Krummrich, rust-for-linux,
netdev, linux-net-drivers, linux-kernel, linux-pci, linux-scsi,
xen-devel, linux-edac
On Tue, Jul 14, 2026 at 09:37:07PM +0200, Maurice Hieronymus wrote:
> +++ b/include/linux/pci.h
> @@ -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);
pci_dev_assign_busmaster() should not have public visibility.
Drivers should really use pci_set_master() / pci_clear_master()
and nothing else.
It seems Xen is the only one in the tree which needs this:
> +++ 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);
> }
> }
> }
Please change these direct assignments to pci_clear_master(),
preferably in a separate patch to ease bisecting if anything
breaks.
Then you don't need pci_dev_assign_busmaster() and can just inline
setting/clearing the bit in __pci_set_master() and pci_disable_device().
Thanks,
Lukas
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-15 4:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 19:37 [PATCH v2] PCI: Move pci_dev->is_busmaster into priv_flags Maurice Hieronymus
2026-07-15 4:54 ` Lukas Wunner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox