Re: [PATCH v3 17/27] rust: auxiliary: generalize Registration over ForLt

["Eliot Courtney" <ecourtney@nvidia.com>]

On Sun May 17, 2026 at 9:01 AM JST, Danilo Krummrich wrote:
> Generalize Registration<T> to Registration<F: ForLt> and
> Device::registration_data<F: ForLt>() to return Pin<&F::Of<'_>>.
>
> The stored 'static lifetime is shortened to the borrow lifetime of &self
> via ForLt::cast_ref; ForLt's covariance guarantee makes this sound.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  drivers/gpu/nova-core/driver.rs       |  4 +-
>  rust/kernel/auxiliary.rs              | 68 +++++++++++++++++----------
>  samples/rust/rust_driver_auxiliary.rs |  8 ++--
>  3 files changed, 52 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index fe4eafe1ebf0..ab0d7f2b516c 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -21,6 +21,7 @@
>          },
>          Arc,
>      },
> +    types::ForLt,
>  };
>  
>  use crate::gpu::Gpu;
> @@ -32,7 +33,8 @@
>  pub(crate) struct NovaCore {
>      #[pin]
>      pub(crate) gpu: Gpu,
> -    _reg: Devres<auxiliary::Registration<()>>,
> +    #[allow(clippy::type_complexity)]
> +    _reg: Devres<auxiliary::Registration<ForLt!(())>>,
>  }
>  
>  const BAR0_SIZE: usize = SZ_16M;
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 3f1e2966c03a..db3d56106ea2 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -20,6 +20,7 @@
>      },
>      prelude::*,
>      types::{
> +        ForLt,
>          ForeignOwnable,
>          Opaque, //
>      },
> @@ -271,12 +272,16 @@ pub fn parent(&self) -> &device::Device<device::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
> +    /// `F` is the [`ForLt`](trait@ForLt) encoding of the data type. The returned
> +    /// reference has its lifetime shortened from `'static` to `&self`'s borrow lifetime via
> +    /// [`ForLt::cast_ref`].
> +    ///
> +    /// Returns [`EINVAL`] if `F` 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>> {
> +    pub fn registration_data<F: ForLt>(&self) -> Result<Pin<&F::Of<'_>>> {
>          // 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() {
> @@ -289,18 +294,23 @@ pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {
>  
>          // 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`.
> +        // at the start of the allocation is valid regardless of `F`.
>          let type_id = unsafe { ptr.cast::<TypeId>().read() };
> -        if type_id != TypeId::of::<T>() {
> +        if type_id != TypeId::of::<F::Of<'static>>() {
>              return Err(EINVAL);
>          }

This seems a little dangerous to me, since the caller can specify the
ForLt type which means it can rewrite the lifetime of the ForLt encoded
type to be whatever it wants. The runtime check here only looks at the
typeid of the encoded type but can't check the mapping from forlt::of
given lifetime to actual underlying type lifetime instantiation.
For example, in the next patch on the sample aux driver you can do:

```
let data = adev.registration_data::<ForLt!(for<'a> Data<'static>)>()?;
let pdev: &'static pci::Device<Bound> = data.parent;
```

and mint a 'static reference to a bound device, which seems unsound to
me. So maybe this needs to store the typeid of F instead? so that it can
store the actual mapping from forlt lifetime to underlying type
lifetimes.

Overall I feel it's unfortunate to have this be a runtime check though.
What about requiring all device ids declared to be supported by an aux
bus driver to have the same rust registration data type? Not sure how
common it is to want to have the same aux bus driver support a very
hetereogenous set of aux bus devices though. I think getting rid of the
runtime check was discussed on the other patch series adding the rust
sideband registration data on the C side, but if there's not much real
use case for hetereogenous registration data types in a single aux bus
driver then it might be worth the added safety. WDYT?