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.