Re: (subset) [PATCH v9 0/7] Rust bindings for gem shmem

[Deborah Brouwer <deborah.brouwer@collabora.com>]

On Thu, Mar 26, 2026 at 02:15:28AM +0100, Danilo Krummrich wrote:
> On Mon Mar 16, 2026 at 10:16 PM CET, Lyude Paul wrote:
> > Lyude Paul (5):
> >   rust: drm: Add gem::impl_aref_for_gem_obj!
> >   rust: gem: Introduce DriverObject::Args
> 
> Applied to drm-rust-next, thanks!
> 
> > Asahi Lina (2):
> >   rust: drm: gem: shmem: Add DRM shmem helper abstraction
> 
> I was about to pick this one up as well, but did run into quite some build
> errors and warnings. I fixed them all up, but I consider this too excessive to
> actually apply the patch. This is the changelog I came up with:
> 
>     [ * DRM_GEM_SHMEM_HELPER is a tristate; when a module driver selects it,
>         it becomes =m. The Rust kernel crate and its C helpers are always
>         built into vmlinux and can't reference symbols from a module,
>         causing link errors.
> 
>         Thus, add RUST_DRM_GEM_SHMEM_HELPER bool Kconfig that selects
>         DRM_GEM_SHMEM_HELPER, forcing it built-in when Rust drivers need it;
>         use cfg(CONFIG_RUST_DRM_GEM_SHMEM_HELPER) for the shmem module.
> 
>       * Add cfg_attr(not(CONFIG_RUST_DRM_GEM_SHMEM_HELPER), expect(unused))
>         on pub(crate) use impl_aref_for_gem_obj and BaseObjectPrivate, so
>         that unused warnings are suppressed when shmem is not enabled.
> 
>       * Enable const_refs_to_static (stabilized in 1.83) to prevent build
>         errors with older compilers.
> 
>       * Use &raw const for bindings::drm_gem_shmem_vm_ops and add
>         #[allow(unused_unsafe, reason = "Safe since Rust 1.82.0")].
> 
>       * Fix incorrect C Header path and minor spelling and formatting
>         issues.
> 
>       * Drop shmem::Object::sg_table() as the current implementation is
>         unsound.
> 
>         - Danilo ]
> 
> Please always consider [1] and [2].
> 
> [1] https://drm.pages.freedesktop.org/maintainer-tools/committer/committer-drm-rust.html#submit-checklist
> [2] https://rust-for-linux.com/contributing#submit-checklist-addendum
> 
> (@Deborah: I assume you were testing this with Tyr built-in?)

Yes, Tyr used to work around this issue by creating a new config
DRM_TYR_STATIC_DEPS, a bool, which selected DRM_GEM_SHMEM_HELPER.
When we build DRM_TYR as a module, it selected DRM_TYR_STATIC_DEPS so
that DRM_GEM_SHMEM_HELPER always ended up built-in even if Tyr was built
as a module.

But now Tyr can just select RUST_DRM_GEM_SHMEM_HELPER directly.

(We still have the same work around for DRM_GPUVM perhaps it could
use the same solution).

> 
> @Lyude, Alice, Miguel: Please have a look at what I came up with below.
> 
> commit 2dc69d77944dbd1494d2b10a4b134b7fead1c8e7
> Author: Asahi Lina <lina+kernel@asahilina.net>
> Date:   Mon Mar 16 17:16:13 2026 -0400
> 
>     rust: drm: gem: shmem: Add DRM shmem helper abstraction
> 
>     The DRM shmem helper includes common code useful for drivers which
>     allocate GEM objects as anonymous shmem. Add a Rust abstraction for
>     this. Drivers can choose the raw GEM implementation or the shmem layer,
>     depending on their needs.
> 
>     Signed-off-by: Asahi Lina <lina@asahilina.net>
>     Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
>     Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>     Signed-off-by: Lyude Paul <lyude@redhat.com>
>     Tested-by: Deborah Brouwer <deborah.brouwer@collabora.com>
>     Link: https://patch.msgid.link/20260316211646.650074-6-lyude@redhat.com
>     [ * DRM_GEM_SHMEM_HELPER is a tristate; when a module driver selects it,
>         it becomes =m. The Rust kernel crate and its C helpers are always
>         built into vmlinux and can't reference symbols from a module,
>         causing link errors.
> 
>         Thus, add RUST_DRM_GEM_SHMEM_HELPER bool Kconfig that selects
>         DRM_GEM_SHMEM_HELPER, forcing it built-in when Rust drivers need it;
>         use cfg(CONFIG_RUST_DRM_GEM_SHMEM_HELPER) for the shmem module.
> 
>       * Add cfg_attr(not(CONFIG_RUST_DRM_GEM_SHMEM_HELPER), expect(unused))
>         on pub(crate) use impl_aref_for_gem_obj and BaseObjectPrivate, so
>         that unused warnings are suppressed when shmem is not enabled.
> 
>       * Enable const_refs_to_static (stabilized in 1.83) to prevent build
>         errors with older compilers.
> 
>       * Use &raw const for bindings::drm_gem_shmem_vm_ops and add
>         #[allow(unused_unsafe, reason = "Safe since Rust 1.82.0")].
> 
>       * Fix incorrect C Header path and minor spelling and formatting
>         issues.
> 
>       * Drop shmem::Object::sg_table() as the current implementation is
>         unsound.
> 
>         - Danilo ]
>     Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 0d0657dd1b41..0f68446c9122 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -258,6 +258,13 @@ config DRM_GEM_SHMEM_HELPER
>  	help
>  	  Choose this if you need the GEM shmem helper functions
> 
> +config RUST_DRM_GEM_SHMEM_HELPER
> +	bool
> +	depends on DRM && MMU
> +	select DRM_GEM_SHMEM_HELPER
> +	help
> +	  Choose this if you need the GEM shmem helper functions In Rust
> +
>  config DRM_SUBALLOC_HELPER
>  	tristate
>  	depends on DRM
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 563863d96d38..eda8f50d3a3c 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -34,6 +34,7 @@
>  #include <drm/drm_drv.h>
>  #include <drm/drm_file.h>
>  #include <drm/drm_gem.h>
> +#include <drm/drm_gem_shmem_helper.h>
>  #include <drm/drm_ioctl.h>
>  #include <kunit/test.h>
>  #include <linux/auxiliary_bus.h>
> @@ -63,6 +64,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io-pgtable.h>
>  #include <linux/ioport.h>
> +#include <linux/iosys-map.h>
>  #include <linux/jiffies.h>
>  #include <linux/jump_label.h>
>  #include <linux/mdio.h>
> diff --git a/rust/helpers/drm.c b/rust/helpers/drm.c
> index fe226f7b53ef..65f3f22b0e1d 100644
> --- a/rust/helpers/drm.c
> +++ b/rust/helpers/drm.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
> 
>  #include <drm/drm_gem.h>
> +#include <drm/drm_gem_shmem_helper.h>
>  #include <drm/drm_vma_manager.h>
> 
>  #ifdef CONFIG_DRM
> @@ -21,4 +22,57 @@ rust_helper_drm_vma_node_offset_addr(struct drm_vma_offset_node *node)
>  	return drm_vma_node_offset_addr(node);
>  }
> 
> -#endif
> +#ifdef CONFIG_DRM_GEM_SHMEM_HELPER
> +__rust_helper void
> +rust_helper_drm_gem_shmem_object_free(struct drm_gem_object *obj)
> +{
> +	return drm_gem_shmem_object_free(obj);
> +}
> +
> +__rust_helper void
> +rust_helper_drm_gem_shmem_object_print_info(struct drm_printer *p, unsigned int indent,
> +					    const struct drm_gem_object *obj)
> +{
> +	drm_gem_shmem_object_print_info(p, indent, obj);
> +}
> +
> +__rust_helper int
> +rust_helper_drm_gem_shmem_object_pin(struct drm_gem_object *obj)
> +{
> +	return drm_gem_shmem_object_pin(obj);
> +}
> +
> +__rust_helper void
> +rust_helper_drm_gem_shmem_object_unpin(struct drm_gem_object *obj)
> +{
> +	drm_gem_shmem_object_unpin(obj);
> +}
> +
> +__rust_helper struct sg_table *
> +rust_helper_drm_gem_shmem_object_get_sg_table(struct drm_gem_object *obj)
> +{
> +	return drm_gem_shmem_object_get_sg_table(obj);
> +}
> +
> +__rust_helper int
> +rust_helper_drm_gem_shmem_object_vmap(struct drm_gem_object *obj,
> +				      struct iosys_map *map)
> +{
> +	return drm_gem_shmem_object_vmap(obj, map);
> +}
> +
> +__rust_helper void
> +rust_helper_drm_gem_shmem_object_vunmap(struct drm_gem_object *obj,
> +					struct iosys_map *map)
> +{
> +	drm_gem_shmem_object_vunmap(obj, map);
> +}
> +
> +__rust_helper int
> +rust_helper_drm_gem_shmem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> +{
> +	return drm_gem_shmem_object_mmap(obj, vma);
> +}
> +
> +#endif /* CONFIG_DRM_GEM_SHMEM_HELPER */
> +#endif /* CONFIG_DRM */
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index 527d86f4ce92..58eb0a3d5686 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -26,6 +26,9 @@
>      ptr::NonNull, //
>  };
> 
> +#[cfg(CONFIG_RUST_DRM_GEM_SHMEM_HELPER)]
> +pub mod shmem;
> +
>  /// A macro for implementing [`AlwaysRefCounted`] for any GEM object type.
>  ///
>  /// Since all GEM objects use the same refcounting scheme.
> @@ -60,6 +63,8 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
>          }
>      };
>  }
> +#[cfg_attr(not(CONFIG_RUST_DRM_GEM_SHMEM_HELPER), allow(unused))]
> +pub(crate) use impl_aref_for_gem_obj;
> 
>  /// A type alias for retrieving a [`Driver`]s [`DriverFile`] implementation from its
>  /// [`DriverObject`] implementation.
> @@ -216,7 +221,7 @@ fn create_mmap_offset(&self) -> Result<u64> {
>  impl<T: IntoGEMObject> BaseObject for T {}
> 
>  /// Crate-private base operations shared by all GEM object classes.
> -#[expect(unused)]
> +#[cfg_attr(not(CONFIG_RUST_DRM_GEM_SHMEM_HELPER), expect(unused))]
>  pub(crate) trait BaseObjectPrivate: IntoGEMObject {
>      /// Return a pointer to this object's dma_resv.
>      fn raw_dma_resv(&self) -> *mut bindings::dma_resv {
> diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
> new file mode 100644
> index 000000000000..d025fb035195
> --- /dev/null
> +++ b/rust/kernel/drm/gem/shmem.rs
> @@ -0,0 +1,228 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! DRM GEM shmem helper objects
> +//!
> +//! C header: [`include/linux/drm/drm_gem_shmem_helper.h`](srctree/include/drm/drm_gem_shmem_helper.h)
> +
> +// TODO:
> +// - There are a number of spots here that manually acquire/release the DMA reservation lock using
> +//   dma_resv_(un)lock(). In the future we should add support for ww mutex, expose a method to
> +//   acquire a reference to the WwMutex, and then use that directly instead of the C functions here.
> +
> +use crate::{
> +    container_of,
> +    drm::{
> +        device,
> +        driver,
> +        gem,
> +        private::Sealed, //
> +    },
> +    error::to_result,
> +    prelude::*,
> +    types::{
> +        ARef,
> +        Opaque, //
> +    }, //
> +};
> +use core::{
> +    ops::{
> +        Deref,
> +        DerefMut, //
> +    },
> +    ptr::NonNull,
> +};
> +use gem::{
> +    BaseObjectPrivate,
> +    DriverObject,
> +    IntoGEMObject, //
> +};
> +
> +/// A struct for controlling the creation of shmem-backed GEM objects.
> +///
> +/// This is used with [`Object::new()`] to control various properties that can only be set when
> +/// initially creating a shmem-backed GEM object.
> +#[derive(Default)]
> +pub struct ObjectConfig<'a, T: DriverObject> {
> +    /// Whether to set the write-combine map flag.
> +    pub map_wc: bool,
> +
> +    /// Reuse the DMA reservation from another GEM object.
> +    ///
> +    /// The newly created [`Object`] will hold an owned refcount to `parent_resv_obj` if specified.
> +    pub parent_resv_obj: Option<&'a Object<T>>,
> +}
> +
> +/// A shmem-backed GEM object.
> +///
> +/// # Invariants
> +///
> +/// `obj` contains a valid initialized `struct drm_gem_shmem_object` for the lifetime of this
> +/// object.
> +#[repr(C)]
> +#[pin_data]
> +pub struct Object<T: DriverObject> {
> +    #[pin]
> +    obj: Opaque<bindings::drm_gem_shmem_object>,
> +    /// Parent object that owns this object's DMA reservation object.
> +    parent_resv_obj: Option<ARef<Object<T>>>,
> +    #[pin]
> +    inner: T,
> +}
> +
> +super::impl_aref_for_gem_obj!(impl<T> for Object<T> where T: DriverObject);
> +
> +// SAFETY: All GEM objects are thread-safe.
> +unsafe impl<T: DriverObject> Send for Object<T> {}
> +
> +// SAFETY: All GEM objects are thread-safe.
> +unsafe impl<T: DriverObject> Sync for Object<T> {}
> +
> +impl<T: DriverObject> Object<T> {
> +    /// `drm_gem_object_funcs` vtable suitable for GEM shmem objects.
> +    const VTABLE: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs {
> +        free: Some(Self::free_callback),
> +        open: Some(super::open_callback::<T>),
> +        close: Some(super::close_callback::<T>),
> +        print_info: Some(bindings::drm_gem_shmem_object_print_info),
> +        export: None,
> +        pin: Some(bindings::drm_gem_shmem_object_pin),
> +        unpin: Some(bindings::drm_gem_shmem_object_unpin),
> +        get_sg_table: Some(bindings::drm_gem_shmem_object_get_sg_table),
> +        vmap: Some(bindings::drm_gem_shmem_object_vmap),
> +        vunmap: Some(bindings::drm_gem_shmem_object_vunmap),
> +        mmap: Some(bindings::drm_gem_shmem_object_mmap),
> +        status: None,
> +        rss: None,
> +        #[allow(unused_unsafe, reason = "Safe since Rust 1.82.0")]
> +        // SAFETY: `drm_gem_shmem_vm_ops` is a valid, static const on the C side.
> +        vm_ops: unsafe { &raw const bindings::drm_gem_shmem_vm_ops },
> +        evict: None,
> +    };
> +
> +    /// Return a raw pointer to the embedded drm_gem_shmem_object.
> +    fn as_raw_shmem(&self) -> *mut bindings::drm_gem_shmem_object {
> +        self.obj.get()
> +    }
> +
> +    /// Create a new shmem-backed DRM object of the given size.
> +    ///
> +    /// Additional config options can be specified using `config`.
> +    pub fn new(
> +        dev: &device::Device<T::Driver>,
> +        size: usize,
> +        config: ObjectConfig<'_, T>,
> +        args: T::Args,
> +    ) -> Result<ARef<Self>> {
> +        let new: Pin<KBox<Self>> = KBox::try_pin_init(
> +            try_pin_init!(Self {
> +                obj <- Opaque::init_zeroed(),
> +                parent_resv_obj: config.parent_resv_obj.map(|p| p.into()),
> +                inner <- T::new(dev, size, args),
> +            }),
> +            GFP_KERNEL,
> +        )?;
> +
> +        // SAFETY: `obj.as_raw()` is guaranteed to be valid by the initialization above.
> +        unsafe { (*new.as_raw()).funcs = &Self::VTABLE };
> +
> +        // SAFETY: The arguments are all valid via the type invariants.
> +        to_result(unsafe { bindings::drm_gem_shmem_init(dev.as_raw(), new.as_raw_shmem(), size) })?;
> +
> +        // SAFETY: We never move out of `self`.
> +        let new = KBox::into_raw(unsafe { Pin::into_inner_unchecked(new) });
> +
> +        // SAFETY: We're taking over the owned refcount from `drm_gem_shmem_init`.
> +        let obj = unsafe { ARef::from_raw(NonNull::new_unchecked(new)) };
> +
> +        // Start filling out values from `config`
> +        if let Some(parent_resv) = config.parent_resv_obj {
> +            // SAFETY: We have yet to expose the new gem object outside of this function, so it is
> +            // safe to modify this field.
> +            unsafe { (*obj.obj.get()).base.resv = parent_resv.raw_dma_resv() };
> +        }
> +
> +        // SAFETY: We have yet to expose this object outside of this function, so we're guaranteed
> +        // to have exclusive access - thus making this safe to hold a mutable reference to.
> +        let shmem = unsafe { &mut *obj.as_raw_shmem() };
> +        shmem.set_map_wc(config.map_wc);
> +
> +        Ok(obj)
> +    }
> +
> +    /// Returns the `Device` that owns this GEM object.
> +    pub fn dev(&self) -> &device::Device<T::Driver> {
> +        // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`.
> +        unsafe { device::Device::from_raw((*self.as_raw()).dev) }
> +    }
> +
> +    extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
> +        // SAFETY:
> +        // - DRM always passes a valid gem object here
> +        // - We used drm_gem_shmem_create() in our create_gem_object callback, so we know that
> +        //   `obj` is contained within a drm_gem_shmem_object
> +        let this = unsafe { container_of!(obj, bindings::drm_gem_shmem_object, base) };
> +
> +        // SAFETY:
> +        // - We're in free_callback - so this function is safe to call.
> +        // - We won't be using the gem resources on `this` after this call.
> +        unsafe { bindings::drm_gem_shmem_release(this) };
> +
> +        // SAFETY:
> +        // - We verified above that `obj` is valid, which makes `this` valid
> +        // - This function is set in AllocOps, so we know that `this` is contained within a
> +        //   `Object<T>`
> +        let this = unsafe { container_of!(Opaque::cast_from(this), Self, obj) }.cast_mut();
> +
> +        // SAFETY: We're recovering the Kbox<> we created in gem_create_object()
> +        let _ = unsafe { KBox::from_raw(this) };
> +    }
> +}
> +
> +impl<T: DriverObject> Deref for Object<T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        &self.inner
> +    }
> +}
> +
> +impl<T: DriverObject> DerefMut for Object<T> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {
> +        &mut self.inner
> +    }
> +}
> +
> +impl<T: DriverObject> Sealed for Object<T> {}
> +
> +impl<T: DriverObject> gem::IntoGEMObject for Object<T> {
> +    fn as_raw(&self) -> *mut bindings::drm_gem_object {
> +        // SAFETY:
> +        // - Our immutable reference is proof that this is safe to dereference.
> +        // - `obj` is always a valid drm_gem_shmem_object via our type invariants.
> +        unsafe { &raw mut (*self.obj.get()).base }
> +    }
> +
> +    unsafe fn from_raw<'a>(obj: *mut bindings::drm_gem_object) -> &'a Object<T> {
> +        // SAFETY: The safety contract of from_gem_obj() guarantees that `obj` is contained within
> +        // `Self`
> +        unsafe {
> +            let obj = Opaque::cast_from(container_of!(obj, bindings::drm_gem_shmem_object, base));
> +
> +            &*container_of!(obj, Object<T>, obj)
> +        }
> +    }
> +}
> +
> +impl<T: DriverObject> driver::AllocImpl for Object<T> {
> +    type Driver = T::Driver;
> +
> +    const ALLOC_OPS: driver::AllocOps = driver::AllocOps {
> +        gem_create_object: None,
> +        prime_handle_to_fd: None,
> +        prime_fd_to_handle: None,
> +        gem_prime_import: None,
> +        gem_prime_import_sg_table: Some(bindings::drm_gem_shmem_prime_import_sg_table),
> +        dumb_create: Some(bindings::drm_gem_shmem_dumb_create),
> +        dumb_map_offset: None,
> +    };
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index e0837ffc91bf..40de00ce4f97 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -38,6 +38,7 @@
>  #![feature(const_option)]
>  #![feature(const_ptr_write)]
>  #![feature(const_refs_to_cell)]
> +#![feature(const_refs_to_static)]
>  //
>  // Stable since Rust 1.84.0.
>  #![feature(strict_provenance)]