* [PATCH v3 1/5] samples: rust: remove the rust_driver_pci sample
2026-08-12 19:52 [PATCH v3 0/5] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
@ 2026-08-12 19:52 ` Maurice Hieronymus
2026-08-12 19:55 ` sashiko-bot
2026-08-12 19:52 ` [PATCH v3 2/5] rust: pci: rework device enabling API Maurice Hieronymus
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Maurice Hieronymus @ 2026-08-12 19:52 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, David Airlie, Simona Vetter
Cc: linux-pci, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
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 | 194 ----------------------------------------
4 files changed, 207 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 95f6791c41bc..92bc1f8c4f8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20915,7 +20915,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 c49ab9106345..0cae695acd84 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 6c0aaa58cccc..70495fed886f 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 1aa8197d8698..000000000000
--- a/samples/rust/rust_driver_pci.rs
+++ /dev/null
@@ -1,194 +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! {
- 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,
- MODULE_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! {
- 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: &'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()
- );
-
- 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 2/5] rust: pci: rework device enabling API
2026-08-12 19:52 [PATCH v3 0/5] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
2026-08-12 19:52 ` [PATCH v3 1/5] samples: rust: remove the rust_driver_pci sample Maurice Hieronymus
@ 2026-08-12 19:52 ` Maurice Hieronymus
2026-08-12 20:08 ` sashiko-bot
2026-08-12 19:52 ` [PATCH v3 3/5] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Maurice Hieronymus @ 2026-08-12 19:52 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, David Airlie, Simona Vetter
Cc: linux-pci, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
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 | 5 ++++-
rust/kernel/pci.rs | 31 ++++++++++++++++++++++++++++---
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5738d4ac521b..99b15da59e81 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -30,6 +30,8 @@ pub(crate) struct NovaCore<'bound> {
bar: pci::Bar<'bound, BAR0_SIZE>,
#[allow(clippy::type_complexity)]
_reg: auxiliary::Registration<'bound, ForLt!(())>,
+ // Declared last so the device stays enabled until everything above is torn down.
+ _enable: pci::DeviceEnableGuard<'bound>,
}
pub(crate) struct NovaCoreDriver;
@@ -75,7 +77,7 @@ fn probe<'bound>(
pin_init::pin_init_scope(move || {
dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
- pdev.enable_device_mem()?;
+ let enable = pdev.enable_device()?;
pdev.set_master();
Ok(try_pin_init!(NovaCore {
@@ -95,6 +97,7 @@ fn probe<'bound>(
crate::MODULE_NAME,
(),
)?,
+ _enable: enable,
}))
})
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4def9ca1824c..bd9a8113af35 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -452,11 +452,36 @@ 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.
+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>> {
- /// 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.
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/5] rust: pci: rework device enabling API
2026-08-12 19:52 ` [PATCH v3 2/5] rust: pci: rework device enabling API Maurice Hieronymus
@ 2026-08-12 20:08 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-12 20:08 UTC (permalink / raw)
To: Maurice Hieronymus
Cc: ojeda, Danilo Krummrich, linux-pci, Alexandre Courbot, dri-devel
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Replacing `pci_enable_device_mem()` with `pci_enable_device()` causes probe failure on platforms with unassigned legacy I/O BARs.
- [Low] Missing `#[must_use]` attribute on `DeviceEnableGuard` allows silent, immediate device disablement.
- [Low] Missing `#[inline]` attribute on the `enable_device()` abstraction method.
Pre-existing issues:
- [High] Incorrect struct field drop order causes MMIO unmap before child device unregistration (Use-After-Free).
--
--- Patch [2]: [PATCH v3 2/5] rust: pci: rework device enabling API ---
commit d9e225d5ab32388fa1a595e83eeef86ed5e5b2b9
Author: Maurice Hieronymus <mhi@mailbox.org>
rust: pci: rework device enabling API
The commit replaces the unmanaged enable_device_mem() with enable_device(),
returning a resource guard DeviceEnableGuard that automatically runs
pci_disable_device() on drop. It also converts the only user, nova-core, to
use the new API.
Link: https://lore.kernel.org/rust-for-linux/DJOEYVBS17MJ.1YD3TNGQBWHNK@kernel.org
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 5738d4ac521b7..99b15da59e819 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -30,6 +30,8 @@ pub(crate) struct NovaCore<'bound> {
> bar: pci::Bar<'bound, BAR0_SIZE>,
> #[allow(clippy::type_complexity)]
> _reg: auxiliary::Registration<'bound, ForLt!(())>,
[Severity: High]
This is a pre-existing issue, but does the struct field drop order in NovaCore
cause memory mapped I/O to be unmapped before child devices are unregistered?
Rust drops struct fields in their exact declaration order. Here, bar is
declared before _reg. When the driver is unbound, bar is dropped (unmapping
memory mapped I/O) before _reg is dropped (unregistering auxiliary devices).
If those child devices attempt to access hardware to quiesce it during their
remove callbacks, could this result in a use-after-free or page fault by
accessing unmapped memory?
> + // Declared last so the device stays enabled until everything above is torn down.
> + _enable: pci::DeviceEnableGuard<'bound>,
> }
[ ... ]
> @@ -75,7 +77,7 @@ fn probe<'bound>(
> pin_init::pin_init_scope(move || {
> dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
>
> - pdev.enable_device_mem()?;
> + let enable = pdev.enable_device()?;
[Severity: High]
Does replacing pci_enable_device_mem() with pci_enable_device() cause probe
failures on platforms with unassigned legacy I/O base address registers?
The previous method, pci_enable_device_mem(), safely bypassed this by only
requesting memory resources, which is typically all modern devices need. Since
pci_enable_device() internally requests both memory and I/O resources, could
this cause probe failures on platforms (such as many arm64 systems or modern
x86 without legacy vga I/O) where a device has an I/O register but the
firmware or pci core did not assign resources for it?
> pdev.set_master();
[ ... ]
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 4def9ca1824ce..bd9a8113af358 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -452,11 +452,36 @@ 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.
> +pub struct DeviceEnableGuard<'a> {
[Severity: Low]
Could the lack of a #[must_use] attribute on DeviceEnableGuard lead to silent
logic errors?
If a caller uses the API as pdev.enable_device()?; without binding the
returned guard to a variable, the guard is immediately dropped, running
pci_disable_device() right after enablement. While nova-core uses it
correctly, omitting #[must_use] on guards might allow logic errors to
compile without warnings.
> + dev: &'a Device<device::Bound>,
> +}
[ ... ]
> impl<'a> Device<device::Core<'a>> {
> - /// 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.
> + pub fn enable_device(&self) -> Result<DeviceEnableGuard<'_>> {
[Severity: Low]
This isn't a bug, but should the enable_device() forwarding abstraction
function be annotated with #[inline]?
The rust guidelines state: "For abstractions ONLY: Functions that are
small or forwarding to a binding call should be annotated with #[inline]."
> // 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 })
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-b4-rust-pci-edu-driver-v3-0-5d0b5594e52b@mailbox.org?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/5] rust: pci: make Vendor::from_raw() public
2026-08-12 19:52 [PATCH v3 0/5] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
2026-08-12 19:52 ` [PATCH v3 1/5] samples: rust: remove the rust_driver_pci sample Maurice Hieronymus
2026-08-12 19:52 ` [PATCH v3 2/5] rust: pci: rework device enabling API Maurice Hieronymus
@ 2026-08-12 19:52 ` Maurice Hieronymus
2026-08-12 19:56 ` sashiko-bot
2026-08-12 19:52 ` [PATCH v3 4/5] rust: completion: add complete() Maurice Hieronymus
2026-08-12 19:52 ` [PATCH v3 5/5] rust: samples: add EDU PCI driver sample Maurice Hieronymus
4 siblings, 1 reply; 11+ messages in thread
From: Maurice Hieronymus @ 2026-08-12 19:52 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, David Airlie, Simona Vetter
Cc: linux-pci, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
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>
Reviewed-by: Onur Özkan <work@onurozkan.dev>
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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 3/5] rust: pci: make Vendor::from_raw() public
2026-08-12 19:52 ` [PATCH v3 3/5] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
@ 2026-08-12 19:56 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-12 19:56 UTC (permalink / raw)
To: Maurice Hieronymus
Cc: Alexandre Courbot, Danilo Krummrich, linux-pci, dri-devel, ojeda
> `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>
> Reviewed-by: Onur Özkan <work@onurozkan.dev>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-b4-rust-pci-edu-driver-v3-0-5d0b5594e52b@mailbox.org?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 4/5] rust: completion: add complete()
2026-08-12 19:52 [PATCH v3 0/5] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
` (2 preceding siblings ...)
2026-08-12 19:52 ` [PATCH v3 3/5] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
@ 2026-08-12 19:52 ` Maurice Hieronymus
2026-08-12 19:56 ` sashiko-bot
2026-08-12 19:52 ` [PATCH v3 5/5] rust: samples: add EDU PCI driver sample Maurice Hieronymus
4 siblings, 1 reply; 11+ messages in thread
From: Maurice Hieronymus @ 2026-08-12 19:52 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, David Airlie, Simona Vetter
Cc: linux-pci, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
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 | 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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 4/5] rust: completion: add complete()
2026-08-12 19:52 ` [PATCH v3 4/5] rust: completion: add complete() Maurice Hieronymus
@ 2026-08-12 19:56 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-12 19:56 UTC (permalink / raw)
To: Maurice Hieronymus
Cc: Danilo Krummrich, Alexandre Courbot, ojeda, linux-pci, dri-devel
> 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. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-b4-rust-pci-edu-driver-v3-0-5d0b5594e52b@mailbox.org?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 5/5] rust: samples: add EDU PCI driver sample
2026-08-12 19:52 [PATCH v3 0/5] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
` (3 preceding siblings ...)
2026-08-12 19:52 ` [PATCH v3 4/5] rust: completion: add complete() Maurice Hieronymus
@ 2026-08-12 19:52 ` Maurice Hieronymus
2026-08-12 20:12 ` sashiko-bot
4 siblings, 1 reply; 11+ messages in thread
From: Maurice Hieronymus @ 2026-08-12 19:52 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, David Airlie, Simona Vetter
Cc: linux-pci, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
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>
---
MAINTAINERS | 1 +
samples/rust/Kconfig | 11 ++
samples/rust/Makefile | 1 +
samples/rust/rust_driver_edu.rs | 421 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 434 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 92bc1f8c4f8a..f8a16d7b8260 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20915,6 +20915,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 0cae695acd84..115f71ed6328 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -107,6 +107,17 @@ config SAMPLE_RUST_I2C_CLIENT
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 70495fed886f..a005d578b7c2 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..52f6c4cf3b08
--- /dev/null
+++ b/samples/rust/rust_driver_edu.rs
@@ -0,0 +1,421 @@
+// 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,
+ dma::{
+ Coherent,
+ Device,
+ DmaMask, //
+ },
+ io::{
+ poll::read_poll_timeout,
+ register,
+ register::Array,
+ Io, //
+ },
+ irq::{
+ self,
+ Flags, //
+ },
+ pci::{
+ self,
+ IrqTypes, //
+ },
+ prelude::*,
+ sync::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<'a> = pci::Bar<'a, { regs::END }>;
+
+struct EduDriver;
+
+#[pin_data(PinnedDrop)]
+struct EduDriverData<'bound> {
+ pdev: &'bound pci::Device,
+ #[pin]
+ irq_handler: irq::Registration<'bound, IrqHandler<'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<u64>,
+}
+
+impl EduDriver {
+ fn init(pdev: &pci::Device<Bound>, bar: &Bar0<'_>, handler: &IrqHandler<'_>) -> Result {
+ Self::config_space(pdev);
+ Self::magic(pdev, bar)?;
+ Self::liveness_check(pdev, bar)?;
+ Self::factorial(pdev, bar)?;
+ Self::test_irq(pdev, handler)?;
+ Self::test_dma(pdev, handler)?;
+ Ok(())
+ }
+
+ fn config_space(pdev: &pci::Device<Bound>) {
+ let config = pdev.config_space();
+
+ // Some PCI configuration space registers.
+ register! {
+ 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(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>, handler: &IrqHandler<'_>) -> Result {
+ dev_dbg!(pdev, "raising irq\n");
+
+ handler.bar.write(regs::IRQ_RAISE, IRQ_MAGIC_VALUE.into());
+
+ handler.irq_test_completion.wait_for_completion();
+
+ dev_info!(pdev, "irq test successful\n");
+ Ok(())
+ }
+
+ fn test_dma(pdev: &pci::Device<Bound>, handler: &IrqHandler<'_>) -> Result {
+ dev_dbg!(pdev, "testing dma\n");
+
+ let dma = &handler.dma;
+
+ const DMA_VALUE: u64 = 42;
+
+ kernel::dma_write!(dma, , DMA_VALUE);
+
+ handler.bar.write(regs::DMA_SRC, dma.dma_handle().into());
+ handler.bar.write(regs::DMA_DST, QEMU_DMA_BASE.into());
+ handler
+ .bar
+ .write(regs::DMA_COUNT, (dma.size() as u64).into());
+ handler.bar.write(
+ regs::DMA_COMMAND,
+ regs::DMA_COMMAND::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
+ kernel::dma_write!(dma, , 0);
+
+ handler.bar.write(regs::DMA_SRC, QEMU_DMA_BASE.into());
+ handler.bar.write(regs::DMA_DST, dma.dma_handle().into());
+ handler
+ .bar
+ .write(regs::DMA_COUNT, (dma.size() as u64).into());
+ handler.bar.write(
+ regs::DMA_COMMAND,
+ regs::DMA_COMMAND::zeroed()
+ .with_start_transfer(true)
+ .with_direction(true)
+ .with_raise_irq(true),
+ );
+
+ handler.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> = EduDriverData<'bound>;
+
+ 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()
+ );
+
+ let enable = 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))?;
+
+ let bar = pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_edu")?;
+
+ // SAFETY: The Registration is not leaked.
+ let req = unsafe {
+ pdev.request_irq(
+ *irq.start(),
+ Flags::TRIGGER_NONE,
+ c"rust_edu_irq",
+ try_pin_init!(IrqHandler {
+ pdev,
+ bar,
+ irq_test_completion <- Completion::new(),
+ irq_dma_completion <- Completion::new(),
+ dma: ca,
+ }? Error),
+ )
+ };
+
+ Ok(try_pin_init!(EduDriverData {
+ irq_handler <- req,
+ // 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).
+ _: {
+ let handler = irq_handler.handler();
+ EduDriver::init(pdev, &handler.bar, handler)?;
+ dev_info!(
+ pdev,
+ "rust_driver_edu successfully initialized\n",
+ );
+ },
+ pdev,
+ _enable: enable,
+ }))
+ })
+ }
+}
+
+impl irq::Handler for IrqHandler<'_> {
+ fn handle(&self) -> irq::IrqReturn {
+ dev_dbg!(self.pdev, "irq handler called\n");
+ let status: u32 = self.bar.read(regs::IRQ_STATUS).into();
+
+ // DMA_IRQ
+ if status & DMA_IRQ != 0 {
+ dev_dbg!(self.pdev, "handling dma completion in irq\n");
+ self.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!(self.pdev, "handling test completion in irq\n");
+ self.bar.write(regs::IRQ_ACK, magic.into());
+ self.irq_test_completion.complete();
+ }
+
+ irq::IrqReturn::Handled
+ }
+}
+
+#[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,
+ 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.54.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 5/5] rust: samples: add EDU PCI driver sample
2026-08-12 19:52 ` [PATCH v3 5/5] rust: samples: add EDU PCI driver sample Maurice Hieronymus
@ 2026-08-12 20:12 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-12 20:12 UTC (permalink / raw)
To: Maurice Hieronymus
Cc: dri-devel, ojeda, linux-pci, Alexandre Courbot, Danilo Krummrich
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unbounded wait_for_completion() during probe can hang the system.
- [High] 64-bit MMIO accesses will cause compilation failures on 32-bit architectures.
- [Medium] Unconditional Handled return in IRQ handler defeats spurious interrupt detection.
--
--- Patch [5]: [PATCH v3 5/5] rust: samples: add EDU PCI driver sample ---
commit c231e5461a247a3fe7e624e02b7930ee66f864b3
Author: Maurice Hieronymus <mhi@mailbox.org>
rust: samples: add EDU PCI driver sample
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.
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index 0cae695acd842..115f71ed63283 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -107,6 +107,17 @@ config SAMPLE_RUST_I2C_CLIENT
>
> If unsure, say N.
>
> +config SAMPLE_RUST_DRIVER_EDU
> + tristate "EDU Driver"
> + depends on PCI
[Severity: High]
Does this missing 64BIT dependency cause build failures on 32-bit
architectures?
The driver defines 64-bit MMIO registers (like DMA_SRC(u64)), but the
IoCapable<u64> trait is only conditionally implemented for Mmio on 64-bit
architectures. Building on a 32-bit target will fail compilation because the
64-bit write method on pci::Bar will be unavailable.
> + help
> + This option builds the Rust EDU driver sample.
[ ... ]
> diff --git a/samples/rust/rust_driver_edu.rs b/samples/rust/rust_driver_edu.rs
> new file mode 100644
> index 0000000000000..52f6c4cf3b08d
> --- /dev/null
> +++ b/samples/rust/rust_driver_edu.rs
> @@ -0,0 +1,421 @@
[ ... ]
> + fn test_irq(pdev: &pci::Device<Bound>, handler: &IrqHandler<'_>) -> Result {
> + dev_dbg!(pdev, "raising irq\n");
> +
> + handler.bar.write(regs::IRQ_RAISE, IRQ_MAGIC_VALUE.into());
> +
> + handler.irq_test_completion.wait_for_completion();
[Severity: High]
Can this unbounded wait hang the system?
Since test_irq() is called synchronously from the driver probe path, a dropped
hardware interrupt or faulty device might block the thread indefinitely.
This could prevent the kernel from booting or loading other drivers.
Would a timed wait like wait_for_completion_timeout() avoid this risk?
> +
> + dev_info!(pdev, "irq test successful\n");
> + Ok(())
> + }
> +
> + fn test_dma(pdev: &pci::Device<Bound>, handler: &IrqHandler<'_>) -> Result {
> + dev_dbg!(pdev, "testing dma\n");
[ ... ]
> + handler.bar.write(
> + regs::DMA_COMMAND,
> + regs::DMA_COMMAND::zeroed()
> + .with_start_transfer(true)
> + .with_direction(false)
> + .with_raise_irq(true),
> + );
> +
> + handler.irq_dma_completion.wait_for_completion();
[Severity: High]
Similarly, could this unbounded wait block the probe sequence indefinitely if
the DMA completion interrupt is never received?
> +
> + // Destroy previous value to test roundtrip
[ ... ]
> +impl irq::Handler for IrqHandler<'_> {
> + fn handle(&self) -> irq::IrqReturn {
> + dev_dbg!(self.pdev, "irq handler called\n");
> + let status: u32 = self.bar.read(regs::IRQ_STATUS).into();
> +
> + // DMA_IRQ
> + if status & DMA_IRQ != 0 {
> + dev_dbg!(self.pdev, "handling dma completion in irq\n");
> + self.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!(self.pdev, "handling test completion in irq\n");
> + self.bar.write(regs::IRQ_ACK, magic.into());
> + self.irq_test_completion.complete();
> + }
> +
> + irq::IrqReturn::Handled
[Severity: Medium]
Does this unconditional return of Handled defeat the kernel's spurious
interrupt detection?
If the status matches neither DMA_IRQ nor IRQ_MAGIC_VALUE, the handler
processes no bits but still returns irq::IrqReturn::Handled.
When a spurious interrupt occurs, the kernel will believe this device handled
it and won't increment the spurious interrupt counter. This could prevent the
IRQ subsystem from disabling a faulty IRQ line, potentially causing an
interrupt storm.
Should this return irq::IrqReturn::None when no expected bits are set?
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-b4-rust-pci-edu-driver-v3-0-5d0b5594e52b@mailbox.org?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread