Rust for Linux List
 help / color / mirror / Atom feed
From: Zhi Wang <zhiw@nvidia.com>
To: Danilo Krummrich <dakr@kernel.org>
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: Thu, 25 Jun 2026 11:24:29 +0300	[thread overview]
Message-ID: <20260625112429.3e89e869@inno-dell> (raw)
In-Reply-To: <DJHGQN654CJR.281FZSV2S5AI8@kernel.org>

On Wed, 24 Jun 2026 19:41:45 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:

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

snip

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

Hi Danilo:

Thanks so much for the pointers. I went through the HRT series. If we
are getting rid of the Devres with HRT, then my PATCH 1 is not required
as well. I actually got this callback example from DRM subsystem.

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

Got it. 

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

Thanks so much. I will take a look and re-spin it today. 

> (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-25  8:25 UTC|newest]

Thread overview: 6+ 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
2026-06-25  8:24     ` Zhi Wang [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=20260625112429.3e89e869@inno-dell \
    --to=zhiw@nvidia.com \
    --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=dakr@kernel.org \
    --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=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