[PATCH v3 0/1] rust: introduce abstractions for fwctl

[PATCH v3 0/1] rust: introduce abstractions for fwctl

[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 patch introduces a Rust abstraction over the fwctl subsystem,
providing safe and idiomatic bindings.

The new `fwctl` module 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`).

  - `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_FWCTL`.

The repo with the patches can be found at [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://github.com/zhiwang-nvidia/driver-core/tree/fwctl-rust-abstraction-v3
[3] https://github.com/zhiwang-nvidia/nova-core/commit/2068da7e8caf58da9584b0aa6c81fed8f547d59f

Zhi Wang (1):
  rust: introduce abstractions for fwctl

 drivers/fwctl/Kconfig           |  12 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/fwctl.c            |  17 ++
 rust/helpers/helpers.c          |   3 +-
 rust/kernel/fwctl.rs            | 449 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 6 files changed, 483 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/fwctl.c
 create mode 100644 rust/kernel/fwctl.rs

-- 
2.43.0

[PATCH v3 1/1] rust: introduce abstractions for fwctl

[Zhi Wang]

Introduce 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.

Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: Joel Fernandes <joelagnelf@nvidia.com>
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            | 449 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 6 files changed, 483 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 b5583b12a011..ce42c0f52d6d 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_MLX5
 	tristate "mlx5 ConnectX control fwctl driver"
 	depends on MLX5_CORE
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 083cc44aa952..c0a7248989f7 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -56,6 +56,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 a3c42e51f00a..38fbae6880a0 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -30,8 +30,9 @@
 #include "dma.c"
 #include "drm.c"
 #include "err.c"
-#include "irq.c"
 #include "fs.c"
+#include "fwctl.c"
+#include "irq.c"
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
diff --git a/rust/kernel/fwctl.rs b/rust/kernel/fwctl.rs
new file mode 100644
index 000000000000..38bb6f9ebca5
--- /dev/null
+++ b/rust/kernel/fwctl.rs
@@ -0,0 +1,449 @@
+// 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 {
+    /// Driver data embedded alongside the `fwctl_device` allocation.
+    type DeviceData;
+
+    /// 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>) -> Result<impl PinInit<Self, Error>, 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: &Self, _device: &Device<Self>) -> Result<KVec<u8>, Error> {
+        Ok(KVec::new())
+    }
+
+    /// Handle a userspace RPC request.
+    fn fw_rpc(
+        this: &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
+///
+/// The `fwctl_device` portion is initialised by the C subsystem during
+/// [`Device::new()`]. The `data` field is initialised in-place via
+/// [`PinInit`] before the struct is exposed.
+#[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>> {
+        let ops = core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();
+
+        // SAFETY: `_fwctl_alloc_device` allocates `size` bytes via kzalloc and
+        // initialises the embedded fwctl_device. `ops` points to a static vtable
+        // that outlives the device. `parent` is bound.
+        let raw = unsafe {
+            bindings::_fwctl_alloc_device(parent.as_raw(), ops, core::mem::size_of::<Self>())
+        };
+
+        if raw.is_null() {
+            return Err(ENOMEM);
+        }
+
+        // CAST: Device<T> is repr(C) with fwctl_device at offset 0.
+        let this = raw as *mut Self;
+
+        // SAFETY: `data` field is within the kzalloc'd allocation, uninitialised.
+        let data_ptr = unsafe { core::ptr::addr_of_mut!((*this).data) };
+        unsafe { data.__pinned_init(data_ptr) }.inspect_err(|_| {
+            // SAFETY: Init failed; release the allocation.
+            unsafe { bindings::fwctl_put(raw) };
+        })?;
+
+        // SAFETY: `_fwctl_alloc_device` returned a valid pointer with refcount 1
+        // and DeviceData is fully initialised.
+        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw as *mut Self)) })
+    }
+
+    /// 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
+    ///
+    /// `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 {
+        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_device always has a valid parent.
+        let parent_dev = unsafe { (*self.as_raw()).dev.parent };
+        let dev: &device::Device = unsafe { device::Device::from_raw(parent_dev) };
+        // SAFETY: The parent is guaranteed to be bound while fwctl ops are active.
+        unsafe { dev.as_bound() }
+    }
+}
+
+impl<T: Operations> AsRef<device::Device> for Device<T> {
+    fn as_ref(&self) -> &device::Device {
+        // SAFETY: self.as_raw() is a valid fwctl_device.
+        let dev = unsafe { core::ptr::addr_of_mut!((*self.as_raw()).dev) };
+        // SAFETY: dev points to a valid struct device.
+        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: The existence of a shared reference guarantees that the refcount is non-zero.
+        unsafe { bindings::fwctl_get(self.as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+        unsafe { bindings::fwctl_put(obj.cast().as_ptr()) };
+    }
+}
+
+/// 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 {
+        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 {
+        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_uctx.fwctl is set by the subsystem and always valid.
+        let raw_fwctl = unsafe { (*self.fwctl_uctx.get()).fwctl };
+        // SAFETY: The fwctl_device is embedded in a Device<T>.
+        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 {
+        // SAFETY: `fwctl_uctx.fwctl` is set by the subsystem before calling open.
+        let raw_fwctl = unsafe { (*uctx).fwctl };
+        // SAFETY: `raw_fwctl` points to a valid fwctl_device embedded in a Device<T>.
+        let device = unsafe { Device::<T>::from_raw(raw_fwctl) };
+
+        let initializer = match T::open(device) {
+            Ok(init) => init,
+            Err(e) => return e.to_errno(),
+        };
+
+        let uctx_offset = core::mem::offset_of!(UserCtx<T>, uctx);
+        // SAFETY: The C side allocated space for the entire UserCtx<T>.
+        let uctx_ptr: *mut T = unsafe { uctx.cast::<u8>().add(uctx_offset).cast() };
+
+        // SAFETY: uctx_ptr points to uninitialised, properly aligned memory.
+        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_uctx.fwctl` is set by the subsystem and always valid.
+        let device = unsafe { Device::<T>::from_raw((*uctx).fwctl) };
+
+        // SAFETY: uctx is a valid pointer promised by C side.
+        let ctx = unsafe { UserCtx::<T>::from_raw_mut(uctx) };
+
+        {
+            // SAFETY: driver uctx is pinned in place by the C allocation.
+            let pinned = unsafe { Pin::new_unchecked(&mut ctx.uctx) };
+            T::close(pinned, device);
+        }
+
+        // SAFETY: After close returns, no further callbacks will access UserCtx.
+        // Drop T before the C side kfree's 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: uctx is a valid pointer promised by C side.
+        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
+        let device = ctx.device();
+
+        match T::info(&ctx.uctx, 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: `uctx` is fully initialised; shared access is sufficient.
+        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
+        let device = ctx.device();
+
+        // SAFETY: `rpc_in` / `in_len` are guaranteed valid by the fwctl subsystem.
+        let rpc_in_slice: &mut [u8] =
+            unsafe { slice::from_raw_parts_mut(rpc_in.cast::<u8>(), in_len) };
+
+        match T::fw_rpc(&ctx.uctx, 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 3da92f18f4ee..d658720276f2 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -97,6 +97,8 @@
 pub mod firmware;
 pub mod fmt;
 pub mod fs;
+#[cfg(CONFIG_RUST_FWCTL_ABSTRACTIONS)]
+pub mod fwctl;
 #[cfg(CONFIG_I2C = "y")]
 pub mod i2c;
 pub mod id_pool;
-- 
2.43.0

Re: [PATCH v3 1/1] rust: introduce abstractions for fwctl

[Jason Gunthorpe]

On Tue, Feb 17, 2026 at 10:49:06PM +0200, Zhi Wang wrote:
> Introduce 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.
> 
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> 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            | 449 ++++++++++++++++++++++++++++++++

The binding is larger than the subystem file:

$ wc -l drivers/fwctl/main.c 
421 drivers/fwctl/main.c

:\

Anyhow is someone waiting for me to do something with it?

> +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>> {
> +        let ops = core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();
> +
> +        // SAFETY: `_fwctl_alloc_device` allocates `size` bytes via kzalloc and
> +        // initialises the embedded fwctl_device. `ops` points to a static vtable
> +        // that outlives the device. `parent` is bound.
> +        let raw = unsafe {
> +            bindings::_fwctl_alloc_device(parent.as_raw(), ops, core::mem::size_of::<Self>())
> +        };
> +
> +        if raw.is_null() {
> +            return Err(ENOMEM);
> +        }
> +
> +        // CAST: Device<T> is repr(C) with fwctl_device at offset 0.
> +        let this = raw as *mut Self;

Should this have some helper? It looks a bit fragile, in C we have a
static_assert on offsetof to prevent errors.

Jason

Re: [PATCH v3 1/1] rust: introduce abstractions for fwctl

[Danilo Krummrich]

On Tue Mar 3, 2026 at 9:15 PM CET, Jason Gunthorpe wrote:
> Anyhow is someone waiting for me to do something with it?

I still have this on my list for review, I can probably do a pass tomorrow. :)

>> +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>> {
>> +        let ops = core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();
>> +
>> +        // SAFETY: `_fwctl_alloc_device` allocates `size` bytes via kzalloc and
>> +        // initialises the embedded fwctl_device. `ops` points to a static vtable
>> +        // that outlives the device. `parent` is bound.
>> +        let raw = unsafe {
>> +            bindings::_fwctl_alloc_device(parent.as_raw(), ops, core::mem::size_of::<Self>())
>> +        };
>> +
>> +        if raw.is_null() {
>> +            return Err(ENOMEM);
>> +        }
>> +
>> +        // CAST: Device<T> is repr(C) with fwctl_device at offset 0.
>> +        let this = raw as *mut Self;
>
> Should this have some helper? It looks a bit fragile, in C we have a
> static_assert on offsetof to prevent errors.

We could add a helper with something like this:

	    const {
	        assert!(
	            core::mem::offset_of!(Self, dev) == 0,
	            "struct fwctl_device must be at offset 0"
	        )
	    };

But we could also just include this in this constructor.

Re: [PATCH v3 1/1] rust: introduce abstractions for fwctl

[Danilo Krummrich]

On Tue Mar 3, 2026 at 9:15 PM CET, Jason Gunthorpe wrote:
>>  rust/kernel/fwctl.rs            | 449 ++++++++++++++++++++++++++++++++
>
> The binding is larger than the subystem file:
>
> $ wc -l drivers/fwctl/main.c 
> 421 drivers/fwctl/main.c

It's probably more a positve than a negative thing, as we should have a lot of
code that is not only mindlessly forwarding things, but actually teaches the
compiler how to prevent users from making mistakes.

But yes, usually the abstractions are smaller than the component they're written
for.

Re: [PATCH v3 1/1] rust: introduce abstractions for fwctl

[Danilo Krummrich]

On Tue Feb 17, 2026 at 9:49 PM CET, Zhi Wang wrote:
> diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
> index b5583b12a011..ce42c0f52d6d 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.

We currently have to ensure that CONFIG_FWCTL=y.

I also gave this a quick shot and I see the following warnings:

	warning: `as` casting between raw pointers without changing their constness
	   --> rust/kernel/fwctl.rs:167:20
	    |
	167 |         let this = raw as *mut Self;
	    |                    ^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `raw.cast::<Self>()`
	    |
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#ptr_as_ptr
	    = note: requested on the command line with `-W clippy::ptr-as-ptr`
	
	warning: unsafe block missing a safety comment
	   --> rust/kernel/fwctl.rs:171:9
	    |
	171 |         unsafe { data.__pinned_init(data_ptr) }.inspect_err(|_| {
	    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    |
	    = help: consider adding a safety comment on the preceding line
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#undocumented_unsafe_blocks
	    = note: requested on the command line with `-W clippy::undocumented-unsafe-blocks`
	
	warning: `as` casting between raw pointers without changing their constness
	   --> rust/kernel/fwctl.rs:178:59
	    |
	178 |         Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw as *mut Self)) })
	    |                                                           ^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `raw.cast::<Self>()`
	    |
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#ptr_as_ptr
	
	warning: unsafe block missing a safety comment
	   --> rust/kernel/fwctl.rs:194:9
	    |
	194 |         unsafe { &*ptr.cast() }
	    |         ^^^^^^^^^^^^^^^^^^^^^^^
	    |
	    = help: consider adding a safety comment on the preceding line
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#undocumented_unsafe_blocks
	
	warning: unsafe block missing a safety comment
	   --> rust/kernel/fwctl.rs:205:36
	    |
	205 |         let dev: &device::Device = unsafe { device::Device::from_raw(parent_dev) };
	    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    |
	    = help: consider adding a safety comment on the preceding line
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#undocumented_unsafe_blocks
	
	warning: unsafe block missing a safety comment
	   --> rust/kernel/fwctl.rs:300:9
	    |
	300 |         unsafe { &*container_of!(Opaque::cast_from(ptr), Self, fwctl_uctx) }
	    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    |
	    = help: consider adding a safety comment on the preceding line
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#undocumented_unsafe_blocks
	
	warning: unsafe block missing a safety comment
	   --> rust/kernel/fwctl.rs:308:9
	    |
	308 |         unsafe { &mut *container_of!(Opaque::cast_from(ptr), Self, fwctl_uctx).cast_mut() }
	    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    |
	    = help: consider adding a safety comment on the preceding line
	    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#undocumented_unsafe_blocks
	
	warning: 7 warnings emitted

From the rustdoc build:

	warning: unresolved link to `include/linux/fwctl.h`
	 --> rust/kernel/fwctl.rs:5:17
	  |
	5 | //! C header: [`include/linux/fwctl.h`]
	  |                 ^^^^^^^^^^^^^^^^^^^^^ no item named `include/linux/fwctl.h` in scope
	  |
	  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
	  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default
	
	warning: 1 warning emitted

Please make sure to test with CLIPPY=1 and make sure to also built the rustdoc
target.

> +/// 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 {

I think this needs Send.

Besides that, are all those callbacks strictly serialized, i.e. can we really
give out a mutable reference?

> +    /// Driver data embedded alongside the `fwctl_device` allocation.
> +    type DeviceData;
> +
> +    /// 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>) -> Result<impl PinInit<Self, Error>, Error>;

This should just return impl PinInit<Self, Error>, the outer result is
redundant.

> +    /// 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>) {}

This can just be self: Pin<&mut Self>.

> +
> +    /// Return device information to userspace.
> +    ///
> +    /// The default implementation returns no device-specific data.
> +    fn info(_this: &Self, _device: &Device<Self>) -> Result<KVec<u8>, Error> {

self: Pin<&Self>, Result<KVec<u8>>

> +        Ok(KVec::new())
> +    }
> +
> +    /// Handle a userspace RPC request.
> +    fn fw_rpc(
> +        this: &Self,

Same here.

> +        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
> +///
> +/// The `fwctl_device` portion is initialised by the C subsystem during
> +/// [`Device::new()`]. The `data` field is initialised in-place via
> +/// [`PinInit`] before the struct is exposed.

Where is this invariant used, do we actually need it?

> +#[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>> {
> +        let ops = core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();

The turbofish shouldn't be needed.

> +
> +        // SAFETY: `_fwctl_alloc_device` allocates `size` bytes via kzalloc and
> +        // initialises the embedded fwctl_device. `ops` points to a static vtable
> +        // that outlives the device. `parent` is bound.
> +        let raw = unsafe {
> +            bindings::_fwctl_alloc_device(parent.as_raw(), ops, core::mem::size_of::<Self>())
> +        };

I suggest to use Kmalloc::aligned_layout() in such a case, please also see [1].

[1] https://lore.kernel.org/r/20250731154919.4132-3-dakr@kernel.org

> +        if raw.is_null() {
> +            return Err(ENOMEM);
> +        }

If you replace this with NonNull::new().ok_or(ENOMEM)? you already have the
NonNull instance for the subsequent call to ARef::from_raw() below.

> +        // CAST: Device<T> is repr(C) with fwctl_device at offset 0.
> +        let this = raw as *mut Self;
> +
> +        // SAFETY: `data` field is within the kzalloc'd allocation, uninitialised.
> +        let data_ptr = unsafe { core::ptr::addr_of_mut!((*this).data) };

Prefer &raw mut.

> +        unsafe { data.__pinned_init(data_ptr) }.inspect_err(|_| {
> +            // SAFETY: Init failed; release the allocation.
> +            unsafe { bindings::fwctl_put(raw) };
> +        })?;
> +
> +        // SAFETY: `_fwctl_alloc_device` returned a valid pointer with refcount 1
> +        // and DeviceData is fully initialised.
> +        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw as *mut Self)) })
> +    }
> +
> +    /// 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
> +    ///
> +    /// `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 {
> +        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_device always has a valid parent.
> +        let parent_dev = unsafe { (*self.as_raw()).dev.parent };
> +        let dev: &device::Device = unsafe { device::Device::from_raw(parent_dev) };
> +        // SAFETY: The parent is guaranteed to be bound while fwctl ops are active.
> +        unsafe { dev.as_bound() }
> +    }
> +}
> +
> +impl<T: Operations> AsRef<device::Device> for Device<T> {
> +    fn as_ref(&self) -> &device::Device {
> +        // SAFETY: self.as_raw() is a valid fwctl_device.
> +        let dev = unsafe { core::ptr::addr_of_mut!((*self.as_raw()).dev) };

NIT: &raw mut

> +        // SAFETY: dev points to a valid struct device.
> +        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: The existence of a shared reference guarantees that the refcount is non-zero.
> +        unsafe { bindings::fwctl_get(self.as_raw()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> +        unsafe { bindings::fwctl_put(obj.cast().as_ptr()) };
> +    }
> +}
> +
> +/// 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 {

This doesn't need PinInit anymore.

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

It doesn't really hurt, but I don't think this comment is needed.

> +    }
> +}
> +
> +// 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,

I'd probably just name this data.

> +}
> +
> +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 {
> +        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 {
> +        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_uctx.fwctl is set by the subsystem and always valid.
> +        let raw_fwctl = unsafe { (*self.fwctl_uctx.get()).fwctl };
> +        // SAFETY: The fwctl_device is embedded in a Device<T>.
> +        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>>(),

We may want to use Kmalloc::aligned_layout() here as well. It should be possible
to make this a const function.

> +        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 {
> +        // SAFETY: `fwctl_uctx.fwctl` is set by the subsystem before calling open.
> +        let raw_fwctl = unsafe { (*uctx).fwctl };
> +        // SAFETY: `raw_fwctl` points to a valid fwctl_device embedded in a Device<T>.
> +        let device = unsafe { Device::<T>::from_raw(raw_fwctl) };
> +
> +        let initializer = match T::open(device) {
> +            Ok(init) => init,
> +            Err(e) => return e.to_errno(),
> +        };
> +
> +        let uctx_offset = core::mem::offset_of!(UserCtx<T>, uctx);
> +        // SAFETY: The C side allocated space for the entire UserCtx<T>.
> +        let uctx_ptr: *mut T = unsafe { uctx.cast::<u8>().add(uctx_offset).cast() };

I think the following is a bit cleaner:

	let user_ctx = container_of!(fw, UserCtx<T>, fwctl_uctx);
	let data = unsafe { &raw mut (*user_ctx).uctx };

> +
> +        // SAFETY: uctx_ptr points to uninitialised, properly aligned memory.
> +        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_uctx.fwctl` is set by the subsystem and always valid.
> +        let device = unsafe { Device::<T>::from_raw((*uctx).fwctl) };
> +
> +        // SAFETY: uctx is a valid pointer promised by C side.

You have to justify exclusive access as well.

> +        let ctx = unsafe { UserCtx::<T>::from_raw_mut(uctx) };
> +
> +        {
> +            // SAFETY: driver uctx is pinned in place by the C allocation.
> +            let pinned = unsafe { Pin::new_unchecked(&mut ctx.uctx) };
> +            T::close(pinned, device);
> +        }
> +
> +        // SAFETY: After close returns, no further callbacks will access UserCtx.
> +        // Drop T before the C side kfree's 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: uctx is a valid pointer promised by C side.
> +        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
> +        let device = ctx.device();
> +
> +        match T::info(&ctx.uctx, 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: `uctx` is fully initialised; shared access is sufficient.
> +        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
> +        let device = ctx.device();
> +
> +        // SAFETY: `rpc_in` / `in_len` are guaranteed valid by the fwctl subsystem.

There are more safety requirements for a mutable slice, please justify them as
well.

> +        let rpc_in_slice: &mut [u8] =
> +            unsafe { slice::from_raw_parts_mut(rpc_in.cast::<u8>(), in_len) };
> +
> +        match T::fw_rpc(&ctx.uctx, 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),
> +        }
> +    }
> +}

Re: [PATCH v3 1/1] rust: introduce abstractions for fwctl

[Zhi Wang]

On Thu, 05 Mar 2026 17:02:38 +0100
"Danilo Krummrich" <dakr@kernel.org> wrote:

> On Tue Feb 17, 2026 at 9:49 PM CET, Zhi Wang wrote:
> > diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
> > index b5583b12a011..ce42c0f52d6d 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.  
> 
> We currently have to ensure that CONFIG_FWCTL=y.
> 

Hi:

Thanks so much for the review efforts.

I just post the nova-core fwctl driver series. You are right. I
encounter a problem when CONFIG_FWCTL=y. Should I move that one within
this series? [1]

[1] https://lore.kernel.org/all/20260305190936.398590-2-zhiw@nvidia.com/

> I also gave this a quick shot and I see the following warnings:
> 
> 	warning: `as` casting between raw pointers without changing

Will double check with the CLIPPY and rustdoc fmt.

> > +/// 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 {  
> 
> I think this needs Send.
> 

Nice catch, I think we also need Sync.

> Besides that, are all those callbacks strictly serialized, i.e. can
> we really give out a mutable reference?
> 

I think open()/close() are serialized by the VFS. But info() and
fw_rpc() are not so I don't give a mutable reference for them. If the
driver wanna modify the members, it should use Mutex<>.

> > +    /// Driver data embedded alongside the `fwctl_device`
> > allocation.
> > +    type DeviceData;
> > +
> > +    /// 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>) -> Result<impl PinInit<Self,
> > Error>, Error>;  
> 

Will fix all the followings in the next re-spin.

> This should just return impl PinInit<Self, Error>, the outer result is
> redundant.
> 
> > +    /// 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>) {}  
> 
> This can just be self: Pin<&mut Self>.
> 
> > +
> > +    /// Return device information to userspace.
> > +    ///
> > +    /// The default implementation returns no device-specific data.
> > +    fn info(_this: &Self, _device: &Device<Self>) ->
> > Result<KVec<u8>, Error> {  
> 
> self: Pin<&Self>, Result<KVec<u8>>
> 
> > +        Ok(KVec::new())
> > +    }
> > +
> > +    /// Handle a userspace RPC request.
> > +    fn fw_rpc(
> > +        this: &Self,  
> 
> Same here.
> 
> > +        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
> > +///
> > +/// The `fwctl_device` portion is initialised by the C subsystem
> > during +/// [`Device::new()`]. The `data` field is initialised
> > in-place via +/// [`PinInit`] before the struct is exposed.  
> 
> Where is this invariant used, do we actually need it?
> 
> > +#[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>> {
> > +        let ops =
> > core::ptr::from_ref::<bindings::fwctl_ops>(&VTable::<T>::VTABLE).cast_mut();
> >  
> 
> The turbofish shouldn't be needed.
> 
> > +
> > +        // SAFETY: `_fwctl_alloc_device` allocates `size` bytes
> > via kzalloc and
> > +        // initialises the embedded fwctl_device. `ops` points to
> > a static vtable
> > +        // that outlives the device. `parent` is bound.
> > +        let raw = unsafe {
> > +            bindings::_fwctl_alloc_device(parent.as_raw(), ops,
> > core::mem::size_of::<Self>())
> > +        };  
> 
> I suggest to use Kmalloc::aligned_layout() in such a case, please
> also see [1].
> 
> [1] https://lore.kernel.org/r/20250731154919.4132-3-dakr@kernel.org
> 
> > +        if raw.is_null() {
> > +            return Err(ENOMEM);
> > +        }  
> 
> If you replace this with NonNull::new().ok_or(ENOMEM)? you already
> have the NonNull instance for the subsequent call to ARef::from_raw()
> below.
> 
> > +        // CAST: Device<T> is repr(C) with fwctl_device at offset
> > 0.
> > +        let this = raw as *mut Self;
> > +
> > +        // SAFETY: `data` field is within the kzalloc'd
> > allocation, uninitialised.
> > +        let data_ptr = unsafe {
> > core::ptr::addr_of_mut!((*this).data) };  
> 
> Prefer &raw mut.
> 
> > +        unsafe { data.__pinned_init(data_ptr) }.inspect_err(|_| {
> > +            // SAFETY: Init failed; release the allocation.
> > +            unsafe { bindings::fwctl_put(raw) };
> > +        })?;
> > +
> > +        // SAFETY: `_fwctl_alloc_device` returned a valid pointer
> > with refcount 1
> > +        // and DeviceData is fully initialised.
> > +        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw as
> > *mut Self)) })
> > +    }
> > +
> > +    /// 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
> > +    ///
> > +    /// `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 {
> > +        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_device always has a valid parent.
> > +        let parent_dev = unsafe { (*self.as_raw()).dev.parent };
> > +        let dev: &device::Device = unsafe {
> > device::Device::from_raw(parent_dev) };
> > +        // SAFETY: The parent is guaranteed to be bound while
> > fwctl ops are active.
> > +        unsafe { dev.as_bound() }
> > +    }
> > +}
> > +
> > +impl<T: Operations> AsRef<device::Device> for Device<T> {
> > +    fn as_ref(&self) -> &device::Device {
> > +        // SAFETY: self.as_raw() is a valid fwctl_device.
> > +        let dev = unsafe {
> > core::ptr::addr_of_mut!((*self.as_raw()).dev) };  
> 
> NIT: &raw mut
> 
> > +        // SAFETY: dev points to a valid struct device.
> > +        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: The existence of a shared reference guarantees
> > that the refcount is non-zero.
> > +        unsafe { bindings::fwctl_get(self.as_raw()) };
> > +    }
> > +
> > +    unsafe fn dec_ref(obj: NonNull<Self>) {
> > +        // SAFETY: The safety requirements guarantee that the
> > refcount is non-zero.
> > +        unsafe { bindings::fwctl_put(obj.cast().as_ptr()) };
> > +    }
> > +}
> > +
> > +/// 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 {  
> 
> This doesn't need PinInit anymore.
> 
> > +        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.  
> 
> It doesn't really hurt, but I don't think this comment is needed.
> 
> > +    }
> > +}
> > +
> > +// 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,  
> 
> I'd probably just name this data.
> 
> > +}
> > +
> > +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 {
> > +        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 {
> > +        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_uctx.fwctl is set by the subsystem and
> > always valid.
> > +        let raw_fwctl = unsafe { (*self.fwctl_uctx.get()).fwctl };
> > +        // SAFETY: The fwctl_device is embedded in a Device<T>.
> > +        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>>(),  
> 
> We may want to use Kmalloc::aligned_layout() here as well. It should
> be possible to make this a const function.
> 
> > +        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 {
> > +        // SAFETY: `fwctl_uctx.fwctl` is set by the subsystem
> > before calling open.
> > +        let raw_fwctl = unsafe { (*uctx).fwctl };
> > +        // SAFETY: `raw_fwctl` points to a valid fwctl_device
> > embedded in a Device<T>.
> > +        let device = unsafe { Device::<T>::from_raw(raw_fwctl) };
> > +
> > +        let initializer = match T::open(device) {
> > +            Ok(init) => init,
> > +            Err(e) => return e.to_errno(),
> > +        };
> > +
> > +        let uctx_offset = core::mem::offset_of!(UserCtx<T>, uctx);
> > +        // SAFETY: The C side allocated space for the entire
> > UserCtx<T>.
> > +        let uctx_ptr: *mut T = unsafe {
> > uctx.cast::<u8>().add(uctx_offset).cast() };  
> 
> I think the following is a bit cleaner:
> 
> 	let user_ctx = container_of!(fw, UserCtx<T>, fwctl_uctx);
> 	let data = unsafe { &raw mut (*user_ctx).uctx };
> 
> > +
> > +        // SAFETY: uctx_ptr points to uninitialised, properly
> > aligned memory.
> > +        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_uctx.fwctl` is set by the subsystem and
> > always valid.
> > +        let device = unsafe { Device::<T>::from_raw((*uctx).fwctl)
> > }; +
> > +        // SAFETY: uctx is a valid pointer promised by C side.  
> 
> You have to justify exclusive access as well.
> 
> > +        let ctx = unsafe { UserCtx::<T>::from_raw_mut(uctx) };
> > +
> > +        {
> > +            // SAFETY: driver uctx is pinned in place by the C
> > allocation.
> > +            let pinned = unsafe { Pin::new_unchecked(&mut
> > ctx.uctx) };
> > +            T::close(pinned, device);
> > +        }
> > +
> > +        // SAFETY: After close returns, no further callbacks will
> > access UserCtx.
> > +        // Drop T before the C side kfree's 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: uctx is a valid pointer promised by C side.
> > +        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
> > +        let device = ctx.device();
> > +
> > +        match T::info(&ctx.uctx, 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: `uctx` is fully initialised; shared access is
> > sufficient.
> > +        let ctx = unsafe { UserCtx::<T>::from_raw(uctx) };
> > +        let device = ctx.device();
> > +
> > +        // SAFETY: `rpc_in` / `in_len` are guaranteed valid by the
> > fwctl subsystem.  
> 
> There are more safety requirements for a mutable slice, please
> justify them as well.
> 
> > +        let rpc_in_slice: &mut [u8] =
> > +            unsafe {
> > slice::from_raw_parts_mut(rpc_in.cast::<u8>(), in_len) }; +
> > +        match T::fw_rpc(&ctx.uctx, 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),
> > +        }
> > +    }
> > +}