NVIDIA GPU driver infrastructure
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Alistair Popple" <apopple@nvidia.com>
Cc: <nova-gpu@lists.linux.dev>, "Alice Ryhl" <aliceryhl@google.com>,
	"David Airlie" <airlied@gmail.com>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Benno Lossin" <lossin@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"John Hubbard" <jhubbard@nvidia.com>,
	<linux-kernel@vger.kernel.org>, <dri-devel@lists.freedesktop.org>,
	<rust-for-linux@vger.kernel.org>
Subject: Re: [RFC 1/3] gpu: nova-core: Add auxiliary bus registration data to nova-core
Date: Sun, 05 Jul 2026 19:09:56 +0200	[thread overview]
Message-ID: <DJQSY9ZY5M5F.3VI537GS00E1G@kernel.org> (raw)
In-Reply-To: <DJQNN9FRZN3C.3DQE4H9HAZ8W6@kernel.org>

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.
>
> 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().

Please see [1] for reference, there's also a branch in [2].

Note that this only works for a covariant NovaCoreApi, once it becomes invariant
we'll need something like this:

	/// 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

  reply	other threads:[~2026-07-05 17:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03  6:45 [RFC 0/3] gpu: nova: Export parameters from nova-core to nova-drm Alistair Popple
2026-07-03  6:45 ` [RFC 1/3] gpu: nova-core: Add auxiliary bus registration data to nova-core Alistair Popple
2026-07-05 13:00   ` Danilo Krummrich
2026-07-05 17:09     ` Danilo Krummrich [this message]
2026-07-06  3:20       ` Alistair Popple
2026-07-03  6:45 ` [RFC 2/3] drm: nova: Add GETPARAM parameter to read the GPU chipset Alistair Popple
2026-07-03  6:45 ` [RFC 3/3] drm: nova: Add a GETPARAM parameter to read usable VRAM size Alistair Popple

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DJQSY9ZY5M5F.3VI537GS00E1G@kernel.org \
    --to=dakr@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox