Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA)
@ 2026-06-20  8:45 Maurice Hieronymus
  2026-06-20  8:45 ` [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Maurice Hieronymus @ 2026-06-20  8:45 UTC (permalink / raw)
  To: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	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, Lyude Paul
  Cc: linux-pci, rust-for-linux, linux-kernel, Maurice Hieronymus,
	Fiona Behrens

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 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 -- each stage waiting on a
Completion that the IRQ handler signals.

Prerequisites, that had to be implemented:

- pci: make Vendor::from_raw() public, so a driver can match a device
  whose vendor ID has no symbolic name in pci_ids.h (QEMU's 0x1234),
  matching what C drivers already do.
- pci: add a managed enable_device() wrapping pcim_enable_device(), so
  the enable count stays balanced across unbind/rebind.
- completion: add complete(), so a single Completion can be reused to
  wait for consecutive events (e.g. back-to-back DMA transfers).

Tested with QEMU `-device edu`;

Signed-off-by: Maurice Hieronymus <mhi@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 (4):
      rust: pci: make Vendor::from_raw() public
      rust: pci: add managed Device::enable_device()
      rust: completion: add complete()
      rust: samples: add EDU PCI driver sample

 rust/kernel/pci.rs              |  16 ++
 rust/kernel/pci/id.rs           |   2 +-
 rust/kernel/sync/completion.rs  |  11 ++
 samples/rust/Kconfig            |  11 ++
 samples/rust/Makefile           |   1 +
 samples/rust/rust_driver_edu.rs | 378 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 418 insertions(+), 1 deletion(-)
---
base-commit: 43a393185e33e573a374c1d4f7ddf6481484ef8d
change-id: 20260614-b4-rust-pci-edu-driver-3e50db2dda0f

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public
  2026-06-20  8:45 [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
@ 2026-06-20  8:45 ` Maurice Hieronymus
  2026-06-20  9:48   ` Onur Özkan
  2026-06-20  8:45 ` [PATCH v2 2/4] rust: pci: add managed Device::enable_device() Maurice Hieronymus
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Maurice Hieronymus @ 2026-06-20  8:45 UTC (permalink / raw)
  To: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	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, Lyude Paul
  Cc: linux-pci, rust-for-linux, linux-kernel, Maurice Hieronymus

`Vendor::from_raw()` is currently `pub(super)`, so a Vendor can only be
obtained through the named constants generated from the
`PCI_VENDOR_ID_*` defines in `<linux/pci_ids.h>`. A driver therefore
cannot match a device whose vendor ID has no symbolic name.

Such devices exist. QEMU's "edu" educational device and the legacy
qemu/Bochs stdvga both use vendor ID 0x1234, which is not registered in
`pci_ids.h`. Per the policy stated at the top of that header, IDs are
only added there when shared between multiple drivers; a single-driver
ID is expected to be open-coded in the driver instead. C drivers already
do this -- see `drivers/gpu/drm/tiny/bochs.c`, which matches with a bare
".vendor = 0x1234".

The Rust abstraction has no equivalent escape hatch: there is no public
way to express an unregistered vendor. Make `Vendor::from_raw()` public
(and const, so it can be used in the const device-ID tables built by
`pci_device_table!`) so that drivers can construct a Vendor from a raw
ID, matching what C drivers can already do.

Reviewed-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/kernel/pci/id.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
index dbaf301666e7..fe3b0047179b 100644
--- a/rust/kernel/pci/id.rs
+++ b/rust/kernel/pci/id.rs
@@ -156,7 +156,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 impl Vendor {
     /// Create a Vendor from a raw 16-bit vendor ID.
     #[inline]
-    pub(super) fn from_raw(vendor_id: u16) -> Self {
+    pub const fn from_raw(vendor_id: u16) -> Self {
         Self(vendor_id)
     }
 

-- 
2.51.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] rust: pci: add managed Device::enable_device()
  2026-06-20  8:45 [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
  2026-06-20  8:45 ` [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
@ 2026-06-20  8:45 ` Maurice Hieronymus
  2026-06-20  9:54   ` Onur Özkan
  2026-06-20  8:45 ` [PATCH v2 3/4] rust: completion: add complete() Maurice Hieronymus
  2026-06-20  8:45 ` [PATCH v2 4/4] rust: samples: add EDU PCI driver sample Maurice Hieronymus
  3 siblings, 1 reply; 12+ messages in thread
From: Maurice Hieronymus @ 2026-06-20  8:45 UTC (permalink / raw)
  To: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	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, Lyude Paul
  Cc: linux-pci, rust-for-linux, linux-kernel, Maurice Hieronymus,
	Fiona Behrens

Add a managed counterpart to Device::enable_device_mem() that wraps
pcim_enable_device(). In addition to enabling the device, it registers a
pci_disable_device() cleanup that runs automatically when the device is
unbound from its driver, keeping the device's enable count balanced
across unbind/rebind cycles.

The existing enable_device_mem() wraps the unmanaged
pci_enable_device_mem() and has no disable counterpart, so the enable
count is leaked on unbind. On the next probe pci_enable_device_flags()
sees a non-zero enable count and returns early, skipping the power-state
transition back to D0. For a device without a PCI power management
capability the power state cannot be re-read from hardware and stays
PCI_UNKNOWN, which makes __pci_enable_msi_range() reject the subsequent
MSI allocation with -EINVAL.

Furthermore, make `enable_device_mem` inline and add a reference to
`enable_device` in the docs.

Reviewed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/kernel/pci.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5071cae6543f..d076a3691091 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -454,11 +454,27 @@ pub fn pci_class(&self) -> Class {
 
 impl<'a> Device<device::Core<'a>> {
     /// Enable memory resources for this device.
+    ///
+    /// This function is unmanaged and does not perform any cleanup when the device is unbound.
+    /// For a managed function take a look at [`Device::enable_device`].
+    #[inline]
     pub fn enable_device_mem(&self) -> Result {
         // 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()) })
     }
 
+    /// Enable I/O and memory resources for this device, with automatic cleanup.
+    ///
+    /// This is the managed version of `pci_enable_device()`: it enables the device's I/O and
+    /// memory resources and registers a `pci_disable_device()` call that runs automatically
+    /// when the device is unbound from its driver. In contrast, [`Device::enable_device_mem`]
+    /// is unmanaged and only enables memory resources.
+    #[inline]
+    pub fn enable_device(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        to_result(unsafe { bindings::pcim_enable_device(self.as_raw()) })
+    }
+
     /// Enable bus-mastering for this device.
     #[inline]
     pub fn set_master(&self) {

-- 
2.51.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/4] rust: completion: add complete()
  2026-06-20  8:45 [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
  2026-06-20  8:45 ` [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
  2026-06-20  8:45 ` [PATCH v2 2/4] rust: pci: add managed Device::enable_device() Maurice Hieronymus
@ 2026-06-20  8:45 ` Maurice Hieronymus
  2026-06-20  9:48   ` Onur Özkan
  2026-06-20  8:45 ` [PATCH v2 4/4] rust: samples: add EDU PCI driver sample Maurice Hieronymus
  3 siblings, 1 reply; 12+ messages in thread
From: Maurice Hieronymus @ 2026-06-20  8:45 UTC (permalink / raw)
  To: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	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, Lyude Paul
  Cc: linux-pci, rust-for-linux, linux-kernel, 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.

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 rust/kernel/sync/completion.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
index 35ff049ff078..a54361b53644 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

-- 
2.51.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] rust: samples: add EDU PCI driver sample
  2026-06-20  8:45 [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
                   ` (2 preceding siblings ...)
  2026-06-20  8:45 ` [PATCH v2 3/4] rust: completion: add complete() Maurice Hieronymus
@ 2026-06-20  8:45 ` Maurice Hieronymus
  2026-06-20  9:45   ` Onur Özkan
  2026-06-21 12:27   ` Danilo Krummrich
  3 siblings, 2 replies; 12+ messages in thread
From: Maurice Hieronymus @ 2026-06-20  8:45 UTC (permalink / raw)
  To: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	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, Lyude Paul
  Cc: linux-pci, rust-for-linux, linux-kernel, Maurice Hieronymus

Add a Rust sample driver for the QEMU EDU device, wired up via a new
SAMPLE_RUST_DRIVER_EDU Kconfig option and the samples Makefile.

Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
 samples/rust/Kconfig            |  11 ++
 samples/rust/Makefile           |   1 +
 samples/rust/rust_driver_edu.rs | 378 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 390 insertions(+)

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index c49ab9106345..742c42262e9b 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -118,6 +118,17 @@ config SAMPLE_RUST_DRIVER_PCI
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_EDU
+	tristate "EDU Driver"
+	depends on PCI
+	help
+	  This option builds the Rust EDU driver sample.
+
+	  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 6c0aaa58cccc..c24a328243b1 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -10,6 +10,7 @@ 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_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..5f4efd514032
--- /dev/null
+++ b/samples/rust/rust_driver_edu.rs
@@ -0,0 +1,378 @@
+// 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::{
+    device::Bound,
+    devres::Devres,
+    dma::{Coherent, Device, DmaMask},
+    io::{
+        poll::read_poll_timeout,
+        register,
+        Io, //
+    },
+    irq::{self, Flags},
+    pci::{self, IrqTypes},
+    prelude::*,
+    sync::{aref::ARef, Arc, Completion},
+    time::Delta, //
+};
+
+const QEMU_VENDOR_ID: u16 = 0x1234;
+const QEMU_EDU_DEVICE_ID: u32 = 0x11e8;
+const QEMU_EDU_DEVICE_MAGIC: u8 = 0xed;
+const QEMU_DMA_BASE: u64 = 0x40000;
+
+const IRQ_MAGIC_VALUE: u32 = 42;
+
+/// Bit set in `IRQ_STATUS` when a DMA transfer has completed.
+const DMA_IRQ: u32 = 0x100;
+
+mod regs {
+    use super::*;
+
+    register! {
+        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 {}
+        pub(super) IRQ_RAISE(u32) @ 0x60 {}
+        pub(super) IRQ_ACK(u32) @ 0x64 {}
+
+        pub(super) DMA_SRC(u64) @ 0x80 {}
+        pub(super) DMA_DST(u64) @ 0x88 {}
+        pub(super) DMA_COUNT(u64) @ 0x90 {}
+        pub(super) DMA_COMMAND(u64) @ 0x98 {
+            0:0 start_transfer;
+            1:1 direction;
+            2:2 raise_irq;
+        }
+    }
+
+    pub(super) const END: usize = 0xA0;
+}
+
+type Bar0 = pci::Bar<'static, { regs::END }>;
+
+#[pin_data(PinnedDrop)]
+struct EduDriver {
+    pdev: ARef<pci::Device>,
+    data: Arc<EduDriverData>,
+    #[pin]
+    irq_handler: irq::Registration<Arc<EduDriverData>>,
+}
+
+#[pin_data]
+struct EduDriverData {
+    #[pin]
+    bar: Devres<Bar0>,
+    #[pin]
+    irq_test_completion: Completion,
+    #[pin]
+    irq_dma_completion: Completion,
+    dma: Coherent<u64>,
+}
+
+impl EduDriver {
+    fn init(pdev: &pci::Device<Bound>, bar: &Bar0, data: &EduDriverData) -> Result {
+        Self::magic(pdev, bar)?;
+        Self::liveness_check(pdev, bar)?;
+        Self::factorial(pdev, bar)?;
+        Self::test_irq(pdev, bar, data)?;
+        Self::test_dma(pdev, bar, data)?;
+        Ok(())
+    }
+
+    fn magic(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
+        let identification = bar.read(regs::IDENTIFICATION);
+
+        let magic: u8 = identification.magic().into();
+
+        if magic != QEMU_EDU_DEVICE_MAGIC {
+            dev_err!(
+                pdev,
+                "magic mismatch: expected {:#x} got {:#x}\n",
+                QEMU_EDU_DEVICE_MAGIC,
+                magic
+            );
+            return Err(ENODEV);
+        }
+
+        dev_info!(
+            pdev,
+            "major: {:#x} minor: {:#x}\n",
+            identification.major(),
+            identification.minor()
+        );
+        Ok(())
+    }
+
+    fn liveness_check(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
+        let test_value = 0xabcd;
+
+        bar.write(regs::LIVENESS_CHECK, test_value.into());
+
+        let inverse_value = bar.read(regs::LIVENESS_CHECK).into_raw();
+
+        if inverse_value != !test_value {
+            dev_err!(
+                pdev,
+                "inverse mismatch: expected {:#x} got {:#x}\n",
+                !test_value,
+                inverse_value
+            );
+            return Err(ENODEV);
+        }
+
+        dev_info!(pdev, "inverse test successful\n");
+        Ok(())
+    }
+
+    fn factorial(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
+        Self::wait_until_compute_has_finished(pdev, bar)?;
+
+        bar.write(regs::FACTORIAL, 4.into());
+
+        Self::wait_until_compute_has_finished(pdev, bar)?;
+
+        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(pdev: &pci::Device<Bound>, bar: &Bar0, data: &EduDriverData) -> Result {
+        dev_dbg!(pdev, "raising irq\n");
+
+        bar.write(regs::IRQ_RAISE, IRQ_MAGIC_VALUE.into());
+
+        data.irq_test_completion.wait_for_completion();
+        Ok(())
+    }
+
+    fn test_dma(pdev: &pci::Device<Bound>, bar: &Bar0, data: &EduDriverData) -> Result {
+        dev_dbg!(pdev, "testing dma\n");
+
+        let dma = &data.dma;
+
+        const DMA_VALUE: u64 = 42;
+
+        kernel::dma_write!(dma, , DMA_VALUE);
+
+        bar.write(regs::DMA_SRC, dma.dma_handle().into());
+        bar.write(regs::DMA_DST, QEMU_DMA_BASE.into());
+        bar.write(regs::DMA_COUNT, (dma.size() as u64).into());
+        bar.write(
+            regs::DMA_COMMAND,
+            regs::DMA_COMMAND::zeroed()
+                .with_start_transfer(true)
+                .with_direction(false)
+                .with_raise_irq(true),
+        );
+
+        data.irq_dma_completion.wait_for_completion();
+
+        // Destroy previous value to test roundtrip
+        kernel::dma_write!(dma, , 0);
+
+        bar.write(regs::DMA_SRC, QEMU_DMA_BASE.into());
+        bar.write(regs::DMA_DST, dma.dma_handle().into());
+        bar.write(regs::DMA_COUNT, (dma.size() as u64).into());
+        bar.write(
+            regs::DMA_COMMAND,
+            regs::DMA_COMMAND::zeroed()
+                .with_start_transfer(true)
+                .with_direction(true)
+                .with_raise_irq(true),
+        );
+
+        data.irq_dma_completion.wait_for_completion();
+
+        let result = kernel::dma_read!(dma,);
+
+        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(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
+        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> = Self;
+
+    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
+
+    fn probe<'bound>(
+        pdev: &'bound pci::Device<kernel::device::Core<'_>>,
+        _id_info: &'bound Self::IdInfo,
+    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
+        pin_init::pin_init_scope(move || {
+            let vendor = pdev.vendor_id();
+            dev_dbg!(
+                pdev,
+                "Probe Rust EDU driver sample (PCI ID: {}, 0x{:x}).\n",
+                vendor,
+                pdev.device_id()
+            );
+
+            pdev.enable_device()?;
+            pdev.set_master();
+
+            let mask = DmaMask::new::<28>();
+
+            // SAFETY: There are no concurrent calls to DMA allocation and mapping primitives.
+            unsafe { pdev.dma_set_mask_and_coherent(mask)? };
+
+            let ca: Coherent<u64> = Coherent::zeroed(pdev.as_ref(), GFP_KERNEL)?;
+
+            let irq = pdev
+                .alloc_irq_vectors(1, 1, IrqTypes::default().with(pci::IrqType::Msi))
+                .inspect_err(|e| dev_err!(pdev, "alloc_irq_vectors failed: {:?}\n", e))?;
+
+            // State shared with the IRQ handler (the BAR and the completion the
+            // handler signals) lives in an `Arc<EduDriverData>`. `EduDriverData`
+            // itself implements `irq::Handler`, and the registration takes an
+            // `Arc<T>` via the `impl Handler for Arc<T>` blanket impl. This keeps
+            // the handler's state out of `EduDriver` and avoids a self-reference.
+            let data = Arc::pin_init(
+                try_pin_init!(EduDriverData {
+                    bar <- pdev
+                        .iomap_region_sized(0, c"rust_driver_edu")
+                        .and_then(|bar| bar.into_devres()),
+                    irq_test_completion <- Completion::new(),
+                    irq_dma_completion <- Completion::new(),
+                    dma: ca,
+                }),
+                GFP_KERNEL,
+            )?;
+
+            let req = irq::Registration::new(
+                (*irq.start()).try_into()?,
+                Flags::TRIGGER_NONE,
+                c"rust_edu_irq",
+                Ok(data.clone()),
+            );
+
+            // Ordering matters: the handler is registered (`irq_handler <- req`)
+            // *before* the `_:` block runs the self-tests, one of which raises an
+            // interrupt and waits for the handler. Raising before the handler is
+            // registered would hang (the completion is never signalled).
+            Ok(try_pin_init!(Self {
+                irq_handler <- req,
+                // Side-effect block: run the staged self-tests against the mapped
+                // BAR now that the handler is live. A failure here aborts probe.
+                _: {
+                    let bar = data.bar.access(pdev.as_ref())?;
+                    EduDriver::init(pdev, bar, &data)?;
+                    dev_info!(
+                        pdev,
+                        "rust_driver_edu successfully initialized\n",
+                    );
+                },
+                data,
+                pdev: pdev.into()
+            }))
+        })
+    }
+}
+
+impl irq::Handler for EduDriverData {
+    fn handle(&self, pdev: &kernel::device::Device<Bound>) -> irq::IrqReturn {
+        dev_dbg!(pdev, "irq handler called\n");
+        // `access()` only fails on device mismatch, so this branch is
+        // structurally unreachable here, but it must be handled.
+        let Ok(bar) = self.bar.access(pdev.as_ref()) else {
+            dev_err!(pdev, "cannot access bar register inside irq handler\n");
+            return irq::IrqReturn::None;
+        };
+        let status: u32 = bar.read(regs::IRQ_STATUS).into();
+
+        // DMA_IRQ
+        if status & DMA_IRQ != 0 {
+            dev_dbg!(pdev, "handling dma completion in irq\n");
+            bar.write(regs::IRQ_ACK, DMA_IRQ.into());
+            self.irq_dma_completion.complete();
+        }
+
+        // TEST_IRQ
+        let magic = status & !DMA_IRQ;
+        if magic == IRQ_MAGIC_VALUE {
+            dev_dbg!(pdev, "handling test completion in irq\n");
+            bar.write(regs::IRQ_ACK, magic.into());
+            self.irq_test_completion.complete();
+        }
+
+        irq::IrqReturn::Handled
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for EduDriver {
+    fn drop(self: Pin<&mut Self>) {
+        dev_dbg!(self.pdev, "Remove Rust EDU driver sample.\n");
+    }
+}
+
+kernel::pci_device_table!(
+    PCI_TABLE,
+    MODULE_PCI_TABLE,
+    <EduDriver as pci::Driver>::IdInfo,
+    [(
+        pci::DeviceId::from_id(pci::Vendor::from_raw(QEMU_VENDOR_ID), 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.51.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] rust: samples: add EDU PCI driver sample
  2026-06-20  8:45 ` [PATCH v2 4/4] rust: samples: add EDU PCI driver sample Maurice Hieronymus
@ 2026-06-20  9:45   ` Onur Özkan
  2026-06-21 12:27   ` Danilo Krummrich
  1 sibling, 0 replies; 12+ messages in thread
From: Onur Özkan @ 2026-06-20  9:45 UTC (permalink / raw)
  To: Maurice Hieronymus
  Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel

On Sat, 20 Jun 2026 10:45:48 +0200
Maurice Hieronymus <mhi@mailbox.org> wrote:

> Add a Rust sample driver for the QEMU EDU device, wired up via a new
> SAMPLE_RUST_DRIVER_EDU Kconfig option and the samples Makefile.
> 
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
> ---
>  samples/rust/Kconfig            |  11 ++
>  samples/rust/Makefile           |   1 +
>  samples/rust/rust_driver_edu.rs | 378 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 390 insertions(+)
> 
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index c49ab9106345..742c42262e9b 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -118,6 +118,17 @@ config SAMPLE_RUST_DRIVER_PCI
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_DRIVER_EDU
> +	tristate "EDU Driver"
> +	depends on PCI
> +	help
> +	  This option builds the Rust EDU driver sample.
> +
> +	  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 6c0aaa58cccc..c24a328243b1 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -10,6 +10,7 @@ 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_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..5f4efd514032
> --- /dev/null
> +++ b/samples/rust/rust_driver_edu.rs
> @@ -0,0 +1,378 @@
> +// 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::{
> +    device::Bound,
> +    devres::Devres,
> +    dma::{Coherent, Device, DmaMask},
> +    io::{
> +        poll::read_poll_timeout,
> +        register,
> +        Io, //
> +    },
> +    irq::{self, Flags},
> +    pci::{self, IrqTypes},
> +    prelude::*,
> +    sync::{aref::ARef, Arc, Completion},
> +    time::Delta, //
> +};

Multiple lines above needs to follow vertical style import.

- Onur

> +
> +const QEMU_VENDOR_ID: u16 = 0x1234;
> +const QEMU_EDU_DEVICE_ID: u32 = 0x11e8;
> +const QEMU_EDU_DEVICE_MAGIC: u8 = 0xed;
> +const QEMU_DMA_BASE: u64 = 0x40000;
> +
> +const IRQ_MAGIC_VALUE: u32 = 42;
> +
> +/// Bit set in `IRQ_STATUS` when a DMA transfer has completed.
> +const DMA_IRQ: u32 = 0x100;
> +
> +mod regs {
> +    use super::*;
> +
> +    register! {
> +        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 {}
> +        pub(super) IRQ_RAISE(u32) @ 0x60 {}
> +        pub(super) IRQ_ACK(u32) @ 0x64 {}
> +
> +        pub(super) DMA_SRC(u64) @ 0x80 {}
> +        pub(super) DMA_DST(u64) @ 0x88 {}
> +        pub(super) DMA_COUNT(u64) @ 0x90 {}
> +        pub(super) DMA_COMMAND(u64) @ 0x98 {
> +            0:0 start_transfer;
> +            1:1 direction;
> +            2:2 raise_irq;
> +        }
> +    }
> +
> +    pub(super) const END: usize = 0xA0;
> +}
> +
> +type Bar0 = pci::Bar<'static, { regs::END }>;
> +
> +#[pin_data(PinnedDrop)]
> +struct EduDriver {
> +    pdev: ARef<pci::Device>,
> +    data: Arc<EduDriverData>,
> +    #[pin]
> +    irq_handler: irq::Registration<Arc<EduDriverData>>,
> +}
> +
> +#[pin_data]
> +struct EduDriverData {
> +    #[pin]
> +    bar: Devres<Bar0>,
> +    #[pin]
> +    irq_test_completion: Completion,
> +    #[pin]
> +    irq_dma_completion: Completion,
> +    dma: Coherent<u64>,
> +}
> +
> +impl EduDriver {
> +    fn init(pdev: &pci::Device<Bound>, bar: &Bar0, data: &EduDriverData) -> Result {
> +        Self::magic(pdev, bar)?;
> +        Self::liveness_check(pdev, bar)?;
> +        Self::factorial(pdev, bar)?;
> +        Self::test_irq(pdev, bar, data)?;
> +        Self::test_dma(pdev, bar, data)?;
> +        Ok(())
> +    }
> +
> +    fn magic(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
> +        let identification = bar.read(regs::IDENTIFICATION);
> +
> +        let magic: u8 = identification.magic().into();
> +
> +        if magic != QEMU_EDU_DEVICE_MAGIC {
> +            dev_err!(
> +                pdev,
> +                "magic mismatch: expected {:#x} got {:#x}\n",
> +                QEMU_EDU_DEVICE_MAGIC,
> +                magic
> +            );
> +            return Err(ENODEV);
> +        }
> +
> +        dev_info!(
> +            pdev,
> +            "major: {:#x} minor: {:#x}\n",
> +            identification.major(),
> +            identification.minor()
> +        );
> +        Ok(())
> +    }
> +
> +    fn liveness_check(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
> +        let test_value = 0xabcd;
> +
> +        bar.write(regs::LIVENESS_CHECK, test_value.into());
> +
> +        let inverse_value = bar.read(regs::LIVENESS_CHECK).into_raw();
> +
> +        if inverse_value != !test_value {
> +            dev_err!(
> +                pdev,
> +                "inverse mismatch: expected {:#x} got {:#x}\n",
> +                !test_value,
> +                inverse_value
> +            );
> +            return Err(ENODEV);
> +        }
> +
> +        dev_info!(pdev, "inverse test successful\n");
> +        Ok(())
> +    }
> +
> +    fn factorial(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
> +        Self::wait_until_compute_has_finished(pdev, bar)?;
> +
> +        bar.write(regs::FACTORIAL, 4.into());
> +
> +        Self::wait_until_compute_has_finished(pdev, bar)?;
> +
> +        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(pdev: &pci::Device<Bound>, bar: &Bar0, data: &EduDriverData) -> Result {
> +        dev_dbg!(pdev, "raising irq\n");
> +
> +        bar.write(regs::IRQ_RAISE, IRQ_MAGIC_VALUE.into());
> +
> +        data.irq_test_completion.wait_for_completion();
> +        Ok(())
> +    }
> +
> +    fn test_dma(pdev: &pci::Device<Bound>, bar: &Bar0, data: &EduDriverData) -> Result {
> +        dev_dbg!(pdev, "testing dma\n");
> +
> +        let dma = &data.dma;
> +
> +        const DMA_VALUE: u64 = 42;
> +
> +        kernel::dma_write!(dma, , DMA_VALUE);
> +
> +        bar.write(regs::DMA_SRC, dma.dma_handle().into());
> +        bar.write(regs::DMA_DST, QEMU_DMA_BASE.into());
> +        bar.write(regs::DMA_COUNT, (dma.size() as u64).into());
> +        bar.write(
> +            regs::DMA_COMMAND,
> +            regs::DMA_COMMAND::zeroed()
> +                .with_start_transfer(true)
> +                .with_direction(false)
> +                .with_raise_irq(true),
> +        );
> +
> +        data.irq_dma_completion.wait_for_completion();
> +
> +        // Destroy previous value to test roundtrip
> +        kernel::dma_write!(dma, , 0);
> +
> +        bar.write(regs::DMA_SRC, QEMU_DMA_BASE.into());
> +        bar.write(regs::DMA_DST, dma.dma_handle().into());
> +        bar.write(regs::DMA_COUNT, (dma.size() as u64).into());
> +        bar.write(
> +            regs::DMA_COMMAND,
> +            regs::DMA_COMMAND::zeroed()
> +                .with_start_transfer(true)
> +                .with_direction(true)
> +                .with_raise_irq(true),
> +        );
> +
> +        data.irq_dma_completion.wait_for_completion();
> +
> +        let result = kernel::dma_read!(dma,);
> +
> +        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(pdev: &pci::Device<Bound>, bar: &Bar0) -> Result {
> +        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> = Self;
> +
> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> +
> +    fn probe<'bound>(
> +        pdev: &'bound pci::Device<kernel::device::Core<'_>>,
> +        _id_info: &'bound Self::IdInfo,
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> +        pin_init::pin_init_scope(move || {
> +            let vendor = pdev.vendor_id();
> +            dev_dbg!(
> +                pdev,
> +                "Probe Rust EDU driver sample (PCI ID: {}, 0x{:x}).\n",
> +                vendor,
> +                pdev.device_id()
> +            );
> +
> +            pdev.enable_device()?;
> +            pdev.set_master();
> +
> +            let mask = DmaMask::new::<28>();
> +
> +            // SAFETY: There are no concurrent calls to DMA allocation and mapping primitives.
> +            unsafe { pdev.dma_set_mask_and_coherent(mask)? };
> +
> +            let ca: Coherent<u64> = Coherent::zeroed(pdev.as_ref(), GFP_KERNEL)?;
> +
> +            let irq = pdev
> +                .alloc_irq_vectors(1, 1, IrqTypes::default().with(pci::IrqType::Msi))
> +                .inspect_err(|e| dev_err!(pdev, "alloc_irq_vectors failed: {:?}\n", e))?;
> +
> +            // State shared with the IRQ handler (the BAR and the completion the
> +            // handler signals) lives in an `Arc<EduDriverData>`. `EduDriverData`
> +            // itself implements `irq::Handler`, and the registration takes an
> +            // `Arc<T>` via the `impl Handler for Arc<T>` blanket impl. This keeps
> +            // the handler's state out of `EduDriver` and avoids a self-reference.
> +            let data = Arc::pin_init(
> +                try_pin_init!(EduDriverData {
> +                    bar <- pdev
> +                        .iomap_region_sized(0, c"rust_driver_edu")
> +                        .and_then(|bar| bar.into_devres()),
> +                    irq_test_completion <- Completion::new(),
> +                    irq_dma_completion <- Completion::new(),
> +                    dma: ca,
> +                }),
> +                GFP_KERNEL,
> +            )?;
> +
> +            let req = irq::Registration::new(
> +                (*irq.start()).try_into()?,
> +                Flags::TRIGGER_NONE,
> +                c"rust_edu_irq",
> +                Ok(data.clone()),
> +            );
> +
> +            // Ordering matters: the handler is registered (`irq_handler <- req`)
> +            // *before* the `_:` block runs the self-tests, one of which raises an
> +            // interrupt and waits for the handler. Raising before the handler is
> +            // registered would hang (the completion is never signalled).
> +            Ok(try_pin_init!(Self {
> +                irq_handler <- req,
> +                // Side-effect block: run the staged self-tests against the mapped
> +                // BAR now that the handler is live. A failure here aborts probe.
> +                _: {
> +                    let bar = data.bar.access(pdev.as_ref())?;
> +                    EduDriver::init(pdev, bar, &data)?;
> +                    dev_info!(
> +                        pdev,
> +                        "rust_driver_edu successfully initialized\n",
> +                    );
> +                },
> +                data,
> +                pdev: pdev.into()
> +            }))
> +        })
> +    }
> +}
> +
> +impl irq::Handler for EduDriverData {
> +    fn handle(&self, pdev: &kernel::device::Device<Bound>) -> irq::IrqReturn {
> +        dev_dbg!(pdev, "irq handler called\n");
> +        // `access()` only fails on device mismatch, so this branch is
> +        // structurally unreachable here, but it must be handled.
> +        let Ok(bar) = self.bar.access(pdev.as_ref()) else {
> +            dev_err!(pdev, "cannot access bar register inside irq handler\n");
> +            return irq::IrqReturn::None;
> +        };
> +        let status: u32 = bar.read(regs::IRQ_STATUS).into();
> +
> +        // DMA_IRQ
> +        if status & DMA_IRQ != 0 {
> +            dev_dbg!(pdev, "handling dma completion in irq\n");
> +            bar.write(regs::IRQ_ACK, DMA_IRQ.into());
> +            self.irq_dma_completion.complete();
> +        }
> +
> +        // TEST_IRQ
> +        let magic = status & !DMA_IRQ;
> +        if magic == IRQ_MAGIC_VALUE {
> +            dev_dbg!(pdev, "handling test completion in irq\n");
> +            bar.write(regs::IRQ_ACK, magic.into());
> +            self.irq_test_completion.complete();
> +        }
> +
> +        irq::IrqReturn::Handled
> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for EduDriver {
> +    fn drop(self: Pin<&mut Self>) {
> +        dev_dbg!(self.pdev, "Remove Rust EDU driver sample.\n");
> +    }
> +}
> +
> +kernel::pci_device_table!(
> +    PCI_TABLE,
> +    MODULE_PCI_TABLE,
> +    <EduDriver as pci::Driver>::IdInfo,
> +    [(
> +        pci::DeviceId::from_id(pci::Vendor::from_raw(QEMU_VENDOR_ID), 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.51.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] rust: completion: add complete()
  2026-06-20  8:45 ` [PATCH v2 3/4] rust: completion: add complete() Maurice Hieronymus
@ 2026-06-20  9:48   ` Onur Özkan
  0 siblings, 0 replies; 12+ messages in thread
From: Onur Özkan @ 2026-06-20  9:48 UTC (permalink / raw)
  To: Maurice Hieronymus
  Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel

On Sat, 20 Jun 2026 10:45:47 +0200
Maurice Hieronymus <mhi@mailbox.org> wrote:

> 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.
> 
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
> ---
>  rust/kernel/sync/completion.rs | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
> index 35ff049ff078..a54361b53644 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`.

I think the safety comment is a bit weak here. It would be nice to add why
self.as_raw() is guaranteed to be a valid pointer.

- Onur

> +        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
> 
> -- 
> 2.51.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public
  2026-06-20  8:45 ` [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
@ 2026-06-20  9:48   ` Onur Özkan
  0 siblings, 0 replies; 12+ messages in thread
From: Onur Özkan @ 2026-06-20  9:48 UTC (permalink / raw)
  To: Maurice Hieronymus
  Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel, Onur Özkan

On Sat, 20 Jun 2026 10:45:45 +0200
Maurice Hieronymus <mhi@mailbox.org> wrote:

> `Vendor::from_raw()` is currently `pub(super)`, so a Vendor can only be
> obtained through the named constants generated from the
> `PCI_VENDOR_ID_*` defines in `<linux/pci_ids.h>`. A driver therefore
> cannot match a device whose vendor ID has no symbolic name.
> 
> Such devices exist. QEMU's "edu" educational device and the legacy
> qemu/Bochs stdvga both use vendor ID 0x1234, which is not registered in
> `pci_ids.h`. Per the policy stated at the top of that header, IDs are
> only added there when shared between multiple drivers; a single-driver
> ID is expected to be open-coded in the driver instead. C drivers already
> do this -- see `drivers/gpu/drm/tiny/bochs.c`, which matches with a bare
> ".vendor = 0x1234".
> 
> The Rust abstraction has no equivalent escape hatch: there is no public
> way to express an unregistered vendor. Make `Vendor::from_raw()` public
> (and const, so it can be used in the const device-ID tables built by
> `pci_device_table!`) so that drivers can construct a Vendor from a raw
> ID, matching what C drivers can already do.
> 
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Reviewed-by: Onur Özkan <work@onurozkan.dev>

> ---
>  rust/kernel/pci/id.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
> index dbaf301666e7..fe3b0047179b 100644
> --- a/rust/kernel/pci/id.rs
> +++ b/rust/kernel/pci/id.rs
> @@ -156,7 +156,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>  impl Vendor {
>      /// Create a Vendor from a raw 16-bit vendor ID.
>      #[inline]
> -    pub(super) fn from_raw(vendor_id: u16) -> Self {
> +    pub const fn from_raw(vendor_id: u16) -> Self {
>          Self(vendor_id)
>      }
>  
> 
> -- 
> 2.51.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] rust: pci: add managed Device::enable_device()
  2026-06-20  8:45 ` [PATCH v2 2/4] rust: pci: add managed Device::enable_device() Maurice Hieronymus
@ 2026-06-20  9:54   ` Onur Özkan
  2026-06-20 22:19     ` Maurice Hieronymus
  0 siblings, 1 reply; 12+ messages in thread
From: Onur Özkan @ 2026-06-20  9:54 UTC (permalink / raw)
  To: Maurice Hieronymus
  Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel, Fiona Behrens

On Sat, 20 Jun 2026 10:45:46 +0200
Maurice Hieronymus <mhi@mailbox.org> wrote:

> Add a managed counterpart to Device::enable_device_mem() that wraps
> pcim_enable_device(). In addition to enabling the device, it registers a
> pci_disable_device() cleanup that runs automatically when the device is
> unbound from its driver, keeping the device's enable count balanced
> across unbind/rebind cycles.
> 
> The existing enable_device_mem() wraps the unmanaged
> pci_enable_device_mem() and has no disable counterpart, so the enable
> count is leaked on unbind. On the next probe pci_enable_device_flags()
> sees a non-zero enable count and returns early, skipping the power-state
> transition back to D0. For a device without a PCI power management
> capability the power state cannot be re-read from hardware and stays
> PCI_UNKNOWN, which makes __pci_enable_msi_range() reject the subsequent
> MSI allocation with -EINVAL.
> 
> Furthermore, make `enable_device_mem` inline and add a reference to
> `enable_device` in the docs.
> 
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
> ---
>  rust/kernel/pci.rs | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 5071cae6543f..d076a3691091 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -454,11 +454,27 @@ pub fn pci_class(&self) -> Class {
>  
>  impl<'a> Device<device::Core<'a>> {
>      /// Enable memory resources for this device.
> +    ///
> +    /// This function is unmanaged and does not perform any cleanup when the device is unbound.
> +    /// For a managed function take a look at [`Device::enable_device`].
> +    #[inline]
>      pub fn enable_device_mem(&self) -> Result {
>          // 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()) })
>      }
>  
> +    /// Enable I/O and memory resources for this device, with automatic cleanup.
> +    ///
> +    /// This is the managed version of `pci_enable_device()`: it enables the device's I/O and
> +    /// memory resources and registers a `pci_disable_device()` call that runs automatically
> +    /// when the device is unbound from its driver. In contrast, [`Device::enable_device_mem`]
> +    /// is unmanaged and only enables memory resources.
> +    #[inline]
> +    pub fn enable_device(&self) -> Result {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> +        to_result(unsafe { bindings::pcim_enable_device(self.as_raw()) })
> +    }
> +

How about adding `_managed` and `_unmanaged` suffixes to the function names?

- Onur

>      /// Enable bus-mastering for this device.
>      #[inline]
>      pub fn set_master(&self) {
> 
> -- 
> 2.51.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] rust: pci: add managed Device::enable_device()
  2026-06-20  9:54   ` Onur Özkan
@ 2026-06-20 22:19     ` Maurice Hieronymus
  2026-06-21 12:02       ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Maurice Hieronymus @ 2026-06-20 22:19 UTC (permalink / raw)
  To: Onur Özkan
  Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel, Fiona Behrens

On Sat, 2026-06-20 at 12:54 +0300, Onur Özkan wrote:
> 
> >  impl<'a> Device<device::Core<'a>> {
> >      /// Enable memory resources for this device.
> > +    ///
> > +    /// This function is unmanaged and does not perform any
> > cleanup when the device is unbound.
> > +    /// For a managed function take a look at
> > [`Device::enable_device`].
> > +    #[inline]
> >      pub fn enable_device_mem(&self) -> Result {
> >          // 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()) })
> >      }
> >  
> > +    /// Enable I/O and memory resources for this device, with
> > automatic cleanup.
> > +    ///
> > +    /// This is the managed version of `pci_enable_device()`: it
> > enables the device's I/O and
> > +    /// memory resources and registers a `pci_disable_device()`
> > call that runs automatically
> > +    /// when the device is unbound from its driver. In contrast,
> > [`Device::enable_device_mem`]
> > +    /// is unmanaged and only enables memory resources.
> > +    #[inline]
> > +    pub fn enable_device(&self) -> Result {
> > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to
> > a valid `struct pci_dev`.
> > +        to_result(unsafe {
> > bindings::pcim_enable_device(self.as_raw()) })
> > +    }
> > +
> 
> How about adding `_managed` and `_unmanaged` suffixes to the function
> names?
> 
In general, I agree with you.

However, both functions differ slightly more than being only managed
and unmanaged. `enable_device` enables I/O and memory resources.
`enable_device_mem` only enables memory resources.

Isn't it confusing when we only have `enable_device_managed` and
`enable_device_mem_unmanaged` but not `enable_device_unmanaged` and
`enable_device_mem_managed`?

What is the general road ahead in the future? Do we want to continue to
have C-Code "magically" clean up our resources or would it be
preferable if we get some handle, which does the clean up when it is
dropped?
> - Onur
> 
> >      /// Enable bus-mastering for this device.
> >      #[inline]
> >      pub fn set_master(&self) {
> > 
> > -- 
> > 2.51.2
> > 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] rust: pci: add managed Device::enable_device()
  2026-06-20 22:19     ` Maurice Hieronymus
@ 2026-06-21 12:02       ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2026-06-21 12:02 UTC (permalink / raw)
  To: Maurice Hieronymus
  Cc: Onur Özkan, Bjorn Helgaas, Krzysztof Wilczyński,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel, Fiona Behrens

On Sun Jun 21, 2026 at 12:19 AM CEST, Maurice Hieronymus wrote:
> Isn't it confusing when we only have `enable_device_managed` and
> `enable_device_mem_unmanaged` but not `enable_device_unmanaged` and
> `enable_device_mem_managed`?

I think we only need enable_device_managed() and a enable_device() /
disable_device() pair.

enable_device_managed() should be preferred, the unmanaged functions may still
be used for runtime PM.

The existing enable_device_mem() should just be removed and nova-core should use
enable_device_managed() in probe() instead.

If you are interested, feel free to create a small separate patch series making
those changes; you can stack this one on top of it.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] rust: samples: add EDU PCI driver sample
  2026-06-20  8:45 ` [PATCH v2 4/4] rust: samples: add EDU PCI driver sample Maurice Hieronymus
  2026-06-20  9:45   ` Onur Özkan
@ 2026-06-21 12:27   ` Danilo Krummrich
  1 sibling, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2026-06-21 12:27 UTC (permalink / raw)
  To: Maurice Hieronymus
  Cc: Bjorn Helgaas, Krzysztof Wilczyński, 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, Lyude Paul,
	linux-pci, rust-for-linux, linux-kernel

On Sat Jun 20, 2026 at 10:45 AM CEST, Maurice Hieronymus wrote:
> Add a Rust sample driver for the QEMU EDU device, wired up via a new
> SAMPLE_RUST_DRIVER_EDU Kconfig option and the samples Makefile.
>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

This is a good example, but it seems a bit redundant with the existing sample
driver. So, I think this one should just replace the existing one. The only
thing that is not covered is PCI configuration space access, but that should be
trivial to demonstrate in this sample too.

> +impl pci::Driver for EduDriver {
> +    type IdInfo = ();
> +    type Data<'bound> = Self;

Please make this a new type and take advantage of Rust native lifetimes for
device resources, just like the existing sample driver and nova-core does.

I understand that the existing IRQ infrastructure is a bit in the way and you
have to work around it.

I'm going to send a patch to make the irq::Registration compatible with the
device driver lifetime rework soon. Please rebase onto that once it's sent, so
this sample can land as idiomatic as possible.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-06-21 12:27 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-20  8:45 [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
2026-06-20  8:45 ` [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
2026-06-20  9:48   ` Onur Özkan
2026-06-20  8:45 ` [PATCH v2 2/4] rust: pci: add managed Device::enable_device() Maurice Hieronymus
2026-06-20  9:54   ` Onur Özkan
2026-06-20 22:19     ` Maurice Hieronymus
2026-06-21 12:02       ` Danilo Krummrich
2026-06-20  8:45 ` [PATCH v2 3/4] rust: completion: add complete() Maurice Hieronymus
2026-06-20  9:48   ` Onur Özkan
2026-06-20  8:45 ` [PATCH v2 4/4] rust: samples: add EDU PCI driver sample Maurice Hieronymus
2026-06-20  9:45   ` Onur Özkan
2026-06-21 12:27   ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox