Re: [PATCH] rust: pci: rework device enabling API
Maurice Hieronymus <[email protected]> Thu, 30 Jul 2026 15:25:58 +0200
| Newsgroups | dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,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