Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Apr 28, 2026 at 7:09 AM JST, Danilo Krummrich wrote:
> Add a registration_data pointer to struct auxiliary_device, allowing the
> registering (parent) driver to attach private data to the device at
> registration time and retrieve it later when called back by the
> auxiliary (child) driver.
>
> By tying the data to the device's registration, Rust drivers can bind
> the lifetime of device resources to it, since the auxiliary bus
> guarantees that the parent driver remains bound while the auxiliary
> device is bound.
>
> On the Rust side, Registration<T> takes ownership of the data via
> ForeignOwnable. A TypeId is stored alongside the data for runtime type
> checking, making Device::registration_data<T>() a safe method.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

This is indeed much, much cleaner!

> ---
>  drivers/gpu/nova-core/driver.rs       |  10 +-
>  include/linux/auxiliary_bus.h         |   4 +
>  rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
>  samples/rust/rust_driver_auxiliary.rs |  40 +++--
>  4 files changed, 180 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 84b0e1703150..8fe484d357f6 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -32,8 +32,7 @@
>  pub(crate) struct NovaCore {
>      #[pin]
>      pub(crate) gpu: Gpu,
> -    #[pin]
> -    _reg: Devres<auxiliary::Registration>,
> +    _reg: Devres<auxiliary::Registration<()>>,
>  }
>  
>  const BAR0_SIZE: usize = SZ_16M;
> @@ -96,14 +95,15 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>  
>              Ok(try_pin_init!(Self {
>                  gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?),
> -                _reg <- auxiliary::Registration::new(
> +                _reg: auxiliary::Registration::new(
>                      pdev.as_ref(),
>                      c"nova-drm",
>                      // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
>                      // now, use a simple atomic counter that never recycles IDs.
>                      AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
> -                    crate::MODULE_NAME
> -                ),
> +                    crate::MODULE_NAME,
> +                    (),
> +                )?,
>              }))
>          })
>      }
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index bc09b55e3682..4e1ad8ccbcdd 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -62,6 +62,9 @@
>   * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
>   * @sysfs.lock: Synchronize irq sysfs creation,
>   * @sysfs.irq_dir_exists: whether "irqs" directory exists,
> + * @registration_data_rust: private data owned by the registering (parent)
> + *                          driver; valid for as long as the device is
> + *                          registered with the driver core,
>   *
>   * An auxiliary_device represents a part of its parent device's functionality.
>   * It is given a name that, combined with the registering drivers
> @@ -148,6 +151,7 @@ struct auxiliary_device {
>  		struct mutex lock; /* Synchronize irq sysfs creation */
>  		bool irq_dir_exists;
>  	} sysfs;
> +	void *registration_data_rust;

I'm wondering whether we could avoid introducing a Rust-only member
here, either by just allowing the aux device private data to be used in
C as well (which might be as simple as a rename, a couple helpers and a
bit more documentation), or using a wrapper type specifically for Rust
drivers:

struct rust_auxiliary_device {
	struct auxiliary_device auxdev;
	void *registration_data;
};

Although I am not sure what the implications would be for e.g. a Rust
auxiliary device spawned by a C driver? Is that even doable with the
current code anyway?

>  };
>  
>  /**
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 93c0db1f6655..467befea8e44 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -19,12 +19,17 @@
>          to_result, //
>      },
>      prelude::*,
> -    types::Opaque,
> +    types::{
> +        ForeignOwnable,
> +        Opaque, //
> +    },
>      ThisModule, //
>  };
>  use core::{
> +    any::TypeId,
>      marker::PhantomData,
>      mem::offset_of,
> +    pin::Pin,
>      ptr::{
>          addr_of_mut,
>          NonNull, //
> @@ -257,6 +262,40 @@ pub fn parent(&self) -> &device::Device<device::Bound> {
>          // SAFETY: A bound auxiliary device always has a bound parent device.
>          unsafe { parent.as_bound() }
>      }
> +
> +    /// Returns a pinned reference to the registration data set by the registering (parent) driver.
> +    ///
> +    /// Returns [`EINVAL`] if `T` does not match the type used by the parent driver when calling
> +    /// [`Registration::new()`].
> +    ///
> +    /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was
> +    /// registered by a C driver.
> +    pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {
> +        // SAFETY: By the type invariant, `self.as_raw()` is a valid `struct auxiliary_device`.
> +        let ptr = unsafe { (*self.as_raw()).registration_data_rust };
> +        if ptr.is_null() {
> +            dev_warn!(
> +                self.as_ref(),
> +                "No registration data set; parent is not a Rust driver.\n"
> +            );
> +            return Err(ENOENT);
> +        }
> +
> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
> +        // at the start of the allocation is valid regardless of `T`.
> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
> +        if type_id != TypeId::of::<T>() {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: The `TypeId` check above confirms that the stored type is `T`; `ptr` remains
> +        // valid until `Registration::drop()` calls `from_foreign()`.
> +        let wrapper = unsafe { Pin::<KBox<RegistrationData<T>>>::borrow(ptr) };
> +
> +        // SAFETY: `data` is a structurally pinned field of `RegistrationData`.
> +        Ok(unsafe { wrapper.map_unchecked(|w| &w.data) })
> +    }
>  }
>  
>  impl Device {
> @@ -326,87 +365,132 @@ unsafe impl Send for Device {}
>  // (i.e. `Device<Normal>) are thread safe.
>  unsafe impl Sync for Device {}
>  
> +/// Wrapper that stores a [`TypeId`] alongside the registration data for runtime type checking.
> +#[repr(C)]
> +#[pin_data]
> +struct RegistrationData<T> {
> +    type_id: TypeId,
> +    #[pin]
> +    data: T,
> +}
> +
>  /// The registration of an auxiliary device.
>  ///
>  /// This type represents the registration of a [`struct auxiliary_device`]. When its parent device
>  /// is unbound, the corresponding auxiliary device will be unregistered from the system.
>  ///
> +/// The type parameter `T` is the type of the registration data owned by the registering (parent)
> +/// driver. It can be accessed by the auxiliary driver through
> +/// [`Device::registration_data()`].
> +///
>  /// # Invariants
>  ///
> -/// `self.0` always holds a valid pointer to an initialized and registered
> -/// [`struct auxiliary_device`].
> -pub struct Registration(NonNull<bindings::auxiliary_device>);
> -
> -impl Registration {
> -    /// Create and register a new auxiliary device.
> -    pub fn new<'a>(
> -        parent: &'a device::Device<device::Bound>,
> -        name: &'a CStr,
> +/// `self.adev` always holds a valid pointer to an initialized and registered
> +/// [`struct auxiliary_device`], and `registration_data` points to a valid

There is no `registration_data` member in this struct, it this referring
to something else?