[PATCH v3 0/7] gpu: nova: Export parameters from nova-core to nova-drm

[PATCH v3 0/7] gpu: nova: Export parameters from nova-core to nova-drm

[Alistair Popple]

This patch series adds some basic GPU properties to the existing nova-drm
GETPARAM ioctl. It builds on top of the "drm: Higher-Ranked Lifetime
private data" series[1] to correctly manage lifetimes of registration
data shared between DRM, auxbus and nova-core. It's also based on top of
"ForLt/CovariantForLt split, auxiliary closure API and DevresLt"[2] although the
functionality of that series isn't actually required.

A tree with this series applied on top of all pre-requisites is available at
[3].

Properties are exported via a new NovaCoreApi type which implements methods to
read data from the GPU. This has been implemented in a separate module to make
the public API implementations obvious and to keep them in one place. Auxiliary
bus drivers can obtain a handle to this type using NovaCoreApi::of().

This handle can then be stored as part of the DRM registration data and used to
interact with the GPU via the nova-core driver.

Changes since v2:

 - Addressed review Danilo and Alex
 - Minor renames to better align with HW based on internal feedback

Changes since v1:

 - Address review comments from Danilo
 - Add an API call to read VRAM PCI BAR size using nova-core
 
[1] - https://lore.kernel.org/rust-for-linux/20260628145406.2107056-1-dakr@kernel.org/
[2] - https://lore.kernel.org/driver-core/20260626183630.2585057-1-dakr@kernel.org/
[3] - https://github.com/apopple-nvidia/linux/tree/nova-drm

Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Benno Lossin <lossin@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: Eliot Courtney <ecourtney@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: linux-kernel@vger.kernel.org
Cc: nova-gpu@lists.linux.dev
Cc: dri-devel@lists.freedesktop.org
Cc: rust-for-linux@vger.kernel.org

Alistair Popple (7):
  gpu: nova-core: Add public driver API to nova-core
  drm: nova: Add DRM registration data
  drm: nova: Add chipid enum to nova-drm UAPI
  drm: nova: Add GETPARAM parameter to read the GPU chipid
  drm: nova: Add GETPARAM parameter to read usable VRAM size
  drm: nova: Use nova-core to read VRAM_BAR_SIZE parameter
  drm: nova: Expose a render node

 drivers/gpu/drm/nova/driver.rs        | 17 +++++-
 drivers/gpu/drm/nova/file.rs          | 24 ++++----
 drivers/gpu/nova-core/api.rs          | 49 ++++++++++++++++
 drivers/gpu/nova-core/driver.rs       | 44 +++++++++++----
 drivers/gpu/nova-core/gpu.rs          | 80 ++++++++++++++-------------
 drivers/gpu/nova-core/gsp/commands.rs |  8 +++
 drivers/gpu/nova-core/gsp/hal.rs      |  2 +-
 drivers/gpu/nova-core/nova_core.rs    |  1 +
 include/uapi/drm/nova_drm.h           | 47 ++++++++++++++++
 9 files changed, 207 insertions(+), 65 deletions(-)
 create mode 100644 drivers/gpu/nova-core/api.rs

-- 
2.54.0

[PATCH v3 1/7] gpu: nova-core: Add public driver API to nova-core

[Alistair Popple]

Nova core will be used to export core functionality to other drivers
which will bind to it via auxiliary bus devices. Add a NovaCoreApi type
which drivers can use to call nova-core methods.

Auxiliary bus drivers can obtain a handle to call nova-core methods on
a particular GPU using NovaCoreApi::of(). This takes a reference to a
bound auxiliary bus device and returns a handle to NovaCoreApi.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes since v2:

 - Add accidentally dropped TODO comment

Changes since v1:

 - Rework unsafe pin-init to make safety comments clearer, suggested
   by Danilo.
 - s/allow(dead_code)/expect(unused)/
---
 drivers/gpu/nova-core/api.rs       | 29 ++++++++++++++++++++
 drivers/gpu/nova-core/driver.rs    | 44 ++++++++++++++++++++++--------
 drivers/gpu/nova-core/gsp/hal.rs   |  2 +-
 drivers/gpu/nova-core/nova_core.rs |  1 +
 4 files changed, 64 insertions(+), 12 deletions(-)
 create mode 100644 drivers/gpu/nova-core/api.rs

diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
new file mode 100644
index 000000000000..610cfc01111e
--- /dev/null
+++ b/drivers/gpu/nova-core/api.rs
@@ -0,0 +1,29 @@
+// 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 core::pin::Pin;
+
+use kernel::{
+    auxiliary,
+    device::Bound,
+    prelude::*,
+    types::CovariantForLt, //
+};
+
+use crate::gpu::Gpu;
+
+/// 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<'_>)>()
+    }
+}
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 48380ac15f68..435aa917d3e7 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -18,6 +18,7 @@
     types::CovariantForLt,
 };
 
+use crate::api::NovaCoreApi;
 use crate::gpu::Gpu;
 
 /// Counter for generating unique auxiliary device IDs.
@@ -25,11 +26,14 @@
 
 #[pin_data]
 pub(crate) struct NovaCore<'bound> {
+    // Fields are dropped in declaration order: unregister the auxiliary
+    // device before dropping `gpu`, and drop `gpu` before `bar` because `Gpu`
+    // borrows `bar`.
+    #[allow(clippy::type_complexity)]
+    _reg: auxiliary::Registration<'bound, CovariantForLt!(NovaCoreApi<'_>)>,
     #[pin]
     pub(crate) gpu: Gpu<'bound>,
     bar: pci::Bar<'bound, BAR0_SIZE>,
-    #[allow(clippy::type_complexity)]
-    _reg: auxiliary::Registration<'bound, CovariantForLt!(())>,
 }
 
 pub(crate) struct NovaCoreDriver;
@@ -86,15 +90,33 @@ 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,
-                    (),
-                )?,
+
+                _reg: {
+                    // TODO: Use `&gpu` self-referential pin-init syntax once available.
+                    //
+                    // 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).
+                    let gpu = unsafe {
+                        Pin::new_unchecked(&*core::ptr::from_ref(gpu.as_ref().get_ref()))
+                    };
+
+                    // SAFETY: `NovaCore` is dropped when the device is unbound;
+                    // i.e. `mem::forget()` is never called on it.
+                    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,
+                            NovaCoreApi { gpu },
+                        )?
+                    }
+                },
             }))
         })
     }
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 34b4bb82a999..b59bcaf2042e 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -29,7 +29,7 @@
 /// The GSP unload code might run in a situation where we cannot load firmware dynamically (e.g.
 /// because we are in shutdown and the file system is not accessible anymore). Thus, the firmware
 /// required for unloading is prepared at load time, and stored here until it needs to be run.
-pub(super) trait UnloadBundle: Send {
+pub(super) trait UnloadBundle: Send + Sync {
     /// Performs the steps required to properly reset the GSP after it has been stopped.
     fn run(&self, ctx: &mut GspBootContext<'_, '_>) -> Result;
 }
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index a61406ba5c0b..9463ae038e81 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -10,6 +10,7 @@
     InPlaceModule, //
 };
 
+pub mod api;
 mod driver;
 mod falcon;
 mod fb;
-- 
2.54.0

[PATCH v3 2/7] drm: nova: Add DRM registration data

[Alistair Popple]

Currently the nova-drm stub driver doesn't really interact with a real
device, so it doesn't have any registration data and has no way to call
into nova-core.

To allow for this add a DrmRegData type which will hold a reference to
the NovaCoreApi associated with the auxbus device.

Signed-off-by: Alistair Popple <apopple@nvidia.com>
Suggested-by: Danilo Krummrich <dakr@kernel.org>

---

Changes from v2:

 - Add newline in crate import

Changes from v1:

 - Drop unrelated device::Bound removal.
 - Switch exisiting import to vertical style.
 - s/allow(dead_code)/expect(unused)/
---
 drivers/gpu/drm/nova/driver.rs | 17 +++++++++++++++--
 drivers/gpu/drm/nova/file.rs   | 12 ++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 739690bc2db5..632137d1c6d7 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::{
@@ -18,6 +20,8 @@
 use crate::file::File;
 use crate::gem::NovaObject;
 
+use nova_core::api::NovaCoreApi;
+
 pub(crate) struct NovaDriver;
 
 pub(crate) struct Nova<'bound> {
@@ -26,6 +30,12 @@ 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> {
+    #[expect(unused)]
+    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>;
 
@@ -60,9 +70,12 @@ fn probe<'bound>(
         _info: &'bound Self::IdInfo,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
+        let reg_data = DrmRegData {
+            api: NovaCoreApi::of(adev)?,
+        };
         // 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, reg_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 298c02bacb4b..1156df51c533 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
-use crate::driver::{NovaDevice, NovaDriver};
+use crate::driver::{
+    DrmRegData,
+    NovaDevice,
+    NovaDriver, //
+};
 use crate::gem::NovaObject;
 use kernel::{
     alloc::flags::*,
@@ -30,7 +34,7 @@ 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> {
@@ -50,7 +54,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> {
@@ -64,7 +68,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> {
-- 
2.54.0

[PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[Alistair Popple]

The chipid contains the GPU architecture and implementation and needs
to be exposed to user-space. This adds a public enum to the userspace
headers for each chip ID. Nova-core can then use this enum to define
it's chipsets.

This does create a coupling between nova-drm and nova-core whereby
nova-core depends on the values defined by the user-space API for
nova-drm. However this is entirely appropriate as nova-core must be
bound by the UAPI headers as the enum values are read by nova-core and
passed through to user-space.

Note that nova-core currently refers to the chipid as a chipset. This
isn't ideal given that chipid is the preferred and more accurate term
and is what should be exposed via the uAPI. Therefore a future patch
series will be posted that aligns nova-core internals to using chipid
rather than chipset.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes since v2:

 - New for v3, split out from "drm: nova: Add GETPARAM parameter to read
   the GPU chipset"
---
 drivers/gpu/nova-core/gpu.rs | 64 +++++++++++++++++++-----------------
 include/uapi/drm/nova_drm.h  | 33 +++++++++++++++++++
 2 files changed, 67 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 442c0979f9c6..738a590e753b 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -10,7 +10,8 @@
     num::Bounded,
     pci,
     prelude::*,
-    sizes::SizeConstants, //
+    sizes::SizeConstants,
+    uapi, //
 };
 
 use crate::{
@@ -35,12 +36,13 @@
 mod hal;
 
 macro_rules! define_chipset {
-    ({ $($variant:ident = $value:expr),* $(,)* }) =>
+    ({ $($variant:ident),* $(,)* }) =>
     {
+        ::kernel::macros::paste!(
         /// Enum representation of the GPU chipset.
         #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
         pub(crate) enum Chipset {
-            $($variant = $value),*,
+            $($variant = uapi::[<drm_nova_chipid_NOVA_DRM_CHIPID_ $variant:upper>] as isize),*,
         }
 
         impl Chipset {
@@ -48,7 +50,6 @@ impl Chipset {
                 $( Chipset::$variant, )*
             ];
 
-            ::kernel::macros::paste!(
             /// Returns the name of this chipset, in lowercase.
             ///
             /// # Examples
@@ -64,7 +65,6 @@ pub(crate) const fn name(&self) -> &'static str {
                 )*
                 }
             }
-            );
         }
 
         // TODO[FPRI]: replace with something like derive(FromPrimitive)
@@ -73,45 +73,49 @@ impl TryFrom<u32> for Chipset {
 
             fn try_from(value: u32) -> Result<Self, Self::Error> {
                 match value {
-                    $( $value => Ok(Chipset::$variant), )*
+                    $(
+                        uapi::[<drm_nova_chipid_NOVA_DRM_CHIPID_ $variant:upper>]
+                            => Ok(Chipset::$variant),
+                    )*
                     _ => Err(ENODEV),
                 }
             }
         }
+    );
     }
 }
 
 define_chipset!({
     // Turing
-    TU102 = 0x162,
-    TU104 = 0x164,
-    TU106 = 0x166,
-    TU117 = 0x167,
-    TU116 = 0x168,
+    TU102,
+    TU104,
+    TU106,
+    TU117,
+    TU116,
     // Ampere
-    GA100 = 0x170,
-    GA102 = 0x172,
-    GA103 = 0x173,
-    GA104 = 0x174,
-    GA106 = 0x176,
-    GA107 = 0x177,
+    GA100,
+    GA102,
+    GA103,
+    GA104,
+    GA106,
+    GA107,
     // Hopper
-    GH100 = 0x180,
+    GH100,
     // Ada
-    AD102 = 0x192,
-    AD103 = 0x193,
-    AD104 = 0x194,
-    AD106 = 0x196,
-    AD107 = 0x197,
+    AD102,
+    AD103,
+    AD104,
+    AD106,
+    AD107,
     // Blackwell GB10x
-    GB100 = 0x1a0,
-    GB102 = 0x1a2,
+    GB100,
+    GB102,
     // Blackwell GB20x
-    GB202 = 0x1b2,
-    GB203 = 0x1b3,
-    GB205 = 0x1b5,
-    GB206 = 0x1b6,
-    GB207 = 0x1b7,
+    GB202,
+    GB203,
+    GB205,
+    GB206,
+    GB207,
 });
 
 impl Chipset {
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index 3ca90ed9d2bb..cb13fba6b952 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -25,6 +25,39 @@ extern "C" {
  */
 #define NOVA_GETPARAM_VRAM_BAR_SIZE	0x1
 
+enum drm_nova_chipid {
+	/* Turing */
+	NOVA_DRM_CHIPID_TU102 = 0x162,
+	NOVA_DRM_CHIPID_TU104 = 0x164,
+	NOVA_DRM_CHIPID_TU106 = 0x166,
+	NOVA_DRM_CHIPID_TU117 = 0x167,
+	NOVA_DRM_CHIPID_TU116 = 0x168,
+	/* Ampere */
+	NOVA_DRM_CHIPID_GA100 = 0x170,
+	NOVA_DRM_CHIPID_GA102 = 0x172,
+	NOVA_DRM_CHIPID_GA103 = 0x173,
+	NOVA_DRM_CHIPID_GA104 = 0x174,
+	NOVA_DRM_CHIPID_GA106 = 0x176,
+	NOVA_DRM_CHIPID_GA107 = 0x177,
+	/* Hopper */
+	NOVA_DRM_CHIPID_GH100 = 0x180,
+	/* Ada */
+	NOVA_DRM_CHIPID_AD102 = 0x192,
+	NOVA_DRM_CHIPID_AD103 = 0x193,
+	NOVA_DRM_CHIPID_AD104 = 0x194,
+	NOVA_DRM_CHIPID_AD106 = 0x196,
+	NOVA_DRM_CHIPID_AD107 = 0x197,
+	/* Blackwell GB10x */
+	NOVA_DRM_CHIPID_GB100 = 0x1a0,
+	NOVA_DRM_CHIPID_GB102 = 0x1a2,
+	/* Blackwell GB20x */
+	NOVA_DRM_CHIPID_GB202 = 0x1b2,
+	NOVA_DRM_CHIPID_GB203 = 0x1b3,
+	NOVA_DRM_CHIPID_GB205 = 0x1b5,
+	NOVA_DRM_CHIPID_GB206 = 0x1b6,
+	NOVA_DRM_CHIPID_GB207 = 0x1b7,
+};
+
 /**
  * struct drm_nova_getparam - query GPU and driver metadata
  */
-- 
2.54.0

[PATCH v3 4/7] drm: nova: Add GETPARAM parameter to read the GPU chipid

[Alistair Popple]

One of the first things a user needs to know about a GPU is its chipid,
so add a GETPARAM parameter to return that.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes since v2:

 - Split out enum definition into its own patch
 - Change uAPI to use chipid rather than chipset

Changes since v1:

 - Vertical import
 - Use a single enum for chipset values
---
 drivers/gpu/drm/nova/driver.rs |  1 -
 drivers/gpu/drm/nova/file.rs   |  3 ++-
 drivers/gpu/nova-core/api.rs   | 11 +++++++++--
 drivers/gpu/nova-core/gpu.rs   | 10 +++++-----
 include/uapi/drm/nova_drm.h    |  7 +++++++
 5 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 632137d1c6d7..f144417b29c9 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -32,7 +32,6 @@ pub(crate) struct Nova<'bound> {
 
 /// DRM registration data, accessible from ioctl handlers via the registration guard.
 pub(crate) struct DrmRegData<'bound> {
-    #[expect(unused)]
     pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
 }
 
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 1156df51c533..855f23877734 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -34,7 +34,7 @@ impl File {
     /// IOCTL: get_param: Query GPU / driver metadata.
     pub(crate) fn get_param(
         dev: &NovaDevice<Registered>,
-        _reg_data: &DrmRegData<'_>,
+        reg_data: &DrmRegData<'_>,
         getparam: &mut uapi::drm_nova_getparam,
         _file: &drm::File<File>,
     ) -> Result<u32> {
@@ -43,6 +43,7 @@ pub(crate) fn get_param(
 
         let value = match getparam.param as u32 {
             uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
+            uapi::NOVA_GETPARAM_GPU_CHIPID => reg_data.api.chipset() as u64,
             _ => return Err(EINVAL),
         };
 
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index 610cfc01111e..09da0b4e9103 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -12,11 +12,13 @@
     types::CovariantForLt, //
 };
 
-use crate::gpu::Gpu;
+use crate::gpu::{
+    Chipset,
+    Gpu, //
+};
 
 /// 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>>,
 }
 
@@ -26,4 +28,9 @@ impl NovaCoreApi<'_> {
     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
+    }
 }
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 738a590e753b..264fe4e65b3f 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -41,7 +41,7 @@ macro_rules! define_chipset {
         ::kernel::macros::paste!(
         /// Enum representation of the GPU chipset.
         #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
-        pub(crate) enum Chipset {
+        pub enum Chipset {
             $($variant = uapi::[<drm_nova_chipid_NOVA_DRM_CHIPID_ $variant:upper>] as isize),*,
         }
 
@@ -119,7 +119,7 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
 });
 
 impl Chipset {
-    pub(crate) const fn arch(self) -> Architecture {
+    pub const fn arch(self) -> Architecture {
         match self {
             Self::TU102 | Self::TU104 | Self::TU106 | Self::TU117 | Self::TU116 => {
                 Architecture::Turing
@@ -161,7 +161,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 bounded_enum! {
     /// Enum representation of the GPU generation.
     #[derive(fmt::Debug, Copy, Clone)]
-    pub(crate) enum Architecture with TryFrom<Bounded<u32, 6>> {
+    pub enum Architecture with TryFrom<Bounded<u32, 6>> {
         Turing = 0x16,
         Ampere = 0x17,
         Hopper = 0x18,
@@ -195,7 +195,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 /// Structure holding a basic description of the GPU: `Chipset` and `Revision`.
 #[derive(Clone, Copy)]
 pub(crate) struct Spec {
-    chipset: Chipset,
+    pub(crate) chipset: Chipset,
     revision: Revision,
 }
 
@@ -281,7 +281,7 @@ struct GspResources<'gpu> {
 /// Structure holding the resources required to operate the GPU.
 #[pin_data]
 pub(crate) struct Gpu<'gpu> {
-    spec: Spec,
+    pub(crate) spec: Spec,
     /// Static GPU information as provided by the GSP.
     gsp_static_info: GetGspStaticInfoReply,
     /// GSP and its resources.
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index cb13fba6b952..c655302a40fd 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -25,6 +25,13 @@ extern "C" {
  */
 #define NOVA_GETPARAM_VRAM_BAR_SIZE	0x1
 
+/*
+ * NOVA_GETPARAM_GPU_CHIPID
+ *
+ * Query the GPU chipid (architecture/implementation).
+ */
+#define NOVA_GETPARAM_GPU_CHIPID	0x2
+
 enum drm_nova_chipid {
 	/* Turing */
 	NOVA_DRM_CHIPID_TU102 = 0x162,
-- 
2.54.0

[PATCH v3 5/7] drm: nova: Add GETPARAM parameter to read usable VRAM size

[Alistair Popple]

Add a parameter to the existing GETPARAM ioctl to return the total
usable framebuffer size. The usable framebuffer excludes GSP carveouts
and other protected regions so may differ in size from BAR size and
total physical VRAM.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes since v2:

 - Commit wording

Changes since v1:

 - Fix rustfmt
---
 drivers/gpu/drm/nova/file.rs          | 1 +
 drivers/gpu/nova-core/api.rs          | 5 +++++
 drivers/gpu/nova-core/gpu.rs          | 6 ++----
 drivers/gpu/nova-core/gsp/commands.rs | 8 ++++++++
 include/uapi/drm/nova_drm.h           | 7 +++++++
 5 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 855f23877734..8c02be4f15bf 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -44,6 +44,7 @@ pub(crate) fn get_param(
         let value = match getparam.param as u32 {
             uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
             uapi::NOVA_GETPARAM_GPU_CHIPID => reg_data.api.chipset() as u64,
+            uapi::NOVA_GETPARAM_VRAM_SIZE => reg_data.api.vram_size(),
             _ => return Err(EINVAL),
         };
 
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index 09da0b4e9103..f025df90dc5d 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -33,4 +33,9 @@ pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
     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/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 264fe4e65b3f..6581c91a0b78 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -283,7 +283,7 @@ struct GspResources<'gpu> {
 pub(crate) struct Gpu<'gpu> {
     pub(crate) spec: Spec,
     /// Static GPU information as provided by the GSP.
-    gsp_static_info: GetGspStaticInfoReply,
+    pub(crate) gsp_static_info: GetGspStaticInfoReply,
     /// GSP and its resources.
     #[pin]
     gsp_resources: GspResources<'gpu>,
@@ -400,9 +400,7 @@ pub(crate) fn new(
                     dev_dbg!(
                         dev,
                         "Total usable VRAM: {} MiB\n",
-                        info.usable_fb_regions.iter().fold(0u64, |res, region| res
-                            .saturating_add(region.end - region.start))
-                            / u64::SZ_1M
+                        info.vram_size() / u64::SZ_1M
                     );
                 }
 
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 08380de39048..49ff3f5edcec 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -252,6 +252,14 @@ pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
             .to_str()
             .map_err(GpuNameError::InvalidUtf8)
     }
+
+    /// Returns the total usable VRAM size in bytes, i.e. the summed lengths of all usable FB
+    /// regions.
+    pub(crate) fn vram_size(&self) -> u64 {
+        self.usable_fb_regions.iter().fold(0, |size, region| {
+            size.saturating_add(region.end - region.start)
+        })
+    }
 }
 
 pub(crate) use fw::commands::PowerStateLevel;
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index c655302a40fd..0f9f4ffa5b90 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -32,6 +32,13 @@ extern "C" {
  */
 #define NOVA_GETPARAM_GPU_CHIPID	0x2
 
+/*
+ * NOVA_GETPARAM_VRAM_SIZE
+ *
+ * Query the total usable VRAM size in bytes.
+ */
+#define NOVA_GETPARAM_VRAM_SIZE	0x3
+
 enum drm_nova_chipid {
 	/* Turing */
 	NOVA_DRM_CHIPID_TU102 = 0x162,
-- 
2.54.0

[PATCH v3 6/7] drm: nova: Use nova-core to read VRAM_BAR_SIZE parameter

[Alistair Popple]

Currently nova-drm reads the VRAM BAR size directly from the PCIe device
which requires trying to cast the parent device into a PCIe device. This
obviously requires the parent device to actually be a PCIe bus device.
Whilst that is true today it may not always be the case, and there
is no reason to make this assumption now that NovaCoreApi can hold a
reference to the bound PCIe device.

So convert nova-drm to using nova-core to obtain the VRAM_BAR_SIZE
parameter.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes from v2:

 - Update nova-core API to use bar1_size() instead of vram_bar_size() as
   this is used internally throughout our chip HW.
---
 drivers/gpu/drm/nova/file.rs    | 10 ++--------
 drivers/gpu/nova-core/api.rs    |  8 ++++++++
 drivers/gpu/nova-core/driver.rs |  2 +-
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 8c02be4f15bf..d65630079237 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -8,14 +8,11 @@
 use crate::gem::NovaObject;
 use kernel::{
     alloc::flags::*,
-    auxiliary,
-    device::Bound,
     drm::{
         self,
         gem::BaseObject,
         Registered, //
     },
-    pci,
     prelude::*,
     uapi,
 };
@@ -33,16 +30,13 @@ fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
 impl File {
     /// IOCTL: get_param: Query GPU / driver metadata.
     pub(crate) fn get_param(
-        dev: &NovaDevice<Registered>,
+        _dev: &NovaDevice<Registered>,
         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 value = match getparam.param as u32 {
-            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
+            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.bar1_size()?,
             uapi::NOVA_GETPARAM_GPU_CHIPID => reg_data.api.chipset() as u64,
             uapi::NOVA_GETPARAM_VRAM_SIZE => reg_data.api.vram_size(),
             _ => return Err(EINVAL),
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index f025df90dc5d..9e2a5ba7c269 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -8,6 +8,7 @@
 use kernel::{
     auxiliary,
     device::Bound,
+    pci,
     prelude::*,
     types::CovariantForLt, //
 };
@@ -20,6 +21,7 @@
 /// API handle for the auxiliary bus child drivers to interact with nova-core.
 pub struct NovaCoreApi<'bound> {
     pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
+    pub(crate) pdev: &'bound pci::Device<Bound>,
 }
 
 impl NovaCoreApi<'_> {
@@ -34,6 +36,12 @@ pub fn chipset(&self) -> Chipset {
         self.gpu.spec.chipset
     }
 
+    /// Returns the size of the PCIe BAR used for accessing VRAM, typically
+    /// BAR1.
+    pub fn bar1_size(&self) -> Result<u64> {
+        self.pdev.resource_len(1)
+    }
+
     /// 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 435aa917d3e7..175275fd7961 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -113,7 +113,7 @@ fn probe<'bound>(
                             // never recycles IDs.
                             AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
                             crate::MODULE_NAME,
-                            NovaCoreApi { gpu },
+                            NovaCoreApi { gpu, pdev },
                         )?
                     }
                 },
-- 
2.54.0

[PATCH v3 7/7] drm: nova: Expose a render node

[Alistair Popple]

nova-drm currently only exposes a primary node even though all of its
ioctls are already marked DRM_RENDER_ALLOW. Set the DRIVER_RENDER
feature so that a render node (/dev/dri/renderDXX) is created as well.

This is required to allow render and compute clients to interact with
nova-drm.

Signed-off-by: Alistair Popple <apopple@nvidia.com>

---

Changes since v2:

 - New for v3.
---
 drivers/gpu/drm/nova/driver.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index f144417b29c9..a9016fcd6518 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -92,6 +92,7 @@ impl drm::Driver for NovaDriver {
     type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
 
     const INFO: drm::DriverInfo = INFO;
+    const FEAT_RENDER: bool = true;
 
     kernel::declare_drm_ioctls! {
         (NOVA_GETPARAM, drm_nova_getparam, ioctl::RENDER_ALLOW, File::get_param),
-- 
2.54.0

Re: [PATCH v3 4/7] drm: nova: Add GETPARAM parameter to read the GPU chipid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Conditional compilation error on 32-bit architectures due to mismatched match arm types.
--

--- Patch [4]: [PATCH v3 4/7] drm: nova: Add GETPARAM parameter to read the GPU chipid ---
commit c560fdc21fe8a1d71109ee27e0b91d35e7283f2a
Author: Alistair Popple <apopple@nvidia.com>

drm: nova: Add GETPARAM parameter to read the GPU chipid

One of the first things a user needs to know about a GPU is its chipid,
so add a GETPARAM parameter to return that.

> diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
> index 1156df51c533a..855f238777349 100644
> --- a/drivers/gpu/drm/nova/file.rs
> +++ b/drivers/gpu/drm/nova/file.rs
[ ... ]
> @@ -43,6 +43,7 @@ pub(crate) fn get_param(
>  
>          let value = match getparam.param as u32 {
>              uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
> +            uapi::NOVA_GETPARAM_GPU_CHIPID => reg_data.api.chipset() as u64,

[Severity: Low]
Does this cause a conditional compilation error on 32-bit architectures?

In File::get_param(), all match arms must evaluate to exactly the same
type. The first arm evaluates to pdev.resource_len(1)? which returns a
bindings::resource_size_t. The newly added second arm evaluates to u64
via the explicit cast.

On 32-bit kernel configurations without CONFIG_PHYS_ADDR_T_64BIT enabled,
resource_size_t resolves to u32. This mismatch between u32 and u64 across
the match arms will result in a hard build error.

>              _ => return Err(EINVAL),
>          };
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723063046.1265791-1-apopple@nvidia.com?part=4

Re: [PATCH v3 1/7] gpu: nova-core: Add public driver API to nova-core

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Forging a `'bound` lifetime for a local field reference inside a `KBox` creates a reference that outlives its referent and violates strict aliasing.
--

--- Patch [1]: [PATCH v3 1/7] gpu: nova-core: Add public driver API to nova-core ---
commit 75bdf4e26c67bf993498d459d03f978972444242
Author: Alistair Popple <apopple@nvidia.com>

gpu: nova-core: Add public driver API to nova-core

Nova core will be used to export core functionality to other drivers
which will bind to it via auxiliary bus devices. Add a NovaCoreApi type
which drivers can use to call nova-core methods.

> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs

[ ... ]

> @@ -86,15 +90,33 @@ 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,
> -                    (),
> -                )?,
> +
> +                _reg: {
> +                    // TODO: Use `&gpu` self-referential pin-init syntax once available.
> +                    //
> +                    // 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).
> +                    let gpu = unsafe {
> +                        Pin::new_unchecked(&*core::ptr::from_ref(gpu.as_ref().get_ref()))
> +                    };

[Severity: High]
Does creating this reference to gpu violate Rust's strict aliasing rules?

Because NovaCore is dynamically allocated in a KBox during device probe,
expanding the lifetime of the local gpu field reference to the longer 'bound
lifetime appears unsafe. The KBox does not strictly outlive 'bound since it
is dropped when the driver unbinds, which could eventually leave child
drivers holding a dangling reference.

Furthermore, when the pin_init macro completes and forms the KBox, the
underlying Box asserts unique ownership over the memory. If the previously
created reference to gpu has escaped into the externally allocated
registration data, it might be invalidated immediately under Stacked Borrows
or Tree Borrows. This could potentially lead to undefined behavior when the
reference is accessed later by the child driver.

> +
> +                    // SAFETY: `NovaCore` is dropped when the device is unbound;
> +                    // i.e. `mem::forget()` is never called on it.
> +                    unsafe {
> +                        auxiliary::Registration::new_with_lt(
> +                            pdev.as_ref(),
> +                            c"nova-drm",

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723063046.1265791-1-apopple@nvidia.com?part=1

Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[Alistair Popple]

On 2026-07-26 at 01:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
> > diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
> > index 3ca90ed9d2bb..cb13fba6b952 100644
> > --- a/include/uapi/drm/nova_drm.h
> > +++ b/include/uapi/drm/nova_drm.h
> > @@ -25,6 +25,39 @@ extern "C" {
> >   */
> >  #define NOVA_GETPARAM_VRAM_BAR_SIZE	0x1
> >  
> > +enum drm_nova_chipid {
> 
> It seems trivial, but let's add at least some documentation.

No, good idea.

> For instance, what's valid for userspace to extract from this? Given chipid is a
> composed value, is userspace e.g. allowed to make assumptions on extracting the
> architecture? Or is this something we want to expose separately and tell
> userspace that the chipid is an opaque value identifying a specific chip only?

I _think_ it makes sense for this to just be an opaque value, but it's a good
question that I should probably try and get some internal feedback on as well.
Reading the arch would then be a separate parameter.

> > +	/* Turing */
> > +	NOVA_DRM_CHIPID_TU102 = 0x162,
> > +	NOVA_DRM_CHIPID_TU104 = 0x164,
> > +	NOVA_DRM_CHIPID_TU106 = 0x166,
> > +	NOVA_DRM_CHIPID_TU117 = 0x167,
> > +	NOVA_DRM_CHIPID_TU116 = 0x168,
> > +	/* Ampere */
> > +	NOVA_DRM_CHIPID_GA100 = 0x170,
> > +	NOVA_DRM_CHIPID_GA102 = 0x172,
> > +	NOVA_DRM_CHIPID_GA103 = 0x173,
> > +	NOVA_DRM_CHIPID_GA104 = 0x174,
> > +	NOVA_DRM_CHIPID_GA106 = 0x176,
> > +	NOVA_DRM_CHIPID_GA107 = 0x177,
> > +	/* Hopper */
> > +	NOVA_DRM_CHIPID_GH100 = 0x180,
> > +	/* Ada */
> > +	NOVA_DRM_CHIPID_AD102 = 0x192,
> > +	NOVA_DRM_CHIPID_AD103 = 0x193,
> > +	NOVA_DRM_CHIPID_AD104 = 0x194,
> > +	NOVA_DRM_CHIPID_AD106 = 0x196,
> > +	NOVA_DRM_CHIPID_AD107 = 0x197,
> > +	/* Blackwell GB10x */
> > +	NOVA_DRM_CHIPID_GB100 = 0x1a0,
> > +	NOVA_DRM_CHIPID_GB102 = 0x1a2,
> > +	/* Blackwell GB20x */
> > +	NOVA_DRM_CHIPID_GB202 = 0x1b2,
> > +	NOVA_DRM_CHIPID_GB203 = 0x1b3,
> > +	NOVA_DRM_CHIPID_GB205 = 0x1b5,
> > +	NOVA_DRM_CHIPID_GB206 = 0x1b6,
> > +	NOVA_DRM_CHIPID_GB207 = 0x1b7,
> > +};

Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[Danilo Krummrich]

On Tue Jul 28, 2026 at 10:00 AM CEST, Alistair Popple wrote:
> On 2026-07-26 at 01:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
>> > diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
>> > index 3ca90ed9d2bb..cb13fba6b952 100644
>> > --- a/include/uapi/drm/nova_drm.h
>> > +++ b/include/uapi/drm/nova_drm.h
>> > @@ -25,6 +25,39 @@ extern "C" {
>> >   */
>> >  #define NOVA_GETPARAM_VRAM_BAR_SIZE	0x1
>> >  
>> > +enum drm_nova_chipid {
>> 
>> It seems trivial, but let's add at least some documentation.
>
> No, good idea.
>
>> For instance, what's valid for userspace to extract from this? Given chipid is a
>> composed value, is userspace e.g. allowed to make assumptions on extracting the
>> architecture? Or is this something we want to expose separately and tell
>> userspace that the chipid is an opaque value identifying a specific chip only?
>
> I _think_ it makes sense for this to just be an opaque value, but it's a good
> question that I should probably try and get some internal feedback on as well.
> Reading the arch would then be a separate parameter.

I'd prefer exposing it as an opaque value and provide other information
separately; the kernel already does the work anyway, so why duplicate it in
userspace. It would also remain much cleaner if encoding details ever change.

>> > +	/* Turing */
>> > +	NOVA_DRM_CHIPID_TU102 = 0x162,
>> > +	NOVA_DRM_CHIPID_TU104 = 0x164,
>> > +	NOVA_DRM_CHIPID_TU106 = 0x166,
>> > +	NOVA_DRM_CHIPID_TU117 = 0x167,
>> > +	NOVA_DRM_CHIPID_TU116 = 0x168,
>> > +	/* Ampere */
>> > +	NOVA_DRM_CHIPID_GA100 = 0x170,
>> > +	NOVA_DRM_CHIPID_GA102 = 0x172,
>> > +	NOVA_DRM_CHIPID_GA103 = 0x173,
>> > +	NOVA_DRM_CHIPID_GA104 = 0x174,
>> > +	NOVA_DRM_CHIPID_GA106 = 0x176,
>> > +	NOVA_DRM_CHIPID_GA107 = 0x177,
>> > +	/* Hopper */
>> > +	NOVA_DRM_CHIPID_GH100 = 0x180,
>> > +	/* Ada */
>> > +	NOVA_DRM_CHIPID_AD102 = 0x192,
>> > +	NOVA_DRM_CHIPID_AD103 = 0x193,
>> > +	NOVA_DRM_CHIPID_AD104 = 0x194,
>> > +	NOVA_DRM_CHIPID_AD106 = 0x196,
>> > +	NOVA_DRM_CHIPID_AD107 = 0x197,
>> > +	/* Blackwell GB10x */
>> > +	NOVA_DRM_CHIPID_GB100 = 0x1a0,
>> > +	NOVA_DRM_CHIPID_GB102 = 0x1a2,
>> > +	/* Blackwell GB20x */
>> > +	NOVA_DRM_CHIPID_GB202 = 0x1b2,
>> > +	NOVA_DRM_CHIPID_GB203 = 0x1b3,
>> > +	NOVA_DRM_CHIPID_GB205 = 0x1b5,
>> > +	NOVA_DRM_CHIPID_GB206 = 0x1b6,
>> > +	NOVA_DRM_CHIPID_GB207 = 0x1b7,
>> > +};

Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[John Hubbard]

On 7/28/26 2:29 PM, Danilo Krummrich wrote:
> On Tue Jul 28, 2026 at 10:00 AM CEST, Alistair Popple wrote:
>> On 2026-07-26 at 01:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>>> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
...
>>> For instance, what's valid for userspace to extract from this? Given chipid is a
>>> composed value, is userspace e.g. allowed to make assumptions on extracting the
>>> architecture? Or is this something we want to expose separately and tell
>>> userspace that the chipid is an opaque value identifying a specific chip only?
>>
>> I _think_ it makes sense for this to just be an opaque value, but it's a good
>> question that I should probably try and get some internal feedback on as well.
>> Reading the arch would then be a separate parameter.
> 
> I'd prefer exposing it as an opaque value and provide other information
> separately; the kernel already does the work anyway, so why duplicate it in
> userspace. It would also remain much cleaner if encoding details ever change.
> 

Just to orient myself, I'm assuming that by "opaque values", you mean using
something that just counts up from, say, 1, instead of using real boot42
values below, right?

That seems like a good approach to me, too. Because I can't imagine anything
particularly good coming from providing unnecessary details to user space
here. :)

>>>> +	/* Turing */
>>>> +	NOVA_DRM_CHIPID_TU102 = 0x162,
>>>> +	NOVA_DRM_CHIPID_TU104 = 0x164,
>>>> +	NOVA_DRM_CHIPID_TU106 = 0x166,
>>>> +	NOVA_DRM_CHIPID_TU117 = 0x167,
>>>> +	NOVA_DRM_CHIPID_TU116 = 0x168,
>>>> +	/* Ampere */
>>>> +	NOVA_DRM_CHIPID_GA100 = 0x170,
>>>> +	NOVA_DRM_CHIPID_GA102 = 0x172,
>>>> +	NOVA_DRM_CHIPID_GA103 = 0x173,
>>>> +	NOVA_DRM_CHIPID_GA104 = 0x174,
>>>> +	NOVA_DRM_CHIPID_GA106 = 0x176,
>>>> +	NOVA_DRM_CHIPID_GA107 = 0x177,
>>>> +	/* Hopper */
>>>> +	NOVA_DRM_CHIPID_GH100 = 0x180,
>>>> +	/* Ada */
>>>> +	NOVA_DRM_CHIPID_AD102 = 0x192,
>>>> +	NOVA_DRM_CHIPID_AD103 = 0x193,
>>>> +	NOVA_DRM_CHIPID_AD104 = 0x194,
>>>> +	NOVA_DRM_CHIPID_AD106 = 0x196,
>>>> +	NOVA_DRM_CHIPID_AD107 = 0x197,
>>>> +	/* Blackwell GB10x */
>>>> +	NOVA_DRM_CHIPID_GB100 = 0x1a0,
>>>> +	NOVA_DRM_CHIPID_GB102 = 0x1a2,
>>>> +	/* Blackwell GB20x */
>>>> +	NOVA_DRM_CHIPID_GB202 = 0x1b2,
>>>> +	NOVA_DRM_CHIPID_GB203 = 0x1b3,
>>>> +	NOVA_DRM_CHIPID_GB205 = 0x1b5,
>>>> +	NOVA_DRM_CHIPID_GB206 = 0x1b6,
>>>> +	NOVA_DRM_CHIPID_GB207 = 0x1b7,
>>>> +};
> 

thanks,
-- 
John Hubbard

Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[Danilo Krummrich]

On Tue Jul 28, 2026 at 11:49 PM CEST, John Hubbard wrote:
> On 7/28/26 2:29 PM, Danilo Krummrich wrote:
>> On Tue Jul 28, 2026 at 10:00 AM CEST, Alistair Popple wrote:
>>> On 2026-07-26 at 01:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>>>> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
> ...
>>>> For instance, what's valid for userspace to extract from this? Given chipid is a
>>>> composed value, is userspace e.g. allowed to make assumptions on extracting the
>>>> architecture? Or is this something we want to expose separately and tell
>>>> userspace that the chipid is an opaque value identifying a specific chip only?
>>>
>>> I _think_ it makes sense for this to just be an opaque value, but it's a good
>>> question that I should probably try and get some internal feedback on as well.
>>> Reading the arch would then be a separate parameter.
>> 
>> I'd prefer exposing it as an opaque value and provide other information
>> separately; the kernel already does the work anyway, so why duplicate it in
>> userspace. It would also remain much cleaner if encoding details ever change.
>> 
>
> Just to orient myself, I'm assuming that by "opaque values", you mean using
> something that just counts up from, say, 1, instead of using real boot42
> values below, right?
>
> That seems like a good approach to me, too. Because I can't imagine anything
> particularly good coming from providing unnecessary details to user space
> here. :)

I'd probably not be too worried about keeping it as is, but just document it as
being an opaque value that shouldn't be messed with.

If we expose the encoded information separately I don't see a huge incentive for
userspace to redo the work and mess with it anyway.

The advantage of keeping the values as they are is that we avoid having to deal
with mapping values back and forth. I kind of like that nova-core uses the
values from the uAPI header to define the Chipset struct, passes it to nova-drm
and nova-drm does use it to fill in the userspace exposed data.

Something else worth to consider in this context is whether we really want to go
with the key-value pair GETPARAM approach, which can become a bit tedious.

I think for the basic GPU information we could have a struct drm_nova_gpu_info,
which contains all the relevant information obtained at probe time, such as
chipid, architecture, VRAM size, etc.

If everything is available in a single struct there should be no reason for
userspace to mess with the chipid value. However, I'd also not object to make it
a "real" opaque value.

In order to deal with additions to the struct we could take a two stage approach
to obtain the information from userspace; probe size first, then allocate with
the returned size:

We can have a DRM_NOVA_INFO ioctl, which takes a struct containing the info
identifier (e.g. DRM_NOVA_GPU_INFO), a size and a pointer to the actual info
structure.

	struct drm_nova_info {
		__u32 id;
		__u32 size;
		__u64 info;
	};

Userspace can call the DRM_NOVA_INFO with the identifier (e.g.
DRM_NOVA_GPU_INFO), size and info pointer set to zero, and the kernel fills in
the size of the info struct (e.g. struct drm_nova_gpu_info).

Subsequently, userspace can allocate memory with the size reported by the kernel
and call the same ioctl with the info pointer set to the allocated memory.

This way existing info structs remain extensible (as long as the layout isn't
changed of course) and new ones can be added at any time.

Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[Alistair Popple]

On 2026-07-29 at 08:57 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Tue Jul 28, 2026 at 11:49 PM CEST, John Hubbard wrote:
> > On 7/28/26 2:29 PM, Danilo Krummrich wrote:
> >> On Tue Jul 28, 2026 at 10:00 AM CEST, Alistair Popple wrote:
> >>> On 2026-07-26 at 01:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> >>>> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
> > ...
> >>>> For instance, what's valid for userspace to extract from this? Given chipid is a
> >>>> composed value, is userspace e.g. allowed to make assumptions on extracting the
> >>>> architecture? Or is this something we want to expose separately and tell
> >>>> userspace that the chipid is an opaque value identifying a specific chip only?
> >>>
> >>> I _think_ it makes sense for this to just be an opaque value, but it's a good
> >>> question that I should probably try and get some internal feedback on as well.
> >>> Reading the arch would then be a separate parameter.
> >> 
> >> I'd prefer exposing it as an opaque value and provide other information
> >> separately; the kernel already does the work anyway, so why duplicate it in
> >> userspace. It would also remain much cleaner if encoding details ever change.

Yep, I agree. Just want to make sure we get everyone in agreement.

> >
> > Just to orient myself, I'm assuming that by "opaque values", you mean using
> > something that just counts up from, say, 1, instead of using real boot42
> > values below, right?

No. We'd just document these as a unique magic value that identifies the GPU
chip rather than something that could be decoded into anything else.

> > That seems like a good approach to me, too. Because I can't imagine anything
> > particularly good coming from providing unnecessary details to user space
> > here. :)
> 
> I'd probably not be too worried about keeping it as is, but just document it as
> being an opaque value that shouldn't be messed with.

Yes, agree with this. I don't think we want to mess with the value. It just
makes life hard when a new chip is added thats only difference from the nova-drm
perspective is a new chipid. You don't want to have to worry about backporting
patches to distros and kernels just to add the identifier for say TU117 even
though it's functionally the same as TU116 from a Nova perspective.

Obviously chipid would still get added to the header and eventually backported,
but having been there and done that it's not a good user story to require it.

> If we expose the encoded information separately I don't see a huge incentive for
> userspace to redo the work and mess with it anyway.
> 
> The advantage of keeping the values as they are is that we avoid having to deal
> with mapping values back and forth. I kind of like that nova-core uses the
> values from the uAPI header to define the Chipset struct, passes it to nova-drm
> and nova-drm does use it to fill in the userspace exposed data.

So do I.

> Something else worth to consider in this context is whether we really want to go
> with the key-value pair GETPARAM approach, which can become a bit tedious.
> 
> I think for the basic GPU information we could have a struct drm_nova_gpu_info,
> which contains all the relevant information obtained at probe time, such as
> chipid, architecture, VRAM size, etc.
> 
> If everything is available in a single struct there should be no reason for
> userspace to mess with the chipid value. However, I'd also not object to make it
> a "real" opaque value.
> 
> In order to deal with additions to the struct we could take a two stage approach
> to obtain the information from userspace; probe size first, then allocate with
> the returned size:
> 
> We can have a DRM_NOVA_INFO ioctl, which takes a struct containing the info
> identifier (e.g. DRM_NOVA_GPU_INFO), a size and a pointer to the actual info
> structure.
> 
> 	struct drm_nova_info {
> 		__u32 id;
> 		__u32 size;
> 		__u64 info;
> 	};
> 
> Userspace can call the DRM_NOVA_INFO with the identifier (e.g.
> DRM_NOVA_GPU_INFO), size and info pointer set to zero, and the kernel fills in
> the size of the info struct (e.g. struct drm_nova_gpu_info).
> 
> Subsequently, userspace can allocate memory with the size reported by the kernel
> and call the same ioctl with the info pointer set to the allocated memory.
> 
> This way existing info structs remain extensible (as long as the layout isn't
> changed of course) and new ones can be added at any time.

I did consider an approach like this but having to do the two ioctl memory
allocation dance doesn't seem any less tedious. Also my experience is these
structs tend to just grow over time and become unwieldy as new bits of info get
added and old ones deprecated forcing userspace to allocate memory for things it
might no longer care about.

Ultimately though I'm thinking (hoping?) there will not be a huge number of
these - a lot of this info should probably get exposed via sysfs anyway.

Re: [PATCH v3 3/7] drm: nova: Add chipid enum to nova-drm UAPI

[Danilo Krummrich]

On Wed Jul 29, 2026 at 10:24 AM CEST, Alistair Popple wrote:
> On 2026-07-29 at 08:57 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>> On Tue Jul 28, 2026 at 11:49 PM CEST, John Hubbard wrote:
>> > On 7/28/26 2:29 PM, Danilo Krummrich wrote:
>> >> On Tue Jul 28, 2026 at 10:00 AM CEST, Alistair Popple wrote:
>> >>> On 2026-07-26 at 01:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>> >>>> On Thu Jul 23, 2026 at 8:30 AM CEST, Alistair Popple wrote:
>> > ...
>> >>>> For instance, what's valid for userspace to extract from this? Given chipid is a
>> >>>> composed value, is userspace e.g. allowed to make assumptions on extracting the
>> >>>> architecture? Or is this something we want to expose separately and tell
>> >>>> userspace that the chipid is an opaque value identifying a specific chip only?
>> >>>
>> >>> I _think_ it makes sense for this to just be an opaque value, but it's a good
>> >>> question that I should probably try and get some internal feedback on as well.
>> >>> Reading the arch would then be a separate parameter.
>> >> 
>> >> I'd prefer exposing it as an opaque value and provide other information
>> >> separately; the kernel already does the work anyway, so why duplicate it in
>> >> userspace. It would also remain much cleaner if encoding details ever change.
>
> Yep, I agree. Just want to make sure we get everyone in agreement.
>
>> >
>> > Just to orient myself, I'm assuming that by "opaque values", you mean using
>> > something that just counts up from, say, 1, instead of using real boot42
>> > values below, right?
>
> No. We'd just document these as a unique magic value that identifies the GPU
> chip rather than something that could be decoded into anything else.
>
>> > That seems like a good approach to me, too. Because I can't imagine anything
>> > particularly good coming from providing unnecessary details to user space
>> > here. :)
>> 
>> I'd probably not be too worried about keeping it as is, but just document it as
>> being an opaque value that shouldn't be messed with.
>
> Yes, agree with this. I don't think we want to mess with the value. It just
> makes life hard when a new chip is added thats only difference from the nova-drm
> perspective is a new chipid. You don't want to have to worry about backporting
> patches to distros and kernels just to add the identifier for say TU117 even
> though it's functionally the same as TU116 from a Nova perspective.
>
> Obviously chipid would still get added to the header and eventually backported,
> but having been there and done that it's not a good user story to require it.
>
>> If we expose the encoded information separately I don't see a huge incentive for
>> userspace to redo the work and mess with it anyway.
>> 
>> The advantage of keeping the values as they are is that we avoid having to deal
>> with mapping values back and forth. I kind of like that nova-core uses the
>> values from the uAPI header to define the Chipset struct, passes it to nova-drm
>> and nova-drm does use it to fill in the userspace exposed data.
>
> So do I.
>
>> Something else worth to consider in this context is whether we really want to go
>> with the key-value pair GETPARAM approach, which can become a bit tedious.
>> 
>> I think for the basic GPU information we could have a struct drm_nova_gpu_info,
>> which contains all the relevant information obtained at probe time, such as
>> chipid, architecture, VRAM size, etc.
>> 
>> If everything is available in a single struct there should be no reason for
>> userspace to mess with the chipid value. However, I'd also not object to make it
>> a "real" opaque value.
>> 
>> In order to deal with additions to the struct we could take a two stage approach
>> to obtain the information from userspace; probe size first, then allocate with
>> the returned size:
>> 
>> We can have a DRM_NOVA_INFO ioctl, which takes a struct containing the info
>> identifier (e.g. DRM_NOVA_GPU_INFO), a size and a pointer to the actual info
>> structure.
>> 
>> 	struct drm_nova_info {
>> 		__u32 id;
>> 		__u32 size;
>> 		__u64 info;
>> 	};
>> 
>> Userspace can call the DRM_NOVA_INFO with the identifier (e.g.
>> DRM_NOVA_GPU_INFO), size and info pointer set to zero, and the kernel fills in
>> the size of the info struct (e.g. struct drm_nova_gpu_info).
>> 
>> Subsequently, userspace can allocate memory with the size reported by the kernel
>> and call the same ioctl with the info pointer set to the allocated memory.
>> 
>> This way existing info structs remain extensible (as long as the layout isn't
>> changed of course) and new ones can be added at any time.
>
> I did consider an approach like this but having to do the two ioctl memory
> allocation dance doesn't seem any less tedious.

Well, it is two ioctls for a structured set of properties, whereas GETPARAM is N
ioctls with no structure and slightly less type information.

> Also my experience is these structs tend to just grow over time and become
> unwieldy as new bits of info get added

Note that there's no need to grow existing info groups endlessly; if something
truly becomes a separate concern we can just add a new group, e.g.
DRM_NOVA_MEMORY_INFO.

> and old ones deprecated forcing userspace to allocate memory for things it
> might no longer care about.

This is equally true of GETPARAM; we accumulate keys that nobody uses anymore.

Allocating for one or two deprecated fields for a single query doesn't seem like
a big deal; plus we can always create a new info group and leave the old one
alone, just like a deprecated GETPARAM key.

> Ultimately though I'm thinking (hoping?) there will not be a huge number of
> these - a lot of this info should probably get exposed via sysfs anyway.

I don't think sysfs is a good fit; it seems tedious for a UMD to do a bunch of
sysfs file reads, each of them requiring at least three syscalls. Plus the
effort to parse the value from the string sysfs provides, which is more error
prone than reading a typed struct.