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.
