On 2026-07-06 at 03:09 +1000, Danilo Krummrich <[email protected]> wrote...
> On Sun Jul 5, 2026 at 3:00 PM CEST, Danilo Krummrich wrote:
> > 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.
Heh. Yeah, I didn't expect this name to survive :-) It was more a straw-man that
I forgot to cleanup - NovaCoreApi works for me.
> > 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.)
>
> Thinking a bit more about this, I'm still not convinced about a design where
> the
> auxiliary::Device itself carries the type information. However, I think with
> the
> new lifetime design this isn't necessary in the first place.
>
> We can just call NovaCoreApi::of() once from nova-drm's probe() function and
> subsequently store the struct NovaCoreApi<'a> in the bus device private data,
> DRM registration data, etc. Such that the only fallible path is in probe().
I agree - and passing a handle to nova_core to access data was my original
instinct and unposted implementation. Of course prior to the higher-ranked
lifetime series that obviously involved some unsafe code (and possibly actual
UB).
Now that we can safely store the handle in the DRM registration I think this
looks cleaner - I just couldn't quite figure out the mechanics so thanks for the
pointers. I will incorporate this into v2.
> Please see [1] for reference, there's also a branch in [2].
Thanks for doing both - I appreciate seeing the proposal in the mail thread.
> Note that this only works for a covariant NovaCoreApi, once it becomes
> invariant
> we'll need something like this:
Yep. But for now at least it is covariant so I think we can avoid doing this
until needed.
- Alistair
> /// Closure-based API handle for invariant registration data types.
> pub struct NovaCoreApiHandle<'a> {
> adev: &'a auxiliary::Device<Bound>,
> }
>
> impl<'a> NovaCoreApiHandle<'a> {
> fn of(adev: &'a auxiliary::Device<Bound>) -> Result<Self> {
> adev.registration_data_with::<ForLt!(NovaCoreApi<'_>), ()>(|_|
> ())?;
> Ok(Self { adev })
> }
>
> /// Access the [`NovaCoreApi`] through a closure.
> pub fn with<R>(&self, f: impl for<'b> FnOnce(Pin<&'b
> NovaCoreApi<'b>>) -> R) -> R {
> // PANIC: TypeId was validated in `of()`, cannot fail.
> self.adev
> .registration_data_with::<ForLt!(NovaCoreApi<'_>), R>(f)
> .unwrap()
> }
> }
>
> We can still improve this a bit from the auxiliary API side, but in general
> this
> should be fine (added this to the branch in [2] as well).
>
> With this the access patterns becomes
>
> // Once from probe(), calls NovaCoreApiHandle::of() internally.
> let handle = NovaCoreApi::handle(adev)?;
>
> and from anywhere else
>
> handle.with(|api| api.chipset());
>
> [1]
>
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index 739690bc2db5..89da0f6c3ba5 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +use core::pin::Pin;
> +
> use kernel::{
> auxiliary,
> device::{
> @@ -17,6 +19,7 @@
>
> use crate::file::File;
> use crate::gem::NovaObject;
> +use nova_core::api::NovaCoreApi;
>
> pub(crate) struct NovaDriver;
>
> @@ -26,6 +29,11 @@ pub(crate) struct Nova<'bound> {
> _reg: drm::Registration<'bound, NovaDriver>,
> }
>
> +/// DRM registration data, accessible from ioctl handlers via the
> registration guard.
> +pub(crate) struct DrmRegData<'bound> {
> + pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
> +}
> +
> /// Convienence type alias for the DRM device type for this driver
> pub(crate) type NovaDevice<Ctx = drm::Normal> = drm::Device<NovaDriver, Ctx>;
>
> @@ -59,10 +67,15 @@ fn probe<'bound>(
> adev: &'bound auxiliary::Device<Core<'_>>,
> _info: &'bound Self::IdInfo,
> ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> + let api = NovaCoreApi::of(adev)?;
> +
> let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
> +
> + let drm_data = DrmRegData { api };
> +
> // SAFETY: `reg` is stored in `Nova` and dropped when the driver is
> unbound; it is
> // never forgotten.
> - let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, (),
> 0)? };
> + let reg = unsafe { drm::Registration::new(adev.as_ref(), drm,
> drm_data, 0)? };
>
> Ok(Nova {
> drm: reg.device().into(),
> @@ -74,7 +87,7 @@ fn probe<'bound>(
> #[vtable]
> impl drm::Driver for NovaDriver {
> type Data = ();
> - type RegistrationData<'a> = ();
> + type RegistrationData<'a> = DrmRegData<'a>;
> type File = File;
> type Object = gem::Object<NovaObject>;
> type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
> diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
> index 57df54ba27bd..2488825c78cf 100644
> --- a/drivers/gpu/drm/nova/file.rs
> +++ b/drivers/gpu/drm/nova/file.rs
> @@ -1,7 +1,14 @@
> // SPDX-License-Identifier: GPL-2.0
>
> -use crate::driver::{NovaDevice, NovaDriver};
> -use crate::gem::NovaObject;
> +use crate::{
> + driver::{
> + DrmRegData,
> + NovaDevice,
> + NovaDriver, //
> + },
> + gem::NovaObject,
> +};
> +
> use kernel::{
> alloc::flags::*,
> auxiliary,
> @@ -16,9 +23,6 @@
> uapi,
> };
>
> -use kernel::types::CovariantForLt;
> -use nova_core::auxdata::AuxData;
> -
> pub(crate) struct File;
>
> impl drm::file::DriverFile for File {
> @@ -33,18 +37,17 @@ impl File {
> /// IOCTL: get_param: Query GPU / driver metadata.
> pub(crate) fn get_param(
> dev: &NovaDevice<Registered>,
> - _reg_data: &(),
> + reg_data: &DrmRegData<'_>,
> getparam: &mut uapi::drm_nova_getparam,
> _file: &drm::File<File>,
> ) -> Result<u32> {
> let adev: &auxiliary::Device<Bound> = dev.as_ref();
> - let pdev: &pci::Device<Bound> = adev.parent().try_into()?;
> - let data = adev.registration_data::<CovariantForLt!(AuxData<'_>)>()?;
> + let pdev: &pci::Device<_> = adev.parent().try_into()?;
>
> let value = match getparam.param as u32 {
> uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
> - uapi::NOVA_GETPARAM_GPU_CHIPSET => data.chipset() as u64,
> - uapi::NOVA_GETPARAM_VRAM_SIZE => data.vram_size(),
> + uapi::NOVA_GETPARAM_GPU_CHIPSET => reg_data.api.chipset() as u64,
> + uapi::NOVA_GETPARAM_VRAM_SIZE => reg_data.api.vram_size(),
> _ => return Err(EINVAL),
> };
>
> @@ -56,7 +59,7 @@ pub(crate) fn get_param(
> /// IOCTL: gem_create: Create a new DRM GEM object.
> pub(crate) fn gem_create(
> dev: &NovaDevice<Registered>,
> - _reg_data: &(),
> + _reg_data: &DrmRegData<'_>,
> req: &mut uapi::drm_nova_gem_create,
> file: &drm::File<File>,
> ) -> Result<u32> {
> @@ -70,7 +73,7 @@ pub(crate) fn gem_create(
> /// IOCTL: gem_info: Query GEM metadata.
> pub(crate) fn gem_info(
> _dev: &NovaDevice<Registered>,
> - _reg_data: &(),
> + _reg_data: &DrmRegData<'_>,
> req: &mut uapi::drm_nova_gem_info,
> file: &drm::File<File>,
> ) -> Result<u32> {
> diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
> new file mode 100644
> index 000000000000..ba99d5a27805
> --- /dev/null
> +++ b/drivers/gpu/nova-core/api.rs
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Nova Core API for auxiliary bus child drivers.
> +
> +use core::pin::Pin;
> +
> +use kernel::{
> + auxiliary,
> + device::Bound,
> + prelude::*,
> + types::CovariantForLt, //
> +};
> +
> +use crate::gpu::{
> + Chipset,
> + Gpu, //
> +};
> +
> +/// API handle for auxiliary bus child drivers to interact with nova-core.
> +pub struct NovaCoreApi<'bound> {
> + 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<'_>)>()
> + }
> +
> + /// Returns the chipset of this GPU.
> + pub fn chipset(&self) -> Chipset {
> + self.gpu.spec.chipset
> + }
> +
> + /// Returns the total usable VRAM size of this GPU in bytes.
> + pub fn vram_size(&self) -> u64 {
> + self.gpu.gsp_static_info.vram_size()
> + }
> +}
> diff --git a/drivers/gpu/nova-core/auxdata.rs
> b/drivers/gpu/nova-core/auxdata.rs
> deleted file mode 100644
> index 0a07a10da327..000000000000
> --- a/drivers/gpu/nova-core/auxdata.rs
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -// 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::Chipset;
> -use crate::gpu::Gpu;
> -
> -/// Auxiliary bus registration data. Used by the auxbus drivers to call
> methods on
> -/// the GPU.
> -pub struct AuxData<'bound> {
> - pub(crate) gpu: &'bound Gpu<'bound>,
> -}
> -
> -impl AuxData<'_> {
> - /// Returns the chipset of this GPU.
> - pub fn chipset(&self) -> Chipset {
> - self.gpu.spec.chipset
> - }
> -
> - /// Returns the total usable VRAM size of this GPU in bytes.
> - pub fn vram_size(&self) -> u64 {
> - self.gpu.gsp_static_info.vram_size()
> - }
> -}
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 8b0bebee0bdf..9cb2eecfae1c 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +use core::pin::Pin;
> +
> use kernel::{
> auxiliary,
> device::Core,
> @@ -18,7 +20,7 @@
> types::CovariantForLt,
> };
>
> -use crate::auxdata::AuxData;
> +use crate::api::NovaCoreApi;
> use crate::gpu::Gpu;
>
> /// Counter for generating unique auxiliary device IDs.
> @@ -30,7 +32,7 @@ pub(crate) struct NovaCore<'bound> {
> // device before dropping `gpu`, and drop `gpu` before `bar` because
> `Gpu`
> // borrows `bar`.
> #[allow(clippy::type_complexity)]
> - _reg: auxiliary::Registration<'bound, CovariantForLt!(AuxData<'_>)>,
> + _reg: auxiliary::Registration<'bound, CovariantForLt!(NovaCoreApi<'_>)>,
> #[pin]
> pub(crate) gpu: Gpu<'bound>,
> bar: pci::Bar<'bound, BAR0_SIZE>,
> @@ -82,7 +84,7 @@ fn probe<'bound>(
> pdev.enable_device_mem()?;
> pdev.set_master();
>
> - Ok(try_pin_init!(&this in NovaCore {
> + Ok(try_pin_init!(NovaCore {
> bar: pdev.iomap_region_sized::<BAR0_SIZE>(0,
> c"nova-core/bar0")?,
> // TODO: Use `&bar` self-referential pin-init syntax once
> available.
> //
> @@ -105,14 +107,16 @@ fn probe<'bound>(
> // For now, use a simple atomic counter that never
> recycles IDs.
> AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
> crate::MODULE_NAME,
> - AuxData {
> + NovaCoreApi {
> // TODO: Use `&gpu` self-referential pin-init
> syntax once available.
> //
> - // SAFETY: `this.gpu` is initialized before this
> expression is evaluated
> + // SAFETY: `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),
> + gpu: Pin::new_unchecked(
> +
> &*core::ptr::from_ref(gpu.as_ref().get_ref()),
> + ),
> },
> )?
> },
> diff --git a/drivers/gpu/nova-core/nova_core.rs
> b/drivers/gpu/nova-core/nova_core.rs
> index d00c8804269d..9463ae038e81 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -10,7 +10,7 @@
> InPlaceModule, //
> };
>
> -pub mod auxdata;
> +pub mod api;
> mod driver;
> mod falcon;
> mod fb;
>
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=nova-api
