Re: [PATCH v5 01/11] gpu: nova-core: Add public driver API to nova-core

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

On Mon Aug 31, 2026 at 9:08 PM BST, Danilo Krummrich wrote:
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
>> +/// API handle for the auxiliary bus child drivers to interact with nova-core.
>> +pub struct NovaCoreApi<'bound> {
>> +    #[expect(unused)]
>> +    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
>> +}
>> +
>> +impl NovaCoreApi<'_> {
>> +    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
>> +    /// by nova-core.
>> +    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
>> +        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
>> +    }
>> +}
>
> CovariantForLt does not hold anymore on latest drm-rust-next, as Cmdq has a
> Mutex. So, this needs ForLt now and therefore the approach that I shared in [1]
> a while ago. I applied the changes in [2] to fix it up.
>
> I think the closure access through api.with(|api| ...) is fine in most cases,
> but there are a few options if we run into cases where we consider it a bit
> inconvinient.
>
> I think it should be possible to support projections into T: 'static and
> covariant fields. The reason I differentiate them is because T: 'static is very
> convinient, but non-'static covariant types need an annoying turbofish.
>
> For T: 'static types it would turn out like this
>
> 	let spec = reg_data.api.project(|api| api.spec());
>
> such that everything that needs spec does not need to be in the closure anymore.
>
> For non-'static covariant fields we could have
>
> 	let foo = reg_data.api.project_lt::<CovariantForLt!(Foo<'_>)>(|api| api.foo());
>
> but as mentioned it unfortunately needs the turbofish. Of course we could invent
> a macro around it to get rid of the turbofish, but we'd still need to explicitly
> mention the type Foo<'_>, so it doesn't buy us a lot.
>
> This is the implementation I came up with in nova-core
>
> 	/// Projects a `'static` sub-field out of the registration data.
> 	///
> 	/// `T` is fully inferred from the closure. For projected types with a lifetime parameter,
> 	/// use [`Self::project_lt`].
> 	pub fn project<T: 'static>(
> 	    &self,
> 	    f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> &'b T,
> 	) -> &'a T {
> 	    self.adev
> 	        .registration_data_field::<ForLt!(NovaCoreApi<'_>), T>(f)
> 	        .expect("TypeId was validated in NovaCoreApiHandle::of()")
> 	}
> 	
> 	/// Projects a covariant sub-field out of the registration data.
> 	///
> 	/// Supports projected types with a lifetime parameter via a
> 	/// [`CovariantForLt`](trait@CovariantForLt) encoding. Unlike [`Self::project`], `G` cannot
> 	/// be inferred and must be specified explicitly.
> 	pub fn project_lt<G: CovariantForLt + 'static>(
> 	    &self,
> 	    f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> &'b G::Of<'b>,
> 	) -> &'a G::Of<'a> {
> 	    self.adev
> 	        .registration_data_project::<ForLt!(NovaCoreApi<'_>), G>(f)
> 	        .expect("TypeId was validated in NovaCoreApiHandle::of()")
> 	}
>
> and this is what we'd need in the auxiliary bus
>
> 	/// Projects a covariant sub-field out of potentially invariant registration data.
> 	///
> 	/// `F` is the [`ForLt`](trait@ForLt) encoding of the registration data type. `G` is the
> 	/// [`CovariantForLt`](trait@CovariantForLt) encoding of the projected sub-field type.
> 	///
> 	/// For projected types that are `'static`, prefer [`Self::registration_data_field`] which
> 	/// does not require a [`CovariantForLt`](trait@CovariantForLt) encoding and fully infers `T`.
> 	///
> 	/// 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.
> 	#[inline]
> 	pub fn registration_data_project<F, G>(
> 	    &self,
> 	    project: impl for<'a> FnOnce(Pin<&'a F::Of<'a>>) -> &'a G::Of<'a>,
> 	) -> Result<&G::Of<'_>>
> 	where
> 	    F: ForLt + 'static,
> 	    G: CovariantForLt + 'static,
> 	{
> 	    let ptr = self.registration_data_with::<F, *const ()>(|data| {
> 	        core::ptr::from_ref::<G::Of<'_>>(project(data)).cast::<()>()
> 	    })?;
> 	
> 	    // SAFETY:
> 	    // - The HRTB bound on `project` ensures the returned pointer is derived from the
> 	    //   registration data (nothing else lives for universally quantified `'a`).
> 	    // - `G: CovariantForLt` guarantees that shortening the lifetime of `G::Of` is sound.
> 	    // - The registration data is heap-allocated and outlives the device's bound state.
> 	    Ok(unsafe { &*ptr.cast::<G::Of<'_>>() })
> 	}

I think this is a bit complex, you should be able to let Rust figure out the
relation between two lifetimes.

I think if you change the signatue of `registration_data_with` slightly:

    pub fn registration_data_with<'this, F: ForLt + 'static, R>(
        &'this self,
        f: impl for<'a> FnOnce(Pin<&'this F::Of<'a>>) -> R,
                                    ^ note this is changed from 'a to 'this
    ) -> Result<R>;

then there will be an implied bound available inside the callback where 'a
outlives 'this, and thus the function callback is able to perform coercion of
any T<'a> to T<'this> provided that `T` is covariant over lifetime `'a`.

[ The coercion won't work when doing abstract `F::Of` on the bus abstraction
side, but for any user it is dealing with concrete types so the compiler sees
specific types and thus can check variance ]

Then your projection can just be

    aux.registration_data_project(|x| &x.field)

I haven't tried it out but I think it should work.

Best,
Gary