Rust for Linux List
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Zhi Wang" <zhiw@nvidia.com>
Cc: <rust-for-linux@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<jgg@nvidia.com>, <gary@garyguo.net>, <joelagnelf@nvidia.com>,
	<aliceryhl@google.com>, <kwilczynski@kernel.org>,
	<ojeda@kernel.org>, <alex.gaynor@gmail.com>,
	<boqun.feng@gmail.com>, <bjorn3_gh@protonmail.com>,
	<lossin@kernel.org>, <a.hindborg@kernel.org>, <tmgross@umich.edu>,
	<cjia@nvidia.com>, <smitra@nvidia.com>, <ankita@nvidia.com>,
	<aniketa@nvidia.com>, <kwankhede@nvidia.com>,
	<targupta@nvidia.com>, <kjaju@nvidia.com>, <alkumar@nvidia.com>,
	<acourbot@nvidia.com>, <jhubbard@nvidia.com>,
	<zhiwang@kernel.org>, <daniel.almeida@collabora.com>
Subject: Re: [PATCH v4 2/2] rust: introduce abstractions for fwctl
Date: Wed, 24 Jun 2026 19:41:45 +0200	[thread overview]
Message-ID: <DJHGQN654CJR.281FZSV2S5AI8@kernel.org> (raw)
In-Reply-To: <20260624091758.1678092-3-zhiw@nvidia.com>

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);

  reply	other threads:[~2026-06-24 17:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-06-24 11:01 ` [PATCH v4 0/2] " Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DJHGQN654CJR.281FZSV2S5AI8@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=alkumar@nvidia.com \
    --cc=aniketa@nvidia.com \
    --cc=ankita@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=cjia@nvidia.com \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=jgg@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=kjaju@nvidia.com \
    --cc=kwankhede@nvidia.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=smitra@nvidia.com \
    --cc=targupta@nvidia.com \
    --cc=tmgross@umich.edu \
    --cc=zhiw@nvidia.com \
    --cc=zhiwang@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox