[PATCH v5 00/11] gpu: nova: Export parameters from nova-core to nova-drm

[PATCH v5 00/11] gpu: nova: Export parameters from nova-core to nova-drm

[Alistair Popple]

This patch series adds some basic GPU properties via a new GPU info
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]

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.

A new info ioctl is introduced which provides an info type field and a
method for reading extendable structs containing GPU information. This
series can be tested using the drm-test[4]. A pull request containing
updated tests will be raised once this has been posted. A pull request for
Mesa has also been raised but is out of date. That will be updated once
this series has been merged.

Changes since v4:

 - No longer export chip-id, instead export architecture and implementation
 - Add a separate info ioctl with types to read GPU info as suggested by
   Danilo

Changes since v3:

 - Use an ioctl to return all parameters rather than multiple key/value
   queries

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
[4] - https://gitlab.freedesktop.org/dakr/drm-test

Cc: M Henning <mhenning@darkrefraction.com>
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 (11):
  gpu: nova-core: Add public driver API to nova-core
  drm: nova: Add DRM registration data
  drm: nova: Add GPU architecture enum to nova-drm UAPI
  rust: uaccess: add UserSliceWriter::write_truncated()
  drm: nova: Add an info ioctl
  drm: nova: Add usable VRAM size to GPU info
  drm: nova: Use nova-core to read VRAM_BAR_SIZE parameter
  drm: nova: Expose a render node
  drm: nova: Report GPU name in GPU info
  drm: nova: Report GPU short name in GPU info
  drm: nova: Report GPU GID in GPU info

 drivers/gpu/drm/nova/driver.rs           | 18 ++++-
 drivers/gpu/drm/nova/file.rs             | 83 ++++++++++++++++++++----
 drivers/gpu/nova-core/api.rs             | 68 +++++++++++++++++++
 drivers/gpu/nova-core/driver.rs          | 46 +++++++++----
 drivers/gpu/nova-core/gpu.rs             | 43 +++++++-----
 drivers/gpu/nova-core/gsp/commands.rs    | 27 ++++++++
 drivers/gpu/nova-core/gsp/fw/commands.rs | 12 ++++
 drivers/gpu/nova-core/gsp/hal.rs         |  2 +-
 drivers/gpu/nova-core/nova_core.rs       |  1 +
 drivers/gpu/nova-core/num.rs             |  2 +-
 include/uapi/drm/nova_drm.h              | 82 +++++++++++++++++++++++
 rust/kernel/uaccess.rs                   | 14 ++++
 12 files changed, 352 insertions(+), 46 deletions(-)
 create mode 100644 drivers/gpu/nova-core/api.rs

-- 
2.54.0

[PATCH v5 01/11] 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 v4:

 - Move the `_reg` field declaration, since it must be dropped before
   `gpu` to satisfy the safety requirements of holding a reference to it.

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    | 46 +++++++++++++++++++++---------
 drivers/gpu/nova-core/gsp/hal.rs   |  2 +-
 drivers/gpu/nova-core/nova_core.rs |  1 +
 4 files changed, 64 insertions(+), 14 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..078a3371a7d6 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -18,18 +18,21 @@
     types::CovariantForLt,
 };
 
-use crate::gpu::Gpu;
+use crate::{
+    api::NovaCoreApi,
+    gpu::Gpu, //
+};
 
 /// Counter for generating unique auxiliary device IDs.
 static AUXILIARY_ID_COUNTER: Atomic<u32> = Atomic::new(0);
 
 #[pin_data]
 pub(crate) struct NovaCore<'bound> {
+    #[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;
@@ -83,18 +86,35 @@ fn probe<'bound>(
                 // TODO: Use `&bar` self-referential pin-init syntax once available.
                 //
                 // SAFETY: `bar` is initialized before this expression is evaluated
-                // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
+                // (`try_pin_init!()` initializes fields in initializer 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 initializer 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 5850fa0fe0e9..0c8670c57bbb 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -24,7 +24,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 35a8b1214b0e..503898cee042 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 v5 02/11] 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 v5 03/11] drm: nova: Add GPU architecture enum to nova-drm UAPI

[Alistair Popple]

The GPU architecture to be exposed to user-space. This adds a public
enum to the userspace headers for each chip architecture. Nova-core can
then use this enum to define its architectures.

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.

It also requires a minor change to the bounded_enum! macro to match the
architecture values in an expression context.

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

---

Changes since v4:

 - Rewritten for v5 as exposing chip-id was dropped.

Changes since v3:

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

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9e4232645a7e..0c12ef145981 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::{
@@ -36,8 +37,9 @@
 mod regs;
 
 macro_rules! define_chipset {
-    ({ $($variant:ident = $value:expr),* $(,)* }) =>
+    ({ $($variant:ident = $value:literal),* $(,)* }) =>
     {
+        ::kernel::macros::paste!(
         /// Enum representation of the GPU chipset.
         #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
         pub(crate) enum Chipset {
@@ -49,7 +51,6 @@ impl Chipset {
                 $( Chipset::$variant, )*
             ];
 
-            ::kernel::macros::paste!(
             /// Returns the name of this chipset, in lowercase.
             ///
             /// # Examples
@@ -65,7 +66,6 @@ pub(crate) const fn name(&self) -> &'static str {
                 )*
                 }
             }
-            );
         }
 
         // TODO[FPRI]: replace with something like derive(FromPrimitive)
@@ -74,11 +74,14 @@ impl TryFrom<u32> for Chipset {
 
             fn try_from(value: u32) -> Result<Self, Self::Error> {
                 match value {
-                    $( $value => Ok(Chipset::$variant), )*
+                    $(
+                        $value => Ok(Chipset::$variant),
+                    )*
                     _ => Err(ENODEV),
                 }
             }
         }
+    );
     }
 }
 
@@ -158,13 +161,16 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 bounded_enum! {
     /// Enum representation of the GPU generation.
     #[derive(fmt::Debug, Copy, Clone)]
+    #[repr(u32)]
     pub(crate) enum Architecture with TryFrom<Bounded<u32, 6>> {
-        Turing = 0x16,
-        Ampere = 0x17,
-        Hopper = 0x18,
-        Ada = 0x19,
-        BlackwellGB10x = 0x1a,
-        BlackwellGB20x = 0x1b,
+        Turing = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_TURING,
+        Ampere = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_AMPERE,
+        Hopper = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_HOPPER,
+        Ada = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_ADA,
+        BlackwellGB10x =
+            uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_BLACKWELL_GB10X,
+        BlackwellGB20x =
+            uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_BLACKWELL_GB20X,
     }
 }
 
diff --git a/drivers/gpu/nova-core/num.rs b/drivers/gpu/nova-core/num.rs
index 6eb174d136ab..f4169235bc24 100644
--- a/drivers/gpu/nova-core/num.rs
+++ b/drivers/gpu/nova-core/num.rs
@@ -263,7 +263,7 @@ fn try_from(
             ) -> kernel::error::Result<Self> {
                 match value.get() {
                     $(
-                        $value => Ok($enum_type::$variant),
+                        value if value == $value => Ok($enum_type::$variant),
                     )*
                     _ => Err(kernel::error::code::EINVAL),
                 }
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index 3ca90ed9d2bb..f0dcbca1908d 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -25,6 +25,18 @@ extern "C" {
  */
 #define NOVA_GETPARAM_VRAM_BAR_SIZE	0x1
 
+/**
+ * enum drm_nova_architecture - GPU architecture identifier
+ */
+enum drm_nova_architecture {
+	NOVA_DRM_ARCHITECTURE_TURING		= 0x16,
+	NOVA_DRM_ARCHITECTURE_AMPERE		= 0x17,
+	NOVA_DRM_ARCHITECTURE_HOPPER		= 0x18,
+	NOVA_DRM_ARCHITECTURE_ADA		= 0x19,
+	NOVA_DRM_ARCHITECTURE_BLACKWELL_GB10X	= 0x1a,
+	NOVA_DRM_ARCHITECTURE_BLACKWELL_GB20X	= 0x1b,
+};
+
 /**
  * struct drm_nova_getparam - query GPU and driver metadata
  */
-- 
2.54.0

[PATCH v5 04/11] rust: uaccess: add UserSliceWriter::write_truncated()

[Alistair Popple]

Add a helper that writes as much of an AsBytes value as fits in the
remaining userspace buffer and returns the number of bytes copied. This
avoids requiring callers of versioned UAPIs to convert values to byte
slices and truncate them manually.

Signed-off-by: Alistair Popple <apopple@nvidia.com>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/uaccess.rs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 5f6c4d7a1a51..c63b91942631 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -624,6 +624,20 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result {
         self.length -= len;
         Ok(())
     }
+
+    /// Writes as much of the provided value as fits in the remaining buffer.
+    ///
+    /// Copies `min(size_of::<T>(), self.len())` bytes to userspace. Returns the number of bytes
+    /// actually written. This is useful for versioned structs where an older userspace may provide
+    /// a smaller buffer than the current kernel struct.
+    ///
+    /// Fails with [`EFAULT`] if the write happens on a bad address. This call may modify the
+    /// associated userspace slice even if it returns an error.
+    pub fn write_truncated<T: AsBytes>(&mut self, value: &T) -> Result<usize> {
+        let len = self.length.min(size_of::<T>());
+        self.write_slice(&value.as_bytes()[..len])?;
+        Ok(len)
+    }
 }
 
 /// Reads a nul-terminated string into `dst` and returns the length.
-- 
2.54.0

[PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

Add an extensible info ioctl and use it to report basic GPU information.
One of the first things userspace needs to know about a GPU is its
architecture and implementation, so add that as the first field in the
GPU info result.

The ioctl selects an information type by ID and writes the result through
a sized userspace buffer. The kernel truncates results to the supplied
size, allowing information structures to grow and new logical information
groups to be added without introducing new ioctls.

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

---

Changes since v4:

 - Add a new info ioctl interface as suggested by Danilo

Changes since v3:

 - New for v4 - chipid was previously returned as a GETPARAM parameter
---
 drivers/gpu/drm/nova/driver.rs |  2 +-
 drivers/gpu/drm/nova/file.rs   | 57 ++++++++++++++++++++++++++++++++++
 drivers/gpu/nova-core/api.rs   | 15 +++++++--
 drivers/gpu/nova-core/gpu.rs   |  9 ++++--
 include/uapi/drm/nova_drm.h    | 49 +++++++++++++++++++++++++++++
 5 files changed, 127 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 632137d1c6d7..38f82a7e1738 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>>,
 }
 
@@ -98,5 +97,6 @@ impl drm::Driver for NovaDriver {
         (NOVA_GETPARAM, drm_nova_getparam, ioctl::RENDER_ALLOW, File::get_param),
         (NOVA_GEM_CREATE, drm_nova_gem_create, ioctl::AUTH | ioctl::RENDER_ALLOW, File::gem_create),
         (NOVA_GEM_INFO, drm_nova_gem_info, ioctl::AUTH | ioctl::RENDER_ALLOW, File::gem_info),
+        (NOVA_INFO, drm_nova_info, ioctl::RENDER_ALLOW, File::info),
     }
 }
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 1156df51c533..097bf607485c 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -17,11 +17,45 @@
     },
     pci,
     prelude::*,
+    transmute::AsBytes,
+    uaccess::UserSlice,
     uapi,
 };
 
 pub(crate) struct File;
 
+/// GPU information returned to userspace.
+///
+/// # Invariants
+///
+/// - The layout of this type is identical to `struct drm_nova_gpu_info`.
+/// - All bytes in the value are initialized.
+#[repr(transparent)]
+struct GpuInfo(uapi::drm_nova_gpu_info);
+
+impl GpuInfo {
+    fn new(reg_data: &DrmRegData<'_>) -> Self {
+        Self(uapi::drm_nova_gpu_info {
+            architecture: reg_data.api.architecture(),
+            implementation: reg_data.api.implementation(),
+        })
+    }
+}
+
+// SAFETY: `GpuInfo` has no implicit padding, kernel pointers, or interior
+// mutability, and all of its fields are initialized before it is written to
+// userspace.
+unsafe impl AsBytes for GpuInfo {}
+
+fn write_info<T: AsBytes>(info: &mut uapi::drm_nova_info, value: &T) -> Result {
+    let mut writer =
+        UserSlice::new(UserPtr::from_addr(info.data as usize), info.size as usize).writer();
+
+    info.size = writer.write_truncated(value)? as u32;
+
+    Ok(())
+}
+
 impl drm::file::DriverFile for File {
     type Driver = NovaDriver;
 
@@ -78,4 +112,27 @@ pub(crate) fn gem_info(
 
         Ok(0)
     }
+
+    /// IOCTL: info: Query device information.
+    pub(crate) fn info(
+        _dev: &NovaDevice<Registered>,
+        reg_data: &DrmRegData<'_>,
+        info: &mut uapi::drm_nova_info,
+        _file: &drm::File<File>,
+    ) -> Result<u32> {
+        if info.data == 0 {
+            info.size = match info.id {
+                uapi::DRM_NOVA_INFO_GPU => size_of::<GpuInfo>() as u32,
+                _ => return Err(EINVAL),
+            };
+            return Ok(0);
+        }
+
+        match info.id {
+            uapi::DRM_NOVA_INFO_GPU => write_info(info, &GpuInfo::new(reg_data))?,
+            _ => return Err(EINVAL),
+        }
+
+        Ok(0)
+    }
 }
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index 610cfc01111e..cff730a38c1d 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -12,11 +12,12 @@
     types::CovariantForLt, //
 };
 
-use crate::gpu::Gpu;
+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>>,
 }
 
@@ -26,4 +27,14 @@ impl NovaCoreApi<'_> {
     pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
         adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
     }
+
+    /// Returns the architecture identifier of this GPU.
+    pub fn architecture(&self) -> u32 {
+        self.gpu.spec.chipset.arch() as u32
+    }
+
+    /// Returns the implementation identifier of this GPU.
+    pub fn implementation(&self) -> u32 {
+        self.gpu.spec.chipset.implementation()
+    }
 }
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 0c12ef145981..740466af268d 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -138,6 +138,11 @@ pub(crate) const fn arch(self) -> Architecture {
         }
     }
 
+    /// Returns the implementation identifier of this chipset.
+    pub(crate) const fn implementation(self) -> u32 {
+        self as u32 & 0xf
+    }
+
     /// Returns the address range of the PCI config mirror space.
     pub(crate) fn pci_config_mirror_range(self) -> Range<u32> {
         hal::gpu_hal(self).pci_config_mirror_range()
@@ -198,7 +203,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,
 }
 
@@ -286,7 +291,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 f0dcbca1908d..300285b520a3 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -92,9 +92,56 @@ struct drm_nova_gem_info {
 	__u64 size;
 };
 
+/**
+ * struct drm_nova_info - query device information
+ */
+struct drm_nova_info {
+	/**
+	 * @id: The identifier of the information to query.
+	 */
+	__u32 id;
+
+	/**
+	 * @size: The amount of space allocated by userspace at @data. The kernel
+	 * will return the number of bytes it wrote.
+	 */
+	__u32 size;
+
+	/**
+	 * @data: Pointer to the userspace buffer into which the queried
+	 * information will be written.
+	 */
+	__u64 data;
+};
+
+/**
+ * DRM_NOVA_INFO_GPU
+ *
+ * Query GPU information. The result is returned in a
+ * &struct drm_nova_gpu_info.
+ */
+#define DRM_NOVA_INFO_GPU		0x00
+
+/**
+ * struct drm_nova_gpu_info - GPU information
+ */
+struct drm_nova_gpu_info {
+	/**
+	 * @architecture: GPU architecture identifier. See
+	 * &enum drm_nova_architecture for currently known architectures.
+	 */
+	__u32 architecture;
+
+	/**
+	 * @implementation: GPU implementation identifier.
+	 */
+	__u32 implementation;
+};
+
 #define DRM_NOVA_GETPARAM		0x00
 #define DRM_NOVA_GEM_CREATE		0x01
 #define DRM_NOVA_GEM_INFO		0x02
+#define DRM_NOVA_INFO			0x03
 
 /* Note: this is an enum so that it can be resolved by Rust bindgen. */
 enum {
@@ -104,6 +151,8 @@ enum {
 						   struct drm_nova_gem_create),
 	DRM_IOCTL_NOVA_GEM_INFO		= DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_GEM_INFO,
 						   struct drm_nova_gem_info),
+	DRM_IOCTL_NOVA_INFO		= DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_INFO,
+						   struct drm_nova_info),
 };
 
 #if defined(__cplusplus)
-- 
2.54.0

[PATCH v5 06/11] drm: nova: Add usable VRAM size to GPU info

[Alistair Popple]

Add a field to the GPU info struct containing the total usable
framebuffer size. The usable framebuffer excludes GSP carveouts and
other protected regions, so it may differ from the BAR size and total
physical VRAM.

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

---

Changes since v3:

 - Partially new for v4 - previously a separate scalar GETPARAM
   parameter
---
 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           | 6 ++++++
 5 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 097bf607485c..cd80c5402eba 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -38,6 +38,7 @@ fn new(reg_data: &DrmRegData<'_>) -> Self {
         Self(uapi::drm_nova_gpu_info {
             architecture: reg_data.api.architecture(),
             implementation: reg_data.api.implementation(),
+            vram_size: reg_data.api.vram_size(),
         })
     }
 }
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index cff730a38c1d..1e73d27d701c 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -37,4 +37,9 @@ pub fn architecture(&self) -> u32 {
     pub fn implementation(&self) -> u32 {
         self.gpu.spec.chipset.implementation()
     }
+
+    /// 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 740466af268d..3e074541af30 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -293,7 +293,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>,
@@ -414,9 +414,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 ffc25fd8c47b..6453184af55b 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -261,6 +261,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 300285b520a3..b80bb50acaa1 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -136,6 +136,12 @@ struct drm_nova_gpu_info {
 	 * @implementation: GPU implementation identifier.
 	 */
 	__u32 implementation;
+
+	/**
+	 * @vram_size: Amount of usable FB, excluding GSP carveouts and protected
+	 * regions.
+	 */
+	__u64 vram_size;
 };
 
 #define DRM_NOVA_GETPARAM		0x00
-- 
2.54.0

[PATCH v5 07/11] 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    | 12 +++---------
 drivers/gpu/nova-core/api.rs    |  8 ++++++++
 drivers/gpu/nova-core/driver.rs |  2 +-
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index cd80c5402eba..6fc169c66d02 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::*,
     transmute::AsBytes,
     uaccess::UserSlice,
@@ -68,16 +65,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>,
-        _reg_data: &DrmRegData<'_>,
+        _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()?,
             _ => return Err(EINVAL),
         };
 
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index 1e73d27d701c..f4fd78f9c321 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, //
 };
@@ -19,6 +20,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<'_> {
@@ -38,6 +40,12 @@ pub fn implementation(&self) -> u32 {
         self.gpu.spec.chipset.implementation()
     }
 
+    /// 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 078a3371a7d6..e0e84f5f9c5c 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -111,7 +111,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 v5 08/11] 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 38f82a7e1738..296fc3c49806 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

[PATCH v5 09/11] drm: nova: Report GPU name in GPU info

[Alistair Popple]

Add the full GPU name to the GPU info structure.

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

---

Changes since v4:

 - New for v5
---
 drivers/gpu/drm/nova/file.rs          | 1 +
 drivers/gpu/nova-core/api.rs          | 5 +++++
 drivers/gpu/nova-core/gsp/commands.rs | 5 +++++
 include/uapi/drm/nova_drm.h           | 5 +++++
 4 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 6fc169c66d02..ab04a10d3353 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -36,6 +36,7 @@ fn new(reg_data: &DrmRegData<'_>) -> Self {
             architecture: reg_data.api.architecture(),
             implementation: reg_data.api.implementation(),
             vram_size: reg_data.api.vram_size(),
+            gpu_name: reg_data.api.gpu_name(),
         })
     }
 }
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index f4fd78f9c321..e6789f1485d1 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -24,6 +24,11 @@ pub struct NovaCoreApi<'bound> {
 }
 
 impl NovaCoreApi<'_> {
+    /// Returns the NUL-terminated full GPU name supplied by GSP-RM.
+    pub fn gpu_name(&self) -> [u8; 64] {
+        *self.gpu.gsp_static_info.gpu_name_bytes()
+    }
+
     /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
     /// by nova-core.
     pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 6453184af55b..b8dc64808620 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -251,6 +251,11 @@ pub(crate) enum GpuNameError {
 }
 
 impl GetGspStaticInfoReply {
+    /// Returns the full GPU name as a NUL-terminated byte string.
+    pub(crate) fn gpu_name_bytes(&self) -> &[u8; 64] {
+        &self.gpu_name
+    }
+
     /// Returns the name of the GPU as a string.
     ///
     /// Returns an error if the string given by the GSP does not contain a null terminator or
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index b80bb50acaa1..7e11d1bfeb6b 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -142,6 +142,11 @@ struct drm_nova_gpu_info {
 	 * regions.
 	 */
 	__u64 vram_size;
+
+	/**
+	 * @gpu_name: NUL-terminated full GPU name.
+	 */
+	__u8 gpu_name[64];
 };
 
 #define DRM_NOVA_GETPARAM		0x00
-- 
2.54.0

[PATCH v5 10/11] drm: nova: Report GPU short name in GPU info

[Alistair Popple]

Add the short GPU name to the GPU info structure.

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

---

Changes since v4:

 - New for v5
---
 drivers/gpu/drm/nova/file.rs             | 1 +
 drivers/gpu/nova-core/api.rs             | 5 +++++
 drivers/gpu/nova-core/gsp/commands.rs    | 7 +++++++
 drivers/gpu/nova-core/gsp/fw/commands.rs | 5 +++++
 include/uapi/drm/nova_drm.h              | 5 +++++
 5 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index ab04a10d3353..64d9f68c7cf0 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -37,6 +37,7 @@ fn new(reg_data: &DrmRegData<'_>) -> Self {
             implementation: reg_data.api.implementation(),
             vram_size: reg_data.api.vram_size(),
             gpu_name: reg_data.api.gpu_name(),
+            gpu_short_name: reg_data.api.gpu_short_name(),
         })
     }
 }
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index e6789f1485d1..c18fa1766892 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -29,6 +29,11 @@ impl NovaCoreApi<'_> {
         *self.gpu.gsp_static_info.gpu_name_bytes()
     }
 
+    /// Returns the NUL-terminated short GPU name supplied by GSP-RM.
+    pub fn gpu_short_name(&self) -> [u8; 64] {
+        *self.gpu.gsp_static_info.gpu_short_name_bytes()
+    }
+
     /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
     /// by nova-core.
     pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index b8dc64808620..b9f19be9b29c 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -214,6 +214,7 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
 /// The reply from the GSP to the [`GetGspStaticInfo`] command.
 pub(crate) struct GetGspStaticInfoReply {
     gpu_name: [u8; 64],
+    gpu_short_name: [u8; 64],
     /// Usable FB (VRAM) regions for driver memory allocation.
     pub(crate) usable_fb_regions: KVec<Range<u64>>,
 }
@@ -234,6 +235,7 @@ fn read(
 
         Ok(GetGspStaticInfoReply {
             gpu_name: msg.gpu_name_str(),
+            gpu_short_name: msg.gpu_short_name_str(),
             usable_fb_regions,
         })
     }
@@ -256,6 +258,11 @@ impl GetGspStaticInfoReply {
         &self.gpu_name
     }
 
+    /// Returns the short GPU name as a NUL-terminated byte string.
+    pub(crate) fn gpu_short_name_bytes(&self) -> &[u8; 64] {
+        &self.gpu_short_name
+    }
+
     /// Returns the name of the GPU as a string.
     ///
     /// Returns an error if the string given by the GSP does not contain a null terminator or
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 6dc31d1bf5ae..8dddd0876145 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -131,6 +131,11 @@ impl GspStaticConfigInfo {
         self.0.gpuNameString
     }
 
+    /// Returns a bytes array containing the NUL-terminated short name of this GPU.
+    pub(crate) fn gpu_short_name_str(&self) -> [u8; 64] {
+        self.0.gpuShortNameString
+    }
+
     /// Returns an iterator over valid FB regions from GSP firmware data.
     fn fb_regions(
         &self,
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index 7e11d1bfeb6b..f912c5bf3b4f 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -147,6 +147,11 @@ struct drm_nova_gpu_info {
 	 * @gpu_name: NUL-terminated full GPU name.
 	 */
 	__u8 gpu_name[64];
+
+	/**
+	 * @gpu_short_name: NUL-terminated short GPU name.
+	 */
+	__u8 gpu_short_name[64];
 };
 
 #define DRM_NOVA_GETPARAM		0x00
-- 
2.54.0

[PATCH v5 11/11] drm: nova: Report GPU GID in GPU info

[Alistair Popple]

Add GPU GID to the reported GPU info.

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

---

Changes since v4:

 - New for v5
---
 drivers/gpu/drm/nova/file.rs             | 1 +
 drivers/gpu/nova-core/api.rs             | 5 +++++
 drivers/gpu/nova-core/gsp/commands.rs    | 7 +++++++
 drivers/gpu/nova-core/gsp/fw/commands.rs | 7 +++++++
 include/uapi/drm/nova_drm.h              | 5 +++++
 5 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 64d9f68c7cf0..e1223a29f8b1 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -38,6 +38,7 @@ fn new(reg_data: &DrmRegData<'_>) -> Self {
             vram_size: reg_data.api.vram_size(),
             gpu_name: reg_data.api.gpu_name(),
             gpu_short_name: reg_data.api.gpu_short_name(),
+            gpu_gid: reg_data.api.gpu_gid(),
         })
     }
 }
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index c18fa1766892..ad4b62db1e5f 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -34,6 +34,11 @@ impl NovaCoreApi<'_> {
         *self.gpu.gsp_static_info.gpu_short_name_bytes()
     }
 
+    /// Returns the 16-byte SHA-1 GPU identifier supplied by GSP-RM.
+    pub fn gpu_gid(&self) -> [u8; 16] {
+        *self.gpu.gsp_static_info.gpu_gid()
+    }
+
     /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
     /// by nova-core.
     pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index b9f19be9b29c..c01b978f46d2 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -215,6 +215,7 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
 pub(crate) struct GetGspStaticInfoReply {
     gpu_name: [u8; 64],
     gpu_short_name: [u8; 64],
+    gpu_gid: [u8; 16],
     /// Usable FB (VRAM) regions for driver memory allocation.
     pub(crate) usable_fb_regions: KVec<Range<u64>>,
 }
@@ -236,6 +237,7 @@ fn read(
         Ok(GetGspStaticInfoReply {
             gpu_name: msg.gpu_name_str(),
             gpu_short_name: msg.gpu_short_name_str(),
+            gpu_gid: msg.gpu_gid(),
             usable_fb_regions,
         })
     }
@@ -263,6 +265,11 @@ impl GetGspStaticInfoReply {
         &self.gpu_short_name
     }
 
+    /// Returns the 16-byte SHA-1 GPU identifier.
+    pub(crate) fn gpu_gid(&self) -> &[u8; 16] {
+        &self.gpu_gid
+    }
+
     /// Returns the name of the GPU as a string.
     ///
     /// Returns an error if the string given by the GSP does not contain a null terminator or
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 8dddd0876145..1020de51a83c 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -136,6 +136,13 @@ impl GspStaticConfigInfo {
         self.0.gpuShortNameString
     }
 
+    /// Returns the 16-byte SHA-1 GPU identifier.
+    pub(crate) fn gpu_gid(&self) -> [u8; 16] {
+        let mut gid = [0; 16];
+        gid.copy_from_slice(&self.0.gidInfo.data[..16]);
+        gid
+    }
+
     /// Returns an iterator over valid FB regions from GSP firmware data.
     fn fb_regions(
         &self,
diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h
index f912c5bf3b4f..3bd9db27c665 100644
--- a/include/uapi/drm/nova_drm.h
+++ b/include/uapi/drm/nova_drm.h
@@ -152,6 +152,11 @@ struct drm_nova_gpu_info {
 	 * @gpu_short_name: NUL-terminated short GPU name.
 	 */
 	__u8 gpu_short_name[64];
+
+	/**
+	 * @gpu_gid: 16-byte SHA-1 GPU identifier supplied by GSP-RM.
+	 */
+	__u8 gpu_gid[16];
 };
 
 #define DRM_NOVA_GETPARAM		0x00
-- 
2.54.0

Re: [PATCH v5 00/11] gpu: nova: Export parameters from nova-core to nova-drm

[Alistair Popple]

On 2026-08-28 at 13:35 +1000, Alistair Popple <apopple@nvidia.com> wrote...
> This patch series adds some basic GPU properties via a new GPU info
> 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]
> 
> 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.
> 
> A new info ioctl is introduced which provides an info type field and a
> method for reading extendable structs containing GPU information. This
> series can be tested using the drm-test[4]. A pull request containing
> updated tests will be raised once this has been posted.

Pull request is here:

https://gitlab.freedesktop.org/dakr/drm-test/-/merge_requests/1

> A pull request for
> Mesa has also been raised but is out of date. That will be updated once
> this series has been merged.
> 
> Changes since v4:
> 
>  - No longer export chip-id, instead export architecture and implementation
>  - Add a separate info ioctl with types to read GPU info as suggested by
>    Danilo
> 
> Changes since v3:
> 
>  - Use an ioctl to return all parameters rather than multiple key/value
>    queries
> 
> 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
> [4] - https://gitlab.freedesktop.org/dakr/drm-test
> 
> Cc: M Henning <mhenning@darkrefraction.com>
> 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 (11):
>   gpu: nova-core: Add public driver API to nova-core
>   drm: nova: Add DRM registration data
>   drm: nova: Add GPU architecture enum to nova-drm UAPI
>   rust: uaccess: add UserSliceWriter::write_truncated()
>   drm: nova: Add an info ioctl
>   drm: nova: Add usable VRAM size to GPU info
>   drm: nova: Use nova-core to read VRAM_BAR_SIZE parameter
>   drm: nova: Expose a render node
>   drm: nova: Report GPU name in GPU info
>   drm: nova: Report GPU short name in GPU info
>   drm: nova: Report GPU GID in GPU info
> 
>  drivers/gpu/drm/nova/driver.rs           | 18 ++++-
>  drivers/gpu/drm/nova/file.rs             | 83 ++++++++++++++++++++----
>  drivers/gpu/nova-core/api.rs             | 68 +++++++++++++++++++
>  drivers/gpu/nova-core/driver.rs          | 46 +++++++++----
>  drivers/gpu/nova-core/gpu.rs             | 43 +++++++-----
>  drivers/gpu/nova-core/gsp/commands.rs    | 27 ++++++++
>  drivers/gpu/nova-core/gsp/fw/commands.rs | 12 ++++
>  drivers/gpu/nova-core/gsp/hal.rs         |  2 +-
>  drivers/gpu/nova-core/nova_core.rs       |  1 +
>  drivers/gpu/nova-core/num.rs             |  2 +-
>  include/uapi/drm/nova_drm.h              | 82 +++++++++++++++++++++++
>  rust/kernel/uaccess.rs                   | 14 ++++
>  12 files changed, 352 insertions(+), 46 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/api.rs
> 
> -- 
> 2.54.0
> 

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-08-28 at 13:35 +1000, Alistair Popple <apopple@nvidia.com> wrote...

[...]

> +/**
> + * DRM_NOVA_INFO_GPU

Now that we have the possiblity for multiple types of info calls I'm going to
rename this and subsequent similar usage to DRM_NOVA_INFO_GPU. That makes future
definitions more consistent and keeps them distinct from other UAPI structs, i.e
DRM_NOVA_INFO_<type>.

 - Alistair

> + *
> + * Query GPU information. The result is returned in a
> + * &struct drm_nova_gpu_info.
> + */
> +#define DRM_NOVA_INFO_GPU		0x00
> +
> +/**
> + * struct drm_nova_gpu_info - GPU information
> + */
> +struct drm_nova_gpu_info {
> +	/**
> +	 * @architecture: GPU architecture identifier. See
> +	 * &enum drm_nova_architecture for currently known architectures.
> +	 */
> +	__u32 architecture;
> +
> +	/**
> +	 * @implementation: GPU implementation identifier.
> +	 */
> +	__u32 implementation;
> +};
> +
>  #define DRM_NOVA_GETPARAM		0x00
>  #define DRM_NOVA_GEM_CREATE		0x01
>  #define DRM_NOVA_GEM_INFO		0x02
> +#define DRM_NOVA_INFO			0x03
>  
>  /* Note: this is an enum so that it can be resolved by Rust bindgen. */
>  enum {
> @@ -104,6 +151,8 @@ enum {
>  						   struct drm_nova_gem_create),
>  	DRM_IOCTL_NOVA_GEM_INFO		= DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_GEM_INFO,
>  						   struct drm_nova_gem_info),
> +	DRM_IOCTL_NOVA_INFO		= DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_INFO,
> +						   struct drm_nova_info),
>  };
>  
>  #if defined(__cplusplus)
> -- 
> 2.54.0
> 

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> +/// GPU information returned to userspace.
> +///
> +/// # Invariants
> +///
> +/// - The layout of this type is identical to `struct drm_nova_gpu_info`.
> +/// - All bytes in the value are initialized.

I don't think we need those invariants. The first one is covered by
#[repr(transparent)] and the second invariant is trivally satisfied by the fact
that we create a value of that type.

Maybe you meant to say that we explicitly initialized everything despite
uapi::drm_nova_gpu_info being FromBytes (i.e. no "random" values)? But I think
even that wouldn't need an invariant.

> +#[repr(transparent)]
> +struct GpuInfo(uapi::drm_nova_gpu_info);
> +
> +impl GpuInfo {
> +    fn new(reg_data: &DrmRegData<'_>) -> Self {
> +        Self(uapi::drm_nova_gpu_info {
> +            architecture: reg_data.api.architecture(),
> +            implementation: reg_data.api.implementation(),
> +        })
> +    }
> +}
> +
> +// SAFETY: `GpuInfo` has no implicit padding, kernel pointers, or interior
> +// mutability, and all of its fields are initialized before it is written to
> +// userspace.
> +unsafe impl AsBytes for GpuInfo {}
> +
> +fn write_info<T: AsBytes>(info: &mut uapi::drm_nova_info, value: &T) -> Result {

We could take T by value I guess? We don't need it anymore after it has been
written to the user buffer.

> +    let mut writer =
> +        UserSlice::new(UserPtr::from_addr(info.data as usize), info.size as usize).writer();
> +
> +    info.size = writer.write_truncated(value)? as u32;
> +
> +    Ok(())
> +}
> +
>  impl drm::file::DriverFile for File {
>      type Driver = NovaDriver;
>  
> @@ -78,4 +112,27 @@ pub(crate) fn gem_info(
>  
>          Ok(0)
>      }
> +
> +    /// IOCTL: info: Query device information.
> +    pub(crate) fn info(
> +        _dev: &NovaDevice<Registered>,
> +        reg_data: &DrmRegData<'_>,
> +        info: &mut uapi::drm_nova_info,
> +        _file: &drm::File<File>,
> +    ) -> Result<u32> {
> +        if info.data == 0 {
> +            info.size = match info.id {
> +                uapi::DRM_NOVA_INFO_GPU => size_of::<GpuInfo>() as u32,
> +                _ => return Err(EINVAL),
> +            };
> +            return Ok(0);
> +        }

I think this check can go into write_info(), so we don't have to repeat this for
every info. I.e. we can just add

	if info.data == 0 {
	    info.size = size_of::<T>() as u32;
	    return Ok(());
	}

at the beginning of write_info(). It may construct the value even if
info.data == 0, but I don't think we care. :)

> +
> +        match info.id {
> +            uapi::DRM_NOVA_INFO_GPU => write_info(info, &GpuInfo::new(reg_data))?,
> +            _ => return Err(EINVAL),
> +        }
> +
> +        Ok(0)
> +    }
>  }
> diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
> index 610cfc01111e..cff730a38c1d 100644
> --- a/drivers/gpu/nova-core/api.rs
> +++ b/drivers/gpu/nova-core/api.rs
> @@ -12,11 +12,12 @@
>      types::CovariantForLt, //
>  };
>  
> -use crate::gpu::Gpu;
> +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>>,
>  }
>  
> @@ -26,4 +27,14 @@ impl NovaCoreApi<'_> {
>      pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
>          adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
>      }
> +
> +    /// Returns the architecture identifier of this GPU.
> +    pub fn architecture(&self) -> u32 {
> +        self.gpu.spec.chipset.arch() as u32
> +    }
> +
> +    /// Returns the implementation identifier of this GPU.
> +    pub fn implementation(&self) -> u32 {
> +        self.gpu.spec.chipset.implementation()
> +    }

We should make the NovaCoreApi just provide an accessor for &Spec and make every
subsequent method we need public. Otherwise we end up with endless forwarding
methods. We can also add as_raw() methods to the specific types as needed.

Re: [PATCH v5 09/11] drm: nova: Report GPU name in GPU info

[Danilo Krummrich]

On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
>  impl NovaCoreApi<'_> {
> +    /// Returns the NUL-terminated full GPU name supplied by GSP-RM.
> +    pub fn gpu_name(&self) -> [u8; 64] {

I don't really like that we have two separate accessors for this, can't we just
use the one we already have, which also does all the validation already?

The constructor of GpuInfo could look like this:

	fn new(reg_data: &DrmRegData<'_>) -> Result<Self> {
	    let mut info = uapi::drm_nova_gpu_info {
	        architecture: reg_data.api.architecture(),
	        implementation: reg_data.api.implementation(),
	        ..Default::default()
	    };
	    let bytes = reg_data.api.gpu_name()?.as_bytes();
	    info.gpu_name[..bytes.len()].copy_from_slice(bytes);
	    Ok(Self(info))
	}

Could also be infallible if we want to go with a fallback name, given that we
consider the firmware not providing something useful as non-fatal so far.

> +        *self.gpu.gsp_static_info.gpu_name_bytes()
> +    }
> +
>      /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
>      /// by nova-core.
>      pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 6453184af55b..b8dc64808620 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -251,6 +251,11 @@ pub(crate) enum GpuNameError {
>  }
>  
>  impl GetGspStaticInfoReply {
> +    /// Returns the full GPU name as a NUL-terminated byte string.
> +    pub(crate) fn gpu_name_bytes(&self) -> &[u8; 64] {
> +        &self.gpu_name
> +    }

AFAICS there's nothing ensuring that this is actually NULL terminated? The
existing gpu_name() method already does this.

Re: [PATCH v5 10/11] drm: nova: Report GPU short name in GPU info

[Danilo Krummrich]

On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> Add the short GPU name to the GPU info structure.
>
> Signed-off-by: Alistair Popple <apopple@nvidia.com>

Same as patch 9.

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

[Danilo Krummrich]

On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> +/// API handle for the auxiliary bus child drivers to interact with nova-core.
> +pub struct NovaCoreApi<'bound> {
> +    #[expect(unused)]
> +    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
> +}
> +
> +impl NovaCoreApi<'_> {
> +    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
> +    /// by nova-core.
> +    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> +        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
> +    }
> +}

CovariantForLt does not hold anymore on latest drm-rust-next, as Cmdq has a
Mutex. So, this needs ForLt now and therefore the approach that I shared in [1]
a while ago. I applied the changes in [2] to fix it up.

I think the closure access through api.with(|api| ...) is fine in most cases,
but there are a few options if we run into cases where we consider it a bit
inconvinient.

I think it should be possible to support projections into T: 'static and
covariant fields. The reason I differentiate them is because T: 'static is very
convinient, but non-'static covariant types need an annoying turbofish.

For T: 'static types it would turn out like this

	let spec = reg_data.api.project(|api| api.spec());

such that everything that needs spec does not need to be in the closure anymore.

For non-'static covariant fields we could have

	let foo = reg_data.api.project_lt::<CovariantForLt!(Foo<'_>)>(|api| api.foo());

but as mentioned it unfortunately needs the turbofish. Of course we could invent
a macro around it to get rid of the turbofish, but we'd still need to explicitly
mention the type Foo<'_>, so it doesn't buy us a lot.

This is the implementation I came up with in nova-core

	/// Projects a `'static` sub-field out of the registration data.
	///
	/// `T` is fully inferred from the closure. For projected types with a lifetime parameter,
	/// use [`Self::project_lt`].
	pub fn project<T: 'static>(
	    &self,
	    f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> &'b T,
	) -> &'a T {
	    self.adev
	        .registration_data_field::<ForLt!(NovaCoreApi<'_>), T>(f)
	        .expect("TypeId was validated in NovaCoreApiHandle::of()")
	}
	
	/// Projects a covariant sub-field out of the registration data.
	///
	/// Supports projected types with a lifetime parameter via a
	/// [`CovariantForLt`](trait@CovariantForLt) encoding. Unlike [`Self::project`], `G` cannot
	/// be inferred and must be specified explicitly.
	pub fn project_lt<G: CovariantForLt + 'static>(
	    &self,
	    f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> &'b G::Of<'b>,
	) -> &'a G::Of<'a> {
	    self.adev
	        .registration_data_project::<ForLt!(NovaCoreApi<'_>), G>(f)
	        .expect("TypeId was validated in NovaCoreApiHandle::of()")
	}

and this is what we'd need in the auxiliary bus

	/// Projects a covariant sub-field out of potentially invariant registration data.
	///
	/// `F` is the [`ForLt`](trait@ForLt) encoding of the registration data type. `G` is the
	/// [`CovariantForLt`](trait@CovariantForLt) encoding of the projected sub-field type.
	///
	/// For projected types that are `'static`, prefer [`Self::registration_data_field`] which
	/// does not require a [`CovariantForLt`](trait@CovariantForLt) encoding and fully infers `T`.
	///
	/// Returns [`EINVAL`] if `F` does not match the type used by the parent driver when calling
	/// [`Registration::new()`]. Returns [`ENOENT`] if no registration data has been set.
	#[inline]
	pub fn registration_data_project<F, G>(
	    &self,
	    project: impl for<'a> FnOnce(Pin<&'a F::Of<'a>>) -> &'a G::Of<'a>,
	) -> Result<&G::Of<'_>>
	where
	    F: ForLt + 'static,
	    G: CovariantForLt + 'static,
	{
	    let ptr = self.registration_data_with::<F, *const ()>(|data| {
	        core::ptr::from_ref::<G::Of<'_>>(project(data)).cast::<()>()
	    })?;
	
	    // SAFETY:
	    // - The HRTB bound on `project` ensures the returned pointer is derived from the
	    //   registration data (nothing else lives for universally quantified `'a`).
	    // - `G: CovariantForLt` guarantees that shortening the lifetime of `G::Of` is sound.
	    // - The registration data is heap-allocated and outlives the device's bound state.
	    Ok(unsafe { &*ptr.cast::<G::Of<'_>>() })
	}
	
	/// Projects a `'static` sub-field out of potentially invariant registration data.
	///
	/// Simplified variant of [`Self::registration_data_project`] for projected types that are
	/// `'static`. Since `&'a T` is trivially covariant when `T: 'static`, no
	/// [`CovariantForLt`](trait@CovariantForLt) encoding is needed and `T` is fully inferred
	/// from the closure.
	///
	/// Returns [`EINVAL`] if `F` does not match the type used by the parent driver when calling
	/// [`Registration::new()`]. Returns [`ENOENT`] if no registration data has been set.
	#[inline]
	pub fn registration_data_field<F: ForLt + 'static, T: 'static>(
	    &self,
	    project: impl for<'a> FnOnce(Pin<&'a F::Of<'a>>) -> &'a T,
	) -> Result<&T> {
	    let ptr = self.registration_data_with::<F, *const T>(|data| {
	        core::ptr::from_ref(project(data))
	    })?;
	
	    // SAFETY:
	    // - The HRTB bound on `project` ensures the returned pointer is derived from the
	    //   registration data (nothing else lives for universally quantified `'a`).
	    // - `T: 'static` means `&T` is trivially covariant; lifetime shortening is sound.
	    // - The registration data is heap-allocated and outlives the device's bound state.
	    Ok(unsafe { &*ptr })
	}

with the documentation being written by an LLM and unchecked.

I think at least the T: 'static projection can provide an ergonomic advantage
and might be useful to add. Please let me know what you think.

Thanks,
Danilo

[1] https://lore.kernel.org/all/DJQSY9ZY5M5F.3VI537GS00E1G@kernel.org/
[2] CovariantForLt to ForLt changes for nova-core

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 7fc0baef5f04..028020213ac5 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0

-use core::pin::Pin;
-
 use kernel::{
     auxiliary,
     device::{
@@ -20,7 +18,10 @@
 use crate::file::File;
 use crate::gem::NovaObject;

-use nova_core::api::NovaCoreApi;
+use nova_core::api::{
+    NovaCoreApi,
+    NovaCoreApiHandle, //
+};

 pub(crate) struct NovaDriver;

@@ -32,7 +33,7 @@ pub(crate) struct Nova<'bound> {

 /// DRM registration data, accessible from ioctl handlers via the registration guard.
 pub(crate) struct DrmRegData<'bound> {
-    pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
+    pub(crate) api: NovaCoreApiHandle<'bound>,
 }

 /// Convienence type alias for the DRM device type for this driver
@@ -69,7 +70,7 @@ fn probe<'bound>(
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
         let reg_data = DrmRegData {
-            api: NovaCoreApi::of(adev)?,
+            api: NovaCoreApi::handle(adev)?,
         };
         // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is
         // never forgotten.
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index e1223a29f8b1..798b14f33e20 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -32,13 +32,15 @@

 impl GpuInfo {
     fn new(reg_data: &DrmRegData<'_>) -> Self {
-        Self(uapi::drm_nova_gpu_info {
-            architecture: reg_data.api.architecture(),
-            implementation: reg_data.api.implementation(),
-            vram_size: reg_data.api.vram_size(),
-            gpu_name: reg_data.api.gpu_name(),
-            gpu_short_name: reg_data.api.gpu_short_name(),
-            gpu_gid: reg_data.api.gpu_gid(),
+        reg_data.api.with(|api| {
+            Self(uapi::drm_nova_gpu_info {
+                architecture: api.architecture(),
+                implementation: api.implementation(),
+                vram_size: api.vram_size(),
+                gpu_name: api.gpu_name(),
+                gpu_short_name: api.gpu_short_name(),
+                gpu_gid: api.gpu_gid(),
+            })
         })
     }
 }
@@ -74,7 +76,7 @@ pub(crate) fn get_param(
         _file: &drm::File<File>,
     ) -> Result<u32> {
         let value = match getparam.param as u32 {
-            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.bar1_size()?,
+            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.with(|api| api.bar1_size())?,
             _ => return Err(EINVAL),
         };

diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index ad4b62db1e5f..c9ae48d278af 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -10,7 +10,7 @@
     device::Bound,
     pci,
     prelude::*,
-    types::CovariantForLt, //
+    types::ForLt, //
 };

 use crate::gpu::{
@@ -39,10 +39,9 @@ impl NovaCoreApi<'_> {
         *self.gpu.gsp_static_info.gpu_gid()
     }

-    /// 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<'_>)>()
+    /// Obtain a [`NovaCoreApiHandle`] from an auxiliary device registered by nova-core.
+    pub fn handle(adev: &auxiliary::Device<Bound>) -> Result<NovaCoreApiHandle<'_>> {
+        NovaCoreApiHandle::of(adev)
     }

     /// Returns the architecture identifier of this GPU.
@@ -66,3 +65,22 @@ pub fn vram_size(&self) -> u64 {
         self.gpu.gsp_static_info.vram_size()
     }
 }
+
+/// 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 {
+        self.adev
+            .registration_data_with::<ForLt!(NovaCoreApi<'_>), R>(f)
+            .expect("TypeId was validated in NovaCoreApiHandle::of()")
+    }
+}
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 4d3c18d6a733..025ba2869d6c 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -15,7 +15,7 @@
         Atomic,
         Relaxed, //
     },
-    types::CovariantForLt,
+    types::ForLt,
 };

 use crate::{
@@ -29,7 +29,7 @@
 #[pin_data]
 pub(crate) struct NovaCore<'bound> {
     #[allow(clippy::type_complexity)]
-    _reg: auxiliary::Registration<'bound, CovariantForLt!(NovaCoreApi<'_>)>,
+    _reg: auxiliary::Registration<'bound, ForLt!(NovaCoreApi<'_>)>,
     #[pin]
     pub(crate) gpu: Gpu<'bound>,
     bar: pci::Bar<'bound, BAR0_SIZE>,

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

[Gary Guo]

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

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

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

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

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

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

Then your projection can just be

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

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

Best,
Gary

Re: [PATCH v5 09/11] drm: nova: Report GPU name in GPU info

[Alistair Popple]

On 2026-09-01 at 00:33 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> >  impl NovaCoreApi<'_> {
> > +    /// Returns the NUL-terminated full GPU name supplied by GSP-RM.
> > +    pub fn gpu_name(&self) -> [u8; 64] {
> 
> I don't really like that we have two separate accessors for this, can't we just
> use the one we already have, which also does all the validation already?

Yeah, I don't know how I forgot we already had an accessor for that. I've been
carrying this patch on my own tree for quite a while, so guess just enough code
moved around change that I missed it.

Anyway I agree, a new acessor is totally unnecessary.

> The constructor of GpuInfo could look like this:
> 
> 	fn new(reg_data: &DrmRegData<'_>) -> Result<Self> {
> 	    let mut info = uapi::drm_nova_gpu_info {
> 	        architecture: reg_data.api.architecture(),
> 	        implementation: reg_data.api.implementation(),
> 	        ..Default::default()
> 	    };
> 	    let bytes = reg_data.api.gpu_name()?.as_bytes();
> 	    info.gpu_name[..bytes.len()].copy_from_slice(bytes);
> 	    Ok(Self(info))
> 	}
> 
> Could also be infallible if we want to go with a fallback name, given that we
> consider the firmware not providing something useful as non-fatal so far.

Yeah, I think keeping this infallible makes sense. 

> > +        *self.gpu.gsp_static_info.gpu_name_bytes()
> > +    }
> > +
> >      /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
> >      /// by nova-core.
> >      pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> > diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> > index 6453184af55b..b8dc64808620 100644
> > --- a/drivers/gpu/nova-core/gsp/commands.rs
> > +++ b/drivers/gpu/nova-core/gsp/commands.rs
> > @@ -251,6 +251,11 @@ pub(crate) enum GpuNameError {
> >  }
> >  
> >  impl GetGspStaticInfoReply {
> > +    /// Returns the full GPU name as a NUL-terminated byte string.
> > +    pub(crate) fn gpu_name_bytes(&self) -> &[u8; 64] {
> > +        &self.gpu_name
> > +    }
> 
> AFAICS there's nothing ensuring that this is actually NULL terminated? The
> existing gpu_name() method already does this.

Yep, relied on FW which is sub-optimal. The existing accessors obviously fix
that.

 - Alistair

Re: [PATCH v5 10/11] drm: nova: Report GPU short name in GPU info

[Alistair Popple]

On 2026-09-01 at 00:41 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> > Add the short GPU name to the GPU info structure.
> >
> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> 
> Same as patch 9.

Ack, will add a better accessor. I don't see an existing one, but makes sense to
enforce NULL termination at the kernel level.

 - Alistair

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-01 at 00:23 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> > +/// GPU information returned to userspace.
> > +///
> > +/// # Invariants
> > +///
> > +/// - The layout of this type is identical to `struct drm_nova_gpu_info`.
> > +/// - All bytes in the value are initialized.
> 
> I don't think we need those invariants. The first one is covered by
> #[repr(transparent)] and the second invariant is trivally satisfied by the fact
> that we create a value of that type.

Ok.

> Maybe you meant to say that we explicitly initialized everything despite
> uapi::drm_nova_gpu_info being FromBytes (i.e. no "random" values)? But I think
> even that wouldn't need an invariant.

Yep, basically trying to justify why As/FromBytes is safe but if the safety
comments for these implementations on their own are adequate I will just use
those and remove this.

> > +#[repr(transparent)]
> > +struct GpuInfo(uapi::drm_nova_gpu_info);
> > +
> > +impl GpuInfo {
> > +    fn new(reg_data: &DrmRegData<'_>) -> Self {
> > +        Self(uapi::drm_nova_gpu_info {
> > +            architecture: reg_data.api.architecture(),
> > +            implementation: reg_data.api.implementation(),
> > +        })
> > +    }
> > +}
> > +
> > +// SAFETY: `GpuInfo` has no implicit padding, kernel pointers, or interior
> > +// mutability, and all of its fields are initialized before it is written to
> > +// userspace.
> > +unsafe impl AsBytes for GpuInfo {}
> > +
> > +fn write_info<T: AsBytes>(info: &mut uapi::drm_nova_info, value: &T) -> Result {
> 
> We could take T by value I guess? We don't need it anymore after it has been
> written to the user buffer.

Makes sense.

> > +    let mut writer =
> > +        UserSlice::new(UserPtr::from_addr(info.data as usize), info.size as usize).writer();
> > +
> > +    info.size = writer.write_truncated(value)? as u32;
> > +
> > +    Ok(())
> > +}
> > +
> >  impl drm::file::DriverFile for File {
> >      type Driver = NovaDriver;
> >  
> > @@ -78,4 +112,27 @@ pub(crate) fn gem_info(
> >  
> >          Ok(0)
> >      }
> > +
> > +    /// IOCTL: info: Query device information.
> > +    pub(crate) fn info(
> > +        _dev: &NovaDevice<Registered>,
> > +        reg_data: &DrmRegData<'_>,
> > +        info: &mut uapi::drm_nova_info,
> > +        _file: &drm::File<File>,
> > +    ) -> Result<u32> {
> > +        if info.data == 0 {
> > +            info.size = match info.id {
> > +                uapi::DRM_NOVA_INFO_GPU => size_of::<GpuInfo>() as u32,
> > +                _ => return Err(EINVAL),
> > +            };
> > +            return Ok(0);
> > +        }
> 
> I think this check can go into write_info(), so we don't have to repeat this for
> every info. I.e. we can just add
> 
> 	if info.data == 0 {
> 	    info.size = size_of::<T>() as u32;
> 	    return Ok(());
> 	}
> 
> at the beginning of write_info(). It may construct the value even if
> info.data == 0, but I don't think we care. :)

Heh. That was why I did it this way, in case obtaining the info was expensive
for some value of "expensive". But info should mostly be cached (ie. cheap) and
I don't mind keeping things simple :)

> 
> > +
> > +        match info.id {
> > +            uapi::DRM_NOVA_INFO_GPU => write_info(info, &GpuInfo::new(reg_data))?,
> > +            _ => return Err(EINVAL),
> > +        }
> > +
> > +        Ok(0)
> > +    }
> >  }
> > diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
> > index 610cfc01111e..cff730a38c1d 100644
> > --- a/drivers/gpu/nova-core/api.rs
> > +++ b/drivers/gpu/nova-core/api.rs
> > @@ -12,11 +12,12 @@
> >      types::CovariantForLt, //
> >  };
> >  
> > -use crate::gpu::Gpu;
> > +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>>,
> >  }
> >  
> > @@ -26,4 +27,14 @@ impl NovaCoreApi<'_> {
> >      pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> >          adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
> >      }
> > +
> > +    /// Returns the architecture identifier of this GPU.
> > +    pub fn architecture(&self) -> u32 {
> > +        self.gpu.spec.chipset.arch() as u32
> > +    }
> > +
> > +    /// Returns the implementation identifier of this GPU.
> > +    pub fn implementation(&self) -> u32 {
> > +        self.gpu.spec.chipset.implementation()
> > +    }
> 
> We should make the NovaCoreApi just provide an accessor for &Spec and make every
> subsequent method we need public. Otherwise we end up with endless forwarding
> methods. We can also add as_raw() methods to the specific types as needed.

Ok. This is where I don't have a good instinct for what we think should be an
accessor/forwarding method vs. where we should just expose the underlying data
structure and required methods to API users.

In the past it seems there's been some resistance to exposing nova-core or gsp
data structures like this which is why I added the forwarding methods. In future
we're going to have other data-structures that NovaCoreApi will need to access
so it would be good to understand what we should do here so we can keep things
somewhat consistent.

It seems pretty arbitrary to say expose self.gpu.spec externally
because of endless forwarding methods but to then require them for say
self.gpu.gsp_static_info. So maybe we can just have a NovaCoreApi method that
returns self.gpu directly instead of writing forwarding methods for each field
of self.gpu that we need to access? Access to genuinely internal nova-core
fields/methods can always be controlled through visibility modifiers.

 - Alistair

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

[Alistair Popple]

On 2026-09-01 at 06:08 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> > +/// API handle for the auxiliary bus child drivers to interact with nova-core.
> > +pub struct NovaCoreApi<'bound> {
> > +    #[expect(unused)]
> > +    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
> > +}
> > +
> > +impl NovaCoreApi<'_> {
> > +    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
> > +    /// by nova-core.
> > +    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> > +        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
> > +    }
> > +}
> 
> CovariantForLt does not hold anymore on latest drm-rust-next, as Cmdq has a
> Mutex. So, this needs ForLt now and therefore the approach that I shared in [1]
> a while ago. I applied the changes in [2] to fix it up.

Thanks. I remember that conversation, just that up until now I could get away
with it being Covariant which was nice :)

I'm in a bit of a painful situation atm where my local test box doesn't work
with older 570.144 firmware so I have to carry this on top of John's r000
patches to test which makes rebasing all kinds of fun. But will rebase to latest
drm-rust-next and see how I go with the below.

> I think the closure access through api.with(|api| ...) is fine in most cases,
> but there are a few options if we run into cases where we consider it a bit
> inconvinient.
> 
> I think it should be possible to support projections into T: 'static and
> covariant fields. The reason I differentiate them is because T: 'static is very
> convinient, but non-'static covariant types need an annoying turbofish.
> 
> For T: 'static types it would turn out like this
> 
> 	let spec = reg_data.api.project(|api| api.spec());
> 
> such that everything that needs spec does not need to be in the closure anymore.
> 
> For non-'static covariant fields we could have
> 
> 	let foo = reg_data.api.project_lt::<CovariantForLt!(Foo<'_>)>(|api| api.foo());
> 
> but as mentioned it unfortunately needs the turbofish. Of course we could invent
> a macro around it to get rid of the turbofish, but we'd still need to explicitly
> mention the type Foo<'_>, so it doesn't buy us a lot.
> 
> This is the implementation I came up with in nova-core
> 
> 	/// Projects a `'static` sub-field out of the registration data.
> 	///
> 	/// `T` is fully inferred from the closure. For projected types with a lifetime parameter,
> 	/// use [`Self::project_lt`].
> 	pub fn project<T: 'static>(
> 	    &self,
> 	    f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> &'b T,
> 	) -> &'a T {
> 	    self.adev
> 	        .registration_data_field::<ForLt!(NovaCoreApi<'_>), T>(f)
> 	        .expect("TypeId was validated in NovaCoreApiHandle::of()")
> 	}
> 	
> 	/// Projects a covariant sub-field out of the registration data.
> 	///
> 	/// Supports projected types with a lifetime parameter via a
> 	/// [`CovariantForLt`](trait@CovariantForLt) encoding. Unlike [`Self::project`], `G` cannot
> 	/// be inferred and must be specified explicitly.
> 	pub fn project_lt<G: CovariantForLt + 'static>(
> 	    &self,
> 	    f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> &'b G::Of<'b>,
> 	) -> &'a G::Of<'a> {
> 	    self.adev
> 	        .registration_data_project::<ForLt!(NovaCoreApi<'_>), G>(f)
> 	        .expect("TypeId was validated in NovaCoreApiHandle::of()")
> 	}
> 
> and this is what we'd need in the auxiliary bus
> 
> 	/// Projects a covariant sub-field out of potentially invariant registration data.
> 	///
> 	/// `F` is the [`ForLt`](trait@ForLt) encoding of the registration data type. `G` is the
> 	/// [`CovariantForLt`](trait@CovariantForLt) encoding of the projected sub-field type.
> 	///
> 	/// For projected types that are `'static`, prefer [`Self::registration_data_field`] which
> 	/// does not require a [`CovariantForLt`](trait@CovariantForLt) encoding and fully infers `T`.
> 	///
> 	/// Returns [`EINVAL`] if `F` does not match the type used by the parent driver when calling
> 	/// [`Registration::new()`]. Returns [`ENOENT`] if no registration data has been set.
> 	#[inline]
> 	pub fn registration_data_project<F, G>(
> 	    &self,
> 	    project: impl for<'a> FnOnce(Pin<&'a F::Of<'a>>) -> &'a G::Of<'a>,
> 	) -> Result<&G::Of<'_>>
> 	where
> 	    F: ForLt + 'static,
> 	    G: CovariantForLt + 'static,
> 	{
> 	    let ptr = self.registration_data_with::<F, *const ()>(|data| {
> 	        core::ptr::from_ref::<G::Of<'_>>(project(data)).cast::<()>()
> 	    })?;
> 	
> 	    // SAFETY:
> 	    // - The HRTB bound on `project` ensures the returned pointer is derived from the
> 	    //   registration data (nothing else lives for universally quantified `'a`).
> 	    // - `G: CovariantForLt` guarantees that shortening the lifetime of `G::Of` is sound.
> 	    // - The registration data is heap-allocated and outlives the device's bound state.
> 	    Ok(unsafe { &*ptr.cast::<G::Of<'_>>() })
> 	}
> 	
> 	/// Projects a `'static` sub-field out of potentially invariant registration data.
> 	///
> 	/// Simplified variant of [`Self::registration_data_project`] for projected types that are
> 	/// `'static`. Since `&'a T` is trivially covariant when `T: 'static`, no
> 	/// [`CovariantForLt`](trait@CovariantForLt) encoding is needed and `T` is fully inferred
> 	/// from the closure.
> 	///
> 	/// Returns [`EINVAL`] if `F` does not match the type used by the parent driver when calling
> 	/// [`Registration::new()`]. Returns [`ENOENT`] if no registration data has been set.
> 	#[inline]
> 	pub fn registration_data_field<F: ForLt + 'static, T: 'static>(
> 	    &self,
> 	    project: impl for<'a> FnOnce(Pin<&'a F::Of<'a>>) -> &'a T,
> 	) -> Result<&T> {
> 	    let ptr = self.registration_data_with::<F, *const T>(|data| {
> 	        core::ptr::from_ref(project(data))
> 	    })?;
> 	
> 	    // SAFETY:
> 	    // - The HRTB bound on `project` ensures the returned pointer is derived from the
> 	    //   registration data (nothing else lives for universally quantified `'a`).
> 	    // - `T: 'static` means `&T` is trivially covariant; lifetime shortening is sound.
> 	    // - The registration data is heap-allocated and outlives the device's bound state.
> 	    Ok(unsafe { &*ptr })
> 	}
> 
> with the documentation being written by an LLM and unchecked.
> 
> I think at least the T: 'static projection can provide an ergonomic advantage
> and might be useful to add. Please let me know what you think.
> 
> Thanks,
> Danilo
> 
> [1] https://lore.kernel.org/all/DJQSY9ZY5M5F.3VI537GS00E1G@kernel.org/
> [2] CovariantForLt to ForLt changes for nova-core
> 
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index 7fc0baef5f04..028020213ac5 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -1,7 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> 
> -use core::pin::Pin;
> -
>  use kernel::{
>      auxiliary,
>      device::{
> @@ -20,7 +18,10 @@
>  use crate::file::File;
>  use crate::gem::NovaObject;
> 
> -use nova_core::api::NovaCoreApi;
> +use nova_core::api::{
> +    NovaCoreApi,
> +    NovaCoreApiHandle, //
> +};
> 
>  pub(crate) struct NovaDriver;
> 
> @@ -32,7 +33,7 @@ pub(crate) struct Nova<'bound> {
> 
>  /// DRM registration data, accessible from ioctl handlers via the registration guard.
>  pub(crate) struct DrmRegData<'bound> {
> -    pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
> +    pub(crate) api: NovaCoreApiHandle<'bound>,
>  }
> 
>  /// Convienence type alias for the DRM device type for this driver
> @@ -69,7 +70,7 @@ fn probe<'bound>(
>      ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
>          let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
>          let reg_data = DrmRegData {
> -            api: NovaCoreApi::of(adev)?,
> +            api: NovaCoreApi::handle(adev)?,
>          };
>          // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is
>          // never forgotten.
> diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
> index e1223a29f8b1..798b14f33e20 100644
> --- a/drivers/gpu/drm/nova/file.rs
> +++ b/drivers/gpu/drm/nova/file.rs
> @@ -32,13 +32,15 @@
> 
>  impl GpuInfo {
>      fn new(reg_data: &DrmRegData<'_>) -> Self {
> -        Self(uapi::drm_nova_gpu_info {
> -            architecture: reg_data.api.architecture(),
> -            implementation: reg_data.api.implementation(),
> -            vram_size: reg_data.api.vram_size(),
> -            gpu_name: reg_data.api.gpu_name(),
> -            gpu_short_name: reg_data.api.gpu_short_name(),
> -            gpu_gid: reg_data.api.gpu_gid(),
> +        reg_data.api.with(|api| {
> +            Self(uapi::drm_nova_gpu_info {
> +                architecture: api.architecture(),
> +                implementation: api.implementation(),
> +                vram_size: api.vram_size(),
> +                gpu_name: api.gpu_name(),
> +                gpu_short_name: api.gpu_short_name(),
> +                gpu_gid: api.gpu_gid(),
> +            })
>          })
>      }
>  }
> @@ -74,7 +76,7 @@ pub(crate) fn get_param(
>          _file: &drm::File<File>,
>      ) -> Result<u32> {
>          let value = match getparam.param as u32 {
> -            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.bar1_size()?,
> +            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.with(|api| api.bar1_size())?,
>              _ => return Err(EINVAL),
>          };
> 
> diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
> index ad4b62db1e5f..c9ae48d278af 100644
> --- a/drivers/gpu/nova-core/api.rs
> +++ b/drivers/gpu/nova-core/api.rs
> @@ -10,7 +10,7 @@
>      device::Bound,
>      pci,
>      prelude::*,
> -    types::CovariantForLt, //
> +    types::ForLt, //
>  };
> 
>  use crate::gpu::{
> @@ -39,10 +39,9 @@ impl NovaCoreApi<'_> {
>          *self.gpu.gsp_static_info.gpu_gid()
>      }
> 
> -    /// 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<'_>)>()
> +    /// Obtain a [`NovaCoreApiHandle`] from an auxiliary device registered by nova-core.
> +    pub fn handle(adev: &auxiliary::Device<Bound>) -> Result<NovaCoreApiHandle<'_>> {
> +        NovaCoreApiHandle::of(adev)
>      }
> 
>      /// Returns the architecture identifier of this GPU.
> @@ -66,3 +65,22 @@ pub fn vram_size(&self) -> u64 {
>          self.gpu.gsp_static_info.vram_size()
>      }
>  }
> +
> +/// 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 {
> +        self.adev
> +            .registration_data_with::<ForLt!(NovaCoreApi<'_>), R>(f)
> +            .expect("TypeId was validated in NovaCoreApiHandle::of()")
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 4d3c18d6a733..025ba2869d6c 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -15,7 +15,7 @@
>          Atomic,
>          Relaxed, //
>      },
> -    types::CovariantForLt,
> +    types::ForLt,
>  };
> 
>  use crate::{
> @@ -29,7 +29,7 @@
>  #[pin_data]
>  pub(crate) struct NovaCore<'bound> {
>      #[allow(clippy::type_complexity)]
> -    _reg: auxiliary::Registration<'bound, CovariantForLt!(NovaCoreApi<'_>)>,
> +    _reg: auxiliary::Registration<'bound, ForLt!(NovaCoreApi<'_>)>,
>      #[pin]
>      pub(crate) gpu: Gpu<'bound>,
>      bar: pci::Bar<'bound, BAR0_SIZE>,

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Dave Airlie]

> >
> > We should make the NovaCoreApi just provide an accessor for &Spec and make every
> > subsequent method we need public. Otherwise we end up with endless forwarding
> > methods. We can also add as_raw() methods to the specific types as needed.
>
> Ok. This is where I don't have a good instinct for what we think should be an
> accessor/forwarding method vs. where we should just expose the underlying data
> structure and required methods to API users.
>
> In the past it seems there's been some resistance to exposing nova-core or gsp
> data structures like this which is why I added the forwarding methods. In future
> we're going to have other data-structures that NovaCoreApi will need to access
> so it would be good to understand what we should do here so we can keep things
> somewhat consistent.

We can expose structure defined in nova-core, we cannot expose
structures defined in gsp bindings or firmware.

In theory we can internally between core/drm but I'd really really
like to keep that boundary as the limits of GSP for auditability
purposes.

Dave.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Dave Airlie]

On Fri, 28 Aug 2026 at 13:36, Alistair Popple <apopple@nvidia.com> wrote:
>
> Add an extensible info ioctl and use it to report basic GPU information.
> One of the first things userspace needs to know about a GPU is its
> architecture and implementation, so add that as the first field in the
> GPU info result.
>
> The ioctl selects an information type by ID and writes the result through
> a sized userspace buffer. The kernel truncates results to the supplied
> size, allowing information structures to grow and new logical information
> groups to be added without introducing new ioctls.
>
>

This is a higher level for the future, I think we have two paths for
getparam type ioctl.

Either we expose a big struct full of values with a add to the end
mentality or we keep a long list of value pairs, and userspace passes
in a list of value keys, and we return a list of the values.

I'm trying to think ahead towards native virtio context support, and I
think having the sequence,

open
get_all_infos
create vm/channel

might have the most optimal ioctl count but also let power management
avoid booting the GPU on every open if we never create a vm/channel.

Dave.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-01 at 14:50 +1000, Dave Airlie <airlied@gmail.com> wrote...
> > >
> > > We should make the NovaCoreApi just provide an accessor for &Spec and make every
> > > subsequent method we need public. Otherwise we end up with endless forwarding
> > > methods. We can also add as_raw() methods to the specific types as needed.
> >
> > Ok. This is where I don't have a good instinct for what we think should be an
> > accessor/forwarding method vs. where we should just expose the underlying data
> > structure and required methods to API users.
> >
> > In the past it seems there's been some resistance to exposing nova-core or gsp
> > data structures like this which is why I added the forwarding methods. In future
> > we're going to have other data-structures that NovaCoreApi will need to access
> > so it would be good to understand what we should do here so we can keep things
> > somewhat consistent.
> 
> We can expose structure defined in nova-core, we cannot expose
> structures defined in gsp bindings or firmware.

All the fields of self.gpu are structures defined as rust native structures in
nova-core. Their values may be decoded or derived from GSP responses, but by
design none of the raw structures from gsp bindings live in self.gpu AFAIK.

 - Alistair

> In theory we can internally between core/drm but I'd really really
> like to keep that boundary as the limits of GSP for auditability
> purposes.
> 
> Dave.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-01 at 14:53 +1000, Dave Airlie <airlied@gmail.com> wrote...
> On Fri, 28 Aug 2026 at 13:36, Alistair Popple <apopple@nvidia.com> wrote:
> >
> > Add an extensible info ioctl and use it to report basic GPU information.
> > One of the first things userspace needs to know about a GPU is its
> > architecture and implementation, so add that as the first field in the
> > GPU info result.
> >
> > The ioctl selects an information type by ID and writes the result through
> > a sized userspace buffer. The kernel truncates results to the supplied
> > size, allowing information structures to grow and new logical information
> > groups to be added without introducing new ioctls.
> >
> >
> 
> This is a higher level for the future, I think we have two paths for
> getparam type ioctl.
> 
> Either we expose a big struct full of values with a add to the end
> mentality or we keep a long list of value pairs, and userspace passes
> in a list of value keys, and we return a list of the values.

There's already been a fair bit of back and forth on this on this series and
this is where we ended up - a bunch of info ioctls returning big structs full
of values.

I did wonder if io_uring could be used by user-space to efficiently pass a list
of value keys as that's trivial enough to add to drm[1] but perf wise didn't
achieve much over just calling lots of ioctls, at least when I tested with a
mock key/value type interface.

[1] - https://lists.freedesktop.org/archives/dri-devel/2025-September/524469.html

> I'm trying to think ahead towards native virtio context support, and I
> think having the sequence,
> 
> open
> get_all_infos
> create vm/channel
> 
> might have the most optimal ioctl count but also let power management
> avoid booting the GPU on every open if we never create a vm/channel.

Power management is a good point - thus far all the info is cached in nova-core
anyway so there shouldn't be much impact there regardless of what interface
we choose.

Thanks.

 - Alistair

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

[Alistair Popple]

On 2026-09-01 at 06:42 +1000, Gary Guo <gary@garyguo.net> wrote...
> On Mon Aug 31, 2026 at 9:08 PM BST, Danilo Krummrich wrote:
> > On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> >> +/// API handle for the auxiliary bus child drivers to interact with nova-core.
> >> +pub struct NovaCoreApi<'bound> {
> >> +    #[expect(unused)]
> >> +    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
> >> +}
> >> +
> >> +impl NovaCoreApi<'_> {
> >> +    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
> >> +    /// by nova-core.
> >> +    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> >> +        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
> >> +    }
> >> +}
> >
> > CovariantForLt does not hold anymore on latest drm-rust-next, as Cmdq has a
> > Mutex. So, this needs ForLt now and therefore the approach that I shared in [1]
> > a while ago. I applied the changes in [2] to fix it up.

I can see why the mutex means covariance no longer holds, but I would kind
of also expect it to not compile given it surely can't be safe here to treat
an invariant type as covariant. Is this just a limitation of the current
CovariantForLt implementation not being able to prove covariance or am I missing
something else? Thanks.

 - Alistair

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

[Danilo Krummrich]

On Tue Sep 1, 2026 at 9:07 AM CEST, Alistair Popple wrote:
> On 2026-09-01 at 06:42 +1000, Gary Guo <gary@garyguo.net> wrote...
>> On Mon Aug 31, 2026 at 9:08 PM BST, Danilo Krummrich wrote:
>> > On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
>> >> +/// API handle for the auxiliary bus child drivers to interact with nova-core.
>> >> +pub struct NovaCoreApi<'bound> {
>> >> +    #[expect(unused)]
>> >> +    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
>> >> +}
>> >> +
>> >> +impl NovaCoreApi<'_> {
>> >> +    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
>> >> +    /// by nova-core.
>> >> +    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
>> >> +        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
>> >> +    }
>> >> +}
>> >
>> > CovariantForLt does not hold anymore on latest drm-rust-next, as Cmdq has a
>> > Mutex. So, this needs ForLt now and therefore the approach that I shared in [1]
>> > a while ago. I applied the changes in [2] to fix it up.
>
> I can see why the mutex means covariance no longer holds, but I would kind
> of also expect it to not compile given it surely can't be safe here to treat
> an invariant type as covariant. Is this just a limitation of the current
> CovariantForLt implementation not being able to prove covariance or am I missing
> something else? Thanks.

Well, it did not compile (as expected) on my end, which is how I caught it.
What's your base revision?

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Tue Sep 1, 2026 at 7:09 AM CEST, Alistair Popple wrote:
> On 2026-09-01 at 14:50 +1000, Dave Airlie <airlied@gmail.com> wrote...
>> > >
>> > > We should make the NovaCoreApi just provide an accessor for &Spec and make every
>> > > subsequent method we need public. Otherwise we end up with endless forwarding
>> > > methods. We can also add as_raw() methods to the specific types as needed.
>> >
>> > Ok. This is where I don't have a good instinct for what we think should be an
>> > accessor/forwarding method vs. where we should just expose the underlying data
>> > structure and required methods to API users.
>> >
>> > In the past it seems there's been some resistance to exposing nova-core or gsp
>> > data structures like this which is why I added the forwarding methods. In future
>> > we're going to have other data-structures that NovaCoreApi will need to access
>> > so it would be good to understand what we should do here so we can keep things
>> > somewhat consistent.
>> 
>> We can expose structure defined in nova-core, we cannot expose
>> structures defined in gsp bindings or firmware.

Yes, those structures are not even exposed to the nova-core layers we are
dealing with here.

There are the structures abstracting firmware interfaces, but despite exposing
trivial cached values, they should not exposed to nova-drm either.

Instead nova-drm should call into nova-core and ask it to do things on its
behalf.

More in general, the same is true for any other device resources. For instance,
we also should not expose the pci::Device or the pci::Bar to nova-drm, but again
provide a higher level API.

> All the fields of self.gpu are structures defined as rust native structures in
> nova-core. Their values may be decoded or derived from GSP responses, but by
> design none of the raw structures from gsp bindings live in self.gpu AFAIK.

We could indeed expose all values from struct Gpu directly and control
everything through visibility, but ...

>> In theory we can internally between core/drm but I'd really really
>> like to keep that boundary as the limits of GSP for auditability
>> purposes.

... exposing struct Gpu and controlling everything through visibility still
means that we could easily miss some device resource being exposed directly to
nova-drm, which also includes GSP abstractions.

So, I think we should provide accessors for the major structures that we
intentionally want to expose (so we have an obvious boundary), but not for every
single fields inside them.

This is also why I said that we can expose an accessor for struct Spec, which I
think serves as a good example.

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

[Alistair Popple]

On 2026-09-01 at 17:14 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Tue Sep 1, 2026 at 9:07 AM CEST, Alistair Popple wrote:
> > On 2026-09-01 at 06:42 +1000, Gary Guo <gary@garyguo.net> wrote...
> >> On Mon Aug 31, 2026 at 9:08 PM BST, Danilo Krummrich wrote:
> >> > On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> >> >> +/// API handle for the auxiliary bus child drivers to interact with nova-core.
> >> >> +pub struct NovaCoreApi<'bound> {
> >> >> +    #[expect(unused)]
> >> >> +    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
> >> >> +}
> >> >> +
> >> >> +impl NovaCoreApi<'_> {
> >> >> +    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
> >> >> +    /// by nova-core.
> >> >> +    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
> >> >> +        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
> >> >> +    }
> >> >> +}
> >> >
> >> > CovariantForLt does not hold anymore on latest drm-rust-next, as Cmdq has a
> >> > Mutex. So, this needs ForLt now and therefore the approach that I shared in [1]
> >> > a while ago. I applied the changes in [2] to fix it up.
> >
> > I can see why the mutex means covariance no longer holds, but I would kind
> > of also expect it to not compile given it surely can't be safe here to treat
> > an invariant type as covariant. Is this just a limitation of the current
> > CovariantForLt implementation not being able to prove covariance or am I missing
> > something else? Thanks.
> 
> Well, it did not compile (as expected) on my end, which is how I caught it.
> What's your base revision?

I'm on drm-rust-next which I believe is:

commit b705c185105762676aa6ec16cf976101df87cc35 (drm-rust/for-linux-next, drm-rust/drm-rust-next)
Author: Alexandre Courbot <acourbot@nvidia.com>
Date:   Thu Jul 23 22:54:37 2026 +0900

    gpu: nova-core: fix incorrect naming/framing of GSP-FMC firmware

Reason it compiles is because the the Gsp has no lifetime parameter in the chain
to the mutex:

Gpu<'a> -> GspResources<'a> -> Gsp -> Cmdq -> Mutex<CmdqInner>

Which I think means using ConvariantForLt is fine in this context, unless there
is some other change coming which requires the lifetime. John's IRQ series hides
the Cmdq behind an Arc<> but that still works as there's no lifetime parameter
there either.

Not sure why you're seeing failures, what base revision are you using? (and
apologies, next time I will tell git to add the base sha1 to the series).

 - Alistair

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

[Danilo Krummrich]

On Tue Sep 1, 2026 at 11:13 AM CEST, Alistair Popple wrote:
> Reason it compiles is because the the Gsp has no lifetime parameter in the chain
> to the mutex:
>
> Gpu<'a> -> GspResources<'a> -> Gsp -> Cmdq -> Mutex<CmdqInner>

Ah, that's on me then, I applied [1] before your series.

In any case, I think we should not write all the code assuming covariance
regardless. Once we do some more series stuff in nova-drm, we will need more
locks in nova-core around types that carry device resources.

[1] https://lore.kernel.org/driver-core/20260830193824.471089-1-dakr@kernel.org/

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

[Danilo Krummrich]

On Mon Aug 31, 2026 at 10:42 PM CEST, Gary Guo wrote:
> I think if you change the signatue of `registration_data_with` slightly:
>
>     pub fn registration_data_with<'this, F: ForLt + 'static, R>(
>         &'this self,
>         f: impl for<'a> FnOnce(Pin<&'this F::Of<'a>>) -> R,
>                                     ^ note this is changed from 'a to 'this
>     ) -> Result<R>;
>
> then there will be an implied bound available inside the callback where 'a
> outlives 'this, and thus the function callback is able to perform coercion of
> any T<'a> to T<'this> provided that `T` is covariant over lifetime `'a`.
>
> [ The coercion won't work when doing abstract `F::Of` on the bus abstraction
> side, but for any user it is dealing with concrete types so the compiler sees
> specific types and thus can check variance ]
>
> Then your projection can just be
>
>     aux.registration_data_project(|x| &x.field)
>
> I haven't tried it out but I think it should work.

I gave this a shot and it seems to work out, it's a good simplification. I think
we don't even need a dedicated project method in this case. We could add an
alias for with() just to clarify the intent, but not sure that's worth.

@Alistair: Here's the diff I tested this with:

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 798b14f33e20..9babaa0b9a5f 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -13,6 +13,7 @@
         gem::BaseObject,
         Registered, //
     },
+    num::Bounded,
     prelude::*,
     transmute::AsBytes,
     uaccess::UserSlice,
@@ -32,10 +33,12 @@

 impl GpuInfo {
     fn new(reg_data: &DrmRegData<'_>) -> Self {
+        let spec = reg_data.api.with(|api| api.get_ref().spec());
+
         reg_data.api.with(|api| {
             Self(uapi::drm_nova_gpu_info {
-                architecture: api.architecture(),
-                implementation: api.implementation(),
+                architecture: u32::from(Bounded::from(spec.chipset.arch())),
+                implementation: spec.chipset.implementation(),
                 vram_size: api.vram_size(),
                 gpu_name: api.gpu_name(),
                 gpu_short_name: api.gpu_short_name(),
diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index c9ae48d278af..6825d3562d56 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -13,6 +13,8 @@
     types::ForLt, //
 };

+pub use crate::gpu::Spec;
+
 use crate::gpu::{
     Gpu, //
 };
@@ -44,14 +46,9 @@ pub fn handle(adev: &auxiliary::Device<Bound>) -> Result<NovaCoreApiHandle<'_>>
         NovaCoreApiHandle::of(adev)
     }

-    /// Returns the architecture identifier of this GPU.
-    pub fn architecture(&self) -> u32 {
-        self.gpu.spec.chipset.arch() as u32
-    }
-
-    /// Returns the implementation identifier of this GPU.
-    pub fn implementation(&self) -> u32 {
-        self.gpu.spec.chipset.implementation()
+    /// Returns the GPU [`Spec`].
+    pub fn spec(&self) -> &Spec {
+        &self.gpu.spec
     }

     /// Returns the size of the PCIe BAR used for accessing VRAM, typically
@@ -78,7 +75,9 @@ fn of(adev: &'a auxiliary::Device<Bound>) -> Result<Self> {
     }

     /// Access the [`NovaCoreApi`] through a closure.
-    pub fn with<R>(&self, f: impl for<'b> FnOnce(Pin<&'b NovaCoreApi<'b>>) -> R) -> R {
+    ///
+    /// References to covariant sub-fields can be returned from the closure directly.
+    pub fn with<R>(&self, f: impl for<'b> FnOnce(Pin<&'a NovaCoreApi<'b>>) -> R) -> R {
         self.adev
             .registration_data_with::<ForLt!(NovaCoreApi<'_>), R>(f)
             .expect("TypeId was validated in NovaCoreApiHandle::of()")
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 166f4bb55752..04b97b0f89f4 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -42,7 +42,8 @@ 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 {
+        #[allow(missing_docs)]
+        pub enum Chipset {
             $($variant = $value),*,
         }

@@ -119,7 +120,8 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
 });

 impl Chipset {
-    pub(crate) const fn arch(self) -> Architecture {
+    /// Returns the [`Architecture`] generation of this chipset.
+    pub const fn arch(self) -> Architecture {
         match self {
             Self::TU102 | Self::TU104 | Self::TU106 | Self::TU117 | Self::TU116 => {
                 Architecture::Turing
@@ -138,8 +140,8 @@ pub(crate) const fn arch(self) -> Architecture {
         }
     }

-    /// Returns the implementation identifier of this chipset.
-    pub(crate) const fn implementation(self) -> u32 {
+    /// Returns the implementation identifier of this chipset within its architecture.
+    pub const fn implementation(self) -> u32 {
         self as u32 & 0xf
     }

@@ -167,7 +169,8 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     /// Enum representation of the GPU generation.
     #[derive(fmt::Debug, Copy, Clone)]
     #[repr(u32)]
-    pub(crate) enum Architecture with TryFrom<Bounded<u32, 6>> {
+    #[allow(missing_docs)]
+    pub enum Architecture with TryFrom<Bounded<u32, 6>> {
         Turing = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_TURING,
         Ampere = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_AMPERE,
         Hopper = uapi::drm_nova_architecture_NOVA_DRM_ARCHITECTURE_HOPPER,
@@ -202,8 +205,9 @@ 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 {
-    pub(crate) chipset: Chipset,
+pub struct Spec {
+    /// The GPU chipset.
+    pub chipset: Chipset,
     revision: Revision,
 }

diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 60dfbec8f330..06f816420790 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -305,6 +305,10 @@ unsafe fn registration_data_pinned<F: ForLt + 'static>(&self) -> Result<Pin<&F::
     /// `F` is the [`ForLt`](trait@ForLt) encoding of the data type. The closure receives a pinned
     /// reference to the registration data.
     ///
+    /// The outer reference carries the `&self` lifetime while the inner type carries the HRTB
+    /// lifetime `'a`, implying `'a` outlives `&self`. This allows the closure to coerce covariant
+    /// sub-fields (e.g. `&'a T` to the caller's lifetime) and return them directly in `R`.
+    ///
     /// For covariant types that implement [`trait@CovariantForLt`], prefer
     /// [`registration_data`](Self::registration_data) which returns a direct reference.
     ///
@@ -314,13 +318,14 @@ unsafe fn registration_data_pinned<F: ForLt + 'static>(&self) -> Result<Pin<&F::
     /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was
     /// registered by a C driver.
     #[inline]
-    pub fn registration_data_with<F: ForLt + 'static, R>(
-        &self,
-        f: impl for<'a> FnOnce(Pin<&'a F::Of<'a>>) -> R,
+    pub fn registration_data_with<'this, F: ForLt + 'static, R>(
+        &'this self,
+        f: impl for<'a> FnOnce(Pin<&'this F::Of<'a>>) -> R,
     ) -> Result<R> {
-        // SAFETY: The HRTB closure prevents the caller from smuggling in references with a
-        // concrete short lifetime, making the round-trip from `'static` sound regardless of
-        // variance.
+        // SAFETY: The HRTB on the inner type prevents the caller from exploiting a specific
+        // choice of `'a`. Covariant sub-fields can be safely coerced to `'this`, while
+        // invariant fields cannot be coerced and thus cannot escape with an incorrect
+        // lifetime.
         let pinned = unsafe { self.registration_data_pinned::<F>()? };

         Ok(f(pinned))

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 0c12ef145981..740466af268d 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -138,6 +138,11 @@ pub(crate) const fn arch(self) -> Architecture {
>          }
>      }
>  
> +    /// Returns the implementation identifier of this chipset.
> +    pub(crate) const fn implementation(self) -> u32 {
> +        self as u32 & 0xf
> +    }

I missed this part in my previous reply. Besides being a bit unfortunate that we
have to reimplement what boot42.implementation() already gives us, I think the
value is not overly useful anyway.

I get the intent, architecture and implementation complement each other, but in
practice we are not interested in the implementation bits, but either in a
unique chip identifier or the architecture.

If you look at the nova-core code you will find exactly that, we either check
for a specific chip or an architecture and I think userspace will be intersted
in the same.

So, I think the uAPI should provide the architecture and a unique chip
identifier.

Before we circle back, I know that the unique chip identifier in nova-core
technically contains the architecture for obvious reasons, but my point has
always been that we can give the decoded architecture to userspace and not
require it to know about and extract it from the chip identifier we consider
opaque in the uAPI.

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

[Gary Guo]

On Tue Sep 1, 2026 at 11:27 AM BST, Danilo Krummrich wrote:
> On Mon Aug 31, 2026 at 10:42 PM CEST, Gary Guo wrote:
>> I think if you change the signatue of `registration_data_with` slightly:
>>
>>     pub fn registration_data_with<'this, F: ForLt + 'static, R>(
>>         &'this self,
>>         f: impl for<'a> FnOnce(Pin<&'this F::Of<'a>>) -> R,
>>                                     ^ note this is changed from 'a to 'this
>>     ) -> Result<R>;
>>
>> then there will be an implied bound available inside the callback where 'a
>> outlives 'this, and thus the function callback is able to perform coercion of
>> any T<'a> to T<'this> provided that `T` is covariant over lifetime `'a`.
>>
>> [ The coercion won't work when doing abstract `F::Of` on the bus abstraction
>> side, but for any user it is dealing with concrete types so the compiler sees
>> specific types and thus can check variance ]
>>
>> Then your projection can just be
>>
>>     aux.registration_data_project(|x| &x.field)
>>
>> I haven't tried it out but I think it should work.
>
> I gave this a shot and it seems to work out, it's a good simplification. I think
> we don't even need a dedicated project method in this case. We could add an
> alias for with() just to clarify the intent, but not sure that's worth.

Right, I don't think it's needed. It's just a typo in the example given that I
write "_project" instead of "_with". Unless you have a case where it actually
needs `&'a F::Of<'a>` instead of `&'this F::Of<'a>` (which I think is not very
likely, changing signatures of existing users should be sufficient), I think we
should just modify the signature of `registration_data_with`.

Best,
Gary

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Tue Sep 1, 2026 at 12:38 PM CEST, Danilo Krummrich wrote:
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 0c12ef145981..740466af268d 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -138,6 +138,11 @@ pub(crate) const fn arch(self) -> Architecture {
>>          }
>>      }
>>  
>> +    /// Returns the implementation identifier of this chipset.
>> +    pub(crate) const fn implementation(self) -> u32 {
>> +        self as u32 & 0xf
>> +    }
>
> I missed this part in my previous reply. Besides being a bit unfortunate that we
> have to reimplement what boot42.implementation() already gives us, I think the
> value is not overly useful anyway.
>
> I get the intent, architecture and implementation complement each other, but in
> practice we are not interested in the implementation bits, but either in a
> unique chip identifier or the architecture.
>
> If you look at the nova-core code you will find exactly that, we either check
> for a specific chip or an architecture and I think userspace will be intersted
> in the same.
>
> So, I think the uAPI should provide the architecture and a unique chip
> identifier.
>
> Before we circle back, I know that the unique chip identifier in nova-core
> technically contains the architecture for obvious reasons, but my point has
> always been that we can give the decoded architecture to userspace and not
> require it to know about and extract it from the chip identifier we consider
> opaque in the uAPI.

IOW, we should not think of this in terms of the numbers/values exposed by some
register. All the users (including nova-core itself) don't really care about the
values behind the enum, how it composes and how it is related to other values,
that's just an implementation detail.

All users care about is that they have an architecture and chip identifier to
compare against. I.e. there's no value letting userspace think of the chip
identifier as architecture/implementation tuple, since the implementation value
by itself is rather useless.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-02 at 03:01 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Tue Sep 1, 2026 at 12:38 PM CEST, Danilo Krummrich wrote:
> > On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> >> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> >> index 0c12ef145981..740466af268d 100644
> >> --- a/drivers/gpu/nova-core/gpu.rs
> >> +++ b/drivers/gpu/nova-core/gpu.rs
> >> @@ -138,6 +138,11 @@ pub(crate) const fn arch(self) -> Architecture {
> >>          }
> >>      }
> >>  
> >> +    /// Returns the implementation identifier of this chipset.
> >> +    pub(crate) const fn implementation(self) -> u32 {
> >> +        self as u32 & 0xf
> >> +    }
> >
> > I missed this part in my previous reply. Besides being a bit unfortunate that we
> > have to reimplement what boot42.implementation() already gives us, I think the
> > value is not overly useful anyway.
> >
> > I get the intent, architecture and implementation complement each other, but in
> > practice we are not interested in the implementation bits, but either in a
> > unique chip identifier or the architecture.
> >
> > If you look at the nova-core code you will find exactly that, we either check
> > for a specific chip or an architecture and I think userspace will be intersted
> > in the same.
> >
> > So, I think the uAPI should provide the architecture and a unique chip
> > identifier.
> >
> > Before we circle back, I know that the unique chip identifier in nova-core
> > technically contains the architecture for obvious reasons, but my point has
> > always been that we can give the decoded architecture to userspace and not
> > require it to know about and extract it from the chip identifier we consider
> > opaque in the uAPI.

This is why I removed the opaque chip-id. If it's opaque, and the kernel
provides the architecture what information is user-space allowed to derive
about a GPU from the chip-id? Nothing? Everything? Or everything except the
chip architecture?

Nothing seemed like the only reasonable answer, as it's always best to have a
single source of truth and deriving everything except arch seemed odd. But then
architecture alone, as currently defined, isn't sufficient for user-space - it
needs the implementation in some form to derive the compute capabilities of the
GPU amoung other things.

> IOW, we should not think of this in terms of the numbers/values exposed by some
> register. All the users (including nova-core itself) don't really care about the
> values behind the enum, how it composes and how it is related to other values,
> that's just an implementation detail.

Right, the question isn't how this information is encoded but what gurantees
the kernel provide to user-space when it returns a particular architecture,
implementation or chip-id and what assumptions is user-space allowed to make
based on the results.

> All users care about is that they have an architecture and chip identifier to
> compare against. I.e. there's no value letting userspace think of the chip
> identifier as architecture/implementation tuple, since the implementation value
> by itself is rather useless.

Except the architecture/implementation tuple is exaclty what user-space needs to
eg. figure out what SM to compile for.

So yes, the implementation value by itself is rather useless, but so is
the architecture. That doesn't imply that providing it as a seperate value
is useless. I've done some research into our current SW stack to see how
implementation is used today and it's basically used to subclass each GPU.

For example the base arch value is used to fill in a *lot* of static information
about the GPU based on architecture, and then the specific implementation is
used to over-ride or fill in yet more static device information. In fact in
userspace it seems we very rarely look at the architecture in isolation from the
implementation, so in many senses providing an arch value on it's own is also
rather useless.

As a concrete example the way this is currently used, in both CUDA and Mesa, is
a lookup table for arch+impl to figure out eg. what SM version a chip supports.
So having architecture seperately decoded but a chip-id instead of a seperately
decoded implementation would mean user-space just has to look at chip-id for
most things and ignore the architecture anyway.

For example Mesa currently has this to figure out SM version:

static uint8_t
sm_for_chipset(uint16_t chipset)
{
	if (chipset >= 0x1b0)
		return 120;
	else if (chipset >= 0x1a0)
		return 100;
	else if (chipset >= 0x190)
		return 89;
	// GH100 is older than AD10X, but is SM90
	else if (chipset >= 0x180)
		return 90;
	else if (chipset == 0x17b)
		return 87;
	else if (chipset >= 0x172)
		return 86;
	else if (chipset >= 0x170)
		return 80;
	...
}

And to be clear we don't care about the specific encodings in this example. The
point is the SM version can't be looked up from architecture alone, it needs the
implementation as well and if the only way to get that is from opaque chip-id
that's all user-space will look at. Eg:

static uint8_t
sm_for_chipset(enum chip_id chip)
{
	switch (chip) {
		case NOVA_GPU_CHIP_GA100:
			return 80;
		case NOVA_GPU_CHIP_GA101:
			return 86;
		case NOVA_GPU_CHIP_GA102:
			return 86;
		case NOVA_GPU_CHIP_GA10B:
			return 87;
		...
	}
}

So I'm ok with providing either a decoded architecture and implementation
xor an opaque chip-id. Or alternatively maybe architecture includes the
implementation (ie. we rename the opaque chip_id to architecture). But treating
the implementation and architecture values differently and only providing one
directly doesn't make much sense IMHO.

- Alistair

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-01 at 17:29 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Tue Sep 1, 2026 at 7:09 AM CEST, Alistair Popple wrote:
> > On 2026-09-01 at 14:50 +1000, Dave Airlie <airlied@gmail.com> wrote...
> >> > >
> >> > > We should make the NovaCoreApi just provide an accessor for &Spec and make every
> >> > > subsequent method we need public. Otherwise we end up with endless forwarding
> >> > > methods. We can also add as_raw() methods to the specific types as needed.
> >> >
> >> > Ok. This is where I don't have a good instinct for what we think should be an
> >> > accessor/forwarding method vs. where we should just expose the underlying data
> >> > structure and required methods to API users.
> >> >
> >> > In the past it seems there's been some resistance to exposing nova-core or gsp
> >> > data structures like this which is why I added the forwarding methods. In future
> >> > we're going to have other data-structures that NovaCoreApi will need to access
> >> > so it would be good to understand what we should do here so we can keep things
> >> > somewhat consistent.
> >> 
> >> We can expose structure defined in nova-core, we cannot expose
> >> structures defined in gsp bindings or firmware.
> 
> Yes, those structures are not even exposed to the nova-core layers we are
> dealing with here.
> 
> There are the structures abstracting firmware interfaces, but despite exposing
> trivial cached values, they should not exposed to nova-drm either.
> 
> Instead nova-drm should call into nova-core and ask it to do things on its
> behalf.
> 
> More in general, the same is true for any other device resources. For instance,
> we also should not expose the pci::Device or the pci::Bar to nova-drm, but again
> provide a higher level API.
> 
> > All the fields of self.gpu are structures defined as rust native structures in
> > nova-core. Their values may be decoded or derived from GSP responses, but by
> > design none of the raw structures from gsp bindings live in self.gpu AFAIK.
> 
> We could indeed expose all values from struct Gpu directly and control
> everything through visibility, but ...
> 
> >> In theory we can internally between core/drm but I'd really really
> >> like to keep that boundary as the limits of GSP for auditability
> >> purposes.
> 
> ... exposing struct Gpu and controlling everything through visibility still
> means that we could easily miss some device resource being exposed directly to
> nova-drm, which also includes GSP abstractions.

The individual fields would still need to be explicitly marked as `pub`, so it's
not like just exposing the top-level data structure suddenly exposes all these
device resources without them being explicitly marked as accessible.

> So, I think we should provide accessors for the major structures that we
> intentionally want to expose (so we have an obvious boundary), but not for every
> single fields inside them.

Sure, I don't really mind either way. I just figured that's what the point
of Rust visibiity was, to provide a simple boiler-plate free way of saying a
type/method/field was used externally. But will switch this to using accessors
for major data structures instead given that seems preferred.
 
> This is also why I said that we can expose an accessor for struct Spec, which I
> think serves as a good example.

Ok, so just to double check are you happy with an accessor to return
gsp_static_info as well?

Thanks.

 - Alistair

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

[Alistair Popple]

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

Not being a Rust variance ninja it took me a while to figure this out :) But now
that I have I agree the projections for 'static at least are quite nice and I
think Gary's suggestion helps a bit as well for non-static. So assuming you're
ok with me adding this code with your Co-developed-by or other appropriate tag
then I will incorporate this idea into v6 so everything keeps working if/when
Covariance no longer holds for Gpu.

Thanks!

 - Alistair

> 
> Thanks,
> Danilo
> 
> [1] https://lore.kernel.org/all/DJQSY9ZY5M5F.3VI537GS00E1G@kernel.org/
> [2] CovariantForLt to ForLt changes for nova-core
> 
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index 7fc0baef5f04..028020213ac5 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -1,7 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> 
> -use core::pin::Pin;
> -
>  use kernel::{
>      auxiliary,
>      device::{
> @@ -20,7 +18,10 @@
>  use crate::file::File;
>  use crate::gem::NovaObject;
> 
> -use nova_core::api::NovaCoreApi;
> +use nova_core::api::{
> +    NovaCoreApi,
> +    NovaCoreApiHandle, //
> +};
> 
>  pub(crate) struct NovaDriver;
> 
> @@ -32,7 +33,7 @@ pub(crate) struct Nova<'bound> {
> 
>  /// DRM registration data, accessible from ioctl handlers via the registration guard.
>  pub(crate) struct DrmRegData<'bound> {
> -    pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
> +    pub(crate) api: NovaCoreApiHandle<'bound>,
>  }
> 
>  /// Convienence type alias for the DRM device type for this driver
> @@ -69,7 +70,7 @@ fn probe<'bound>(
>      ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
>          let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
>          let reg_data = DrmRegData {
> -            api: NovaCoreApi::of(adev)?,
> +            api: NovaCoreApi::handle(adev)?,
>          };
>          // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is
>          // never forgotten.
> diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
> index e1223a29f8b1..798b14f33e20 100644
> --- a/drivers/gpu/drm/nova/file.rs
> +++ b/drivers/gpu/drm/nova/file.rs
> @@ -32,13 +32,15 @@
> 
>  impl GpuInfo {
>      fn new(reg_data: &DrmRegData<'_>) -> Self {
> -        Self(uapi::drm_nova_gpu_info {
> -            architecture: reg_data.api.architecture(),
> -            implementation: reg_data.api.implementation(),
> -            vram_size: reg_data.api.vram_size(),
> -            gpu_name: reg_data.api.gpu_name(),
> -            gpu_short_name: reg_data.api.gpu_short_name(),
> -            gpu_gid: reg_data.api.gpu_gid(),
> +        reg_data.api.with(|api| {
> +            Self(uapi::drm_nova_gpu_info {
> +                architecture: api.architecture(),
> +                implementation: api.implementation(),
> +                vram_size: api.vram_size(),
> +                gpu_name: api.gpu_name(),
> +                gpu_short_name: api.gpu_short_name(),
> +                gpu_gid: api.gpu_gid(),
> +            })
>          })
>      }
>  }
> @@ -74,7 +76,7 @@ pub(crate) fn get_param(
>          _file: &drm::File<File>,
>      ) -> Result<u32> {
>          let value = match getparam.param as u32 {
> -            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.bar1_size()?,
> +            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.with(|api| api.bar1_size())?,
>              _ => return Err(EINVAL),
>          };
> 
> diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
> index ad4b62db1e5f..c9ae48d278af 100644
> --- a/drivers/gpu/nova-core/api.rs
> +++ b/drivers/gpu/nova-core/api.rs
> @@ -10,7 +10,7 @@
>      device::Bound,
>      pci,
>      prelude::*,
> -    types::CovariantForLt, //
> +    types::ForLt, //
>  };
> 
>  use crate::gpu::{
> @@ -39,10 +39,9 @@ impl NovaCoreApi<'_> {
>          *self.gpu.gsp_static_info.gpu_gid()
>      }
> 
> -    /// 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<'_>)>()
> +    /// Obtain a [`NovaCoreApiHandle`] from an auxiliary device registered by nova-core.
> +    pub fn handle(adev: &auxiliary::Device<Bound>) -> Result<NovaCoreApiHandle<'_>> {
> +        NovaCoreApiHandle::of(adev)
>      }
> 
>      /// Returns the architecture identifier of this GPU.
> @@ -66,3 +65,22 @@ pub fn vram_size(&self) -> u64 {
>          self.gpu.gsp_static_info.vram_size()
>      }
>  }
> +
> +/// 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 {
> +        self.adev
> +            .registration_data_with::<ForLt!(NovaCoreApi<'_>), R>(f)
> +            .expect("TypeId was validated in NovaCoreApiHandle::of()")
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 4d3c18d6a733..025ba2869d6c 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -15,7 +15,7 @@
>          Atomic,
>          Relaxed, //
>      },
> -    types::CovariantForLt,
> +    types::ForLt,
>  };
> 
>  use crate::{
> @@ -29,7 +29,7 @@
>  #[pin_data]
>  pub(crate) struct NovaCore<'bound> {
>      #[allow(clippy::type_complexity)]
> -    _reg: auxiliary::Registration<'bound, CovariantForLt!(NovaCoreApi<'_>)>,
> +    _reg: auxiliary::Registration<'bound, ForLt!(NovaCoreApi<'_>)>,
>      #[pin]
>      pub(crate) gpu: Gpu<'bound>,
>      bar: pci::Bar<'bound, BAR0_SIZE>,

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-02 at 15:21 +1000, Alistair Popple <apopple@nvidia.com> wrote...
> On 2026-09-01 at 17:29 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> > On Tue Sep 1, 2026 at 7:09 AM CEST, Alistair Popple wrote:
> > > On 2026-09-01 at 14:50 +1000, Dave Airlie <airlied@gmail.com> wrote...
> > >> > >
> > >> > > We should make the NovaCoreApi just provide an accessor for &Spec and make every
> > >> > > subsequent method we need public. Otherwise we end up with endless forwarding
> > >> > > methods. We can also add as_raw() methods to the specific types as needed.
> > >> >
> > >> > Ok. This is where I don't have a good instinct for what we think should be an
> > >> > accessor/forwarding method vs. where we should just expose the underlying data
> > >> > structure and required methods to API users.
> > >> >
> > >> > In the past it seems there's been some resistance to exposing nova-core or gsp
> > >> > data structures like this which is why I added the forwarding methods. In future
> > >> > we're going to have other data-structures that NovaCoreApi will need to access
> > >> > so it would be good to understand what we should do here so we can keep things
> > >> > somewhat consistent.
> > >> 
> > >> We can expose structure defined in nova-core, we cannot expose
> > >> structures defined in gsp bindings or firmware.
> > 
> > Yes, those structures are not even exposed to the nova-core layers we are
> > dealing with here.
> > 
> > There are the structures abstracting firmware interfaces, but despite exposing
> > trivial cached values, they should not exposed to nova-drm either.
> > 
> > Instead nova-drm should call into nova-core and ask it to do things on its
> > behalf.
> > 
> > More in general, the same is true for any other device resources. For instance,
> > we also should not expose the pci::Device or the pci::Bar to nova-drm, but again
> > provide a higher level API.
> > 
> > > All the fields of self.gpu are structures defined as rust native structures in
> > > nova-core. Their values may be decoded or derived from GSP responses, but by
> > > design none of the raw structures from gsp bindings live in self.gpu AFAIK.
> > 
> > We could indeed expose all values from struct Gpu directly and control
> > everything through visibility, but ...
> > 
> > >> In theory we can internally between core/drm but I'd really really
> > >> like to keep that boundary as the limits of GSP for auditability
> > >> purposes.
> > 
> > ... exposing struct Gpu and controlling everything through visibility still
> > means that we could easily miss some device resource being exposed directly to
> > nova-drm, which also includes GSP abstractions.
> 
> The individual fields would still need to be explicitly marked as `pub`, so it's
> not like just exposing the top-level data structure suddenly exposes all these
> device resources without them being explicitly marked as accessible.
> 
> > So, I think we should provide accessors for the major structures that we
> > intentionally want to expose (so we have an obvious boundary), but not for every
> > single fields inside them.
> 
> Sure, I don't really mind either way. I just figured that's what the point
> of Rust visibiity was, to provide a simple boiler-plate free way of saying a
> type/method/field was used externally. But will switch this to using accessors
> for major data structures instead given that seems preferred.

That said with the changes required for Invariant Gpu and being able to project
Covariant fields out I can see why accessors for top-level data structures might
be useful, particularly if some of them (eg. Spec) are Covariant.

> > This is also why I said that we can expose an accessor for struct Spec, which I
> > think serves as a good example.
> 
> Ok, so just to double check are you happy with an accessor to return
> gsp_static_info as well?

Still interested in your thoughts here though, just to avoid churn because I'll
probably pick the wrong answer :-)

 - Alistair

> Thanks.
> 
>  - Alistair

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Wed Sep 2, 2026 at 4:38 AM CEST, Alistair Popple wrote:
> So I'm ok with providing either a decoded architecture and implementation
> xor an opaque chip-id. Or alternatively maybe architecture includes the
> implementation (ie. we rename the opaque chip_id to architecture). But treating
> the implementation and architecture values differently and only providing one
> directly doesn't make much sense IMHO.

I think this is you still thinking about this in terms of register encoding.

Look at nova-core, there we are caring about two things chipid and architecture,
but never about the implementation bits.

You won't see any

	if (arch == X && impl == Y)

checks anywhere, because it would be unnecessarily complicated and error prone.
You will instead find checks for the architecture or the chipid directly, such
as in:

	/// Returns the HAL corresponding to `chipset`.
	pub(super) fn fb_hal(chipset: Chipset) -> &'static dyn FbHal {
	    match chipset.arch() {
	        Architecture::Turing => tu102::TU102_HAL,
	        Architecture::Ampere if chipset == Chipset::GA100 => ga100::GA100_HAL,
	        Architecture::Ampere | Architecture::Ada => ga102::GA102_HAL,
	        Architecture::Hopper => gh100::GH100_HAL,
	        Architecture::BlackwellGB10x => gb100::GB100_HAL,
	        Architecture::BlackwellGB20x => gb202::GB202_HAL,
	    }
	}

See? We never care about the implementation part, as it is just an
implementation detail of the encoding of the chipid that no one should ever
bother with.

Yes, the check in fb_hal() could technically be

	arch == Architecture::Ampere && impl == 0

but that's arguably worse than what fb_hal() does today for many reasons.

To name just one of them: We'd entirely lose the guarantee that the combination
of arch and impl even exists in the first place. Even with a new type this
wouldn't go away, since not every Implementation would be valid for any
Architecture.

Also, look at the code that you had to write to even expose the implementation
bits in the first place.

	pub(crate) const fn implementation(self) -> u32 {
	    self as u32 & 0xf
	} 

Notice the tension it creates? You have to reimplement what the lower layer of
the register encoding already does and intentionally hides in favor of providing
a chipset() accessor.

Also note that with this you get a raw integer that is kinda ugly, because you
can't even make up a useful new type:

For Architecture the variants are obvious and meaningful (Turing, Ampere, etc.),
for Chipset the variants are obvious and meaningful too (TU102, AD107, etc.).
But what would the variants for Implementation look like? "Zero", "Two", etc.?

> Except the architecture/implementation tuple is exaclty what user-space needs to
> eg. figure out what SM to compile for.

It doesn't need an architecture/implementation tuple, it needs a chipid for this
lookup. (Although it might be questionable whether userspace should have this
lookup table in the first place; see below.)

Both the KMD and the UMD only ever care about the architecture or the chipid.
The fact that the chipid is defined by an architecture/implementation tuple is
an irrelevant implementation detail not even the kernel cares about.

> And to be clear we don't care about the specific encodings in this example. The
> point is the SM version can't be looked up from architecture alone, it needs the
> implementation as well and if the only way to get that is from opaque chip-id
> that's all user-space will look at. Eg:

Again, it doesn't need the implementation, it needs the chipid. Which also your
code below correctly considers.

That said, if we know that userspace will never need to do an architecture based
check, but always a chipid specific check, it is obviously pointless to expose
it in the first place. But otherwise it should just be chipid and architecture.

> static uint8_t
> sm_for_chipset(enum chip_id chip)
> {
> 	switch (chip) {
> 		case NOVA_GPU_CHIP_GA100:
> 			return 80;
> 		case NOVA_GPU_CHIP_GA101:
> 			return 86;
> 		case NOVA_GPU_CHIP_GA102:
> 			return 86;
> 		case NOVA_GPU_CHIP_GA10B:
> 			return 87;
> 		...
> 	}
> }

That looks reasonable and much better than what mesa has, but if we'd ever care
about the SM value in the kernel, then the kernel should be the single source of
truth for this value and expose it to userspace.

Now, in this case I don't think the kernel really needs the value, but I think
the value is provided by GSP through GR_INFO_INDEX_SM_VERSION?

Given that, the kernel should query it and provide it via its GPU info structure
rather than having userspace invent another lookup table?

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Wed Sep 2, 2026 at 9:05 AM CEST, Alistair Popple wrote:
> On 2026-09-02 at 15:21 +1000, Alistair Popple <apopple@nvidia.com> wrote...
>> Ok, so just to double check are you happy with an accessor to return
>> gsp_static_info as well?
>
> Still interested in your thoughts here though, just to avoid churn because I'll
> probably pick the wrong answer :-)

Yeah, exposing that for gpu_name() seems fine.

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

[Danilo Krummrich]

On Wed Sep 2, 2026 at 8:57 AM CEST, Alistair Popple wrote:
> Not being a Rust variance ninja it took me a while to figure this out :) But now
> that I have I agree the projections for 'static at least are quite nice and I
> think Gary's suggestion helps a bit as well for non-static. So assuming you're
> ok with me adding this code with your Co-developed-by or other appropriate tag
> then I will incorporate this idea into v6 so everything keeps working if/when
> Covariance no longer holds for Gpu.

Sure, I'd suggest to base it off of this diff [1].

Thanks,
Danilo

[1] https://lore.kernel.org/nova-gpu/DL3WPTVM033J.33RWYCZOC67Z1@kernel.org/

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Dave Airlie]

On Thu, 3 Sept 2026 at 05:27, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Sep 2, 2026 at 9:05 AM CEST, Alistair Popple wrote:
> > On 2026-09-02 at 15:21 +1000, Alistair Popple <apopple@nvidia.com> wrote...
> >> Ok, so just to double check are you happy with an accessor to return
> >> gsp_static_info as well?
> >
> > Still interested in your thoughts here though, just to avoid churn because I'll
> > probably pick the wrong answer :-)
>
> Yeah, exposing that for gpu_name() seems fine.

Though userspace doesn't probably need this, but we could I suppose.

Dave.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Wed Sep 2, 2026 at 9:38 PM CEST, Dave Airlie wrote:
> On Thu, 3 Sept 2026 at 05:27, Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed Sep 2, 2026 at 9:05 AM CEST, Alistair Popple wrote:
>> > On 2026-09-02 at 15:21 +1000, Alistair Popple <apopple@nvidia.com> wrote...
>> >> Ok, so just to double check are you happy with an accessor to return
>> >> gsp_static_info as well?
>> >
>> > Still interested in your thoughts here though, just to avoid churn because I'll
>> > probably pick the wrong answer :-)
>>
>> Yeah, exposing that for gpu_name() seems fine.
>
> Though userspace doesn't probably need this, but we could I suppose.

Yeah, it shouldn't be needed for anything, but I also don't think it hurts and
it could be nice to be consistent about the name throughout the different
layers.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-02 at 19:40 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Wed Sep 2, 2026 at 4:38 AM CEST, Alistair Popple wrote:
> > So I'm ok with providing either a decoded architecture and implementation
> > xor an opaque chip-id. Or alternatively maybe architecture includes the
> > implementation (ie. we rename the opaque chip_id to architecture). But treating
> > the implementation and architecture values differently and only providing one
> > directly doesn't make much sense IMHO.
> 
> I think this is you still thinking about this in terms of register encoding.

No, this is just me trying to figure out what user-space would use the
architecture value for, as on it's own it's a pretty a useless value from a
user-space perspective. For user-space to do anything useful it needs to know
the implementation as well regardless of how that's encoded. It can't for
example run anything reliably knowing only the arch AFAICT.

Originally I thought the point of providing the decoded arch rather than
chip-id might have been to allow a kernel to support chips with a known arch
but unknown implementation, because as you rightfully point out the kernel
doesn't (currently at least) care much about the implementation, so an updated
user-space could maybe run fine on the same arch, different implementation.

But we agreed that wasn't desirable, so now I'm still left figuring out why we
need to provide an arch value? The chip-id already entirely encapsulates the
concept, so what can/should user-space use the arch value for?

> > Except the architecture/implementation tuple is exaclty what user-space needs to
> > eg. figure out what SM to compile for.
>
> It doesn't need an architecture/implementation tuple, it needs a chipid for this
> lookup. (Although it might be questionable whether userspace should have this
> lookup table in the first place; see below.)

Maybe I could've been clearer here - what I meant is that userspace needs the
architecture *and* implementation. Whether or not that comes via opaque chip-id
or some other means doesn't matter to me, what matters is that user-space needs
both bits of information - the architecture on it's own is not very helpful.

> Both the KMD and the UMD only ever care about the architecture or the chipid.
> The fact that the chipid is defined by an architecture/implementation tuple is
> an irrelevant implementation detail not even the kernel cares about.
>
> > And to be clear we don't care about the specific encodings in this example. The
> > point is the SM version can't be looked up from architecture alone, it needs the
> > implementation as well and if the only way to get that is from opaque chip-id
> > that's all user-space will look at. Eg:
> 
> Again, it doesn't need the implementation, it needs the chipid. Which also your
> code below correctly considers.

Right. This is what we had agreed on previously - provide an opaque chip-id
userspace can use to lookup whatever chip info it needs. You asked for the arch
value to be provided separately, but I can't figure out what the use-case for it
is and I don't think we should be adding things to the uAPI that we can't even
imagine use-cases for. We can always add it to the uAPI later if we do find a
need, but it's much harder to remove stuff and right now I can't see any reason
to provide it.

Also for a bit of background on this series, which I should have put in the
cover-letter and will next time, the goal is to get a basic deviceQuery sample
running. That's why I started with these properties first, as they're what CUDA
or Mesa need to fully populate their device properties.

> That said, if we know that userspace will never need to do an architecture based
> check, but always a chipid specific check, it is obviously pointless to expose
> it in the first place. But otherwise it should just be chipid and architecture.

That is where I ended and what I had originally - lets just provide the opaque
chip-id. If we provide the chipid why would user-space ever need the arch? What
can it do with an architecture value that it can't do with the chipid? Again I
don't think we should be adding things to the UAPI that userspace has no known
use for.

But it sounds like you might actually be ok with us just providing chip-id?

We already know the full chip-id is needed, so it makes sense to expose that
now.

> > static uint8_t
> > sm_for_chipset(enum chip_id chip)
> > {
> > 	switch (chip) {
> > 		case NOVA_GPU_CHIP_GA100:
> > 			return 80;
> > 		case NOVA_GPU_CHIP_GA101:
> > 			return 86;
> > 		case NOVA_GPU_CHIP_GA102:
> > 			return 86;
> > 		case NOVA_GPU_CHIP_GA10B:
> > 			return 87;
> > 		...
> > 	}
> > }
> 
> That looks reasonable and much better than what mesa has, but if we'd ever care
> about the SM value in the kernel, then the kernel should be the single source of
> truth for this value and expose it to userspace.

Yes, no problem with this.

> Now, in this case I don't think the kernel really needs the value, but I think
> the value is provided by GSP through GR_INFO_INDEX_SM_VERSION?

Argh. There is, shall we say, some nuance here :-) HW have not made our lives
easy and actually figuring out what the version that matters to user-space is
is not as straight forward as one might like, and may well not match what the
kernel would want (if it ever needs it).

> Given that, the kernel should query it and provide it via its GPU info structure
> rather than having userspace invent another lookup table?

It really boils down to how much static config info we want to encode into
kernel lookup tables vs. user-space lookup tables. I don't this we should be
filling the kernel with a bunch of static struct lookups just to ship those
struct definitions to user-space. It seems more reasonable to put that in
user-space, unless of course the kernel needs it.

So I think your "does the kernel need this" is a reasonable benchmark for
deciding this.

As I think you're hinting at it would be ideal if GSP or some other firmware
table could just provide everything in some nice static config table, but I
don't think that's currently possible and looking at other user-space drivers
we're certainly not alone there.

 - Alistair

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Thu Sep 3, 2026 at 3:12 AM CEST, Alistair Popple wrote:
> Originally I thought the point of providing the decoded arch rather than
> chip-id

I think there never was a "rather than". The point was that if userspace has
conditionals based on the architecture it shouldn't have to figure it out based
on the chipid, as this has been done by the kernel already.

> because as you rightfully point out the kernel doesn't (currently at least)
> care much about the implementation

I still don't see how that value will ever be meaningful, there is no such thing
as "all GPUs of a certain implementation regardless of architecture" have
something in common, is it?

But since you say "currently at least", are there any plans to give this value
some meaning beyond being a unique counter for chips within a certain
architecture?

If so, I think that'd be a horrible way to encode some chip commonality.

> But we agreed that wasn't desirable, so now I'm still left figuring out why we
> need to provide an arch value? The chip-id already entirely encapsulates the
> concept, so what can/should user-space use the arch value for?

Because userspace otherwise has to figure out the architecture itself based on
the chipid, while the kernel already did derive this information.

There's many ways userspace could do this, and I don't want to incentivise any
of them.

For instance, you previously showed how userspace derives the SM value from the
chipid with sm_for_chipset() in mesa with its own lookup table.

Then in NAK (src/nouveau/compiler/nak/ir.rs), there's this code.

	fn is_turing(&self) -> bool {
	    self.sm() >= 73 && self.sm() < 80
	}
	
	fn is_ampere(&self) -> bool {
	    self.sm() >= 80 && self.sm() < 89
	}
	
	fn is_ada(&self) -> bool {
	    self.sm() == 89
	}
	
	#[allow(dead_code)]
	fn is_hopper(&self) -> bool {
	    self.sm() >= 90 && self.sm() < 100
	}
	
	fn is_blackwell_a(&self) -> bool {
	    self.sm() >= 100 && self.sm() < 110
	}
	
	fn is_blackwell_b(&self) -> bool {
	    self.sm() >= 120 && self.sm() < 130
	}
	
	fn is_blackwell(&self) -> bool {
	    self.is_blackwell_a() || self.is_blackwell_b()
	}

That's two unnecessary indirections for something the kernel already has
available.

> It really boils down to how much static config info we want to encode into
> kernel lookup tables vs. user-space lookup tables. I don't this we should be
> filling the kernel with a bunch of static struct lookups just to ship those
> struct definitions to user-space. It seems more reasonable to put that in
> user-space, unless of course the kernel needs it.
>
> So I think your "does the kernel need this" is a reasonable benchmark for
> deciding this.
>
> As I think you're hinting at it would be ideal if GSP or some other firmware
> table could just provide everything in some nice static config table, but I
> don't think that's currently possible and looking at other user-space drivers
> we're certainly not alone there.

Yeah, although if SM is correctly reported by the GSP, I'd rather have it
exported in an info structure than have userspace create its own lookup table.

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Alistair Popple]

On 2026-09-03 at 20:42 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
> On Thu Sep 3, 2026 at 3:12 AM CEST, Alistair Popple wrote:
> > Originally I thought the point of providing the decoded arch rather than
> > chip-id
> 
> I think there never was a "rather than". The point was that if userspace has
> conditionals based on the architecture it shouldn't have to figure it out based
> on the chipid, as this has been done by the kernel already.

I see. Maybe that was a bad assumption on my behalf, because I assumed that if
you make a precise hardware description available (chip-id) there'd be no point
making an imprecise subset of that description (arch) available as it isn't
particularly useful if you can't use it in isolation for any generic purpose.

Which is to say it's unclear what generic properties users are allowed to derive
from the kernel saying "this is a Hopper GPU". It's clearer for a chip-id as it
represents an (almost) exact piece of HW with the exact properties of that HW,
so userspace can derive everything from that.

Anyway guess I will just leave it in, time will tell what use it has, even
though I think it would be better to know that use upfront.

> > because as you rightfully point out the kernel doesn't (currently at least)
> > care much about the implementation
> 
> I still don't see how that value will ever be meaningful, there is no such thing
> as "all GPUs of a certain implementation regardless of architecture" have
> something in common, is it?
> 
> But since you say "currently at least", are there any plans to give this value
> some meaning beyond being a unique counter for chips within a certain
> architecture?

Oh I just meant AFAIK the kernel isn't currently looking at the implementation
because everything it cares about (like the HAL) can be keyed off the
architecture and that's been my experience with most of our GPU kernel drivers.

Or IOW, at the moment, from a kernel perspective the kernel doesn't care about
implementation beyond needing to encode that into a type. So supporting for
example GB203 instead of GB202 amounts to just adding the encoding for that.
If we didn't need to add the encoding the kernel could say claim support for all
Blackwell implementations.

Of course whether this remains true generally requires a fair bit of
co-operation from the HW, and I'm not sure we're actually there yet although I
think there has been some discussions in the past.

But it would be a nice place to be - having dealt with similar problems in
the past on the CPU side it all becomes a bit of a pain constantly backporting
patches to a bunch of distros just to allow a +1 on your minor HW version that
the kernel doesn't actually care about anyway.

But we already agreed this isn't something we can or want to solve here and now,
so this is really just an aside. Maybe one day we can get there, but not now.

> If so, I think that'd be a horrible way to encode some chip commonality.
> 
> > But we agreed that wasn't desirable, so now I'm still left figuring out why we
> > need to provide an arch value? The chip-id already entirely encapsulates the
> > concept, so what can/should user-space use the arch value for?
> 
> Because userspace otherwise has to figure out the architecture itself based on
> the chipid, while the kernel already did derive this information.

That wasn't my question. My question wasn't where should userspace get the
arch, it is what can user-space use the arch value for? Again, architecture
alone isn't entirely sufficient for user-space. There isn't for example a neat
mapping of arch to sm version for code generation once you start looking at all
our GPUs.

> There's many ways userspace could do this, and I don't want to incentivise any
> of them.
> 
> For instance, you previously showed how userspace derives the SM value from the
> chipid with sm_for_chipset() in mesa with its own lookup table.
> 
> Then in NAK (src/nouveau/compiler/nak/ir.rs), there's this code.
> 
> 	fn is_turing(&self) -> bool {
> 	    self.sm() >= 73 && self.sm() < 80
> 	}
> 	
> 	fn is_ampere(&self) -> bool {
> 	    self.sm() >= 80 && self.sm() < 89
> 	}
> 	
> 	fn is_ada(&self) -> bool {
> 	    self.sm() == 89
> 	}
> 	
> 	#[allow(dead_code)]
> 	fn is_hopper(&self) -> bool {
> 	    self.sm() >= 90 && self.sm() < 100
> 	}
> 	
> 	fn is_blackwell_a(&self) -> bool {
> 	    self.sm() >= 100 && self.sm() < 110
> 	}
> 	
> 	fn is_blackwell_b(&self) -> bool {
> 	    self.sm() >= 120 && self.sm() < 130
> 	}
> 	
> 	fn is_blackwell(&self) -> bool {
> 	    self.is_blackwell_a() || self.is_blackwell_b()
> 	}
> 
> That's two unnecessary indirections for something the kernel already has
> available.

Again though is what the kernel provides in the form of an arch actually useful
to user-space? Obviously the code above makes it look nice and simple like that,
but as I have been saying more complete implementations can't just rely on arch
alone and so are still going to have lookup tables both for SM and for other
info the kernel can't provide.

Anyway at this point I'll just leave chipset and arch in. Clearly you think
there is some value in providing it separately, and I suppose it doesn't make
much difference just to have it there.

> > It really boils down to how much static config info we want to encode into
> > kernel lookup tables vs. user-space lookup tables. I don't this we should be
> > filling the kernel with a bunch of static struct lookups just to ship those
> > struct definitions to user-space. It seems more reasonable to put that in
> > user-space, unless of course the kernel needs it.
> >
> > So I think your "does the kernel need this" is a reasonable benchmark for
> > deciding this.
> >
> > As I think you're hinting at it would be ideal if GSP or some other firmware
> > table could just provide everything in some nice static config table, but I
> > don't think that's currently possible and looking at other user-space drivers
> > we're certainly not alone there.
> 
> Yeah, although if SM is correctly reported by the GSP, I'd rather have it
> exported in an info structure than have userspace create its own lookup table.

Yeah, although that would need to be a contract between HW and GSP and GSP and
user-space. That's fine, and can could probably do it one day, it just doesn't
currently exist AFAICT.

 - Alistair

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Fri Sep 4, 2026 at 9:49 AM CEST, Alistair Popple wrote:
> On 2026-09-03 at 20:42 +1000, Danilo Krummrich <dakr@kernel.org> wrote...
>> I think there never was a "rather than". The point was that if userspace has
>> conditionals based on the architecture it shouldn't have to figure it out based
>> on the chipid, as this has been done by the kernel already.
>
> I see. Maybe that was a bad assumption on my behalf, because I assumed that if
> you make a precise hardware description available (chip-id) there'd be no point
> making an imprecise subset of that description (arch) available as it isn't
> particularly useful if you can't use it in isolation for any generic purpose.

Except that it is used in isolation for a generic purpose e.g. in NAK.

>> Then in NAK (src/nouveau/compiler/nak/ir.rs), there's this code.
>> 
>> 	fn is_turing(&self) -> bool {
>> 	    self.sm() >= 73 && self.sm() < 80
>> 	}
>> 	
>> 	fn is_ampere(&self) -> bool {
>> 	    self.sm() >= 80 && self.sm() < 89
>> 	}
>> 	
>> 	fn is_ada(&self) -> bool {
>> 	    self.sm() == 89
>> 	}
>> 	
>> 	#[allow(dead_code)]
>> 	fn is_hopper(&self) -> bool {
>> 	    self.sm() >= 90 && self.sm() < 100
>> 	}
>> 	
>> 	fn is_blackwell_a(&self) -> bool {
>> 	    self.sm() >= 100 && self.sm() < 110
>> 	}
>> 	
>> 	fn is_blackwell_b(&self) -> bool {
>> 	    self.sm() >= 120 && self.sm() < 130
>> 	}
>> 	
>> 	fn is_blackwell(&self) -> bool {
>> 	    self.is_blackwell_a() || self.is_blackwell_b()
>> 	}
>> 
>> That's two unnecessary indirections for something the kernel already has
>> available.
>
> Again though is what the kernel provides in the form of an arch actually useful
> to user-space? Obviously the code above makes it look nice and simple like that,
> but as I have been saying more complete implementations can't just rely on arch
> alone and so are still going to have lookup tables both for SM and for other
> info the kernel can't provide.

Are you saying that NAK oversimplifies things and hence gets away with per
architecture checks in src/nouveau/compiler/nak/sm70.rs? And that a "more
complete implementation" would be specialized to a point where it becomes
impossible to have any common code per architecture and in general?

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Gary Guo]

On Thu Sep 3, 2026 at 11:42 AM BST, Danilo Krummrich wrote:
> On Thu Sep 3, 2026 at 3:12 AM CEST, Alistair Popple wrote:
>> Originally I thought the point of providing the decoded arch rather than
>> chip-id
>
> I think there never was a "rather than". The point was that if userspace has
> conditionals based on the architecture it shouldn't have to figure it out based
> on the chipid, as this has been done by the kernel already.
>
>> because as you rightfully point out the kernel doesn't (currently at least)
>> care much about the implementation
>
> I still don't see how that value will ever be meaningful, there is no such thing
> as "all GPUs of a certain implementation regardless of architecture" have
> something in common, is it?

I don't think it's an issue that implementation field is only meaningful when
you get the architecture. That's pretty common for other IDs, e.g. PCI product
ID is meaningless without having the vendor ID.

You mentioned in an earlier email about the typing, but we could still have
meaningful impl IDs fully typed like this:

    pub enum Arch {
        Turing(impl_id),
        Ampere(impl_id),
        ...
    }

and some langauges can do better, e.g. TypeScript allow you to write

    enumn ArchId {
        Turing,
        Ampere,
        ...
    }

    enum TuringImplId { ... }
    enum AmpereImplId { ... }

    type Id =
        { arch: ArchId.Turing, impl: TuringImplId } |
        { arch: ArchId.Ampere, impl: AmpereImplId };

So I think it's a reasonable design to have it.

>
> But since you say "currently at least", are there any plans to give this value
> some meaning beyond being a unique counter for chips within a certain
> architecture?
>
> If so, I think that'd be a horrible way to encode some chip commonality.

True, but I think the chip ID is as bad as impl ID. Feature detections should
use dedicated featuire detection mechanism, not looking up IDs directly. I.e. we
should have userspace not having to use either chip ID or impl ID as much as we
can.

That said, I do think looking up tables are unavoidable, for getting names or
applying some quirk fixes. And I agree with Alistair that if we include it,
including impl ID is better than the chip ID, as we should rather not having
user space relying on an arbitrary encoded chip ID.

Best,
Gary

Re: [PATCH v5 05/11] drm: nova: Add an info ioctl

[Danilo Krummrich]

On Fri Sep 4, 2026 at 1:13 PM CEST, Gary Guo wrote:
> You mentioned in an earlier email about the typing, but we could still have
> meaningful impl IDs fully typed like this:
>
>     pub enum Arch {
>         Turing(impl_id),
>         Ampere(impl_id),
>         ...
>     }
>
> and some langauges can do better, e.g. TypeScript allow you to write
>
>     enumn ArchId {
>         Turing,
>         Ampere,
>         ...
>     }
>
>     enum TuringImplId { ... }
>     enum AmpereImplId { ... }
>
>     type Id =
>         { arch: ArchId.Turing, impl: TuringImplId } |
>         { arch: ArchId.Ampere, impl: AmpereImplId };
>
> So I think it's a reasonable design to have it.

Of course we could encode this implementation detail, but I think there's no
reason to do so.

For instance currently we refer to GA100 as

	Chipset::GA100

but with the above we'd refer to GA100 as

	Arch::Ampere(AmpereImpl::GA100)

which is not buying us anything, is it?

It also would be confusing because both the architecture and the specific chip
would now be both represented by a type called Arch.

> True, but I think the chip ID is as bad as impl ID. Feature detections should
> use dedicated featuire detection mechanism, not looking up IDs directly. I.e. we
> should have userspace not having to use either chip ID or impl ID as much as we
> can.

This I agree with, which is also why I mentioned we should export SM if GSP
already provides it to us.

> That said, I do think looking up tables are unavoidable, for getting names or
> applying some quirk fixes. And I agree with Alistair that if we include it,
> including impl ID is better than the chip ID, as we should rather not having
> user space relying on an arbitrary encoded chip ID.

No, the only thing userspace ever uses to distinguish between things is either
by architecture or by chip. If we instead provide architecture and some ID
userspace will just go

	if (arch == Ampere && id == 0)
		chip = GA100;
	else if (arch == Ampere && id == 2)
		chip = GA102;
	[...]

and after that never care about the ID again. Whereas with giving userspace the
chip and architecture as separate fields userspace is done.