Re: [RFC 1/3] gpu: nova-core: Add auxiliary bus registration data to nova-core

["Danilo Krummrich" <dakr@kernel.org>]

On Fri Jul 3, 2026 at 8:45 AM CEST, Alistair Popple wrote:
> diff --git a/drivers/gpu/nova-core/auxdata.rs b/drivers/gpu/nova-core/auxdata.rs
> new file mode 100644
> index 000000000000..266b5ce4ee89
> --- /dev/null
> +++ b/drivers/gpu/nova-core/auxdata.rs

I think it'd just call it something along the lines of api.rs, as this is going
to be the file where the entry points into nova-core will live.

> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Nova-core auxbus data. Contains all the methods used by the auxbus drivers
> +//! to interact with nova-core.
> +
> +use crate::gpu::Gpu;
> +
> +/// Auxiliary bus registration data. Used by the auxbus drivers to call methods on
> +/// the GPU.
> +pub struct AuxData<'bound> {

Since this will also be the data type that will be exposed as an API entry point
into nova-core, I'd rather call it e.g. NovaCoreApi.

We should provide an assoicated function NovaCoreApi::of(), which takes an &'a
auxiliary::Device<Bound> as argument and returns &'a NovaCoreApi<'a> (at least
as long as the type remains covariant).

This makes it a bit cleaner since it keeps the type assertion local to a single
place and makes it transparent to nova-drm that this also is the
auxiliary::Registration private data of nova-core.

(I'm already working on getting rid of the type ID check, but going through
NovaCoreApi::of() is cleaner regardless.)

> +    pub(crate) _gpu: &'bound Gpu<'bound>,

I'd store this as Pin<&'bound Gpu<'bound>> as it better documents the pinning
guarantee and allows calling Pin<&Self> methods on Gpu.

> @@ -78,7 +82,7 @@ fn probe<'bound>(
>              pdev.enable_device_mem()?;
>              pdev.set_master();
>  
> -            Ok(try_pin_init!(NovaCore {
> +            Ok(try_pin_init!(&this in NovaCore {

This change shouldn't be needed.

>                  bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?,
>                  // TODO: Use `&bar` self-referential pin-init syntax once available.
>                  //
> @@ -86,15 +90,32 @@ fn probe<'bound>(
>                  // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
>                  // stable address, and is dropped after `gpu` (struct field drop order).
>                  gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
> -                _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,
> -                    (),
> -                )?,
> +
> +                // SAFETY:
> +                // - `NovaCore` is dropped when the device is unbound; i.e.
> +                //   `mem::forget()` is never called on it.
> +                // - `gpu` is initialized above, lives at a pinned stable
> +                //   address, and is dropped after `_reg` (struct field drop
> +                //   order).
> +                _reg: unsafe {
> +                    auxiliary::Registration::new_with_lt(
> +                        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,
> +                        AuxData {
> +                            // TODO: Use `&gpu` self-referential pin-init syntax once available.
> +                            //
> +                            // SAFETY: `this.gpu` is initialized before this expression is evaluated
> +                            // (`try_pin_init!()` initializes fields in declaration order), lives at
> +                            // a pinned stable address, and is dropped after `_reg` (struct field
> +                            // drop order).
> +                            _gpu: &*core::ptr::from_ref(&this.as_ref().gpu),

This is UB, we can't create a reference from 'this' before it is fully
initialized.

	gpu: &*core::ptr::from_ref(gpu.as_ref().get_ref()),

should work instead and also gets us rid of this entirely. Or rather

	gpu: Pin::new_unchecked(
	    &*core::ptr::from_ref(gpu.as_ref().get_ref()),
	),

if stored as Pin<&'bound Gpu<'bound>>.

> +                        },
> +                    )?
> +                },
>              }))
>          })
>      }