* [PATCH v2] samples: rust: add QEMU EDU sample driver
@ 2026-10-30 18:15 Lucas Costa
2026-10-30 18:30 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Lucas Costa @ 2026-10-30 18:15 UTC (permalink / raw)
To: dakr, ojeda
Cc: bhelgaas, kwilczynski, boqun, gary, bjorn3_gh, lossin, a.hindborg,
aliceryhl, tmgross, daniel.almeida, tamird, acourbot, work, lyude,
linux-pci, rust-for-linux, linux-kernel
Add a Rust PCI driver sample for QEMU's educational ("edu") device.
It exercises the Rust PCI, IRQ, and DMA abstractions: MMIO
identification/liveness checks, interrupt status handling, an
MSI-backed factorial computation with a bounded wait, a coherent-DMA
round trip, and PCI config-space reads.
Also add the PCI teardown helpers and Completion timeout support used
by the sample.
Run QEMU with `-device edu` to probe it.
Signed-off-by: Lucas Costa <2000.costalucas@gmail.com>
---
rust/kernel/pci.rs | 14 ++
rust/kernel/pci/id.rs | 8 +-
rust/kernel/sync/completion.rs | 20 +-
samples/rust/Kconfig | 11 +
samples/rust/Makefile | 1 +
samples/rust/rust_driver_edu.rs | 423 ++++++++++++++++++++++++++++++++
6 files changed, 474 insertions(+), 3 deletions(-)
create mode 100644 samples/rust/rust_driver_edu.rs
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 3ec897709e89..810703d7a2fb 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -445,6 +445,20 @@ pub fn pci_class(&self) -> Class {
// SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
Class::from_raw(unsafe { (*self.as_raw()).class })
}
+
+ /// Disable bus-mastering for this device.
+ #[inline]
+ pub fn clear_master(&self) {
+ // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+ unsafe { bindings::pci_clear_master(self.as_raw()) };
+ }
+
+ /// Disable the device.
+ #[inline]
+ pub fn disable_device(&self) {
+ // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+ unsafe { bindings::pci_disable_device(self.as_raw()) };
+ }
}
impl<'a> Device<device::Core<'a>> {
diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
index dbaf301666e7..2747d92be91b 100644
--- a/rust/kernel/pci/id.rs
+++ b/rust/kernel/pci/id.rs
@@ -154,9 +154,13 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// Once constructed, a `Vendor` contains a valid PCI Vendor ID.
impl Vendor {
- /// Create a Vendor from a raw 16-bit vendor ID.
+ /// Create a [`Vendor`] from a raw 16-bit vendor ID.
+ ///
+ /// Prefer one of the named [`Vendor`] constants where one exists. This is intended for
+ /// vendor IDs that have no dedicated constant, e.g. test or emulated devices whose IDs
+ /// are not shared between multiple drivers and thus do not belong in `pci_ids.h`.
#[inline]
- pub(super) fn from_raw(vendor_id: u16) -> Self {
+ pub const fn from_raw(vendor_id: u16) -> Self {
Self(vendor_id)
}
diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
index 35ff049ff078..83a22d25b61d 100644
--- a/rust/kernel/sync/completion.rs
+++ b/rust/kernel/sync/completion.rs
@@ -6,7 +6,12 @@
//!
//! C header: [`include/linux/completion.h`](srctree/include/linux/completion.h)
-use crate::{bindings, prelude::*, types::Opaque};
+use crate::{
+ bindings,
+ prelude::*,
+ time::Delta,
+ types::Opaque, //
+};
/// Synchronization primitive to signal when a certain task has been completed.
///
@@ -111,4 +116,17 @@ pub fn wait_for_completion(&self) {
// SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
unsafe { bindings::wait_for_completion(self.as_raw()) };
}
+
+ /// Wait for completion of a task, with a timeout.
+ ///
+ /// Returns `true` if the task completed before `timeout` elapsed, and `false` otherwise.
+ pub fn wait_for_completion_timeout(&self, timeout: Delta) -> bool {
+ let timeout = timeout.as_millis().clamp(0, u32::MAX as i64) as u32;
+ let jiffies = crate::time::msecs_to_jiffies(timeout);
+
+ // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
+ let remaining = unsafe { bindings::wait_for_completion_timeout(self.as_raw(), jiffies) };
+
+ remaining != 0
+ }
}
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 31d62533ef25..b654751041fa 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -190,4 +190,15 @@ config SAMPLE_RUST_HOSTPROGS
If unsure, say N.
+config SAMPLE_RUST_DRIVER_EDU
+ tristate "QEMU EDU Driver"
+ depends on PCI
+ help
+ This option builds the Rust QEMU EDU PCI driver sample.
+
+ To compile this as a module, choose M here:
+ the module will be called rust_driver_edu.
+
+ If unsure, say N.
+
endif # SAMPLES_RUST
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index b986b681cde5..57d944baa781 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY) += rust_driver_auxiliary.o
obj-$(CONFIG_SAMPLE_RUST_DRIVER_SERDEV) += rust_driver_serdev.o
obj-$(CONFIG_SAMPLE_RUST_CONFIGFS) += rust_configfs.o
obj-$(CONFIG_SAMPLE_RUST_SOC) += rust_soc.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_EDU) += rust_driver_edu.o
rust_print-y := rust_print_main.o rust_print_events.o
diff --git a/samples/rust/rust_driver_edu.rs b/samples/rust/rust_driver_edu.rs
new file mode 100644
index 000000000000..f21619e7bdc7
--- /dev/null
+++ b/samples/rust/rust_driver_edu.rs
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust QEMU EDU driver sample (based on QEMU's edu device).
+//!
+//! To make this driver probe, QEMU must be run with `-device edu`.
+
+use kernel::{
+ device::{
+ Bound,
+ Core, //
+ },
+ dma::{
+ Coherent,
+ CoherentBox,
+ Device as _,
+ DmaMask, //
+ },
+ io::{
+ poll::read_poll_timeout,
+ register,
+ register::Array,
+ Io, //
+ },
+ irq::{
+ self,
+ Flags,
+ Registration, //
+ },
+ pci,
+ prelude::*,
+ sync::Completion,
+ time::Delta,
+ types::ScopeGuard, //
+};
+
+mod regs {
+ use super::*;
+
+ register! {
+ pub(super) IDENTIFICATION(u32) @ 0x0 {
+ 31:0 version;
+ }
+
+ pub(super) LIVENESS(u32) @ 0x4 {
+ 31:0 value;
+ }
+
+ pub(super) FACTORIAL(u32) @ 0x8 {
+ 31:0 value;
+ }
+
+ pub(super) STATUS(u32) @ 0x20 {
+ 7:7 irq_on_done; // 0x80: raise an IRQ when the factorial is done
+ 0:0 computing; // 0x01: set while a factorial is being computed
+ }
+
+ pub(super) INTERRUPT_STATUS(u32) @ 0x24 {
+ 31:0 status;
+ }
+
+ pub(super) INTERRUPT_RAISE(u32) @ 0x60 {
+ 31:0 raise;
+ }
+
+ pub(super) INTERRUPT_ACK(u32) @ 0x64 {
+ 31:0 ack;
+ }
+
+ // The EDU DMA registers are documented as 64-bit, but QEMU's edu device
+ // accepts 4-byte accesses to these registers and zero-extends the
+ // written value, and this driver only uses addresses/counts that fit in
+ // 32 bits (a 28-bit DMA mask), so a single `u32` access per register is
+ // correct and sufficient here.
+ pub(super) DMA_SRC_ADDR(u32) @ 0x80 {
+ 31:0 src;
+ }
+
+ pub(super) DMA_DST_ADDR(u32) @ 0x88 {
+ 31:0 dst;
+ }
+
+ pub(super) DMA_CNT(u32) @ 0x90 {
+ 31:0 size;
+ }
+
+ pub(super) DMA_CMD(u32) @ 0x98 {
+ 31:0 cmd;
+ }
+ }
+
+ pub(super) const END: usize = 0x9C;
+}
+
+type Bar0<'bound> = pci::Bar<'bound, { regs::END }>;
+
+struct SampleDriverData<'bound> {
+ pdev: &'bound pci::Device,
+ // Held for the lifetime of the bound device so the BAR mapping stays alive;
+ // all register access happens during probe, so the field is never read again.
+ #[expect(dead_code)]
+ bar: Bar0<'bound>,
+}
+
+struct SampleDriver;
+
+// The QEMU EDU device's PCI IDs. These are specific to this emulated device and are not
+// shared with any other driver, so they are kept local here rather than in `pci_ids.h`.
+// `0x1234` is QEMU's conventional vendor ID, which has no dedicated `pci::Vendor` constant.
+const VENDOR_ID_QEMU: u16 = 0x1234;
+const DEVICE_ID_EDU: u32 = 0x11e8;
+
+kernel::pci_device_table!(
+ PCI_TABLE,
+ <SampleDriver as pci::Driver>::IdInfo,
+ [(
+ pci::DeviceId::from_id(pci::Vendor::from_raw(VENDOR_ID_QEMU), DEVICE_ID_EDU),
+ ()
+ )]
+);
+
+impl SampleDriverData<'_> {
+ /// The value the EDU device reports from its `IDENTIFICATION` register.
+ const EDU_ID: u32 = 0x010000ed;
+
+ /// Confirm we are really talking to an EDU device by reading its ID register.
+ fn identification(bar: &Bar0<'_>) -> Result {
+ if bar.read(regs::IDENTIFICATION).version().get() != Self::EDU_ID {
+ return Err(ENODEV);
+ }
+
+ Ok(())
+ }
+
+ /// Liveness check: the EDU device inverts (bitwise NOT) whatever we write to the
+ /// `LIVENESS` register, so writing `x` and reading back `!x` proves MMIO works.
+ fn liveness(bar: &Bar0<'_>) -> Result {
+ let pattern: u32 = 0xdead_beef;
+
+ bar.write_reg(regs::LIVENESS::zeroed().with_value(pattern));
+
+ let readback = bar.read(regs::LIVENESS).value().get();
+
+ if readback != !pattern {
+ return Err(EIO);
+ }
+
+ Ok(())
+ }
+
+ /// Exercise the interrupt registers at the MMIO level.
+ ///
+ /// Writing a bitmask to `INTERRUPT_RAISE` asserts those bits in
+ /// `INTERRUPT_STATUS` (and, if an IRQ is enabled, makes the device raise an
+ /// interrupt); writing the same bitmask to `INTERRUPT_ACK` clears them
+ /// again. Here we raise a test bit, confirm the status register reflects it,
+ /// then acknowledge it and confirm it clears -- all without registering a
+ /// handler. Wiring up MSI and an actual IRQ handler is a later step.
+ fn interrupt_status(bar: &Bar0<'_>) -> Result {
+ let pattern: u32 = 0x0000_0001;
+
+ bar.write_reg(regs::INTERRUPT_RAISE::zeroed().with_raise(pattern));
+
+ if bar.read(regs::INTERRUPT_STATUS).status().get() != pattern {
+ return Err(EIO);
+ }
+
+ bar.write_reg(regs::INTERRUPT_ACK::zeroed().with_ack(pattern));
+
+ if bar.read(regs::INTERRUPT_STATUS).status().get() != 0 {
+ return Err(EIO);
+ }
+
+ Ok(())
+ }
+
+ /// Make the edu device compute a factorial and inform the cpu
+ /// of completion via an IRQ
+ fn factorial_irq(pdev: &pci::Device<Bound>, bar: &Bar0<'_>) -> Result {
+ let operand: u32 = 6;
+
+ let msi = pci::IrqTypes::default().with(pci::IrqType::Msi);
+ let vectors = pdev.alloc_irq_vectors(1, 1, msi)?;
+
+ let registration = KBox::pin_init(
+ unsafe {
+ Registration::new(
+ vectors.index(0)?.into(),
+ Flags::TRIGGER_NONE,
+ c"rust_driver_edu",
+ try_pin_init!(EduIrq {
+ done <- Completion::new(),
+ bar,
+ }? Error),
+ )
+ },
+ GFP_KERNEL,
+ )?;
+
+ // Arm the "raise an IRQ when done" bit, then start the computation.
+ bar.write_reg(regs::STATUS::zeroed().with_irq_on_done(true));
+ bar.write_reg(regs::FACTORIAL::zeroed().with_value(operand));
+
+ // Sleep until the handler calls `complete_all()` from IRQ context.
+ if !registration
+ .handler()
+ .done
+ .wait_for_completion_timeout(Delta::from_secs(1))
+ {
+ bar.write_reg(regs::STATUS::zeroed());
+
+ let pending = bar.read(regs::INTERRUPT_STATUS).status().get();
+ bar.write_reg(regs::INTERRUPT_ACK::zeroed().with_ack(pending));
+
+ registration.synchronize();
+
+ return Err(ETIMEDOUT);
+ }
+
+ let result = bar.read(regs::FACTORIAL).value().get();
+
+ if result != factorial_helper(operand) {
+ return Err(EIO);
+ }
+
+ Ok(())
+ }
+
+ /// Tests a full DMA round-trip, writing from RAM -> device buffer,
+ /// and then from device buffer -> RAM
+ fn dma_test(pdev: &pci::Device<Core<'_>>, bar: &Bar0<'_>) -> Result {
+ const DEV_BUF: u32 = 0x40_000;
+ const LEN: usize = 512;
+ const CMD_START: u32 = 1 << 0;
+ const CMD_TO_RAM: u32 = 1 << 1;
+
+ unsafe { pdev.dma_set_mask_and_coherent(DmaMask::new::<28>())? };
+
+ let mut src: CoherentBox<[u8]> = CoherentBox::zeroed_slice(pdev.as_ref(), LEN, GFP_KERNEL)?;
+ src.fill(0xa5);
+
+ let src: Coherent<[u8]> = src.into();
+
+ let dst: Coherent<[u8]> =
+ CoherentBox::<[u8]>::zeroed_slice(pdev.as_ref(), LEN, GFP_KERNEL)?.into();
+
+ let src_dma = src.dma_address() as u32;
+ let dst_dma = dst.dma_address() as u32;
+
+ let wait_dma = || {
+ read_poll_timeout(
+ || Ok(bar.read(regs::DMA_CMD).cmd().get()),
+ |cmd| *cmd & CMD_START == 0,
+ Delta::from_millis(10),
+ Delta::from_secs(1),
+ )
+ };
+
+ // 1) RAM -> device internal buffer. CMD is written last: it triggers the transfer.
+ bar.write_reg(regs::DMA_SRC_ADDR::zeroed().with_src(src_dma));
+ bar.write_reg(regs::DMA_DST_ADDR::zeroed().with_dst(DEV_BUF));
+ bar.write_reg(regs::DMA_CNT::zeroed().with_size(LEN as u32));
+ bar.write_reg(regs::DMA_CMD::zeroed().with_cmd(CMD_START));
+ if wait_dma().is_err() {
+ // The EDU device cannot abort an in-flight DMA. Keep the buffer
+ // alive so the device cannot write to freed memory.
+ core::mem::forget(src);
+ return Err(ETIMEDOUT);
+ }
+
+ // 2) device internal buffer -> RAM, landing in `dst`.
+ bar.write_reg(regs::DMA_SRC_ADDR::zeroed().with_src(DEV_BUF));
+ bar.write_reg(regs::DMA_DST_ADDR::zeroed().with_dst(dst_dma));
+ bar.write_reg(regs::DMA_CNT::zeroed().with_size(LEN as u32));
+ bar.write_reg(regs::DMA_CMD::zeroed().with_cmd(CMD_START | CMD_TO_RAM));
+ if wait_dma().is_err() {
+ // The EDU device cannot abort an in-flight DMA. Keep the buffer
+ // alive so the device cannot write to freed memory.
+ core::mem::forget(dst);
+ return Err(ETIMEDOUT);
+ }
+
+ // Transfer completed, check dst is the expected value
+ let out = unsafe { dst.as_ref() };
+ if out.iter().any(|&b| b != 0xa5) {
+ return Err(EIO);
+ }
+
+ 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,
+ "edu config space read8 rev ID: {:x}\n",
+ config.read(REVISION_ID).revision_id()
+ );
+
+ dev_info!(
+ pdev,
+ "edu config space read16 vendor ID: {:x}\n",
+ config.read(VENDOR_ID).vendor_id()
+ );
+
+ dev_info!(
+ pdev,
+ "edu config space read32 BAR 0: {:x}\n",
+ config.read(BAR::at(0)).value()
+ );
+ }
+}
+
+impl pci::Driver for SampleDriver {
+ type IdInfo = ();
+ type Data<'bound> = SampleDriverData<'bound>;
+
+ const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
+
+ fn probe<'bound>(
+ pdev: &'bound pci::Device<Core<'_>>,
+ _info: Option<&'bound Self::IdInfo>,
+ ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
+ dev_dbg!(
+ pdev,
+ "Probe Rust QEMU EDU driver sample (PCI ID: {}, 0x{:x}).\n",
+ pdev.vendor_id(),
+ pdev.device_id()
+ );
+
+ pdev.enable_device_mem()?;
+ pdev.set_master();
+
+ let cleanup = ScopeGuard::new(|| {
+ pdev.clear_master();
+ pdev.disable_device();
+ });
+
+ let bar = pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_edu")?;
+
+ SampleDriverData::identification(&bar)?;
+ SampleDriverData::liveness(&bar)?;
+ SampleDriverData::interrupt_status(&bar)?;
+ SampleDriverData::factorial_irq(pdev, &bar)?;
+ SampleDriverData::dma_test(pdev, &bar)?;
+ SampleDriverData::config_space(pdev);
+
+ dev_info!(pdev, "EDU device probed: all checks passed.\n");
+
+ cleanup.dismiss();
+
+ Ok(SampleDriverData { pdev, bar })
+ }
+}
+
+impl Drop for SampleDriverData<'_> {
+ fn drop(&mut self) {
+ dev_dbg!(self.pdev, "Remove Rust QEMU EDU driver sample.\n");
+
+ self.pdev.clear_master();
+ self.pdev.disable_device();
+ }
+}
+
+/// Per-interrupt state shared between process context and IRQ context.
+///
+/// The factorial computation signals `done` from the IRQ handler; the process
+/// context that started it waits on the same [`Completion`].
+#[pin_data]
+struct EduIrq<'bar, 'bound> {
+ #[pin]
+ done: Completion,
+ bar: &'bar Bar0<'bound>,
+}
+
+impl irq::Handler for EduIrq<'_, '_> {
+ // Runs in IRQ context when the EDU device raises its interrupt.
+ fn handle(&self) -> irq::IrqReturn {
+ let pending = self.bar.read(regs::INTERRUPT_STATUS).status().get();
+
+ if pending == 0 {
+ return irq::IrqReturn::None;
+ }
+
+ self.bar
+ .write_reg(regs::INTERRUPT_ACK::zeroed().with_ack(pending));
+ self.done.complete_all();
+
+ irq::IrqReturn::Handled
+ }
+}
+
+// Helper functions
+fn factorial_helper(n: u32) -> u32 {
+ if n <= 1 {
+ return 1;
+ }
+
+ n * factorial_helper(n - 1)
+}
+
+kernel::module_pci_driver! {
+ type: SampleDriver,
+ name: "rust_driver_edu",
+ authors: ["Lucas Costa"],
+ description: "Rust QEMU EDU driver",
+ license: "GPL v2",
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] samples: rust: add QEMU EDU sample driver
2026-10-30 18:15 [PATCH v2] samples: rust: add QEMU EDU sample driver Lucas Costa
@ 2026-10-30 18:30 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-30 18:30 UTC (permalink / raw)
To: Lucas Costa; +Cc: linux-pci, ojeda, gary
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rust: pci: clear_master() and disable_device() exposed on Sync Device<Normal>
- [Medium] rust: sync: bad timeout conversion in wait_for_completion_timeout()
--
commit 49001463072fd010a27e67166e414638fec06345
Author: Lucas Costa <2000.costalucas@gmail.com>
samples: rust: add QEMU EDU sample driver
This commit adds a Rust PCI sample driver for QEMU's educational ("edu")
device along with PCI teardown helpers and completion timeout support.
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 3ec897709e890..810703d7a2fb5 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -445,6 +445,20 @@ pub fn pci_class(&self) -> Class {
> // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
> Class::from_raw(unsafe { (*self.as_raw()).class })
> }
> +
> + /// Disable bus-mastering for this device.
> + #[inline]
> + pub fn clear_master(&self) {
> + // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> + unsafe { bindings::pci_clear_master(self.as_raw()) };
> + }
> +
> + /// Disable the device.
> + #[inline]
> + pub fn disable_device(&self) {
> + // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> + unsafe { bindings::pci_disable_device(self.as_raw()) };
> + }
> }
>
> impl<'a> Device<device::Core<'a>> {
[Severity: High]
Should clear_master() and disable_device() be restricted to
Device<device::Core<'_>> alongside enable_device_mem() and set_master()
rather than exposed on impl Device (Device<device::Normal>)?
Device<device::Normal> and Device<device::Bound> implement Sync under the
safety invariant that all methods on Device<device::Normal> are thread-safe,
and Device<device::Normal> implements AlwaysRefCounted so ARef<pci::Device>
can be shared across threads and outlive driver unbind:
rust/kernel/pci.rs:
// SAFETY: `Device` can be shared among threads because all methods of `Device`
// (i.e. `Device<Normal>) are thread safe.
unsafe impl Sync for Device {}
However, pci_clear_master() (via __pci_set_master()) and
pci_disable_device() perform unsynchronized read-modify-write sequences on
PCI_COMMAND in configuration space and non-atomic writes to
dev->is_busmaster, which is a 1-bit C bitfield sharing a storage word with
msi_enabled, msix_enabled, block_cfg_access, is_managed, and state_saved in
struct pci_dev.
Furthermore, in samples/rust/rust_driver_edu.rs, SampleDriverData::drop()
calls self.pdev.clear_master() and self.pdev.disable_device():
samples/rust/rust_driver_edu.rs:
impl Drop for SampleDriverData<'_> {
fn drop(&mut self) {
dev_dbg!(self.pdev, "Remove Rust QEMU EDU driver sample.\n");
self.pdev.clear_master();
self.pdev.disable_device();
}
}
During unbind, Registration::post_unbind_callback() drops SampleDriverData
in device_unbind_cleanup() only after pci_device_remove() has already
dropped the runtime PM reference and cleared pci_dev->driver and after
pci_dma_cleanup() has detached the IOMMU default domain. In addition,
SampleDriverData::drop() runs before its bar field (Bar0) is dropped, so
pci_disable_device() executes before Bar::drop() calls pci_iounmap() and
pci_release_region().
Would moving clear_master() and disable_device() to
impl Device<device::Core<'_>> and performing teardown in
pci::Driver::unbind() (or managing device enablement via devres) avoid both
the Sync soundness issue and the out-of-phase teardown?
[ ... ]
> diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
> index 35ff049ff0783..83a22d25b61dd 100644
> --- a/rust/kernel/sync/completion.rs
> +++ b/rust/kernel/sync/completion.rs
> @@ -111,4 +116,17 @@ pub fn wait_for_completion(&self) {
> // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
> unsafe { bindings::wait_for_completion(self.as_raw()) };
> }
> +
> + /// Wait for completion of a task, with a timeout.
> + ///
> + /// Returns `true` if the task completed before `timeout` elapsed, and `false` otherwise.
> + pub fn wait_for_completion_timeout(&self, timeout: Delta) -> bool {
> + let timeout = timeout.as_millis().clamp(0, u32::MAX as i64) as u32;
> + let jiffies = crate::time::msecs_to_jiffies(timeout);
> +
> + // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
> + let remaining = unsafe { bindings::wait_for_completion_timeout(self.as_raw(), jiffies) };
> +
> + remaining != 0
> + }
> }
[Severity: Medium]
Can this timeout conversion in Completion::wait_for_completion_timeout()
mishandle both sub-millisecond and large Delta values?
First, timeout.as_millis() truncates towards zero rather than rounding up
with timeout.as_millis_ceil(), so any positive Delta under 1 ms (such as
Delta::from_micros(500)) is converted to 0 jiffies and times out
immediately without sleeping if the completion is not already done.
Second, clamping the upper bound to u32::MAX passes values in
(i32::MAX as u32 + 1)..=u32::MAX to __msecs_to_jiffies(), which treats any
value where (int)m < 0 as a negative timeout and returns MAX_JIFFY_OFFSET
(an infinite timeout on 64-bit systems):
kernel/time/time.c:__msecs_to_jiffies()
if ((int)m < 0)
return MAX_JIFFY_OFFSET;
return _msecs_to_jiffies(m);
Should this use timeout.as_millis_ceil().clamp(0, i32::MAX as i64) as u32
(and also be marked #[inline] like the other methods on Completion)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030181540.102665-1-2000.costalucas@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-30 18:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 18:15 [PATCH v2] samples: rust: add QEMU EDU sample driver Lucas Costa
2026-10-30 18:30 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox