linux-kernel - Re: [PATCH 2/2] rust: drm: Add GPUVM abstraction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m2pli62sfx.fsf@posteo.net>
Date: Mon, 24 Mar 2025 18:29:22 +0000
From: Charalampos Mitrodimas <charmitro@...teo.net>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...nel.org>,  Alex Gaynor <alex.gaynor@...il.com>,
  Boqun Feng <boqun.feng@...il.com>,  Gary Guo <gary@...yguo.net>,
  Björn
 Roy Baron <bjorn3_gh@...tonmail.com>,  Benno Lossin
 <benno.lossin@...ton.me>,  Andreas Hindborg <a.hindborg@...nel.org>,
  Alice Ryhl <aliceryhl@...gle.com>,  Trevor Gross <tmgross@...ch.edu>,
  Sumit Semwal <sumit.semwal@...aro.org>,  Christian König
 <christian.koenig@....com>,  Boris Brezillon
 <boris.brezillon@...labora.com>,  linux-kernel@...r.kernel.org,
  rust-for-linux@...r.kernel.org,  dri-devel@...ts.freedesktop.org,  Asahi
 Lina <lina@...hilina.net>
Subject: Re: [PATCH 2/2] rust: drm: Add GPUVM abstraction

Daniel Almeida <daniel.almeida@...labora.com> writes:

> +/// The object returned after a call to [`GpuVm::lock`].
> +///
> +/// This object has access to operations that modify the VM's interval tree.
> +pub struct LockedGpuVm<'a, T: DriverGpuVm> {
> +    gpuvm: &'a GpuVm<T>,
> +}
> +
> +impl<T: DriverGpuVm> LockedGpuVm<'_, T> {
> +    /// Finds the [`GpuVmBo`] object that connects `obj` to this VM.
> +    ///
> +    /// If found, increases the reference count of the GpuVmBo object
> +    /// accordingly.
> +    pub fn find_bo(&mut self, obj: &DriverObject<T>) -> Option<ARef<GpuVmBo<T>>> {
> +        // SAFETY: LockedGpuVm implies the right locks are held.
> +        let p = unsafe {
> +            bindings::drm_gpuvm_bo_find(
> +                self.gpuvm.gpuvm() as *mut _,
> +                obj.gem_obj() as *const _ as *mut _,
> +            )
> +        };
> +        if p.is_null() {
> +            None
> +        } else {
> +            // SAFETY: All the drm_gpuvm_bo objects in this GpuVm are always allocated by us as GpuVmBo<T>.
> +            let p = unsafe { crate::container_of!(p, GpuVmBo<T>, bo) as *mut GpuVmBo<T> };
> +            // SAFETY: We checked for NULL above, and the types ensure that
> +            // this object was created by vm_bo_alloc_callback<T>.
> +            Some(unsafe { ARef::from_raw(NonNull::new_unchecked(p)) })
> +        }

Hi Daniel,

This is mostly eye candy—maybe we can simplify it to just:
    if p.is_null() {
       return None;
    }

    // SAFETY: All the drm_gpuvm_bo objects in this GpuVm are always allocated by us as GpuVmBo<T>.
    let p = unsafe { crate::container_of!(p, GpuVmBo<T>, bo) as *mut GpuVmBo<T> };
    // SAFETY: We checked for NULL above, and the types ensure that
    // this object was created by vm_bo_alloc_callback<T>.
    Some(unsafe { ARef::from_raw(NonNull::new_unchecked(p)) })


Same with `fn obtain_bo`?

--
C. Mitrodimas

> +    }
> +
> +    /// Obtains the [`GpuVmBo`] object that connects `obj` to this VM.
> +    ///
> +    /// This connection is unique, so an instane of [`GpuVmBo`] will be
> +    /// allocated for `obj` once, and that instance will be returned from that
> +    /// point forward.
> +    pub fn obtain_bo(&mut self, obj: &DriverObject<T>) -> Result<ARef<GpuVmBo<T>>> {
> +        // SAFETY: LockedGpuVm implies the right locks are held.
> +        let p = unsafe {
> +            bindings::drm_gpuvm_bo_obtain(
> +                self.gpuvm.gpuvm() as *mut _,
> +                obj.gem_obj() as *const _ as *mut _,
> +            )
> +        };
> +        if p.is_null() {
> +            Err(ENOMEM)
> +        } else {
> +            // SAFETY: Container invariant is guaranteed for GpuVmBo objects for this GpuVm.
> +            let p = unsafe { crate::container_of!(p, GpuVmBo<T>, bo) as *mut GpuVmBo<T> };
> +            // SAFETY: We checked for NULL above, and the types ensure that
> +            // this object was created by vm_bo_alloc_callback<T>.
> +            Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(p)) })
> +        }
> +    }