[PATCH v4 0/7] samples: rust: add an EDU PCI driver sample (MMIO + IRQ + DMA)

[PATCH v4 0/7] samples: rust: add an EDU PCI driver sample (MMIO + IRQ + DMA)

[Maurice Hieronymus]

The Rust sample drivers currently exercise PCI facilities in isolation:
rust_driver_pci covers MMIO and rust_dma covers DMA, but there is no
single in-tree example that combines memory-mapped I/O, interrupts and
DMA in one driver.

This series adds one. It targets QEMU's `edu` device -- a small,
well-documented educational PCI device that supports exactly these three
facilities and ships with any recent QEMU (`-device edu`), so the sample
runs without special hardware.

The sample maps BAR0, reads the PCI configuration space, and runs a set
of MMIO self-tests (identification, liveness, factorial), allocates an
MSI vector and registers an IRQ handler, and performs a DMA round-trip.
The IRQ and DMA tests wait on a `Completion` signalled by the IRQ
handler; the factorial test polls for completion.

As requested by Danilo in the v2 review, the sample replaces
rust_driver_pci, which it covers a superset of; the removal is the
first patch.

Prerequisites that had to be implemented:

- pci: rework the device enabling API: replace enable_device_mem()
  with enable_device(), which returns a DeviceEnableGuard that
  disables the device again on drop, so the enable count stays
  balanced across unbind/rebind. This follows the design Danilo
  proposed in the review of the standalone patch [1], which this
  series absorbs.
- pci: keep `set_master()` on `Device<Core>` and require a borrowed
  `DeviceEnableGuard` for the same device, preserving callback
  synchronization while ensuring the device is enabled.
- pci: add PCI_VENDOR_ID_QEMU (0x1234) to pci_ids.h and expose it as
  Vendor::QEMU, so the sample can match the device.
- completion: add complete(), so a single Completion can be reused to
  wait for consecutive events (e.g. back-to-back DMA transfers).

The series is based on rust/rust-next.

Note: DeviceEnableGuard drops from a bound scope, so pci_disable_device()
can race the pci_dev bitfield word as discussed in [1]; that race
predates this series and is triggerable from sysfs today. The bitops
conversion is under way separately on linux-pci [3].

Tested with QEMU `-device edu`.

Not taken from the v3 review (Sashiko):

- The IRQ and DMA completion waits remain unbounded; missing
  interrupts can block probe.
- enable_device() uses pci_enable_device(), not
  pci_enable_device_mem(): it is the single enable API Danilo proposed,
  and enabling I/O decoding is harmless for memory-only devices.

[1] https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org
[2] https://lore.kernel.org/r/20260719153631.559341-1-dakr@kernel.org
[3] https://lore.kernel.org/linux-pci/20260714-pci-dev-flags-v2-1-a1d7dc441cf3@mailbox.org/

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
Changes in v4:
- Drop "rust: pci: make Vendor::from_raw() public" (and its R-b tags);
  instead add PCI_VENDOR_ID_QEMU to pci_ids.h and expose it as
  Vendor::QEMU (new patches 4-5) (Alice, John).
- New patch: pci: require a borrowed enable guard for the same device
  in `set_master()`, retaining `Device<Core>` callback synchronization;
  convert nova-core. This enforces enabling before bus-mastering
  (Danilo) while addressing the concurrency concern (Sashiko).
- pci: mark DeviceEnableGuard #[must_use] and enable_device() #[inline]
  (Sashiko, Fiona).
- completion: reference complete() from wait_for_completion()'s docs
  (Gary).
- samples/edu: turn init() and the test helpers into methods on
  EduDriverData; rename init() to selftest() (Danilo).
- samples/edu: initialize EduDriverData in a single try_pin_init!(),
  holding the IRQ vectors and registering the handler with
  irq::Registration::new() instead of pci::Device::request_irq(); run
  selftest() via pin_chain() (Danilo).
- samples/edu: make the magic and DMA base constants local to their
  users; check the magic via an IDENTIFICATION constant (Danilo).
- samples/edu: describe the DMA and test IRQ bits as fields of
  IRQ_STATUS/IRQ_RAISE/IRQ_ACK instead of a DMA_IRQ constant (Danilo).
- samples/edu: model the DMA registers as a struct accessed through
  io_project!()/io_write!().
- samples/edu: compare the liveness pattern via a Not impl on
  LIVENESS_CHECK (Danilo).
- samples/edu: replace `as` casts with safe casts (Danilo).
- samples/edu: return IrqReturn::None for interrupts that are not ours
  (Sashiko).
- samples/edu: depend on 64BIT for the 64-bit MMIO accesses (Sashiko).
- samples/edu: expand the commit message and Kconfig help to describe
  the sample's coverage, QEMU usage and the 64BIT requirement; mention
  the MAINTAINERS entry in the commit message (Sashiko).
- samples/edu: ack stale interrupts before registering the handler
  (Sashiko).
- samples/edu: spell out why the IRQ Registration is not leaked (Ewan).
- Drop the dependency on Danilo's irq Registration rework [2], now in
  mainline.
- Rebase on rust-next; adapt to register!() `base:`, Option<&IdInfo>
  in probe() and the Coherent read_val()/write_val() API.
- Link to v3: https://lore.kernel.org/r/20260812-b4-rust-pci-edu-driver-v3-0-5d0b5594e52b@mailbox.org

Changes in v3:
- Absorb the device enabling rework [1] into this series, redesigned
  around DeviceEnableGuard as proposed by Danilo; convert nova-core.
- Remove rust_driver_pci, replaced by this sample (Danilo).
- Add access to PCI configuration space (Danilo).
- Rebase on rust/rust-next on top of Danilo's irq Registration rework
  [2]; register the handler via pci::Device::request_irq().
- Use vertical import style.
- Link to v2: https://lore.kernel.org/r/20260620-b4-rust-pci-edu-driver-v2-0-6fd6684f2c14@mailbox.org

Changes in v2:
- pci: Vendor::from_raw(): collected Reviewed-by from Gary Guo;
  wrapped code identifiers in the commit message in backticks (Gary).
- pci: enable_device(): collected Reviewed-by from Fiona Behrens;
  made enable_device_mem() #[inline] and added a cross-reference to
  enable_device() in its docs (Fiona).
- completion: complete(): tightened the doc comment per Gary's review
  (emphasise "single", drop the internal-counter detail, drop the
  complete_all comparison).
- samples/edu: take &EduDriverData instead of &Arc<EduDriverData> in
  init()/test_irq()/test_dma() (Ewan Chorynski).
- samples/edu: simplify wait_until_compute_has_finished() to forward
  read_poll_timeout()'s error via inspect_err() instead of returning a
  hard-coded ETIMEDOUT (Ewan Chorynski / Miguel Ojeda).
- samples/edu: Rebased on rust/rust-next and adapt to the updated
  pci::Bar / device::Core lifetimes and pci::Driver::Data<'bound>, and
  obtain the BAR via into_devres().
- Link to v1: https://lore.kernel.org/r/20260614-b4-rust-pci-edu-driver-v1-0-e3f2471b595c@mailbox.org

---
Maurice Hieronymus (7):
      samples: rust: remove the rust_driver_pci sample
      rust: pci: rework device enabling API
      rust: pci: require an enable guard for `set_master()`
      PCI: Add QEMU vendor ID
      rust: pci: add Vendor::QEMU
      rust: completion: add complete()
      samples: rust: add EDU PCI driver sample

 MAINTAINERS                     |   2 +-
 drivers/gpu/nova-core/driver.rs |   9 +-
 include/linux/pci_ids.h         |   2 +
 rust/kernel/pci.rs              |  45 +++-
 rust/kernel/pci/id.rs           |   1 +
 rust/kernel/sync/completion.rs  |  13 +-
 samples/rust/Kconfig            |  14 +-
 samples/rust/Makefile           |   2 +-
 samples/rust/rust_driver_edu.rs | 475 ++++++++++++++++++++++++++++++++++++++++
 samples/rust/rust_driver_pci.rs | 198 -----------------
 10 files changed, 547 insertions(+), 214 deletions(-)
---
base-commit: d89e1d186ec9271bf78e62771779e9fc03bf0fc3
change-id: 20260614-b4-rust-pci-edu-driver-3e50db2dda0f

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

[PATCH v4 1/7] samples: rust: remove the rust_driver_pci sample

[Maurice Hieronymus]

The EDU sample added later in this series covers everything
rust_driver_pci demonstrates (device matching, BAR mapping, MMIO) and
exercises interrupts and DMA on top, against a device every QEMU ships.
Remove the old sample in favor of it, as requested by Danilo during
review of the EDU series.

Link: https://lore.kernel.org/rust-for-linux/DJEQ64V8HE19.2DMBHY4XRPMG6@kernel.org
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 MAINTAINERS                     |   1 -
 samples/rust/Kconfig            |  11 ---
 samples/rust/Makefile           |   1 -
 samples/rust/rust_driver_pci.rs | 198 ----------------------------------------
 4 files changed, 211 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a162e9bcab1..8f7f1e14b31e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21595,7 +21595,6 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
 F:	rust/helpers/pci.c
 F:	rust/kernel/pci.rs
 F:	rust/kernel/pci/
-F:	samples/rust/rust_driver_pci.rs
 
 PCIE BANDWIDTH CONTROLLER
 M:	Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 31d62533ef25..7fbd49df30ce 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -107,17 +107,6 @@ config SAMPLE_RUST_I2C_CLIENT
 
 	  If unsure, say N.
 
-config SAMPLE_RUST_DRIVER_PCI
-	tristate "PCI Driver"
-	depends on PCI
-	help
-	  This option builds the Rust PCI driver sample.
-
-	  To compile this as a module, choose M here:
-	  the module will be called rust_driver_pci.
-
-	  If unsure, say N.
-
 config SAMPLE_RUST_DRIVER_PLATFORM
 	tristate "Platform Driver"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index b986b681cde5..20b55e0bdbb7 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -9,7 +9,6 @@ obj-$(CONFIG_SAMPLE_RUST_DEBUGFS_SCOPED)	+= rust_debugfs_scoped.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_I2C)		+= rust_driver_i2c.o
 obj-$(CONFIG_SAMPLE_RUST_I2C_CLIENT)		+= rust_i2c_client.o
-obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_USB)		+= rust_driver_usb.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
deleted file mode 100644
index 13b035a95756..000000000000
--- a/samples/rust/rust_driver_pci.rs
+++ /dev/null
@@ -1,198 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Rust PCI driver sample (based on QEMU's `pci-testdev`).
-//!
-//! To make this driver probe, QEMU must be run with `-device pci-testdev`.
-
-use kernel::{
-    device::{
-        Bound,
-        Core, //
-    },
-    io::{
-        register,
-        register::Array,
-        Io, //
-    },
-    num::Bounded,
-    pci,
-    prelude::*, //
-};
-
-mod regs {
-    use super::*;
-
-    register! {
-        base: kernel::io::Region<END>;
-
-        pub(super) TEST(u8) @ 0x0 {
-            7:0 index => TestIndex;
-        }
-
-        pub(super) OFFSET(u32) @ 0x4 {
-            31:0 offset;
-        }
-
-        pub(super) DATA(u8) @ 0x8 {
-            7:0 data;
-        }
-
-        pub(super) COUNT(u32) @ 0xC {
-            31:0 count;
-        }
-    }
-
-    pub(super) const END: usize = 0x10;
-}
-
-type Bar0<'bound> = pci::Bar<'bound, { regs::END }>;
-
-#[derive(Copy, Clone, Debug)]
-struct TestIndex(u8);
-
-impl From<Bounded<u8, 8>> for TestIndex {
-    fn from(value: Bounded<u8, 8>) -> Self {
-        Self(value.into())
-    }
-}
-
-impl From<TestIndex> for Bounded<u8, 8> {
-    fn from(value: TestIndex) -> Self {
-        value.0.into()
-    }
-}
-
-impl TestIndex {
-    const NO_EVENTFD: Self = Self(0);
-}
-
-struct SampleDriverData<'bound> {
-    pdev: &'bound pci::Device,
-    bar: Bar0<'bound>,
-    index: TestIndex,
-}
-
-struct SampleDriver;
-
-kernel::pci_device_table!(
-    PCI_TABLE,
-    <SampleDriver as pci::Driver>::IdInfo,
-    [(
-        pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5),
-        TestIndex::NO_EVENTFD
-    )]
-);
-
-impl SampleDriverData<'_> {
-    fn testdev(index: &TestIndex, bar: &Bar0<'_>) -> Result<u32> {
-        // Select the test.
-        bar.write_reg(regs::TEST::zeroed().with_index(*index));
-
-        let offset = bar.read(regs::OFFSET).into_raw() as usize;
-        let data = bar.read(regs::DATA).into();
-
-        // Write `data` to `offset` to increase `count` by one.
-        //
-        // Note that we need `try_write8`, since `offset` can't be checked at compile-time.
-        bar.try_write8(data, offset)?;
-
-        Ok(bar.read(regs::COUNT).into())
-    }
-
-    fn config_space(pdev: &pci::Device<Bound>) {
-        let config = pdev.config_space();
-
-        // Some PCI configuration space registers.
-        register! {
-            base: pci::Normal;
-
-            VENDOR_ID(u16) @ 0x0 {
-                15:0 vendor_id;
-            }
-
-            REVISION_ID(u8) @ 0x8 {
-                7:0 revision_id;
-            }
-
-            BAR(u32)[6] @ 0x10 {
-                31:0 value;
-            }
-        }
-
-        dev_info!(
-            pdev,
-            "pci-testdev config space read8 rev ID: {:x}\n",
-            config.read(REVISION_ID).revision_id()
-        );
-
-        dev_info!(
-            pdev,
-            "pci-testdev config space read16 vendor ID: {:x}\n",
-            config.read(VENDOR_ID).vendor_id()
-        );
-
-        dev_info!(
-            pdev,
-            "pci-testdev config space read32 BAR 0: {:x}\n",
-            config.read(BAR::at(0)).value()
-        );
-    }
-}
-
-impl pci::Driver for SampleDriver {
-    type IdInfo = TestIndex;
-    type Data<'bound> = SampleDriverData<'bound>;
-
-    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
-
-    fn probe<'bound>(
-        pdev: &'bound pci::Device<Core<'_>>,
-        info: Option<&'bound Self::IdInfo>,
-    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
-        let vendor = pdev.vendor_id();
-        dev_dbg!(
-            pdev,
-            "Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n",
-            vendor,
-            pdev.device_id()
-        );
-        let info = info.ok_or(ENODEV)?;
-
-        pdev.enable_device_mem()?;
-        pdev.set_master();
-
-        let bar = pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_pci")?;
-
-        dev_info!(
-            pdev,
-            "pci-testdev data-match count: {}\n",
-            SampleDriverData::testdev(info, &bar)?
-        );
-        SampleDriverData::config_space(pdev);
-
-        Ok(SampleDriverData {
-            pdev,
-            bar,
-            index: *info,
-        })
-    }
-
-    fn unbind<'bound>(_pdev: &'bound pci::Device<Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        this.bar
-            .write_reg(regs::TEST::zeroed().with_index(this.index));
-    }
-}
-
-impl Drop for SampleDriverData<'_> {
-    fn drop(&mut self) {
-        dev_dbg!(self.pdev, "Remove Rust PCI driver sample.\n");
-    }
-}
-
-kernel::module_pci_driver! {
-    type: SampleDriver,
-    name: "rust_driver_pci",
-    authors: ["Danilo Krummrich"],
-    description: "Rust PCI driver",
-    license: "GPL v2",
-}

-- 
2.54.0

[PATCH v4 2/7] rust: pci: rework device enabling API

[Maurice Hieronymus]

`enable_device_mem()` wraps the unmanaged `pci_enable_device_mem()` and
has no disable counterpart, so the enable count is leaked on driver
unbind and the device does not come back up on a subsequent probe.

Replace it with `enable_device()`, which wraps `pci_enable_device()`
and returns a `DeviceEnableGuard<'a>`: dropping the guard runs
`pci_disable_device()`. The guard borrows the device's bound scope
(`&'a Device<Bound>`), so it cannot outlive the driver binding, and
since it is the only way to enable the device from safe code, the
enable count always stays balanced.

Obtaining the guard still requires a `&Device<Core>`, i.e. a bus
callback. Unlike `pci_enable_device_mem()`, `pci_enable_device()`
enables I/O and memory resources.

Convert nova-core, the only user of `enable_device_mem()`, storing the
guard as the last field of `NovaCore` so the device is disabled only
after the GPU teardown.

Link: https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 drivers/gpu/nova-core/driver.rs |  7 +++++--
 rust/kernel/pci.rs              | 33 ++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 560a70c4f5a7..f0aebfe705ba 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -38,6 +38,8 @@ pub(crate) struct NovaCore<'bound> {
     pub(crate) gpu: Gpu<'bound>,
     bar: pci::Bar<'bound, BAR0_SIZE>,
     bar1: pci::Bar<'bound>,
+    // Declared last so the device stays enabled until everything above is torn down.
+    _enable: pci::DeviceEnableGuard<'bound>,
 }
 
 pub(crate) struct NovaCoreDriver;
@@ -103,9 +105,10 @@ fn probe<'bound>(
         dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
 
         try_pin_init!(NovaCore {
-            _: {
-                pdev.enable_device_mem()?;
+            _enable: {
+                let enable = pdev.enable_device()?;
                 pdev.set_master();
+                enable
             },
 
             bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?,
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 769d01d23429..75efd5c5ee97 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -469,6 +469,25 @@ pub fn pci_class(&self) -> Class {
     }
 }
 
+/// A guard that keeps the device's I/O and memory resources enabled.
+///
+/// # Invariants
+///
+/// The device's enable count was incremented once for this guard; dropping the guard decrements
+/// it again.
+#[must_use = "if unused, the device will be immediately disabled"]
+pub struct DeviceEnableGuard<'a> {
+    dev: &'a Device<device::Bound>,
+}
+
+impl Drop for DeviceEnableGuard<'_> {
+    fn drop(&mut self) {
+        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`, and by the type
+        // invariant this guard holds one increment of the device's enable count.
+        unsafe { bindings::pci_disable_device(self.dev.as_raw()) };
+    }
+}
+
 impl<'a> Device<device::Core<'a>> {
     /// Returns the total number of VFs, or [`None`] if SR-IOV is not available.
     #[inline]
@@ -482,10 +501,18 @@ pub fn sriov_get_totalvfs(&self) -> Option<NonZero<u16>> {
         NonZero::new(total_vfs as u16)
     }
 
-    /// Enable memory resources for this device.
-    pub fn enable_device_mem(&self) -> Result {
+    /// Enable I/O and memory resources for this device.
+    ///
+    /// The device stays enabled for the lifetime of the returned guard; dropping the guard
+    /// disables the device again. The guard borrows the device's bound scope, so it cannot
+    /// outlive the driver binding.
+    #[inline]
+    pub fn enable_device(&self) -> Result<DeviceEnableGuard<'_>> {
         // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
-        to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) })
+        to_result(unsafe { bindings::pci_enable_device(self.as_raw()) })?;
+
+        // INVARIANT: `pci_enable_device()` succeeded, so the enable count was incremented once.
+        Ok(DeviceEnableGuard { dev: self })
     }
 
     /// Enable bus-mastering for this device.

-- 
2.54.0

[PATCH v4 3/7] rust: pci: require an enable guard for `set_master()`

[Maurice Hieronymus]

Require a borrowed `DeviceEnableGuard` for the same device before enabling
bus-mastering. Check device identity so a guard for another device cannot
satisfy this requirement.

Keep `set_master()` on `Device<Core>` to retain callback synchronization:
the guard is shareable, while `pci_set_master()` modifies device state
without locking. Convert nova-core to pass its enable guard.

Link: https://lore.kernel.org/rust-for-linux/DLNQ5C9BTIAJ.1XEWLZ13TU1VF@kernel.org
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 drivers/gpu/nova-core/driver.rs |  2 +-
 rust/kernel/pci.rs              | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index f0aebfe705ba..94f207c6078d 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -107,7 +107,7 @@ fn probe<'bound>(
         try_pin_init!(NovaCore {
             _enable: {
                 let enable = pdev.enable_device()?;
-                pdev.set_master();
+                pdev.set_master(&enable)?;
                 enable
             },
 
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 75efd5c5ee97..20f45ca960cd 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -516,10 +516,18 @@ pub fn enable_device(&self) -> Result<DeviceEnableGuard<'_>> {
     }
 
     /// Enable bus-mastering for this device.
+    ///
+    /// Returns `EINVAL` if `enable` belongs to another device.
     #[inline]
-    pub fn set_master(&self) {
-        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+    pub fn set_master(&self, enable: &DeviceEnableGuard<'_>) -> Result {
+        if self.as_raw() != enable.dev.as_raw() {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: `self` is a valid device in core context, and `enable`
+        // keeps this device enabled for the duration of the call.
         unsafe { bindings::pci_set_master(self.as_raw()) };
+        Ok(())
     }
 }
 

-- 
2.54.0

[PATCH v4 4/7] PCI: Add QEMU vendor ID

[Maurice Hieronymus]

QEMU's emulated PCI devices that predate its Red Hat vendor IDs (stdvga,
bochs-display, edu) use vendor ID 0x1234. It is not registered with
PCI-SIG, but it is a de-facto QEMU ID matched by more than one driver:
`drivers/gpu/drm/tiny/bochs.c` open-codes it, and the Rust EDU sample
needs it too.

Add `PCI_VENDOR_ID_QEMU` so these drivers can share the definition.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Suggested-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 include/linux/pci_ids.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 030c86dd6d67..cf384fd96f6a 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1754,6 +1754,8 @@
 #define PCI_DEVICE_ID_3DFX_VOODOO3	0x0005
 #define PCI_DEVICE_ID_3DFX_VOODOO5	0x0009
 
+#define PCI_VENDOR_ID_QEMU		0x1234
+
 #define PCI_VENDOR_ID_AVM		0x1244
 #define PCI_DEVICE_ID_AVM_B1		0x0700
 #define PCI_DEVICE_ID_AVM_C4		0x0800

-- 
2.54.0

[PATCH v4 5/7] rust: pci: add Vendor::QEMU

[Maurice Hieronymus]

Expose `PCI_VENDOR_ID_QEMU` as `Vendor::QEMU`, so Rust drivers can match
QEMU's 0x1234 devices without constructing a `Vendor` from a raw ID.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Suggested-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/kernel/pci/id.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
index dbaf301666e7..d17706485402 100644
--- a/rust/kernel/pci/id.rs
+++ b/rust/kernel/pci/id.rs
@@ -418,6 +418,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     CYCLADES                 = bindings::PCI_VENDOR_ID_CYCLADES,                 // 0x120e
     O2                       = bindings::PCI_VENDOR_ID_O2,                       // 0x1217
     THREEDX                  = bindings::PCI_VENDOR_ID_3DFX,                     // 0x121a
+    QEMU                     = bindings::PCI_VENDOR_ID_QEMU,                     // 0x1234
     AVM                      = bindings::PCI_VENDOR_ID_AVM,                      // 0x1244
     STALLION                 = bindings::PCI_VENDOR_ID_STALLION,                 // 0x124d
     AT                       = bindings::PCI_VENDOR_ID_AT,                       // 0x1259

-- 
2.54.0

[PATCH v4 6/7] rust: completion: add complete()

[Maurice Hieronymus]

The initial completion abstraction only added `complete_all()` and
`wait_for_completion()`. `complete_all()` marks the completion permanently
done, which makes a single `Completion` unsuitable for signalling the same
event repeatedly: once `complete_all()` has run, every subsequent
`wait_for_completion()` returns immediately without waiting.

Add `complete()`, which wakes a single waiter and increments the internal
counter by one. Paired one-to-one with `wait_for_completion()`, it allows
the same completion to be reused across multiple cycles, e.g. to wait for
consecutive DMA transfers to finish.

Acked-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/kernel/sync/completion.rs | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
index 1771bfc0ade2..d8150b7b0acb 100644
--- a/rust/kernel/sync/completion.rs
+++ b/rust/kernel/sync/completion.rs
@@ -90,6 +90,17 @@ fn as_raw(&self) -> *mut bindings::completion {
         self.inner.get()
     }
 
+    /// Signal a single task waiting on this completion.
+    ///
+    /// This method wakes up a single task waiting on this completion.
+    /// If no task is currently waiting, the next
+    /// [`Completion::wait_for_completion`] returns immediately.
+    #[inline]
+    pub fn complete(&self) {
+        // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
+        unsafe { bindings::complete(self.as_raw()) };
+    }
+
     /// Signal all tasks waiting on this completion.
     ///
     /// This method wakes up all tasks waiting on this completion; after this operation the
@@ -105,7 +116,7 @@ pub fn complete_all(&self) {
     /// This method waits for the completion of a task; it is not interruptible and there is no
     /// timeout.
     ///
-    /// See also [`Completion::complete_all`].
+    /// See also [`Completion::complete`] and [`Completion::complete_all`].
     #[inline]
     pub fn wait_for_completion(&self) {
         // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.

-- 
2.54.0

[PATCH v4 7/7] samples: rust: add EDU PCI driver sample

[Maurice Hieronymus]

Add a driver for QEMU's EDU device to demonstrate and exercise Rust PCI
driver abstractions without physical hardware. Its probe-time selftest
reads PCI config space, checks BAR MMIO and factorial computation, tests
MSI delivery through an IRQ handler, and performs a coherent DMA roundtrip.

The sample uses `pci::Driver`, `DeviceEnableGuard`, typed I/O registers,
PCI IRQ vectors, `irq::Registration`, `dma::Coherent` and `Completion` to
manage resources and wait for interrupts.

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 MAINTAINERS                     |   1 +
 samples/rust/Kconfig            |  15 ++
 samples/rust/Makefile           |   1 +
 samples/rust/rust_driver_edu.rs | 475 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 492 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8f7f1e14b31e..ec4d2cb59662 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21595,6 +21595,7 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
 F:	rust/helpers/pci.c
 F:	rust/kernel/pci.rs
 F:	rust/kernel/pci/
+F:	samples/rust/rust_driver_edu.rs
 
 PCIE BANDWIDTH CONTROLLER
 M:	Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 7fbd49df30ce..8226a21fc5e4 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -107,6 +107,21 @@ config SAMPLE_RUST_I2C_CLIENT
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_EDU
+	tristate "EDU Driver"
+	depends on PCI && 64BIT
+	help
+	  This option builds the Rust PCI driver sample for QEMU's EDU device.
+	  Start QEMU with -device edu to use this sample.
+	  The sample requires 64BIT because it accesses the device's DMA
+	  registers using 64-bit MMIO operations, which the Rust I/O
+	  abstractions only support on 64-bit kernels.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_driver_edu.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_PLATFORM
 	tristate "Platform Driver"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 20b55e0bdbb7..9b0b3d7f8cc2 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_SAMPLE_RUST_DEBUGFS_SCOPED)	+= rust_debugfs_scoped.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_I2C)		+= rust_driver_i2c.o
 obj-$(CONFIG_SAMPLE_RUST_I2C_CLIENT)		+= rust_i2c_client.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_EDU)		+= rust_driver_edu.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_USB)		+= rust_driver_usb.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
diff --git a/samples/rust/rust_driver_edu.rs b/samples/rust/rust_driver_edu.rs
new file mode 100644
index 000000000000..a83095d2a988
--- /dev/null
+++ b/samples/rust/rust_driver_edu.rs
@@ -0,0 +1,475 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust EDU driver sample (based on QEMU's `edu`).
+//!
+//! To make this driver probe, QEMU must be run with `-device edu`.
+
+use kernel::{
+    bitfield,
+    device::Bound,
+    dma::{
+        Coherent,
+        Device,
+        DmaMask, //
+    },
+    io::{
+        io_project,
+        io_write,
+        poll::read_poll_timeout,
+        register,
+        register::Array,
+        Io, //
+    },
+    irq::{
+        self,
+        Flags, //
+    },
+    num::casts::IntoSafeCast,
+    pci,
+    prelude::*,
+    sync::Completion,
+    time::Delta, //
+};
+
+const QEMU_EDU_DEVICE_ID: u32 = 0x11e8;
+
+const IRQ_MAGIC_VALUE: u8 = 42;
+
+mod regs {
+    use kernel::io::Region;
+
+    use super::*;
+
+    register! {
+        base: Region<{ END }>;
+
+        pub(super) IDENTIFICATION(u32) @ 0x0 {
+            31:24 major;
+            23:16 minor;
+            7:0 magic;
+        }
+
+        pub(super) LIVENESS_CHECK(u32) @ 0x04 {}
+
+        pub(super) FACTORIAL(u32) @ 0x08 {}
+
+        pub(super) STATUS(u32) @ 0x20 {
+            0:0 computing;
+            7:7 raise_interrupt;
+        }
+
+        pub(super) IRQ_STATUS(u32) @ 0x24 {
+            8:8 dma;
+            7:0 test;
+        }
+
+        pub(super) IRQ_RAISE(u32) @ 0x60 {
+            7:0 test;
+        }
+
+        pub(super) IRQ_ACK(u32) @ 0x64 {
+            8:8 dma;
+            7:0 test;
+        }
+
+        pub(super) DMA: Dma @ 0x80;
+    }
+
+    impl core::ops::Not for LIVENESS_CHECK {
+        type Output = Self;
+
+        fn not(self) -> Self::Output {
+            Self::from_raw(!self.into_raw())
+        }
+    }
+
+    bitfield! {
+        #[derive(FromBytes, IntoBytes)]
+        pub(super) struct DmaCommand(u64) {
+            0:0 start_transfer;
+            1:1 direction;
+            2:2 raise_irq;
+        }
+    }
+
+    /// DMA register block. Its 64-bit registers need more alignment than `Region` guarantees, so
+    /// project it with `io_project!(bar, try: DMA)`, which checks the alignment at runtime.
+    #[repr(C)]
+    #[derive(FromBytes, IntoBytes)]
+    pub(super) struct Dma {
+        pub(super) src: u64,
+        pub(super) dst: u64,
+        pub(super) count: u64,
+        pub(super) command: DmaCommand,
+    }
+
+    pub(super) const END: usize = 0xA0;
+}
+
+type Bar0<'a> = pci::Bar<'a, { regs::END }>;
+
+struct EduDriver;
+
+#[pin_data(PinnedDrop)]
+struct EduDriverData<'bound> {
+    pdev: &'bound pci::Device<Bound>,
+    #[pin]
+    irq_handler: irq::Registration<'vectors, IrqHandler<'vectors>>,
+    vectors: pci::IrqVectorRegistration<'bound>,
+    // Declared last so the device stays enabled until the IRQ handler is freed.
+    _enable: pci::DeviceEnableGuard<'bound>,
+}
+
+#[pin_data]
+struct IrqHandler<'a> {
+    pdev: &'a pci::Device,
+    bar: Bar0<'a>,
+    #[pin]
+    irq_test_completion: Completion,
+    #[pin]
+    irq_dma_completion: Completion,
+    dma: Coherent<'a, u64>,
+}
+
+impl EduDriverData<'_> {
+    fn selftest(&self) -> Result {
+        self.config_space();
+        self.magic()?;
+        self.liveness_check()?;
+        self.factorial()?;
+        self.test_irq()?;
+        self.test_dma()?;
+        Ok(())
+    }
+
+    fn config_space(&self) {
+        let pdev = self.pdev;
+        let config = pdev.config_space();
+
+        // Some PCI configuration space registers.
+        register! {
+            base: pci::Normal;
+
+            VENDOR_ID(u16) @ 0x0 {
+                15:0 vendor_id;
+            }
+
+            REVISION_ID(u8) @ 0x8 {
+                7:0 revision_id;
+            }
+
+            BAR(u32)[6] @ 0x10 {
+                31:0 value;
+            }
+        }
+
+        dev_info!(
+            pdev,
+            "config space read8 rev ID: {:x}\n",
+            config.read(REVISION_ID).revision_id()
+        );
+
+        dev_info!(
+            pdev,
+            "config space read16 vendor ID: {:x}\n",
+            config.read(VENDOR_ID).vendor_id()
+        );
+
+        dev_info!(
+            pdev,
+            "config space read32 BAR 0: {:x}\n",
+            config.read(BAR::at(0)).value()
+        );
+    }
+
+    fn magic(&self) -> Result {
+        const EXPECTED: regs::IDENTIFICATION =
+            regs::IDENTIFICATION::from_raw(0).with_const_magic::<0xed>();
+
+        let pdev = self.pdev;
+        let bar = &self.irq_handler().handler().bar;
+
+        let identification = bar.read(regs::IDENTIFICATION);
+
+        if identification.magic() != EXPECTED.magic() {
+            dev_err!(
+                pdev,
+                "magic mismatch: expected {:#x} got {:#x}\n",
+                EXPECTED.magic(),
+                identification.magic()
+            );
+            return Err(ENODEV);
+        }
+
+        dev_info!(
+            pdev,
+            "major: {:#x} minor: {:#x}\n",
+            identification.major(),
+            identification.minor()
+        );
+        Ok(())
+    }
+
+    fn liveness_check(&self) -> Result {
+        let pdev = self.pdev;
+        let bar = &self.irq_handler().handler().bar;
+
+        let pattern = regs::LIVENESS_CHECK::from_raw(0xdead_beef);
+        bar.write_reg(pattern);
+
+        let inversed = bar.read(regs::LIVENESS_CHECK);
+        if inversed != !pattern {
+            dev_err!(
+                pdev,
+                "inverse mismatch: expected {:#x} got {:#x}\n",
+                !pattern.into_raw(),
+                inversed.into_raw()
+            );
+            return Err(ENODEV);
+        }
+
+        dev_info!(pdev, "inverse test successful\n");
+        Ok(())
+    }
+
+    fn factorial(&self) -> Result {
+        let pdev = self.pdev;
+        let bar = &self.irq_handler().handler().bar;
+
+        self.wait_until_compute_has_finished()?;
+
+        bar.write(regs::FACTORIAL, 4.into());
+
+        self.wait_until_compute_has_finished()?;
+
+        let result: u32 = bar.read(regs::FACTORIAL).into();
+
+        let expected = 24;
+
+        if result != expected {
+            dev_err!(
+                pdev,
+                "factorial result wrong: expected {} got {}\n",
+                expected,
+                result
+            );
+            return Err(ENODEV);
+        }
+
+        dev_info!(pdev, "factorial test successful\n");
+        Ok(())
+    }
+
+    fn test_irq(&self) -> Result {
+        let pdev = self.pdev;
+        let handler = self.irq_handler().handler();
+
+        dev_dbg!(pdev, "raising irq\n");
+
+        handler.bar.write(
+            regs::IRQ_RAISE,
+            regs::IRQ_RAISE::zeroed().with_test(IRQ_MAGIC_VALUE),
+        );
+
+        handler.irq_test_completion.wait_for_completion();
+
+        dev_info!(pdev, "irq test successful\n");
+        Ok(())
+    }
+
+    fn test_dma(&self) -> Result {
+        let pdev = self.pdev;
+        let handler = self.irq_handler().handler();
+
+        const QEMU_DMA_BASE: u64 = 0x40000;
+
+        dev_dbg!(pdev, "testing dma\n");
+
+        let dma = &handler.dma;
+
+        const DMA_VALUE: u64 = 42;
+
+        dma.write_val(DMA_VALUE);
+
+        let dma_regs = io_project!(handler.bar, try: regs::DMA);
+
+        io_write!(dma_regs, .src, dma.dma_address());
+        io_write!(dma_regs, .dst, QEMU_DMA_BASE);
+        io_write!(dma_regs, .count, dma.size().into_safe_cast());
+        io_write!(
+            dma_regs,
+            .command,
+            regs::DmaCommand::zeroed()
+                .with_start_transfer(true)
+                .with_direction(false)
+                .with_raise_irq(true)
+        );
+
+        handler.irq_dma_completion.wait_for_completion();
+
+        // Destroy previous value to test roundtrip
+        dma.write_val(0);
+
+        io_write!(dma_regs, .src, QEMU_DMA_BASE);
+        io_write!(dma_regs, .dst, dma.dma_address());
+        io_write!(dma_regs, .count, dma.size().into_safe_cast());
+        io_write!(
+            dma_regs,
+            .command,
+            regs::DmaCommand::zeroed()
+                .with_start_transfer(true)
+                .with_direction(true)
+                .with_raise_irq(true)
+        );
+
+        handler.irq_dma_completion.wait_for_completion();
+
+        let result = dma.read_val();
+
+        if result != DMA_VALUE {
+            dev_err!(
+                pdev,
+                "dma result wrong: expected {} got {}\n",
+                DMA_VALUE,
+                result
+            );
+            return Err(ENODEV);
+        }
+
+        dev_info!(pdev, "dma test successful\n");
+        Ok(())
+    }
+
+    fn wait_until_compute_has_finished(&self) -> Result {
+        let pdev = self.pdev;
+        let bar = &self.irq_handler().handler().bar;
+
+        read_poll_timeout(
+            || Ok(bar.read(regs::STATUS)),
+            |status| status.computing() == 0,
+            Delta::from_millis(10),
+            Delta::from_millis(100),
+        )
+        .inspect_err(|_| dev_err!(pdev, "computation bit did not clear before timeout\n"))
+        .map(|_| ())
+    }
+}
+
+impl pci::Driver for EduDriver {
+    type IdInfo = ();
+    type Data<'bound> = EduDriverData<'bound>;
+
+    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
+
+    fn probe<'bound>(
+        pdev: &'bound pci::Device<kernel::device::Core<'_>>,
+        _id_info: Option<&'bound Self::IdInfo>,
+    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
+        try_pin_init!(EduDriverData {
+            _: {
+                dev_dbg!(
+                    pdev,
+                    "Probe Rust EDU driver sample (PCI ID: {}, 0x{:x}).\n",
+                    pdev.vendor_id(),
+                    pdev.device_id()
+                );
+            },
+
+            _enable: {
+                let enable = pdev.enable_device()?;
+                pdev.set_master(&enable)?;
+                enable
+            },
+
+            _: {
+                // SAFETY: There are no concurrent calls to DMA allocation and mapping primitives.
+                unsafe { pdev.dma_set_mask_and_coherent(DmaMask::new::<28>())? };
+            },
+
+            vectors: pdev
+                .alloc_irq_vectors(1, 1, pci::IrqType::Msi.into())
+                .inspect_err(|e| dev_err!(pdev, "alloc_irq_vectors failed: {:?}\n", e))?,
+
+            // SAFETY: `irq_handler` is owned by `EduDriverData`, which the driver core drops on
+            // unbind, so the Registration is not leaked.
+            irq_handler <- unsafe {
+                irq::Registration::new(
+                    vectors.index(0)?.into(),
+                    Flags::TRIGGER_NONE,
+                    c"rust_edu_irq",
+                    try_pin_init!(IrqHandler {
+                        bar: pdev.iomap_region_sized(0, c"rust_driver_edu")?,
+                        // Ack interrupts left pending by a previous binding.
+                        _: {
+                            let status = bar.read(regs::IRQ_STATUS);
+                            bar.write(regs::IRQ_ACK, regs::IRQ_ACK::from_raw(status.into_raw()));
+                        },
+                        irq_test_completion <- Completion::new(),
+                        irq_dma_completion <- Completion::new(),
+                        dma: Coherent::zeroed(pdev.as_ref(), GFP_KERNEL)?,
+                        pdev,
+                    }? Error),
+                )
+            },
+
+            pdev,
+        })
+        .pin_chain(|this| {
+            this.selftest()?;
+            dev_info!(this.pdev, "rust_driver_edu successfully initialized\n");
+            Ok(())
+        })
+    }
+}
+
+impl irq::Handler for IrqHandler<'_> {
+    fn handle(&self) -> irq::IrqReturn {
+        dev_dbg!(self.pdev, "irq handler called\n");
+        let status = self.bar.read(regs::IRQ_STATUS);
+        let mut ret = irq::IrqReturn::None;
+
+        if status.dma() != 0 {
+            dev_dbg!(self.pdev, "handling dma completion in irq\n");
+            self.bar
+                .write(regs::IRQ_ACK, regs::IRQ_ACK::zeroed().with_dma(true));
+            self.irq_dma_completion.complete();
+            ret = irq::IrqReturn::Handled;
+        }
+
+        if u8::from(status.test()) == IRQ_MAGIC_VALUE {
+            dev_dbg!(self.pdev, "handling test completion in irq\n");
+            self.bar.write(
+                regs::IRQ_ACK,
+                regs::IRQ_ACK::zeroed().with_test(IRQ_MAGIC_VALUE),
+            );
+            self.irq_test_completion.complete();
+            ret = irq::IrqReturn::Handled;
+        }
+
+        ret
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for EduDriverData<'_> {
+    fn drop(self: Pin<&mut Self>) {
+        dev_dbg!(self.pdev, "Remove Rust EDU driver sample.\n");
+    }
+}
+
+kernel::pci_device_table!(
+    PCI_TABLE,
+    <EduDriver as pci::Driver>::IdInfo,
+    [(
+        pci::DeviceId::from_id(pci::Vendor::QEMU, QEMU_EDU_DEVICE_ID),
+        ()
+    )]
+);
+
+kernel::module_pci_driver! {
+    type: EduDriver,
+    name: "rust_driver_edu",
+    authors: ["Maurice Hieronymus"],
+    description: "Rust EDU driver",
+    license: "GPL v2",
+}

-- 
2.54.0

Re: [PATCH v4 4/7] PCI: Add QEMU vendor ID

[sashiko-bot]

> QEMU's emulated PCI devices that predate its Red Hat vendor IDs (stdvga,
> bochs-display, edu) use vendor ID 0x1234. It is not registered with
> PCI-SIG, but it is a de-facto QEMU ID matched by more than one driver:
> `drivers/gpu/drm/tiny/bochs.c` open-codes it, and the Rust EDU sample
> needs it too.
> 
> Add `PCI_VENDOR_ID_QEMU` so these drivers can share the definition.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Suggested-by: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=4

Re: [PATCH v4 5/7] rust: pci: add Vendor::QEMU

[sashiko-bot]

> Expose `PCI_VENDOR_ID_QEMU` as `Vendor::QEMU`, so Rust drivers can match
> QEMU's 0x1234 devices without constructing a `Vendor` from a raw ID.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Suggested-by: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=5

Re: [PATCH v4 1/7] samples: rust: remove the rust_driver_pci sample

[sashiko-bot]

> The EDU sample added later in this series covers everything
> rust_driver_pci demonstrates (device matching, BAR mapping, MMIO) and
> exercises interrupts and DMA on top, against a device every QEMU ships.
> Remove the old sample in favor of it, as requested by Danilo during
> review of the EDU series.
> 
> Link: https://lore.kernel.org/rust-for-linux/DJEQ64V8HE19.2DMBHY4XRPMG6@kernel.org
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=1

Re: [PATCH v4 6/7] rust: completion: add complete()

[sashiko-bot]

> The initial completion abstraction only added `complete_all()` and
> `wait_for_completion()`. `complete_all()` marks the completion permanently
> done, which makes a single `Completion` unsuitable for signalling the same
> event repeatedly: once `complete_all()` has run, every subsequent
> `wait_for_completion()` returns immediately without waiting.
> 
> Add `complete()`, which wakes a single waiter and increments the internal
> counter by one. Paired one-to-one with `wait_for_completion()`, it allows
> the same completion to be reused across multiple cycles, e.g. to wait for
> consecutive DMA transfers to finish.
> 
> Acked-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=6

Re: [PATCH v4 3/7] rust: pci: require an enable guard for `set_master()`

[sashiko-bot]

> Require a borrowed `DeviceEnableGuard` for the same device before enabling
> bus-mastering. Check device identity so a guard for another device cannot
> satisfy this requirement.
> 
> Keep `set_master()` on `Device<Core>` to retain callback synchronization:
> the guard is shareable, while `pci_set_master()` modifies device state
> without locking. Convert nova-core to pass its enable guard.
> 
> Link: https://lore.kernel.org/rust-for-linux/DLNQ5C9BTIAJ.1XEWLZ13TU1VF@kernel.org
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=3

Re: [PATCH v4 2/7] rust: pci: rework device enabling API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: pci: missing #[inline] on DeviceEnableGuard::drop()
--

commit 74e19693f9a2d2ce6a2ed986dc5fdc9a88535014
Author: Maurice Hieronymus <mhi@mailbox.org>

rust: pci: rework device enabling API

This commit replaces Device::enable_device_mem() with
Device::enable_device(), returning a DeviceEnableGuard<'a> that calls
pci_disable_device() on drop to keep the PCI device enable count balanced.
It also updates the nova-core driver to store the guard in NovaCore.

Link: https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 769d01d234296..75efd5c5ee97f 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -469,6 +469,25 @@ pub fn pci_class(&self) -> Class {
>      }
>  }
>  
> +/// A guard that keeps the device's I/O and memory resources enabled.
> +///
> +/// # Invariants
> +///
> +/// The device's enable count was incremented once for this guard; dropping the guard decrements
> +/// it again.
> +#[must_use = "if unused, the device will be immediately disabled"]
> +pub struct DeviceEnableGuard<'a> {
> +    dev: &'a Device<device::Bound>,
> +}
> +
> +impl Drop for DeviceEnableGuard<'_> {
> +    fn drop(&mut self) {
> +        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`, and by the type
> +        // invariant this guard holds one increment of the device's enable count.
> +        unsafe { bindings::pci_disable_device(self.dev.as_raw()) };
> +    }
> +}

[Severity: Low]
This isn't a bug, but since DeviceEnableGuard::drop() in rust/kernel/pci.rs
is a small non-generic abstraction method in the kernel crate that forwards
directly to bindings::pci_disable_device() when a DeviceEnableGuard is
dropped on driver unbind or probe error unwind, should it be marked with
#[inline] to allow cross-crate inlining into drivers (matching
Device::enable_device() below and IrqVectorRegistration::drop() in
rust/kernel/pci/irq.rs)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=2

Re: [PATCH v4 7/7] samples: rust: add EDU PCI driver sample

[sashiko-bot]

> Add a driver for QEMU's EDU device to demonstrate and exercise Rust PCI
> driver abstractions without physical hardware. Its probe-time selftest
> reads PCI config space, checks BAR MMIO and factorial computation, tests
> MSI delivery through an IRQ handler, and performs a coherent DMA roundtrip.
> 
> The sample uses `pci::Driver`, `DeviceEnableGuard`, typed I/O registers,
> PCI IRQ vectors, `irq::Registration`, `dma::Coherent` and `Completion` to
> manage resources and wait for interrupts.
> 
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-b4-rust-pci-edu-driver-v4-0-20ef027bb707@mailbox.org?part=7