Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH v4 0/2] rust: introduce abstractions for fwctl
@ 2026-06-24  9:17 Zhi Wang
  2026-06-24  9:17 ` [PATCH v4 1/2] fwctl: add device release hook Zhi Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Zhi Wang @ 2026-06-24  9:17 UTC (permalink / raw)
  To: rust-for-linux, linux-kernel
  Cc: dakr, jgg, gary, joelagnelf, aliceryhl, kwilczynski, ojeda,
	alex.gaynor, boqun.feng, bjorn3_gh, lossin, a.hindborg, tmgross,
	cjia, smitra, ankita, aniketa, kwankhede, targupta, kjaju,
	alkumar, acourbot, jhubbard, zhiwang, daniel.almeida, Zhi Wang

In the NVIDIA vGPU RFC [1], the vGPU type blobs must be provided to the GSP
before userspace can enumerate available vGPU types and create vGPU
instances. The original design relied on the firmware loading interface,
but fwctl is a more natural fit for this use case, as it is designed for
uploading configuration or firmware data required before the device becomes
operational.

This series introduces a Rust abstraction over the fwctl subsystem,
providing safe and idiomatic bindings.

Patch 1 adds a small fwctl core release hook for private tail data. Rust
wrappers need such a hook to run drop glue for data embedded in the fwctl
allocation before the allocation is freed.

Patch 2 adds the Rust fwctl module. It allows Rust drivers to integrate with
the existing C-side fwctl core through a typed trait interface. It provides:

  - `Operations` trait: defines driver-specific callbacks: `open()`,
    `close()`, `info()`, and `fw_rpc()`. The implementing type itself
    serves as the per-FD user context, one instance per open().

  - `Device<T>`: wraps `struct fwctl_device` with embedded driver-specific
    data (`T::DeviceData`). The data is dropped from the fwctl device
    release hook.

  - `Registration<T>`: safe registration and automatic unregistration of
    `struct fwctl_device` objects, living inside `Devres` to ensure teardown
    before the parent device unbinds.

  - `RpcScope` / `FwRpcResponse`: type-safe enums for RPC scope and response
    handling, keeping unsafe pointer manipulation confined to the abstraction
    layer.

`rust/kernel/lib.rs` is updated to conditionally include this module under
`CONFIG_RUST_FWCTL_ABSTRACTIONS`.

v4:

- Rebase on top of the current drm-rust-next.
- Split out the fwctl core release hook before the Rust abstraction.
- Drop the fwctl init-ordering change from this series; it is already in
  drm-rust-next as commit a55f80233f38 ("fwctl: Fix class init ordering to
  avoid NULL pointer dereference on device removal").
- Add compile-time layout checks for the embedded `struct fwctl_device` and
  `struct fwctl_uctx` offset assumptions. (Jason)
- Use `const_assert!()` for generic layout assertions. (Zhi)
- Require `Operations` and its `DeviceData` to be `Send + Sync`. (Danilo)
- Pass pinned shared references to `info()` and `fw_rpc()`. (Danilo)
- Make `Operations::open()` return an initializer directly and report open
  failures through the initializer error path. (Danilo)
- Drop `DeviceData` from the fwctl device release hook.
- Fix clippy warnings for raw pointer casts and unsafe blocks. (Danilo)
- Fix the rustdoc broken link warning. (Danilo)

Link to v3: [2]

v3:

Quite some updates in this version. Here you can find the example
nova-core fwctl driver [3]. The interface is still WIP so it is just to
demonstrate the use of the rust fwctl abstractions.

Comments from folks:

- Use an enum for the return of fw_rpc. (Joel)
- Remove FWCTL_DEVICE_TYPE_RUST_FWCTL_TEST together with the sample
  driver. (Jason)
- Remove DeviceType:Error. (Gary)
- Add __rust_helper for fwctl_get/fwctl_put. (Gary)
- Refine the design of the device private data. Now it has a similar
  device private data structure as DRM. (Danilo)
- Separate fwctl alloc and register in the abstractions. (Jason)
- Registration::new() now takes &fwctl::Device<T> and the parent
  &Device<Bound> to align with other class device abstractions. (Danilo)
- Update the Registration SAFETY comments. (Danilo & Jason)
- Take self as per-FD user context in callbacks. (Danilo)
- {open, close}_uctx -> {open, close}(). open() now takes &Device<Self>.
  (Danilo)

Updates from me:

- Introduce enums for fwctl RPC scope.
- Introduce AlwaysRefCounted to avoid hacks after introducing the
  refined flow of device private data.
- Introduce default implementation of close()/info().
- Fix a leak: Drop T::UserCtx in the close_uctx_callback explicitly.

v2:

- Don't open fwctl_put(). Add a rust helper. (Jason/Danilo)
- Wrap Registration with Devres to guarantee proper lifetime management.
  (Jason/Danilo)
- Rename FwctlOps to Operations, FwctlUserCtx to UserCtx, FwctlDevice to
  Device. (Danilo)
- Use fwctl::DeviceType enum instead of raw u32 for DEVICE_TYPE. (Danilo)
- Change fwctl_uctx field in UserCtx to Opaque<bindings::fwctl_uctx> and
  make it private. (Danilo)
- Provide Deref and DerefMut implementations for UserCtx::uctx. (Danilo)
- Add UserCtx::parent_device_from_raw() to simplify parent device access.
- Use cast() and cast_mut() instead of manual pointer casts. (Danilo)
- Implement AlwaysRefCounted for Device and use ARef<Device> in
  Registration. (Danilo)
- Add rust_helper_fwctl_get() for reference counting.
- Improve safety comments for slice::from_raw_parts_mut() in
  fw_rpc_callback. (Danilo)
- Convert imports to vertical style.
- Fix all clippy warnings.

v1:

- Initial submission introducing fwctl Rust abstractions.

[1] https://lore.kernel.org/all/20250903221111.3866249-1-zhiw@nvidia.com/
[2] https://lore.kernel.org/all/20260217204909.211793-1-zhiw@nvidia.com/
[3] https://github.com/zhiwang-nvidia/nova-core/commit/2068da7e8caf58da9584b0aa6c81fed8f547d59f

Zhi Wang (2):
  fwctl: add device release hook
  rust: introduce abstractions for fwctl

 drivers/fwctl/Kconfig           |  12 +
 drivers/fwctl/main.c            |   2 +
 include/linux/fwctl.h           |   2 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/fwctl.c            |  17 ++
 rust/helpers/helpers.c          |   3 +-
 rust/kernel/fwctl.rs            | 486 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 8 files changed, 524 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/fwctl.c
 create mode 100644 rust/kernel/fwctl.rs

-- 
2.51.0


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

* [PATCH v4 1/2] fwctl: add device release hook
  2026-06-24  9:17 [PATCH v4 0/2] rust: introduce abstractions for fwctl Zhi Wang
@ 2026-06-24  9:17 ` Zhi Wang
  2026-06-24  9:17 ` [PATCH v4 2/2] rust: introduce abstractions for fwctl Zhi Wang
  2026-06-24 11:01 ` [PATCH v4 0/2] " Miguel Ojeda
  2 siblings, 0 replies; 5+ messages in thread
From: Zhi Wang @ 2026-06-24  9:17 UTC (permalink / raw)
  To: rust-for-linux, linux-kernel
  Cc: dakr, jgg, gary, joelagnelf, aliceryhl, kwilczynski, ojeda,
	alex.gaynor, boqun.feng, bjorn3_gh, lossin, a.hindborg, tmgross,
	cjia, smitra, ankita, aniketa, kwankhede, targupta, kjaju,
	alkumar, acourbot, jhubbard, zhiwang, daniel.almeida, Zhi Wang

Add an optional per-device release callback to struct fwctl_device and
invoke it from fwctl_device_release() before kfree().

This gives higher-level abstractions a final teardown point for tail data
that requires explicit destruction.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/fwctl/main.c  | 2 ++
 include/linux/fwctl.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/fwctl/main.c b/drivers/fwctl/main.c
index 098c3824ad75..7bc76e3a5306 100644
--- a/drivers/fwctl/main.c
+++ b/drivers/fwctl/main.c
@@ -275,6 +275,8 @@ static void fwctl_device_release(struct device *device)
 
 	ida_free(&fwctl_ida, fwctl->dev.devt - fwctl_dev);
 	mutex_destroy(&fwctl->uctx_list_lock);
+	if (fwctl->release_data)
+		fwctl->release_data(fwctl);
 	kfree(fwctl);
 }
 
diff --git a/include/linux/fwctl.h b/include/linux/fwctl.h
index 5d61fc8a6871..25834f6b79a8 100644
--- a/include/linux/fwctl.h
+++ b/include/linux/fwctl.h
@@ -79,6 +79,8 @@ struct fwctl_device {
 	 */
 	struct rw_semaphore registration_lock;
 	const struct fwctl_ops *ops;
+	/* Optional release hook for driver-private tail data. */
+	void (*release_data)(struct fwctl_device *fwctl);
 };
 
 struct fwctl_device *_fwctl_alloc_device(struct device *parent,
-- 
2.51.0


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

* [PATCH v4 2/2] rust: introduce abstractions for fwctl
  2026-06-24  9:17 [PATCH v4 0/2] rust: introduce abstractions for fwctl Zhi Wang
  2026-06-24  9:17 ` [PATCH v4 1/2] fwctl: add device release hook Zhi Wang
@ 2026-06-24  9:17 ` Zhi Wang
  2026-06-24 17:41   ` Danilo Krummrich
  2026-06-24 11:01 ` [PATCH v4 0/2] " Miguel Ojeda
  2 siblings, 1 reply; 5+ messages in thread
From: Zhi Wang @ 2026-06-24  9:17 UTC (permalink / raw)
  To: rust-for-linux, linux-kernel
  Cc: dakr, jgg, gary, joelagnelf, aliceryhl, kwilczynski, ojeda,
	alex.gaynor, boqun.feng, bjorn3_gh, lossin, a.hindborg, tmgross,
	cjia, smitra, ankita, aniketa, kwankhede, targupta, kjaju,
	alkumar, acourbot, jhubbard, zhiwang, daniel.almeida, Zhi Wang

Introduce safe Rust wrappers around struct fwctl_device and
struct fwctl_uctx. This lets Rust drivers register fwctl devices and
implement firmware RPC callbacks through a typed trait interface.

The abstraction keeps lifetime and reference-count handling inside the
wrapper, exposes pinned per-FD user contexts to drivers, and validates the
layout assumptions required by the C fwctl allocation model.

DeviceData is destroyed from the fwctl device release hook, so Rust driver
data is dropped at the same point as the C allocation is released.

Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/fwctl/Kconfig           |  12 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/fwctl.c            |  17 ++
 rust/helpers/helpers.c          |   3 +-
 rust/kernel/fwctl.rs            | 486 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 6 files changed, 520 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/fwctl.c
 create mode 100644 rust/kernel/fwctl.rs

diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
index d1b1925bdaec..bbfc31b0681c 100644
--- a/drivers/fwctl/Kconfig
+++ b/drivers/fwctl/Kconfig
@@ -9,6 +9,18 @@ menuconfig FWCTL
 	  fit neatly into an existing subsystem.
 
 if FWCTL
+
+config RUST_FWCTL_ABSTRACTIONS
+	bool "Rust fwctl abstractions"
+	depends on RUST
+	help
+	  This enables the Rust abstractions for the fwctl device firmware
+	  access framework. It provides safe wrappers around struct fwctl_device
+	  and struct fwctl_uctx, allowing Rust drivers to register fwctl devices
+	  and implement their control and RPC logic in safe Rust.
+
+	  If unsure, say N.
+
 config FWCTL_BNXT
 	tristate "bnxt control fwctl driver"
 	depends on BNXT
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 1124785e210b..3d0511e4ab4f 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -60,6 +60,7 @@
 #include <linux/fdtable.h>
 #include <linux/file.h>
 #include <linux/firmware.h>
+#include <linux/fwctl.h>
 #include <linux/fs.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
diff --git a/rust/helpers/fwctl.c b/rust/helpers/fwctl.c
new file mode 100644
index 000000000000..c7eecd4336a7
--- /dev/null
+++ b/rust/helpers/fwctl.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/fwctl.h>
+
+#if IS_ENABLED(CONFIG_RUST_FWCTL_ABSTRACTIONS)
+
+__rust_helper struct fwctl_device *rust_helper_fwctl_get(struct fwctl_device *fwctl)
+{
+	return fwctl_get(fwctl);
+}
+
+__rust_helper void rust_helper_fwctl_put(struct fwctl_device *fwctl)
+{
+	fwctl_put(fwctl);
+}
+
+#endif
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 4488a87223b9..b360bd837569 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -61,10 +61,11 @@
 #include "drm.c"
 #include "drm_gpuvm.c"
 #include "err.c"
-#include "irq.c"
 #include "fs.c"
+#include "fwctl.c"
 #include "gpu.c"
 #include "io.c"
+#include "irq.c"
 #include "jump_label.c"
 #include "kunit.c"
 #include "list.c"
diff --git a/rust/kernel/fwctl.rs b/rust/kernel/fwctl.rs
new file mode 100644
index 000000000000..f5f802f5299c
--- /dev/null
+++ b/rust/kernel/fwctl.rs
@@ -0,0 +1,486 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+//! Abstractions for the fwctl subsystem.
+//!
+//! C header: `include/linux/fwctl.h`
+
+use crate::{
+    bindings,
+    container_of,
+    device,
+    devres::Devres,
+    prelude::*,
+    sync::aref::{
+        ARef,
+        AlwaysRefCounted, //
+    },
+    types::Opaque, //
+};
+use core::{
+    marker::PhantomData,
+    ptr::NonNull,
+    slice, //
+};
+
+/// Represents a fwctl device type.
+///
+/// Corresponds to the C `enum fwctl_device_type`.
+#[repr(u32)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum DeviceType {
+    /// Mellanox ConnectX (mlx5) device.
+    Mlx5 = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_MLX5,
+    /// CXL (Compute Express Link) device.
+    Cxl = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_CXL,
+    /// AMD/Pensando PDS device.
+    Pds = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_PDS,
+}
+
+impl From<DeviceType> for u32 {
+    fn from(device_type: DeviceType) -> Self {
+        device_type as u32
+    }
+}
+
+/// Scope of access for an RPC request.
+///
+/// Corresponds to the C `enum fwctl_rpc_scope`.
+#[repr(u32)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum RpcScope {
+    /// Read/write access to device configuration.
+    Configuration = bindings::fwctl_rpc_scope_FWCTL_RPC_CONFIGURATION,
+    /// Read-only access to debug information.
+    DebugReadOnly = bindings::fwctl_rpc_scope_FWCTL_RPC_DEBUG_READ_ONLY,
+    /// Write access to lockdown-compatible debug information.
+    DebugWrite = bindings::fwctl_rpc_scope_FWCTL_RPC_DEBUG_WRITE,
+    /// Full read/write access to all debug information (requires `CAP_SYS_RAWIO`).
+    DebugWriteFull = bindings::fwctl_rpc_scope_FWCTL_RPC_DEBUG_WRITE_FULL,
+}
+
+impl TryFrom<u32> for RpcScope {
+    type Error = Error;
+
+    fn try_from(value: u32) -> Result<Self, Error> {
+        match value {
+            v if v == Self::Configuration as u32 => Ok(Self::Configuration),
+            v if v == Self::DebugReadOnly as u32 => Ok(Self::DebugReadOnly),
+            v if v == Self::DebugWrite as u32 => Ok(Self::DebugWrite),
+            v if v == Self::DebugWriteFull as u32 => Ok(Self::DebugWriteFull),
+            _ => Err(ENOTSUPP),
+        }
+    }
+}
+
+/// Response from a [`Operations::fw_rpc`] call.
+pub enum FwRpcResponse {
+    /// Reuse the input buffer as the output, with the given output length.
+    InPlace(usize),
+    /// Return a newly allocated buffer as the output.
+    NewBuffer(KVec<u8>),
+}
+
+/// Trait implemented by each Rust driver that integrates with the fwctl subsystem.
+///
+/// The implementing type **is** the per-FD user context: one instance is
+/// created for each `open()` call and dropped when the FD is closed.
+///
+/// Each implementation corresponds to a specific device type and provides the
+/// vtable used by the core `fwctl` layer to manage per-FD user contexts and
+/// handle RPC requests.
+pub trait Operations: Sized + Send + Sync {
+    /// Driver data embedded alongside the `fwctl_device` allocation.
+    type DeviceData: Send + Sync;
+
+    /// fwctl device type identifier.
+    const DEVICE_TYPE: DeviceType;
+
+    /// Called when a new user context is opened.
+    ///
+    /// Returns a [`PinInit`] initializer for `Self`. The instance is dropped
+    /// automatically when the FD is closed (after [`close`](Self::close)).
+    fn open(device: &Device<Self>) -> impl PinInit<Self, Error>;
+
+    /// Called when the user context is closed.
+    ///
+    /// The driver may perform additional cleanup here that requires access
+    /// to the owning [`Device`]. `Self` is dropped automatically after this
+    /// returns.
+    fn close(_this: Pin<&mut Self>, _device: &Device<Self>) {}
+
+    /// Return device information to userspace.
+    ///
+    /// The default implementation returns no device-specific data.
+    fn info(_this: Pin<&Self>, _device: &Device<Self>) -> Result<KVec<u8>, Error> {
+        Ok(KVec::new())
+    }
+
+    /// Handle a userspace RPC request.
+    fn fw_rpc(
+        this: Pin<&Self>,
+        device: &Device<Self>,
+        scope: RpcScope,
+        rpc_in: &mut [u8],
+    ) -> Result<FwRpcResponse, Error>;
+}
+
+/// A fwctl device with embedded driver data.
+///
+/// `#[repr(C)]` with the `fwctl_device` at offset 0, matching the C
+/// `fwctl_alloc_device()` layout convention.
+///
+/// # Invariants
+///
+/// - `dev` is embedded at offset 0 and is initialised by fwctl.
+/// - The fwctl refcount owns the allocation lifetime.
+/// - `data` is dropped from the fwctl core release hook before `kfree()`.
+#[repr(C)]
+pub struct Device<T: Operations> {
+    dev: Opaque<bindings::fwctl_device>,
+    data: T::DeviceData,
+}
+
+impl<T: Operations> Device<T> {
+    /// Allocate a new fwctl device with embedded driver data.
+    ///
+    /// Returns an [`ARef`] that can be passed to [`Registration::new()`]
+    /// to make the device visible to userspace. The caller may inspect or
+    /// configure the device between allocation and registration.
+    pub fn new(
+        parent: &device::Device<device::Bound>,
+        data: impl PinInit<T::DeviceData, Error>,
+    ) -> Result<ARef<Self>> {
+        const_assert!(
+            core::mem::offset_of!(Self, dev) == 0,
+            "struct fwctl_device must be at offset 0"
+        );
+
+        let ops = core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();
+
+        // SAFETY: `ops` is static, `parent` is bound, and `size` includes the
+        // offset-0 `fwctl_device` plus `DeviceData`.
+        let raw = unsafe {
+            bindings::_fwctl_alloc_device(parent.as_raw(), ops, core::mem::size_of::<Self>())
+        };
+
+        if raw.is_null() {
+            return Err(ENOMEM);
+        }
+
+        let this = raw.cast::<Self>();
+
+        // SAFETY: `this` points to the allocation just returned by fwctl.
+        let data_ptr = unsafe { core::ptr::addr_of_mut!((*this).data) };
+        // SAFETY: `data_ptr` addresses the uninitialised tail data.
+        unsafe { data.__pinned_init(data_ptr) }.inspect_err(|_| {
+            // SAFETY: `raw` still owns the initial reference.
+            unsafe { bindings::fwctl_put(raw) };
+        })?;
+
+        // SAFETY: `raw` is a live fwctl_device allocated above.
+        unsafe { (*raw).release_data = Some(Self::release_data_callback) };
+
+        // SAFETY: `raw` owns the initial reference and `DeviceData` is ready.
+        Ok(unsafe { ARef::from_raw(NonNull::new(raw.cast::<Self>()).ok_or(ENOMEM)?) })
+    }
+
+    /// Returns a reference to the embedded driver data.
+    pub fn data(&self) -> &T::DeviceData {
+        &self.data
+    }
+
+    fn as_raw(&self) -> *mut bindings::fwctl_device {
+        self.dev.get()
+    }
+
+    /// # Safety
+    ///
+    /// `raw` must point to an offset-0 `fwctl_device` embedded in `Device<T>`.
+    /// fwctl calls this exactly once from the device release path.
+    unsafe extern "C" fn release_data_callback(raw: *mut bindings::fwctl_device) {
+        let this = raw.cast::<Self>();
+
+        // SAFETY: fwctl invokes this callback once during the final device
+        // release, before freeing the allocation.
+        unsafe { core::ptr::drop_in_place(core::ptr::addr_of_mut!((*this).data)) };
+    }
+
+    /// # Safety
+    ///
+    /// `ptr` must point to a valid `fwctl_device` embedded in a `Device<T>`.
+    unsafe fn from_raw<'a>(ptr: *mut bindings::fwctl_device) -> &'a Self {
+        // SAFETY: The caller upholds the offset-0 `Device<T>` invariant.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the parent device.
+    ///
+    /// The parent is guaranteed to be bound while any fwctl callback is
+    /// running (ensured by the `registration_lock` read lock on the ioctl
+    /// path and by `Devres` on the teardown path).
+    pub fn parent(&self) -> &device::Device<device::Bound> {
+        // SAFETY: fwctl sets the parent during allocation.
+        let parent_dev = unsafe { (*self.as_raw()).dev.parent };
+        // SAFETY: The parent stays live while fwctl ops run.
+        let dev: &device::Device = unsafe { device::Device::from_raw(parent_dev) };
+        // SAFETY: Devres teardown keeps the parent bound here.
+        unsafe { dev.as_bound() }
+    }
+}
+
+impl<T: Operations> AsRef<device::Device> for Device<T> {
+    fn as_ref(&self) -> &device::Device {
+        // SAFETY: `self` contains a live fwctl_device.
+        let dev = unsafe { core::ptr::addr_of_mut!((*self.as_raw()).dev) };
+        // SAFETY: The embedded device is initialised by fwctl.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+// SAFETY: `fwctl_get` increments the refcount of a valid fwctl_device.
+// `fwctl_put` decrements it and frees the device when it reaches zero.
+unsafe impl<T: Operations> AlwaysRefCounted for Device<T> {
+    fn inc_ref(&self) {
+        // SAFETY: `self` holds a live reference.
+        unsafe { bindings::fwctl_get(self.as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The caller owns a live reference.
+        unsafe { bindings::fwctl_put(obj.cast().as_ptr()) };
+    }
+}
+
+// SAFETY: `Device<T>` is refcounted by the fwctl core and may be released from
+// any thread. The embedded driver data is `Send`.
+unsafe impl<T: Operations> Send for Device<T> {}
+
+// SAFETY: Shared access to the embedded `fwctl_device` is protected by the
+// fwctl core, and the embedded driver data is `Sync`.
+unsafe impl<T: Operations> Sync for Device<T> {}
+
+/// A registered fwctl device.
+///
+/// Must live inside a [`Devres`] to guarantee that [`fwctl_unregister`] runs
+/// before the parent driver unbinds. `Devres` prevents the `Registration`
+/// from being moved to a context that could outlive the parent device.
+///
+/// On drop the device is unregistered (all user contexts are closed and
+/// `ops` is set to `NULL`) and the [`ARef`] is released.
+///
+/// [`fwctl_unregister`]: srctree/drivers/fwctl/main.c
+pub struct Registration<T: Operations> {
+    dev: ARef<Device<T>>,
+}
+
+impl<T: Operations> Registration<T> {
+    /// Register a previously allocated fwctl device.
+    pub fn new<'a>(
+        parent: &'a device::Device<device::Bound>,
+        dev: &'a Device<T>,
+    ) -> impl PinInit<Devres<Self>, Error> + 'a {
+        pin_init::pin_init_scope(move || {
+            // SAFETY: `dev` is a valid fwctl_device backed by an ARef.
+            let ret = unsafe { bindings::fwctl_register(dev.as_raw()) };
+            if ret != 0 {
+                return Err(Error::from_errno(ret));
+            }
+
+            Ok(Devres::new(parent, Self { dev: dev.into() }))
+        })
+    }
+}
+
+impl<T: Operations> Drop for Registration<T> {
+    fn drop(&mut self) {
+        // SAFETY: `Registration` lives inside a `Devres`, which guarantees
+        // that drop runs while the parent device is still bound.
+        unsafe { bindings::fwctl_unregister(self.dev.as_raw()) };
+        // ARef<Device<T>> is dropped after this, calling fwctl_put.
+    }
+}
+
+// SAFETY: `Registration` can be sent between threads; the underlying
+// fwctl_device uses internal locking.
+unsafe impl<T: Operations> Send for Registration<T> {}
+
+// SAFETY: `Registration` provides no mutable access; the underlying
+// fwctl_device is protected by internal locking.
+unsafe impl<T: Operations> Sync for Registration<T> {}
+
+/// Internal per-FD user context wrapping `struct fwctl_uctx` and `T`.
+///
+/// Not exposed to drivers; they work with `&T` / `Pin<&mut T>` directly.
+#[repr(C)]
+#[pin_data]
+struct UserCtx<T: Operations> {
+    #[pin]
+    fwctl_uctx: Opaque<bindings::fwctl_uctx>,
+    #[pin]
+    uctx: T,
+}
+
+impl<T: Operations> UserCtx<T> {
+    /// # Safety
+    ///
+    /// `ptr` must point to a `fwctl_uctx` embedded in a live `UserCtx<T>`.
+    unsafe fn from_raw<'a>(ptr: *mut bindings::fwctl_uctx) -> &'a Self {
+        // SAFETY: The caller upholds the `UserCtx<T>` embedding invariant.
+        unsafe { &*container_of!(Opaque::cast_from(ptr), Self, fwctl_uctx) }
+    }
+
+    /// # Safety
+    ///
+    /// `ptr` must point to a `fwctl_uctx` embedded in a live `UserCtx<T>`.
+    /// The caller must ensure exclusive access to the `UserCtx<T>`.
+    unsafe fn from_raw_mut<'a>(ptr: *mut bindings::fwctl_uctx) -> &'a mut Self {
+        // SAFETY: The caller upholds the embedding and exclusivity invariants.
+        unsafe { &mut *container_of!(Opaque::cast_from(ptr), Self, fwctl_uctx).cast_mut() }
+    }
+
+    /// Returns a reference to the fwctl [`Device`] that owns this context.
+    fn device(&self) -> &Device<T> {
+        // SAFETY: fwctl initialises this pointer before any driver callback.
+        let raw_fwctl = unsafe { (*self.fwctl_uctx.get()).fwctl };
+        // SAFETY: Rust fwctl devices use the offset-0 `Device<T>` layout.
+        unsafe { Device::from_raw(raw_fwctl) }
+    }
+}
+
+/// Static vtable mapping Rust trait methods to C callbacks.
+pub struct VTable<T: Operations>(PhantomData<T>);
+
+impl<T: Operations> VTable<T> {
+    /// The fwctl operations vtable for this driver type.
+    pub const VTABLE: bindings::fwctl_ops = bindings::fwctl_ops {
+        device_type: T::DEVICE_TYPE as u32,
+        uctx_size: core::mem::size_of::<UserCtx<T>>(),
+        open_uctx: Some(Self::open_uctx_callback),
+        close_uctx: Some(Self::close_uctx_callback),
+        info: Some(Self::info_callback),
+        fw_rpc: Some(Self::fw_rpc_callback),
+    };
+
+    /// # Safety
+    ///
+    /// `uctx` must be a valid `fwctl_uctx` embedded in a `UserCtx<T>` with
+    /// sufficient allocated space for the uctx field.
+    unsafe extern "C" fn open_uctx_callback(uctx: *mut bindings::fwctl_uctx) -> ffi::c_int {
+        const_assert!(
+            core::mem::offset_of!(UserCtx<T>, fwctl_uctx) == 0,
+            "struct fwctl_uctx must be at offset 0"
+        );
+
+        // SAFETY: fwctl sets this pointer before calling `open_uctx`.
+        let raw_fwctl = unsafe { (*uctx).fwctl };
+        // SAFETY: Rust fwctl devices use the offset-0 `Device<T>` layout.
+        let device = unsafe { Device::<T>::from_raw(raw_fwctl) };
+
+        let initializer = T::open(device);
+
+        let uctx_offset = core::mem::offset_of!(UserCtx<T>, uctx);
+        // SAFETY: `uctx_size` reserves space for the full `UserCtx<T>`.
+        let uctx_ptr: *mut T = unsafe { uctx.cast::<u8>().add(uctx_offset).cast() };
+
+        // SAFETY: `uctx_ptr` addresses the uninitialised pinned context.
+        match unsafe { initializer.__pinned_init(uctx_ptr.cast()) } {
+            Ok(()) => 0,
+            Err(e) => e.to_errno(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `uctx` must point to a fully initialised `UserCtx<T>`.
+    unsafe extern "C" fn close_uctx_callback(uctx: *mut bindings::fwctl_uctx) {
+        // SAFETY: fwctl keeps the owning device live for this callback.
+        let device = unsafe { Device::<T>::from_raw((*uctx).fwctl) };
+
+        // SAFETY: close is called for an opened Rust user context.
+        let ctx = unsafe { UserCtx::<T>::from_raw_mut(uctx) };
+
+        {
+            // SAFETY: fwctl never moves an opened user context.
+            let pinned = unsafe { Pin::new_unchecked(&mut ctx.uctx) };
+            T::close(pinned, device);
+        }
+
+        // SAFETY: close is the last callback before fwctl frees the allocation.
+        unsafe { core::ptr::drop_in_place(&mut ctx.uctx) };
+    }
+
+    /// # Safety
+    ///
+    /// `uctx` must point to a fully initialised `UserCtx<T>`.
+    /// `length` must be a valid pointer.
+    unsafe extern "C" fn info_callback(
+        uctx: *mut bindings::fwctl_uctx,
+        length: *mut usize,
+    ) -> *mut ffi::c_void {
+        // SAFETY: info is called for an opened Rust user context.
+        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
+        let device = ctx.device();
+
+        // SAFETY: fwctl never moves an opened user context.
+        let pinned = unsafe { Pin::new_unchecked(&ctx.uctx) };
+
+        match T::info(pinned, device) {
+            Ok(kvec) if kvec.is_empty() => {
+                // SAFETY: `length` is a valid out-parameter.
+                unsafe { *length = 0 };
+                // Return NULL for empty data; kfree(NULL) is safe.
+                core::ptr::null_mut()
+            }
+            Ok(kvec) => {
+                let (ptr, len, _cap) = kvec.into_raw_parts();
+                // SAFETY: `length` is a valid out-parameter.
+                unsafe { *length = len };
+                ptr.cast::<ffi::c_void>()
+            }
+            Err(e) => Error::to_ptr(e),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `uctx` must point to a fully initialised `UserCtx<T>`.
+    /// `rpc_in` must be valid for `in_len` bytes. `out_len` must be valid.
+    unsafe extern "C" fn fw_rpc_callback(
+        uctx: *mut bindings::fwctl_uctx,
+        scope: u32,
+        rpc_in: *mut ffi::c_void,
+        in_len: usize,
+        out_len: *mut usize,
+    ) -> *mut ffi::c_void {
+        let scope = match RpcScope::try_from(scope) {
+            Ok(s) => s,
+            Err(e) => return Error::to_ptr(e),
+        };
+
+        // SAFETY: RPC is called for an opened Rust user context.
+        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
+        let device = ctx.device();
+
+        // SAFETY: fwctl passes a valid in/out buffer for this callback.
+        let rpc_in_slice: &mut [u8] =
+            unsafe { slice::from_raw_parts_mut(rpc_in.cast::<u8>(), in_len) };
+
+        // SAFETY: fwctl never moves an opened user context.
+        let pinned = unsafe { Pin::new_unchecked(&ctx.uctx) };
+
+        match T::fw_rpc(pinned, device, scope, rpc_in_slice) {
+            Ok(FwRpcResponse::InPlace(len)) => {
+                // SAFETY: `out_len` is valid.
+                unsafe { *out_len = len };
+                rpc_in
+            }
+            Ok(FwRpcResponse::NewBuffer(kvec)) => {
+                let (ptr, len, _cap) = kvec.into_raw_parts();
+                // SAFETY: `out_len` is valid.
+                unsafe { *out_len = len };
+                ptr.cast::<ffi::c_void>()
+            }
+            Err(e) => Error::to_ptr(e),
+        }
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b72b2fbe046d..ee0ae6d9f1dd 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -72,6 +72,8 @@
 pub mod firmware;
 pub mod fmt;
 pub mod fs;
+#[cfg(CONFIG_RUST_FWCTL_ABSTRACTIONS)]
+pub mod fwctl;
 #[cfg(CONFIG_GPU_BUDDY = "y")]
 pub mod gpu;
 #[cfg(CONFIG_I2C = "y")]
-- 
2.51.0


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

* Re: [PATCH v4 0/2] rust: introduce abstractions for fwctl
  2026-06-24  9:17 [PATCH v4 0/2] rust: introduce abstractions for fwctl Zhi Wang
  2026-06-24  9:17 ` [PATCH v4 1/2] fwctl: add device release hook Zhi Wang
  2026-06-24  9:17 ` [PATCH v4 2/2] rust: introduce abstractions for fwctl Zhi Wang
@ 2026-06-24 11:01 ` Miguel Ojeda
  2 siblings, 0 replies; 5+ messages in thread
From: Miguel Ojeda @ 2026-06-24 11:01 UTC (permalink / raw)
  To: Zhi Wang, Dave Jiang, Jason Gunthorpe, Saeed Mahameed,
	Jonathan Cameron
  Cc: rust-for-linux, linux-kernel, dakr, gary, joelagnelf, aliceryhl,
	kwilczynski, ojeda, alex.gaynor, boqun.feng, bjorn3_gh, lossin,
	a.hindborg, tmgross, cjia, smitra, ankita, aniketa, kwankhede,
	targupta, kjaju, alkumar, acourbot, jhubbard, zhiwang,
	daniel.almeida

On Wed, Jun 24, 2026 at 11:20 AM Zhi Wang <zhiw@nvidia.com> wrote:
>
> This series introduces a Rust abstraction over the fwctl subsystem,
> providing safe and idiomatic bindings.

Cc'ing FWCTL SUBSYSTEM (Jason is there, but not the rest from what I can see).

The new files should likely be added to `MAINTAINERS` too.

Thanks!

Cheers,
Miguel

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

* Re: [PATCH v4 2/2] rust: introduce abstractions for fwctl
  2026-06-24  9:17 ` [PATCH v4 2/2] rust: introduce abstractions for fwctl Zhi Wang
@ 2026-06-24 17:41   ` Danilo Krummrich
  0 siblings, 0 replies; 5+ messages in thread
From: Danilo Krummrich @ 2026-06-24 17:41 UTC (permalink / raw)
  To: Zhi Wang
  Cc: rust-for-linux, linux-kernel, jgg, gary, joelagnelf, aliceryhl,
	kwilczynski, ojeda, alex.gaynor, boqun.feng, bjorn3_gh, lossin,
	a.hindborg, tmgross, cjia, smitra, ankita, aniketa, kwankhede,
	targupta, kjaju, alkumar, acourbot, jhubbard, zhiwang,
	daniel.almeida

On Wed Jun 24, 2026 at 11:17 AM CEST, Zhi Wang wrote:
> +impl<T: Operations> Device<T> {

[...]

> +    /// Returns the parent device.
> +    ///
> +    /// The parent is guaranteed to be bound while any fwctl callback is
> +    /// running (ensured by the `registration_lock` read lock on the ioctl
> +    /// path and by `Devres` on the teardown path).
> +    pub fn parent(&self) -> &device::Device<device::Bound> {
> +        // SAFETY: fwctl sets the parent during allocation.
> +        let parent_dev = unsafe { (*self.as_raw()).dev.parent };
> +        // SAFETY: The parent stays live while fwctl ops run.
> +        let dev: &device::Device = unsafe { device::Device::from_raw(parent_dev) };
> +        // SAFETY: Devres teardown keeps the parent bound here.
> +        unsafe { dev.as_bound() }
> +    }

You can't return a Device<Bound> from fwctl::Device; it is reference counted and
can outlive driver unbind. But I think this method isn't needed anyways, so I'd
just drop it.

> +impl<T: Operations> Registration<T> {
> +    /// Register a previously allocated fwctl device.
> +    pub fn new<'a>(
> +        parent: &'a device::Device<device::Bound>,
> +        dev: &'a Device<T>,
> +    ) -> impl PinInit<Devres<Self>, Error> + 'a {
> +        pin_init::pin_init_scope(move || {
> +            // SAFETY: `dev` is a valid fwctl_device backed by an ARef.
> +            let ret = unsafe { bindings::fwctl_register(dev.as_raw()) };
> +            if ret != 0 {
> +                return Err(Error::from_errno(ret));
> +            }
> +
> +            Ok(Devres::new(parent, Self { dev: dev.into() }))
> +        })

I have recently been working on getting rid of Devres for Registration types and
device resources in favor of Rust-native lifetimes using higher-ranked types
(HRT). This has a couple of advantages, e.g. it simplifies accessing device
resources from destructors and (with pin-init self-referential support) allows
us to share (Rust) references between private data structures.

This has been merged for existing device resources, bus device private data and
auxiliary registration data [1]. There's also a follow-up series to support
invariance [2] and another one to enable it for the DRM subsystem [3].

Class devices infrastructure should follow that same pattern, i.e. the
fwctl::Registration type should gain a lifetime and the fwctl private data
provided via fwctl callbacks should be tied to the lifetime of the Registration,
i.e. the lifetime the underlying bus device is bound to the driver.

Please find a diff in [4] implementing this for fwctl and a diff in [5]
demonstrating how this is used in nova-core. (Feel free to pick up the provided
code and use it in any way.)

(Please ignore that I use fwctl::DeviceType::Mlx5 in the nova-core diff, it is
just there to make the code compile, so I could do a smoke test.)

Thanks,
Danilo

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2c7c65933600e8db2ec1a78dec5008de876dd3ad
[2] https://lore.kernel.org/driver-core/20260618230834.812007-1-dakr@kernel.org/
[3] https://lore.kernel.org/driver-core/20260620184924.2247517-1-dakr@kernel.org/

[4] FWCTL diff:

diff --git a/rust/kernel/fwctl.rs b/rust/kernel/fwctl.rs
index f5f802f5299c..65abea866b22 100644
--- a/rust/kernel/fwctl.rs
+++ b/rust/kernel/fwctl.rs
@@ -8,16 +8,20 @@
     bindings,
     container_of,
     device,
-    devres::Devres,
     prelude::*,
     sync::aref::{
         ARef,
         AlwaysRefCounted, //
     },
-    types::Opaque, //
+    types::{
+        ForLt,
+        Opaque, //
+    }, //
 };
 use core::{
+    cell::UnsafeCell,
     marker::PhantomData,
+    mem,
     ptr::NonNull,
     slice, //
 };
@@ -89,8 +93,12 @@ pub enum FwRpcResponse {
 /// vtable used by the core `fwctl` layer to manage per-FD user contexts and
 /// handle RPC requests.
 pub trait Operations: Sized + Send + Sync {
-    /// Driver data embedded alongside the `fwctl_device` allocation.
-    type DeviceData: Send + Sync;
+    /// Data owned by the [`Registration`] and accessible during callbacks.
+    ///
+    /// This is a [`ForLt`](trait@ForLt) type whose lifetime is tied to the parent bus device
+    /// binding scope. Drivers use this to store references to resources bound to this scope, such as PCI
+    /// BARs or typed bus device references.
+    type RegistrationData: ForLt;
 
     /// fwctl device type identifier.
     const DEVICE_TYPE: DeviceType;
@@ -99,57 +107,68 @@ pub trait Operations: Sized + Send + Sync {
     ///
     /// Returns a [`PinInit`] initializer for `Self`. The instance is dropped
     /// automatically when the FD is closed (after [`close`](Self::close)).
-    fn open(device: &Device<Self>) -> impl PinInit<Self, Error>;
+    fn open<'a>(
+        device: &'a Device<Self>,
+        reg_data: &'a <Self::RegistrationData as ForLt>::Of<'a>,
+    ) -> impl PinInit<Self, Error>;
 
     /// Called when the user context is closed.
     ///
     /// The driver may perform additional cleanup here that requires access
     /// to the owning [`Device`]. `Self` is dropped automatically after this
     /// returns.
-    fn close(_this: Pin<&mut Self>, _device: &Device<Self>) {}
+    fn close<'a>(
+        _this: Pin<&mut Self>,
+        _device: &'a Device<Self>,
+        _reg_data: &'a <Self::RegistrationData as ForLt>::Of<'a>,
+    ) {
+    }
 
     /// Return device information to userspace.
     ///
     /// The default implementation returns no device-specific data.
-    fn info(_this: Pin<&Self>, _device: &Device<Self>) -> Result<KVec<u8>, Error> {
+    fn info<'a>(
+        _this: Pin<&Self>,
+        _device: &'a Device<Self>,
+        _reg_data: &'a <Self::RegistrationData as ForLt>::Of<'a>,
+    ) -> Result<KVec<u8>, Error> {
         Ok(KVec::new())
     }
 
     /// Handle a userspace RPC request.
-    fn fw_rpc(
+    fn fw_rpc<'a>(
         this: Pin<&Self>,
-        device: &Device<Self>,
+        device: &'a Device<Self>,
+        reg_data: &'a <Self::RegistrationData as ForLt>::Of<'a>,
         scope: RpcScope,
         rpc_in: &mut [u8],
     ) -> Result<FwRpcResponse, Error>;
 }
 
-/// A fwctl device with embedded driver data.
+/// A fwctl device.
 ///
-/// `#[repr(C)]` with the `fwctl_device` at offset 0, matching the C
-/// `fwctl_alloc_device()` layout convention.
+/// `#[repr(C)]` with the `fwctl_device` at offset 0, matching the C `fwctl_alloc_device()` layout
+/// convention. Contains a pointer to the [`Registration`]'s data, set at registration time and
+/// cleared on unregistration.
 ///
 /// # Invariants
 ///
 /// - `dev` is embedded at offset 0 and is initialised by fwctl.
 /// - The fwctl refcount owns the allocation lifetime.
-/// - `data` is dropped from the fwctl core release hook before `kfree()`.
+/// - `registration_data` is either `NonNull::dangling()` (before registration / after
+///   unregistration) or points to a valid `RegistrationData` owned by the [`Registration`].
 #[repr(C)]
 pub struct Device<T: Operations> {
     dev: Opaque<bindings::fwctl_device>,
-    data: T::DeviceData,
+    registration_data: UnsafeCell<NonNull<<T::RegistrationData as ForLt>::Of<'static>>>,
 }
 
 impl<T: Operations> Device<T> {
-    /// Allocate a new fwctl device with embedded driver data.
+    /// Allocate a new fwctl device.
     ///
     /// Returns an [`ARef`] that can be passed to [`Registration::new()`]
-    /// to make the device visible to userspace. The caller may inspect or
-    /// configure the device between allocation and registration.
-    pub fn new(
-        parent: &device::Device<device::Bound>,
-        data: impl PinInit<T::DeviceData, Error>,
-    ) -> Result<ARef<Self>> {
+    /// to make the device visible to userspace.
+    pub fn new(parent: &device::Device<device::Bound>) -> Result<ARef<Self>> {
         const_assert!(
             core::mem::offset_of!(Self, dev) == 0,
             "struct fwctl_device must be at offset 0"
@@ -157,8 +176,7 @@ pub fn new(
 
         let ops = core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();
 
-        // SAFETY: `ops` is static, `parent` is bound, and `size` includes the
-        // offset-0 `fwctl_device` plus `DeviceData`.
+        // SAFETY: `ops` is static, `parent` is bound, and `size` covers the full `Device<T>`.
         let raw = unsafe {
             bindings::_fwctl_alloc_device(parent.as_raw(), ops, core::mem::size_of::<Self>())
         };
@@ -169,42 +187,21 @@ pub fn new(
 
         let this = raw.cast::<Self>();
 
+        // INVARIANT: Set `registration_data` to dangling (no registration yet).
         // SAFETY: `this` points to the allocation just returned by fwctl.
-        let data_ptr = unsafe { core::ptr::addr_of_mut!((*this).data) };
-        // SAFETY: `data_ptr` addresses the uninitialised tail data.
-        unsafe { data.__pinned_init(data_ptr) }.inspect_err(|_| {
-            // SAFETY: `raw` still owns the initial reference.
-            unsafe { bindings::fwctl_put(raw) };
-        })?;
-
-        // SAFETY: `raw` is a live fwctl_device allocated above.
-        unsafe { (*raw).release_data = Some(Self::release_data_callback) };
-
-        // SAFETY: `raw` owns the initial reference and `DeviceData` is ready.
-        Ok(unsafe { ARef::from_raw(NonNull::new(raw.cast::<Self>()).ok_or(ENOMEM)?) })
-    }
+        unsafe {
+            core::ptr::addr_of_mut!((*this).registration_data)
+                .write(UnsafeCell::new(NonNull::dangling()));
+        };
 
-    /// Returns a reference to the embedded driver data.
-    pub fn data(&self) -> &T::DeviceData {
-        &self.data
+        // SAFETY: `raw` owns the initial reference.
+        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(this)) })
     }
 
     fn as_raw(&self) -> *mut bindings::fwctl_device {
         self.dev.get()
     }
 
-    /// # Safety
-    ///
-    /// `raw` must point to an offset-0 `fwctl_device` embedded in `Device<T>`.
-    /// fwctl calls this exactly once from the device release path.
-    unsafe extern "C" fn release_data_callback(raw: *mut bindings::fwctl_device) {
-        let this = raw.cast::<Self>();
-
-        // SAFETY: fwctl invokes this callback once during the final device
-        // release, before freeing the allocation.
-        unsafe { core::ptr::drop_in_place(core::ptr::addr_of_mut!((*this).data)) };
-    }
-
     /// # Safety
     ///
     /// `ptr` must point to a valid `fwctl_device` embedded in a `Device<T>`.
@@ -213,18 +210,17 @@ unsafe fn from_raw<'a>(ptr: *mut bindings::fwctl_device) -> &'a Self {
         unsafe { &*ptr.cast() }
     }
 
-    /// Returns the parent device.
+    /// Returns a reference to the registration data.
+    ///
+    /// # Safety
     ///
-    /// The parent is guaranteed to be bound while any fwctl callback is
-    /// running (ensured by the `registration_lock` read lock on the ioctl
-    /// path and by `Devres` on the teardown path).
-    pub fn parent(&self) -> &device::Device<device::Bound> {
-        // SAFETY: fwctl sets the parent during allocation.
-        let parent_dev = unsafe { (*self.as_raw()).dev.parent };
-        // SAFETY: The parent stays live while fwctl ops run.
-        let dev: &device::Device = unsafe { device::Device::from_raw(parent_dev) };
-        // SAFETY: Devres teardown keeps the parent bound here.
-        unsafe { dev.as_bound() }
+    /// The caller must ensure the device is registered, i.e. that this is called within a fwctl
+    /// callback protected by `registration_lock`. The pointer cast from `Of<'static>` to `Of<'_>`
+    /// is sound because [`ForLt`] guarantees covariance.
+    unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
+        // SAFETY: Caller guarantees the device is registered, so the pointer is valid.
+        // Lifetimes do not affect layout, so the cast is sound.
+        unsafe { (*self.registration_data.get()).cast::<_>().as_ref() }
     }
 }
 
@@ -251,62 +247,98 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-// SAFETY: `Device<T>` is refcounted by the fwctl core and may be released from
-// any thread. The embedded driver data is `Send`.
+// SAFETY: `Device<T>` is refcounted by the fwctl core and may be released from any thread.
 unsafe impl<T: Operations> Send for Device<T> {}
 
-// SAFETY: Shared access to the embedded `fwctl_device` is protected by the
-// fwctl core, and the embedded driver data is `Sync`.
+// SAFETY: Shared access to the embedded `fwctl_device` is protected by the fwctl core. The
+// `registration_data` field is only mutated before registration and after unregistration (both
+// single-threaded with respect to callbacks).
 unsafe impl<T: Operations> Sync for Device<T> {}
 
 /// A registered fwctl device.
 ///
-/// Must live inside a [`Devres`] to guarantee that [`fwctl_unregister`] runs
-/// before the parent driver unbinds. `Devres` prevents the `Registration`
-/// from being moved to a context that could outlive the parent device.
+/// Carries the lifetime `'a` of the parent device to ensure that [`fwctl_unregister`] runs (via
+/// [`Drop`]) before the parent driver unbinds. Owns the
+/// [`RegistrationData`](Operations::RegistrationData) which is accessible during callbacks via the
+/// pointer stored in [`Device`].
 ///
-/// On drop the device is unregistered (all user contexts are closed and
-/// `ops` is set to `NULL`) and the [`ARef`] is released.
+/// On drop the device is unregistered (all user contexts are closed and `ops` is set to `NULL`)
+/// and the registration data is dropped.
 ///
 /// [`fwctl_unregister`]: srctree/drivers/fwctl/main.c
-pub struct Registration<T: Operations> {
+pub struct Registration<'a, T: Operations> {
     dev: ARef<Device<T>>,
+    _reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>>,
 }
 
-impl<T: Operations> Registration<T> {
-    /// Register a previously allocated fwctl device.
-    pub fn new<'a>(
-        parent: &'a device::Device<device::Bound>,
-        dev: &'a Device<T>,
-    ) -> impl PinInit<Devres<Self>, Error> + 'a {
-        pin_init::pin_init_scope(move || {
-            // SAFETY: `dev` is a valid fwctl_device backed by an ARef.
-            let ret = unsafe { bindings::fwctl_register(dev.as_raw()) };
-            if ret != 0 {
-                return Err(Error::from_errno(ret));
-            }
+impl<'a, T: Operations> Registration<'a, T>
+where
+    for<'b> <T::RegistrationData as ForLt>::Of<'b>: Send + Sync,
+{
+    /// Register a previously allocated fwctl device with the given registration data.
+    ///
+    /// The `reg_data` is owned by the registration and accessible during callbacks via
+    /// `Device::registration_data_unchecked()`.
+    ///
+    /// # Safety
+    ///
+    /// Callers must not `mem::forget()` the returned [`Registration`] or otherwise prevent its
+    /// [`Drop`] implementation from running, since `fwctl_unregister` must be called before the
+    /// parent device is unbound.
+    pub unsafe fn new(
+        _parent: &'a device::Device<device::Bound>,
+        dev: &Device<T>,
+        reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, Error>,
+    ) -> Result<Self> {
+        let reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>> =
+            KBox::pin_init(reg_data, GFP_KERNEL)?;
+
+        // Store the registration data pointer in the device before registration, so that it is
+        // visible once callbacks can be invoked.
+        //
+        // SAFETY: Lifetimes do not affect layout, so the pointer cast is sound.
+        let ptr: NonNull<<T::RegistrationData as ForLt>::Of<'static>> =
+            unsafe { mem::transmute(NonNull::from(Pin::get_ref(reg_data.as_ref()))) };
+
+        // SAFETY: No concurrent access; the device is not yet registered.
+        unsafe { *dev.registration_data.get() = ptr };
+
+        // SAFETY: `dev` is a valid fwctl_device backed by an ARef.
+        let ret = unsafe { bindings::fwctl_register(dev.as_raw()) };
+        if ret != 0 {
+            // SAFETY: No concurrent readers; registration failed.
+            unsafe { *dev.registration_data.get() = NonNull::dangling() };
+            return Err(Error::from_errno(ret));
+        }
 
-            Ok(Devres::new(parent, Self { dev: dev.into() }))
+        Ok(Self {
+            dev: dev.into(),
+            _reg_data: reg_data,
         })
     }
 }
 
-impl<T: Operations> Drop for Registration<T> {
+impl<T: Operations> Drop for Registration<'_, T> {
     fn drop(&mut self) {
-        // SAFETY: `Registration` lives inside a `Devres`, which guarantees
-        // that drop runs while the parent device is still bound.
+        // SAFETY: The Registration lifetime guarantees that the parent device is still bound.
+        // `fwctl_unregister` takes the write lock, closes all user contexts, and sets ops=NULL.
+        // After it returns, no callbacks can be running or will run.
         unsafe { bindings::fwctl_unregister(self.dev.as_raw()) };
-        // ARef<Device<T>> is dropped after this, calling fwctl_put.
+
+        // SAFETY: `fwctl_unregister` guarantees no concurrent readers.
+        unsafe { *self.dev.registration_data.get() = NonNull::dangling() };
+
+        // `self._reg_data` is dropped here — safe because no concurrent readers remain.
     }
 }
 
-// SAFETY: `Registration` can be sent between threads; the underlying
-// fwctl_device uses internal locking.
-unsafe impl<T: Operations> Send for Registration<T> {}
+// SAFETY: `Registration` can be sent between threads; the underlying fwctl_device uses internal
+// locking.
+unsafe impl<T: Operations> Send for Registration<'_, T> {}
 
-// SAFETY: `Registration` provides no mutable access; the underlying
-// fwctl_device is protected by internal locking.
-unsafe impl<T: Operations> Sync for Registration<T> {}
+// SAFETY: `Registration` provides no mutable access; the underlying fwctl_device is protected by
+// internal locking.
+unsafe impl<T: Operations> Sync for Registration<'_, T> {}
 
 /// Internal per-FD user context wrapping `struct fwctl_uctx` and `T`.
 ///
@@ -376,7 +408,10 @@ impl<T: Operations> VTable<T> {
         // SAFETY: Rust fwctl devices use the offset-0 `Device<T>` layout.
         let device = unsafe { Device::<T>::from_raw(raw_fwctl) };
 
-        let initializer = T::open(device);
+        // SAFETY: open_uctx is called under registration_lock read; the device is registered.
+        let reg_data = unsafe { device.registration_data_unchecked() };
+
+        let initializer = T::open(device, reg_data);
 
         let uctx_offset = core::mem::offset_of!(UserCtx<T>, uctx);
         // SAFETY: `uctx_size` reserves space for the full `UserCtx<T>`.
@@ -396,13 +431,17 @@ impl<T: Operations> VTable<T> {
         // SAFETY: fwctl keeps the owning device live for this callback.
         let device = unsafe { Device::<T>::from_raw((*uctx).fwctl) };
 
+        // SAFETY: close_uctx is called under registration_lock write (from fwctl_unregister) or
+        // under registration_lock read (from fwctl_fops_release); the device is registered.
+        let reg_data = unsafe { device.registration_data_unchecked() };
+
         // SAFETY: close is called for an opened Rust user context.
         let ctx = unsafe { UserCtx::<T>::from_raw_mut(uctx) };
 
         {
             // SAFETY: fwctl never moves an opened user context.
             let pinned = unsafe { Pin::new_unchecked(&mut ctx.uctx) };
-            T::close(pinned, device);
+            T::close(pinned, device, reg_data);
         }
 
         // SAFETY: close is the last callback before fwctl frees the allocation.
@@ -421,10 +460,13 @@ impl<T: Operations> VTable<T> {
         let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
         let device = ctx.device();
 
+        // SAFETY: info is called under registration_lock read; the device is registered.
+        let reg_data = unsafe { device.registration_data_unchecked() };
+
         // SAFETY: fwctl never moves an opened user context.
         let pinned = unsafe { Pin::new_unchecked(&ctx.uctx) };
 
-        match T::info(pinned, device) {
+        match T::info(pinned, device, reg_data) {
             Ok(kvec) if kvec.is_empty() => {
                 // SAFETY: `length` is a valid out-parameter.
                 unsafe { *length = 0 };
@@ -461,6 +503,9 @@ impl<T: Operations> VTable<T> {
         let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
         let device = ctx.device();
 
+        // SAFETY: fw_rpc is called under registration_lock read; the device is registered.
+        let reg_data = unsafe { device.registration_data_unchecked() };
+
         // SAFETY: fwctl passes a valid in/out buffer for this callback.
         let rpc_in_slice: &mut [u8] =
             unsafe { slice::from_raw_parts_mut(rpc_in.cast::<u8>(), in_len) };
@@ -468,7 +513,7 @@ impl<T: Operations> VTable<T> {
         // SAFETY: fwctl never moves an opened user context.
         let pinned = unsafe { Pin::new_unchecked(&ctx.uctx) };
 
-        match T::fw_rpc(pinned, device, scope, rpc_in_slice) {
+        match T::fw_rpc(pinned, device, reg_data, scope, rpc_in_slice) {
             Ok(FwRpcResponse::InPlace(len)) => {
                 // SAFETY: `out_len` is valid.
                 unsafe { *out_len = len };

[5] nova-core diff:

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5738d4ac521b..34d595b655fc 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -3,6 +3,7 @@
 use kernel::{
     auxiliary,
     device::Core,
+    fwctl,
     pci,
     pci::{
         Class,
@@ -15,7 +16,7 @@
         Atomic,
         Relaxed, //
     },
-    types::ForLt,
+    types::ForLt, //
 };

 use crate::gpu::Gpu;
@@ -23,6 +24,34 @@
 /// Counter for generating unique auxiliary device IDs.
 static AUXILIARY_ID_COUNTER: Atomic<u32> = Atomic::new(0);

+struct FwctlRegData<'a> {
+    _bar: Bar0<'a>,
+}
+
+struct FwctlUctx;
+
+impl fwctl::Operations for FwctlUctx {
+    type RegistrationData = ForLt!(FwctlRegData<'_>);
+    const DEVICE_TYPE: fwctl::DeviceType = fwctl::DeviceType::Mlx5;
+
+    fn open(
+        _device: &fwctl::Device<Self>,
+        _reg_data: &FwctlRegData<'_>,
+    ) -> impl PinInit<Self, Error> {
+        Ok(FwctlUctx)
+    }
+
+    fn fw_rpc(
+        _this: core::pin::Pin<&Self>,
+        _device: &fwctl::Device<Self>,
+        _reg_data: &FwctlRegData<'_>,
+        _scope: fwctl::RpcScope,
+        _rpc_in: &mut [u8],
+    ) -> Result<fwctl::FwRpcResponse, Error> {
+        Err(ENOTSUPP)
+    }
+}
+
 #[pin_data]
 pub(crate) struct NovaCore<'bound> {
     #[pin]
@@ -30,6 +59,7 @@ pub(crate) struct NovaCore<'bound> {
     bar: pci::Bar<'bound, BAR0_SIZE>,
     #[allow(clippy::type_complexity)]
     _reg: auxiliary::Registration<'bound, ForLt!(())>,
+    _fwctl_reg: fwctl::Registration<'bound, FwctlUctx>,
 }

 pub(crate) struct NovaCoreDriver;
@@ -78,6 +108,8 @@ fn probe<'bound>(
             pdev.enable_device_mem()?;
             pdev.set_master();

+            let fwctl_dev = fwctl::Device::<FwctlUctx>::new(pdev.as_ref())?;
+
             Ok(try_pin_init!(NovaCore {
                 bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?,
                 // TODO: Use `&bar` self-referential pin-init syntax once available.
@@ -95,6 +127,17 @@ fn probe<'bound>(
                     crate::MODULE_NAME,
                     (),
                 )?,
+                // SAFETY: `_fwctl_reg` is dropped before `bar` (struct field drop order), and its
+                // drop calls `fwctl_unregister` before the parent device is unbound.
+                _fwctl_reg: unsafe {
+                    fwctl::Registration::new(
+                        pdev.as_ref(),
+                        &fwctl_dev,
+                        Ok(FwctlRegData {
+                            _bar: &*core::ptr::from_ref(bar),
+                        }),
+                    )?
+                },
             }))
         })
     }
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 735b8e17c6b6..d5f050b2b55e 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -74,6 +74,7 @@ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
     description: "Nova Core GPU driver",
     license: "GPL v2",
     firmware: [],
+    imports_ns: ["FWCTL"],
 }

 kernel::module_firmware!(firmware::ModInfoBuilder);

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

end of thread, other threads:[~2026-06-24 17:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-24  9:17 [PATCH v4 0/2] rust: introduce abstractions for fwctl Zhi Wang
2026-06-24  9:17 ` [PATCH v4 1/2] fwctl: add device release hook Zhi Wang
2026-06-24  9:17 ` [PATCH v4 2/2] rust: introduce abstractions for fwctl Zhi Wang
2026-06-24 17:41   ` Danilo Krummrich
2026-06-24 11:01 ` [PATCH v4 0/2] " Miguel Ojeda

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