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

["Gary Guo" <gary@garyguo.net>]

On Mon Apr 27, 2026 at 11:09 PM BST, 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.

I wonder if we could require auxillary drivers to specify a type, and then the
abstraction would check if the type matches; if it matches, it create a
`&Device<Core, T>` and `probe`, otherwise it skips over the driver completely.

This gets rid of the failure path.

>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  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;
>  };
>  
>  /**
> 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>> {

Any reason that this is not just `Result<&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() {

I think we could treat C-registered aux devices as if their data is `()`.

So you could do

    if TypeId::of::<T>() != TypeId::of::<()> {
        return Err(EINVAL);
    }
    return Ok(Pin::static_ref(&()));

here.

Best,
Gary

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