Re: [PATCH] rust: pci: rework device enabling API

Maurice Hieronymus <[email protected]>
Newsgroups org.freedesktop.lists.dri-devel,dev.linux.lists.nova-gpu,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Sat Jul 11, 2026 at 5:25 PM CEST, Maurice Hieronymus wrote:
> I went ahead and sent a series implementing the second option, a
> public flags bitmap with generated accessors following the driver
> core precedent, converting is_busmaster and broken_parity_status:
>
> https://lore.kernel.org/r/[email protected]

A status update on that, and two questions.

v2 [1] moved the bit into `priv_flags` instead, as Lukas preferred. He
then asked me to convert xen-pciback's two direct `dev->is_busmaster = 0`
assignments to `pci_clear_master()`, so that nothing outside drivers/pci
writes the flag [2].

That conversion is not behavior-preserving: the assignments clear the
software flag only, while `pci_clear_master()` also clears
PCI_COMMAND_MASTER in config space. Commit 7681f31ec9cd ("xen/pciback:
Don't disable PCI_COMMAND on PCI device reset.") deliberately removed a
PCI_COMMAND write from directly above the first assignment, so putting
one back at that spot looked wrong to me.

I said so in that thread on Jul 18 [3], and asked the xen maintainers
directly in a separate mail on Jul 23 [4]. Neither has had a reply, so
the C-side prerequisite has not moved since.

On proceeding on the Rust side in the meantime:

I would like to keep the enable/disable rework in the EDU series rather
than drop it. `enable_device_mem()` has no disable counterpart, so the
enable count is never dropped on unbind, and the sample does not come
back up after rmmod + modprobe.

The races the guard would expose are not introduced by this series.
`is_busmaster` shares one bitfield word in `struct pci_dev` with a
number of other flags, and those are written from unrelated contexts, so
several of them can be updated concurrently with it.
`broken_parity_status` is only the clearest example:
`broken_parity_status_store()` (drivers/pci/pci-sysfs.c) writes it from
sysfs without taking any lock at all, so userspace can already race an
`is_busmaster` update today.

So my first question: would you take the `DeviceEnableGuard` design
while that word is still a C bitfield, given that it can already be
raced from userspace today, and let the EDU sample land on top of it,
replacing `rust_driver_pci` as you asked? Or do you want the bitops
conversion merged first, in which case the series stays blocked on
linux-pci.

My second question is a behavioural one, since I do not have much
experience here: what is the accepted way forward when a list does not
respond? I do not want to resend the same question every week and turn
into noise, but I also do not want to guess at xen-pciback semantics and
break someone's setup.

[1] https://lore.kernel.org/linux-pci/[email protected]/
[2] https://lore.kernel.org/linux-pci/[email protected]/
[3] https://lore.kernel.org/linux-pci/[email protected]/
[4] https://lore.kernel.org/linux-pci/[email protected]/

Best,

Maurice
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.