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

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

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