This is the next version of the shmem backed GEM objects series originally from Asahi, previously posted by Daniel Almeida.
The previous version of the patch series can be found here:
https://patchwork.freedesktop.org/series/156093/
This patch series may be applied on top of the driver-core/driver-core-testing branch:
https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/...
Changelogs are per-patch
Asahi Lina (2): rust: helpers: Add bindings/wrappers for dma_resv_lock rust: drm: gem: shmem: Add DRM shmem helper abstraction
Lyude Paul (5): rust: drm: Add gem::impl_aref_for_gem_obj! rust: drm: gem: Add raw_dma_resv() function rust: gem: Introduce DriverObject::Args rust: drm: gem: Introduce shmem::SGTable rust: drm/gem: Add vmap functions to shmem bindings
drivers/gpu/drm/nova/gem.rs | 5 +- drivers/gpu/drm/tyr/gem.rs | 3 +- rust/bindings/bindings_helper.h | 3 + rust/helpers/dma-resv.c | 13 + rust/helpers/drm.c | 56 ++- rust/helpers/helpers.c | 1 + rust/kernel/drm/gem/mod.rs | 79 +++- rust/kernel/drm/gem/shmem.rs | 654 ++++++++++++++++++++++++++++++++ 8 files changed, 792 insertions(+), 22 deletions(-) create mode 100644 rust/helpers/dma-resv.c create mode 100644 rust/kernel/drm/gem/shmem.rs
base-commit: dc33ae50d32b509af5ae61030912fa20c79ef112 prerequisite-patch-id: c631986f96e2073263e97e82a65b96fc5ada6924 prerequisite-patch-id: ae853e8eb8d58c77881371960be4ae92755e83c6 prerequisite-patch-id: 0ab78b50648c7d8f66b83c32ed2af0ec3ede42a3 prerequisite-patch-id: 636ec7f913f4047e5e1a1788f3e835b7259698c2 prerequisite-patch-id: d75e4d7140eadeeed8017af8cd093bfd2766ee8e prerequisite-patch-id: 67a8010c1bc95bca1d2cf6b246c67bc79d24e766
In the future we're going to be introducing more GEM object types in rust then just gem::Object<T>. Since all types of GEM objects have refcounting, let's introduce a macro that we can use in the gem crate in order to copy this boilerplate implementation for each type: impl_aref_for_gem_obj!().
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Reviewed-by: Janne Grunau j@jananu.net
--- V5: * Move .as_raw() call to `let obj` in dec_ref, to ensure that the reference to object is not live by the time that we call drm_gem_object_put(). * Add missing #[macro_export] annotation V6: * Add missing IntoGEMObject trait bound
Signed-off-by: Lyude Paul lyude@redhat.com --- rust/kernel/drm/gem/mod.rs | 51 +++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index d49a9ba026356..94e7c2a7293d0 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -15,6 +15,41 @@ }; use core::{ops::Deref, ptr::NonNull};
+/// A macro for implementing [`AlwaysRefCounted`] for any GEM object type. +/// +/// Since all GEM objects use the same refcounting scheme. +#[macro_export] +macro_rules! impl_aref_for_gem_obj { + ( + impl $( <$( $tparam_id:ident ),+> )? for $type:ty + $( + where + $( $bind_param:path : $bind_trait:path ),+ + )? + ) => { + // SAFETY: All gem objects are refcounted + unsafe impl $( <$( $tparam_id ),+> )? $crate::types::AlwaysRefCounted for $type + where + Self: IntoGEMObject, + $( $( $bind_param : $bind_trait ),+ )? + { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is + // non-zero. + unsafe { bindings::drm_gem_object_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) { + // SAFETY: `obj` is a valid pointer to an `Object<T>`. + let obj = unsafe { obj.as_ref() }.as_raw(); + + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::drm_gem_object_put(obj) }; + } + } + }; +} + /// A type alias for retrieving a [`Driver`]s [`DriverFile`] implementation from its /// [`DriverObject`] implementation. /// @@ -252,21 +287,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { } }
-// SAFETY: Instances of `Object<T>` are always reference-counted. -unsafe impl<T: DriverObject> crate::sync::aref::AlwaysRefCounted for Object<T> { - fn inc_ref(&self) { - // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::drm_gem_object_get(self.as_raw()) }; - } - - unsafe fn dec_ref(obj: NonNull<Self>) { - // SAFETY: `obj` is a valid pointer to an `Object<T>`. - let obj = unsafe { obj.as_ref() }; - - // SAFETY: The safety requirements guarantee that the refcount is non-zero. - unsafe { bindings::drm_gem_object_put(obj.as_raw()) } - } -} +impl_aref_for_gem_obj!(impl<T> for Object<T> where T: DriverObject);
impl<T: DriverObject> super::private::Sealed for Object<T> {}
For retrieving a pointer to the struct dma_resv for a given GEM object. We also introduce it in a new trait, BaseObjectPrivate, which we automatically implement for all gem objects and don't expose to users outside of the crate.
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Janne Grunau j@jananu.net --- rust/kernel/drm/gem/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 94e7c2a7293d0..bcec62155c02d 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -197,6 +197,18 @@ 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)] +pub(crate) trait BaseObjectPrivate: IntoGEMObject { + /// Return a pointer to this object's dma_resv. + fn raw_dma_resv(&self) -> *mut bindings::dma_resv { + // SAFETY: `as_gem_obj()` always returns a valid pointer to the base DRM gem object + unsafe { (*self.as_raw()).resv } + } +} + +impl<T: IntoGEMObject> BaseObjectPrivate for T {} + /// A base GEM object. /// /// # Invariants
From: Asahi Lina lina@asahilina.net
This is just for basic usage in the DRM shmem abstractions for implied locking, not intended as a full DMA Reservation abstraction yet.
Signed-off-by: Asahi Lina lina@asahilina.net Signed-off-by: Daniel Almeida daniel.almeida@collabora.com Reviewed-by: Alice Ryhl aliceryhl@google.com Reviewed-by: Janne Grunau j@jananu.net Signed-off-by: Lyude Paul lyude@redhat.com --- rust/bindings/bindings_helper.h | 1 + rust/helpers/dma-resv.c | 13 +++++++++++++ rust/helpers/helpers.c | 1 + 3 files changed, 15 insertions(+) create mode 100644 rust/helpers/dma-resv.c
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 083cc44aa952c..39a8f15603692 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -48,6 +48,7 @@ #include <linux/cpumask.h> #include <linux/cred.h> #include <linux/debugfs.h> +#include <linux/dma-resv.h> #include <linux/device/faux.h> #include <linux/dma-direction.h> #include <linux/dma-mapping.h> diff --git a/rust/helpers/dma-resv.c b/rust/helpers/dma-resv.c new file mode 100644 index 0000000000000..05501cb814513 --- /dev/null +++ b/rust/helpers/dma-resv.c @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/dma-resv.h> + +int rust_helper_dma_resv_lock(struct dma_resv *obj, struct ww_acquire_ctx *ctx) +{ + return dma_resv_lock(obj, ctx); +} + +void rust_helper_dma_resv_unlock(struct dma_resv *obj) +{ + dma_resv_unlock(obj); +} diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index a3c42e51f00a0..d090e2c7c7ea8 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -28,6 +28,7 @@ #include "cred.c" #include "device.c" #include "dma.c" +#include "dma-resv.c" #include "drm.c" #include "err.c" #include "irq.c"
This is an associated type that may be used in order to specify a data-type to pass to gem objects when construction them, allowing for drivers to more easily initialize their private-data for gem objects.
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Alice Ryhl aliceryhl@google.com Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Reviewed-by: Janne Grunau j@jananu.net
--- V3: * s/BaseDriverObject/DriverObject/ V4: * Fix leftover reference to BaseObjectDriver in rustdoc for DriverObject::Args V6: * Fix build errors in Tyr
Signed-off-by: Lyude Paul lyude@redhat.com --- drivers/gpu/drm/nova/gem.rs | 5 +++-- drivers/gpu/drm/tyr/gem.rs | 3 ++- rust/kernel/drm/gem/mod.rs | 13 ++++++++++--- 3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs index 6ccfa5da57617..e073e174e2578 100644 --- a/drivers/gpu/drm/nova/gem.rs +++ b/drivers/gpu/drm/nova/gem.rs @@ -19,8 +19,9 @@ pub(crate) struct NovaObject {}
impl gem::DriverObject for NovaObject { type Driver = NovaDriver; + type Args = ();
- fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> { + fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl PinInit<Self, Error> { try_pin_init!(NovaObject {}) } } @@ -33,7 +34,7 @@ pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self } let aligned_size = page::page_align(size).ok_or(EINVAL)?;
- gem::Object::new(dev, aligned_size) + gem::Object::new(dev, aligned_size, ()) }
/// Look up a GEM object handle for a `File` and return an `ObjectRef` for it. diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs index 1273bf89dbd5d..bb5e7871efa94 100644 --- a/drivers/gpu/drm/tyr/gem.rs +++ b/drivers/gpu/drm/tyr/gem.rs @@ -11,8 +11,9 @@ pub(crate) struct TyrObject {}
impl gem::DriverObject for TyrObject { type Driver = TyrDriver; + type Args = ();
- fn new(_dev: &TyrDevice, _size: usize) -> impl PinInit<Self, Error> { + fn new(_dev: &TyrDevice, _size: usize, _args: ()) -> impl PinInit<Self, Error> { try_pin_init!(TyrObject {}) } } diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index bcec62155c02d..68bf33969a7d4 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -62,8 +62,15 @@ pub trait DriverObject: Sync + Send + Sized { /// Parent `Driver` for this object. type Driver: drm::Driver;
+ /// The data type to use for passing arguments to [`DriverObject::new`]. + type Args; + /// Create a new driver data object for a GEM object of a given size. - fn new(dev: &drm::DeviceSelf::Driver, size: usize) -> impl PinInit<Self, Error>; + fn new( + dev: &drm::DeviceSelf::Driver, + size: usize, + args: Self::Args, + ) -> impl PinInit<Self, Error>;
/// Open a new handle to an existing object, associated with a File. fn open(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) -> Result { @@ -242,11 +249,11 @@ impl<T: DriverObject> Object<T> { };
/// Create a new GEM object. - pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> { + pub fn new(dev: &drm::Device<T::Driver>, size: usize, args: T::Args) -> Result<ARef<Self>> { let obj: Pin<KBox<Self>> = KBox::pin_init( try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), - data <- T::new(dev, size), + data <- T::new(dev, size, args), }), GFP_KERNEL, )?;
From: Asahi Lina lina@asahilina.net
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
---
V2: * Use the drm_gem_shmem_init() and drm_gem_shmem_release() that I extracted so we can handle memory allocation in rust, which means we no longer have to handle freeing rust members of the struct by hand and have a closer implementation to the main gem object (this also gets rid of gem_create_object) * Get rid of GemObjectRef and UniqueGemObjectRef, we have ARef<T> at home. * Use Device<T::Driver> in Object<T> * Cleanup Object::<T>::new() a bit: * Cleanup safety comment * Use cast_mut() * Just import container_of!(), we use it all over anyhow * mut_shmem() -> as_shmem(), make it safe (there's no reason for being unsafe) * Remove any *const and *muts in structs, just use NonNull * Get rid of the previously hand-rolled sg_table bindings in shmem, use the bindings from Abdiel's sg_table patch series * Add a TODO at the top about DMA reservation APIs and a desire for WwMutex * Get rid of map_wc() and replace it with a new ObjectConfig struct. While it currently only specifies the map_wc flag, the idea here is that settings like map_wc() and parent_resv_obj() shouldn't be exposed as normal functions since the only place where it's safe to set them is when we're still guaranteed unique access to the GEM object, e.g. before returning it to the caller. Using a struct instead of individual arguments here is mainly because we'll be adding at least one more argument, and there's enough other gem shmem settings that trying to add all of them as individual function arguments in the future would be a bit messy. * Get rid of vm_numa_fields!, Lina didn't like this macro much either and I think that it's fine for us to just specify the #[cfg(…)] attributes by hand since we only need to do it twice. * Set drm_gem_object_funcs.vm_ops directly to drm_gem_shmem_vm_ops, don't export the various shmem funcs. I'm not sure why this wasn't possible before but it seems to work fine now. V4: * Switch from OpaqueObject to a normal Object<T> now that we've removed it * Remove `dev` from Object, it's redundant as drm_gem_object already has a device pointer we can use. * Use &raw instead of addr_of!() V5: * Fix typo in shmem::Object::as_raw() * Add type invariant around `obj` always being a valid `drm_gem_shmem_object` for the duration of the lifetime of `Object`. * Use Opaque::cast_from() instead of unrestricted casts * Use IS_ENABLED() for gem shmem C helpers. V7: * Fix import style * Don't forget to make Object<T> Send/Sync V8: * s/as_shmem()/as_raw_shmem()
Signed-off-by: Lyude Paul lyude@redhat.com --- rust/bindings/bindings_helper.h | 2 + rust/helpers/drm.c | 56 ++++++- rust/kernel/drm/gem/mod.rs | 5 +- rust/kernel/drm/gem/shmem.rs | 250 ++++++++++++++++++++++++++++++++ 4 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 rust/kernel/drm/gem/shmem.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 39a8f15603692..a343c684e8d06 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -33,6 +33,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> @@ -62,6 +63,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 fe226f7b53ef0..a49aff4fb6350 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 +#if IS_ENABLED(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 68bf33969a7d4..e7ebfb3f6ce6b 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -3,6 +3,8 @@ //! DRM GEM API //! //! C header: [`include/drm/drm_gem.h`](srctree/include/drm/drm_gem.h) +#[cfg(CONFIG_DRM_GEM_SHMEM_HELPER)] +pub mod shmem;
use crate::{ alloc::flags::*, @@ -50,6 +52,8 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) { }; }
+pub(crate) use impl_aref_for_gem_obj; + /// A type alias for retrieving a [`Driver`]s [`DriverFile`] implementation from its /// [`DriverObject`] implementation. /// @@ -205,7 +209,6 @@ 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)] 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 0000000000000..6c77ace05d30a --- /dev/null +++ b/rust/kernel/drm/gem/shmem.rs @@ -0,0 +1,250 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! DRM GEM shmem helper objects +//! +//! C header: [`include/linux/drm/drm_gem_shmem_helper.h`](srctree/include/linux/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::{ + from_err_ptr, + to_result, // + }, + prelude::*, + scatterlist, + 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: Opaquebindings::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, + // SAFETY: `drm_gem_shmem_vm_ops` is static const on the C side, so immutable references are + // safe here and such references shall be valid forever + vm_ops: unsafe { &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) }; + } + + /// Creates (if necessary) and returns an immutable reference to a scatter-gather table of DMA + /// pages for this object. + /// + /// This will pin the object in memory. + #[inline] + pub fn sg_table(&self) -> Result<&scatterlist::SGTable> { + // SAFETY: + // - drm_gem_shmem_get_pages_sgt is thread-safe. + // - drm_gem_shmem_get_pages_sgt returns either a valid pointer to a scatterlist, or an + // error pointer. + let sgt = + from_err_ptr(unsafe { bindings::drm_gem_shmem_get_pages_sgt(self.as_raw_shmem()) })?; + + // SAFETY: We checked above that `sgt` is not an error pointer, so it must be a valid + // pointer to a scatterlist + Ok(unsafe { scatterlist::SGTable::from_raw(sgt) }) + } +} + +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, + }; +}
On Mon Mar 16, 2026 at 10:16 PM CET, Lyude Paul wrote:
- /// Creates (if necessary) and returns an immutable reference to a scatter-gather table of DMA
- /// pages for this object.
- ///
- /// This will pin the object in memory.
- #[inline]
- pub fn sg_table(&self) -> Result<&scatterlist::SGTable> {
// SAFETY:// - drm_gem_shmem_get_pages_sgt is thread-safe.// - drm_gem_shmem_get_pages_sgt returns either a valid pointer to a scatterlist, or an// error pointer.let sgt =from_err_ptr(unsafe { bindings::drm_gem_shmem_get_pages_sgt(self.as_raw_shmem()) })?;
This is unsound as nothing guarantees that the device used by drm_gem_shmem_get_pages_sgt() is actually bound to the calling driver. It is also not guaranteed that the DMA mapping within the returned &SGTable does not out-live driver unbind.
There are two possible solutions.
(1) Change drm_gem_shmem_get_pages_sgt() to provide this guarantee.
(2) Don't use drm_gem_shmem_get_pages_sgt() in the first place and instead use SGTable::new(), which guarantees to destroy the backing DMA mapping on driver unbind.
In any case, this function needs to take a &Device<Bound> argument that matches the bus devices stored in the backing GEM object.
// SAFETY: We checked above that `sgt` is not an error pointer, so it must be a valid// pointer to a scatterlistOk(unsafe { scatterlist::SGTable::from_raw(sgt) })- }
+}
Currently we expose the ability to retrieve an SGTable for an shmem gem object using gem::shmem::Object::<T>::sg_table(). However, this only gives us a borrowed reference. This being said - retrieving an SGTable is a fallible operation, and as such it's reasonable that a driver may want to hold onto an SGTable for longer then a reference would allow in order to avoid having to deal with fallibility every time they want to access the SGTable. One such driver with this usecase is the Asahi driver.
So to support this, let's introduce shmem::SGTable - which both holds a pointer to the SGTable and a reference to its respective GEM object in order to keep the GEM object alive for as long as the shmem::SGTable. The type can be used identically to a normal SGTable.
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Janne Grunau j@jananu.net
--- V3: * Rename OwnedSGTable to shmem::SGTable. Since the current version of the SGTable abstractions now has a `Owned` and `Borrowed` variant, I think renaming this to shmem::SGTable makes things less confusing. We do however, keep the name of owned_sg_table() as-is. V4: * Clarify safety comments for SGTable to explain why the object is thread-safe. * Rename from SGTableRef to SGTable
Signed-off-by: Lyude Paul lyude@redhat.com --- rust/kernel/drm/gem/shmem.rs | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 6c77ace05d30a..3fab5d76c197b 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -198,6 +198,25 @@ pub fn sg_table(&self) -> Result<&scatterlist::SGTable> { // pointer to a scatterlist Ok(unsafe { scatterlist::SGTable::from_raw(sgt) }) } + + /// Creates (if necessary) and returns an owned reference to a scatter-gather table of DMA pages + /// for this object. + /// + /// This is the same as [`sg_table`](Self::sg_table), except that it instead returns an + /// [`shmem::SGTable`] which holds a reference to the associated gem object, instead of a + /// reference to an [`scatterlist::SGTable`]. + /// + /// This will pin the object in memory. + /// + /// [`shmem::SGTable`]: SGTable + pub fn owned_sg_table(&self) -> Result<SGTable<T>> { + Ok(SGTable { + sgt: self.sg_table()?.into(), + // INVARIANT: We take an owned refcount to `self` here, ensuring that `sgt` remains + // valid for as long as this `SGTable`. + _owner: self.into(), + }) + } }
impl<T: DriverObject> Deref for Object<T> { @@ -248,3 +267,34 @@ impl<T: DriverObject> driver::AllocImpl for Object<T> { dumb_map_offset: None, }; } + +/// An owned reference to a scatter-gather table of DMA address spans for a GEM shmem object. +/// +/// This object holds an owned reference to the underlying GEM shmem object, ensuring that the +/// [`scatterlist::SGTable`] referenced by this type remains valid for the lifetime of this object. +/// +/// # Invariants +/// +/// - `sgt` is kept alive by `_owner`, ensuring it remains valid for as long as `Self`. +/// - `sgt` corresponds to the owned object in `_owner`. +/// - This object is only exposed in situations where we know the underlying `SGTable` will not be +/// modified for the lifetime of this object. Thus, it is safe to send/access this type across +/// threads. +pub struct SGTable<T: DriverObject> { + sgt: NonNullscatterlist::SGTable, + _owner: ARef<Object<T>>, +} + +// SAFETY: This object is thread-safe via our type invariants. +unsafe impl<T: DriverObject> Send for SGTable<T> {} +// SAFETY: This object is thread-safe via our type invariants. +unsafe impl<T: DriverObject> Sync for SGTable<T> {} + +impl<T: DriverObject> Deref for SGTable<T> { + type Target = scatterlist::SGTable; + + fn deref(&self) -> &Self::Target { + // SAFETY: Creating an immutable reference to this is safe via our type invariants. + unsafe { self.sgt.as_ref() } + } +}
One of the more obvious use cases for gem shmem objects is the ability to create mappings into their contents. So, let's hook this up in our rust bindings.
Similar to how we handle SGTables, we make sure there's two different types of mappings: owned mappings (kernel::drm::gem::shmem::VMap) and borrowed mappings (kernel::drm::gem::shmem::VMapRef).
Signed-off-by: Lyude Paul lyude@redhat.com
--- V7: * Switch over to the new iosys map bindings that use the Io trait V8: * Get rid of iosys_map bindings for now, only support non-iomem types * s/as_shmem()/as_raw_shmem() V9: * Get rid of some outdated comments I missed * Add missing SIZE check to raw_vmap() * Add a proper unit test that ensures that we actually validate SIZE at compile-time. Turns out it takes only 34 lines to make a boilerplate DRM driver for a kunit test :) * Add unit tests * Add some missing #[inline]s
Signed-off-by: Lyude Paul lyude@redhat.com --- rust/kernel/drm/gem/shmem.rs | 358 ++++++++++++++++++++++++++++++++++- 1 file changed, 356 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 3fab5d76c197b..5254338bad394 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -2,7 +2,7 @@
//! DRM GEM shmem helper objects //! -//! C header: [`include/linux/drm/drm_gem_shmem_helper.h`](srctree/include/linux/drm/drm_gem_shmem_helper.h) +//! 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 @@ -21,6 +21,11 @@ from_err_ptr, to_result, // }, + io::{ + Io, + IoCapable, + IoKnownSize, // + }, prelude::*, scatterlist, types::{ @@ -29,13 +34,22 @@ }, // }; use core::{ + ffi::c_void, + mem::{ + self, + MaybeUninit, // + }, ops::{ Deref, DerefMut, // }, - ptr::NonNull, + ptr::{ + self, + NonNull, // + }, }; use gem::{ + BaseObject, BaseObjectPrivate, DriverObject, IntoGEMObject, // @@ -217,6 +231,88 @@ pub fn owned_sg_table(&self) -> Result<SGTable<T>> { _owner: self.into(), }) } + + /// Attempt to create a vmap from the gem object, and confirm the size of said vmap. + fn raw_vmap(&self, min_size: usize) -> Result<*mut c_void> { + if self.size() < min_size { + return Err(ENOSPC); + } + + let mut map: MaybeUninitbindings::iosys_map = MaybeUninit::uninit(); + + // SAFETY: drm_gem_shmem_vmap can be called with the DMA reservation lock held + to_result(unsafe { + // TODO: see top of file + bindings::dma_resv_lock(self.raw_dma_resv(), ptr::null_mut()); + let ret = bindings::drm_gem_shmem_vmap_locked(self.as_raw_shmem(), map.as_mut_ptr()); + bindings::dma_resv_unlock(self.raw_dma_resv()); + ret + })?; + + // SAFETY: The call to drm_gem_shmem_vunmap_locked succeeded above, so we are guaranteed + // that map is properly initialized. + let map = unsafe { map.assume_init() }; + + // XXX: We don't currently support iomem allocations + if map.is_iomem { + // SAFETY: + // - The vmap operation above succeeded, making it safe to call vunmap + // - We checked that this is an iomem allocation, making it safe to read vaddr_iomem + unsafe { self.raw_vunmap(map.__bindgen_anon_1.vaddr_iomem) }; + + Err(ENOTSUPP) + } else { + // SAFETY: We checked that this is not an iomem allocation, making it safe to read vaddr + Ok(unsafe { map.__bindgen_anon_1.vaddr }) + } + } + + /// Unmap a vmap from the gem object. + /// + /// # Safety + /// + /// - The caller promises that addr came from a prior call to [`Self::raw_vmap`] on this gem + /// object. + /// - The caller promises that the memory pointed to by addr will no longer be accesed through + /// this instance. + unsafe fn raw_vunmap(&self, vaddr: *mut c_void) { + let resv = self.raw_dma_resv(); + let mut map = bindings::iosys_map { + is_iomem: false, + __bindgen_anon_1: bindings::iosys_map__bindgen_ty_1 { vaddr }, + }; + + // SAFETY: + // - This function is safe to call with the DMA reservation lock held + // - Our `ARef` is proof that the underlying gem object here is initialized and thus safe to + // dereference. + unsafe { + // TODO: see top of file + bindings::dma_resv_lock(resv, ptr::null_mut()); + bindings::drm_gem_shmem_vunmap_locked(self.as_raw_shmem(), &mut map); + bindings::dma_resv_unlock(resv); + } + } + + /// Creates and returns a virtual kernel memory mapping for this object. + #[inline] + pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, SIZE>> { + Ok(VMapRef { + // INVARIANT: `raw_vmap()` checks that the gem object is at least as large as `SIZE`. + addr: self.raw_vmap(SIZE)?, + owner: self, + }) + } + + /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. + #[inline] + pub fn owned_vmap<const SIZE: usize>(&self) -> Result<VMap<T, SIZE>> { + Ok(VMap { + // INVARIANT: `raw_vmap()` checks that the gem object is at least as large as `SIZE`. + addr: self.raw_vmap(SIZE)?, + owner: self.into(), + }) + } }
impl<T: DriverObject> Deref for Object<T> { @@ -268,6 +364,147 @@ impl<T: DriverObject> driver::AllocImpl for Object<T> { }; }
+macro_rules! impl_vmap_io_capable { + ($impl:ident, $ty:ty $(, $lifetime:lifetime )?) => { + impl<$( $lifetime ,)? D: DriverObject, const SIZE: usize> IoCapable<$ty> + for $impl<$( $lifetime ,)? D, SIZE> + { + #[inline(always)] + unsafe fn io_read(&self, address: usize) -> $ty { + let ptr = address as *mut $ty; + + // SAFETY: The safety contract of `io_read` guarantees that address is a valid + // address within the bounds of `Self` of at least the size of $ty, and is properly + // aligned. + unsafe { ptr::read(ptr) } + } + + #[inline(always)] + unsafe fn io_write(&self, value: $ty, address: usize) { + let ptr = address as *mut $ty; + + // SAFETY: The safety contract of `io_write` guarantees that address is a valid + // address within the bounds of `Self` of at least the size of $ty, and is properly + // aligned. + unsafe { ptr::write(ptr, value) } + } + } + }; +} + +// Implement various traits common to both VMap types +macro_rules! impl_vmap_common { + ($impl:ident $(, $lifetime:lifetime )?) => { + impl<$( $lifetime ,)? D, const SIZE: usize> $impl<$( $lifetime ,)? D, SIZE> + where + D: DriverObject, + { + /// Borrows a reference to the object that owns this virtual mapping. + #[inline(always)] + pub fn owner(&self) -> &Object<D> { + &self.owner + } + } + + impl<$( $lifetime ,)? D, const SIZE: usize> Drop for $impl<$( $lifetime ,)? D, SIZE> + where + D: DriverObject, + { + #[inline(always)] + fn drop(&mut self) { + // SAFETY: Our existence is proof that this map was previously created using + // self.owner + unsafe { self.owner.raw_vunmap(self.addr) }; + } + } + + impl<$( $lifetime ,)? D, const SIZE: usize> Io for $impl<$( $lifetime ,)? D, SIZE> + where + D: DriverObject, + { + #[inline(always)] + fn addr(&self) -> usize { + self.addr as usize + } + + #[inline(always)] + fn maxsize(&self) -> usize { + self.owner.size() + } + } + + impl<$( $lifetime ,)? D, const SIZE: usize> IoKnownSize for $impl<$( $lifetime ,)? D, SIZE> + where + D: DriverObject, + { + const MIN_SIZE: usize = SIZE; + } + + impl_vmap_io_capable!($impl, u8 $( , $lifetime )?); + impl_vmap_io_capable!($impl, u16 $( , $lifetime )?); + impl_vmap_io_capable!($impl, u32 $( , $lifetime )?); + #[cfg(CONFIG_64BIT)] + impl_vmap_io_capable!($impl, u64 $( , $lifetime )?); + }; +} + +/// An owned reference to a virtual mapping for a shmem-based GEM object in kernel address space. +/// +/// # Invariants +/// +/// - The size of `owner` is >= SIZE. +/// - The memory pointed to by addr remains valid at least until this object is dropped. +pub struct VMap<D: DriverObject, const SIZE: usize = 0> { + addr: *mut c_void, + owner: ARef<Object<D>>, +} + +impl_vmap_common!(VMap); + +impl<D: DriverObject, const SIZE: usize> Clone for VMap<D, SIZE> { + #[inline] + fn clone(&self) -> Self { + // SAFETY: We have a successful vmap already, so this can't fail + unsafe { self.owner.owned_vmap().unwrap_unchecked() } + } +} + +impl<'a, D: DriverObject, const SIZE: usize> From<VMapRef<'a, D, SIZE>> for VMap<D, SIZE> { + #[inline] + fn from(value: VMapRef<'a, D, SIZE>) -> Self { + let this = Self { + addr: value.addr, + owner: value.owner.into(), + }; + + mem::forget(value); + this + } +} + +// SAFETY: addr is guaranteed to be valid and accessible for the lifetime of VMap, ensuring its +// safe to send across threads. +unsafe impl<D: DriverObject, const SIZE: usize> Send for VMap<D, SIZE> {} +// SAFETY: addr is guaranteed to be valid and accessible for the lifetime of VMap, ensuring its +// safe to send across threads. +unsafe impl<D: DriverObject, const SIZE: usize> Sync for VMap<D, SIZE> {} + +/// A borrowed reference to a virtual mapping for a shmem-based GEM object in kernel address space. +pub struct VMapRef<'a, D: DriverObject, const SIZE: usize = 0> { + addr: *mut c_void, + owner: &'a Object<D>, +} + +impl_vmap_common!(VMapRef, 'a); + +impl<'a, D: DriverObject, const SIZE: usize> Clone for VMapRef<'a, D, SIZE> { + #[inline] + fn clone(&self) -> Self { + // SAFETY: We have a successful vmap already, so this can't fail + unsafe { self.owner.vmap().unwrap_unchecked() } + } +} + /// An owned reference to a scatter-gather table of DMA address spans for a GEM shmem object. /// /// This object holds an owned reference to the underlying GEM shmem object, ensuring that the @@ -298,3 +535,120 @@ fn deref(&self) -> &Self::Target { unsafe { self.sgt.as_ref() } } } + +#[kunit_tests(rust_drm_gem_shmem)] +mod tests { + use super::*; + use crate::{ + drm, + faux, + page::PAGE_SIZE, // + }; + + // The bare minimum needed to create a fake drm driver for kunit + + #[pin_data] + struct KunitData {} + struct KunitDriver; + struct KunitFile; + #[pin_data] + struct KunitObject {} + + const INFO: drm::DriverInfo = drm::DriverInfo { + major: 0, + minor: 0, + patchlevel: 0, + name: c"kunit", + desc: c"Kunit", + }; + + impl drm::file::DriverFile for KunitFile { + type Driver = KunitDriver; + + fn open(_dev: &drm::Device<KunitDriver>) -> Result<Pin<KBox<Self>>> { + Ok(KBox::new(Self, GFP_KERNEL)?.into()) + } + } + + impl gem::DriverObject for KunitObject { + type Driver = KunitDriver; + type Args = (); + + fn new( + _dev: &drm::Device<KunitDriver>, + _size: usize, + _args: Self::Args, + ) -> impl PinInit<Self, Error> { + try_pin_init!(KunitObject {}) + } + } + + #[vtable] + impl drm::Driver for KunitDriver { + type Data = KunitData; + type File = KunitFile; + type Object = Object<KunitObject>; + + const INFO: drm::DriverInfo = INFO; + const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[]; + } + + fn create_drm_dev() -> Result<(faux::Registration, ARef<drm::Device<KunitDriver>>)> { + // Create a faux DRM device so we can test gem object creation. + let data = try_pin_init!(KunitData {}); + let dev = faux::Registration::new(c"Kunit", None)?; + let drm = drm::Device::<KunitDriver>::new(dev.as_ref(), data)?; + + Ok((dev, drm)) + } + + #[test] + fn compile_time_vmap_sizes() -> Result { + let (_dev, drm) = create_drm_dev()?; + + // Create a gem object to test with + let cfg_ = ObjectConfig::<KunitObject> { + map_wc: false, + parent_resv_obj: None, + }; + let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, cfg_, ())?; + + // Try creating a normal vmap + obj.vmap::<PAGE_SIZE>()?; + + // Try creating a vmap that's smaller then the size we specified + obj.vmap::<{ PAGE_SIZE - 100 }>()?; + + // Make sure creating a vmap that's too large fails + assert!(obj.vmap::<{ PAGE_SIZE + 200 }>().is_err()); + + Ok(()) + } + + #[test] + fn vmap_io() -> Result { + let (_dev, drm) = create_drm_dev()?; + + // Create a gem object to test with + let cfg_ = ObjectConfig::<KunitObject> { + map_wc: false, + parent_resv_obj: None, + }; + let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, cfg_, ())?; + + let vmap = obj.vmap::<PAGE_SIZE>()?; + + vmap.write8(0xDE, 0x0); + assert_eq!(vmap.read8(0x0), 0xDE); + vmap.write32(0xFFFFFFFF, 0x20); + + assert_eq!(vmap.read32(0x20), 0xFFFFFFFF); + + assert_eq!(vmap.read8(0x20), 0xFF); + assert_eq!(vmap.read8(0x21), 0xFF); + assert_eq!(vmap.read8(0x22), 0xFF); + assert_eq!(vmap.read8(0x23), 0xFF); + + Ok(()) + } +}
On 3/17/26 05:16, Lyude Paul wrote:
One of the more obvious use cases for gem shmem objects is the ability to create mappings into their contents. So, let's hook this up in our rust bindings.
Similar to how we handle SGTables, we make sure there's two different types of mappings: owned mappings (kernel::drm::gem::shmem::VMap) and borrowed mappings (kernel::drm::gem::shmem::VMapRef).
Signed-off-by: Lyude Paul lyude@redhat.com
V7:
- Switch over to the new iosys map bindings that use the Io trait
V8:
- Get rid of iosys_map bindings for now, only support non-iomem types
- s/as_shmem()/as_raw_shmem()
V9:
- Get rid of some outdated comments I missed
- Add missing SIZE check to raw_vmap()
- Add a proper unit test that ensures that we actually validate SIZE at compile-time. Turns out it takes only 34 lines to make a boilerplate DRM driver for a kunit test :)
- Add unit tests
- Add some missing #[inline]s
Signed-off-by: Lyude Paul lyude@redhat.com
rust/kernel/drm/gem/shmem.rs | 358 ++++++++++++++++++++++++++++++++++- 1 file changed, 356 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 3fab5d76c197b..5254338bad394 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -2,7 +2,7 @@ //! DRM GEM shmem helper objects //! -//! C header: [`include/linux/drm/drm_gem_shmem_helper.h`](srctree/include/linux/drm/drm_gem_shmem_helper.h) +//! 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 @@ -21,6 +21,11 @@ from_err_ptr, to_result, // },
- io::{
Io,IoCapable,IoKnownSize, //- }, prelude::*, scatterlist, types::{
@@ -29,13 +34,22 @@ }, // }; use core::{
- ffi::c_void,
- mem::{
self,MaybeUninit, //- }, ops::{ Deref, DerefMut, // },
- ptr::NonNull,
- ptr::{
self,NonNull, //- }, }; use gem::{
- BaseObject, BaseObjectPrivate, DriverObject, IntoGEMObject, //
@@ -217,6 +231,88 @@ pub fn owned_sg_table(&self) -> Result<SGTable<T>> { _owner: self.into(), }) }
- /// Attempt to create a vmap from the gem object, and confirm the size of said vmap.
- fn raw_vmap(&self, min_size: usize) -> Result<*mut c_void> {
if self.size() < min_size {return Err(ENOSPC);}let mut map: MaybeUninit<bindings::iosys_map> = MaybeUninit::uninit();// SAFETY: drm_gem_shmem_vmap can be called with the DMA reservation lock heldto_result(unsafe {// TODO: see top of filebindings::dma_resv_lock(self.raw_dma_resv(), ptr::null_mut());let ret = bindings::drm_gem_shmem_vmap_locked(self.as_raw_shmem(), map.as_mut_ptr());bindings::dma_resv_unlock(self.raw_dma_resv());ret})?;// SAFETY: The call to drm_gem_shmem_vunmap_locked succeeded above, so we are guaranteed// that map is properly initialized.let map = unsafe { map.assume_init() };// XXX: We don't currently support iomem allocationsif map.is_iomem {// SAFETY:// - The vmap operation above succeeded, making it safe to call vunmap// - We checked that this is an iomem allocation, making it safe to read vaddr_iomemunsafe { self.raw_vunmap(map.__bindgen_anon_1.vaddr_iomem) };Err(ENOTSUPP)} else {// SAFETY: We checked that this is not an iomem allocation, making it safe to read vaddrOk(unsafe { map.__bindgen_anon_1.vaddr })}- }
- /// Unmap a vmap from the gem object.
- ///
- /// # Safety
- ///
- /// - The caller promises that addr came from a prior call to [`Self::raw_vmap`] on this gem
- /// object.
- /// - The caller promises that the memory pointed to by addr will no longer be accesed through
- /// this instance.
- unsafe fn raw_vunmap(&self, vaddr: *mut c_void) {
let resv = self.raw_dma_resv();let mut map = bindings::iosys_map {is_iomem: false,__bindgen_anon_1: bindings::iosys_map__bindgen_ty_1 { vaddr },};// SAFETY:// - This function is safe to call with the DMA reservation lock held// - Our `ARef` is proof that the underlying gem object here is initialized and thus safe to// dereference.unsafe {// TODO: see top of filebindings::dma_resv_lock(resv, ptr::null_mut());bindings::drm_gem_shmem_vunmap_locked(self.as_raw_shmem(), &mut map);bindings::dma_resv_unlock(resv);}- }
- /// Creates and returns a virtual kernel memory mapping for this object.
- #[inline]
- pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, SIZE>> {
Ok(VMapRef {// INVARIANT: `raw_vmap()` checks that the gem object is at least as large as `SIZE`.addr: self.raw_vmap(SIZE)?,owner: self,})- }
- /// Creates and returns an owned reference to a virtual kernel memory mapping for this object.
- #[inline]
- pub fn owned_vmap<const SIZE: usize>(&self) -> Result<VMap<T, SIZE>> {
Ok(VMap {// INVARIANT: `raw_vmap()` checks that the gem object is at least as large as `SIZE`.addr: self.raw_vmap(SIZE)?,owner: self.into(),})- } }
impl<T: DriverObject> Deref for Object<T> { @@ -268,6 +364,147 @@ impl<T: DriverObject> driver::AllocImpl for Object<T> { }; } +macro_rules! impl_vmap_io_capable {
- ($impl:ident, $ty:ty $(, $lifetime:lifetime )?) => {
impl<$( $lifetime ,)? D: DriverObject, const SIZE: usize> IoCapable<$ty>for $impl<$( $lifetime ,)? D, SIZE>{#[inline(always)]unsafe fn io_read(&self, address: usize) -> $ty {let ptr = address as *mut $ty;// SAFETY: The safety contract of `io_read` guarantees that address is a valid// address within the bounds of `Self` of at least the size of $ty, and is properly// aligned.unsafe { ptr::read(ptr) }}#[inline(always)]unsafe fn io_write(&self, value: $ty, address: usize) {let ptr = address as *mut $ty;// SAFETY: The safety contract of `io_write` guarantees that address is a valid// address within the bounds of `Self` of at least the size of $ty, and is properly// aligned.unsafe { ptr::write(ptr, value) }}}- };
+}
+// Implement various traits common to both VMap types +macro_rules! impl_vmap_common {
- ($impl:ident $(, $lifetime:lifetime )?) => {
impl<$( $lifetime ,)? D, const SIZE: usize> $impl<$( $lifetime ,)? D, SIZE>whereD: DriverObject,{/// Borrows a reference to the object that owns this virtual mapping.#[inline(always)]pub fn owner(&self) -> &Object<D> {&self.owner}}impl<$( $lifetime ,)? D, const SIZE: usize> Drop for $impl<$( $lifetime ,)? D, SIZE>whereD: DriverObject,{#[inline(always)]fn drop(&mut self) {// SAFETY: Our existence is proof that this map was previously created using// self.ownerunsafe { self.owner.raw_vunmap(self.addr) };}}impl<$( $lifetime ,)? D, const SIZE: usize> Io for $impl<$( $lifetime ,)? D, SIZE>whereD: DriverObject,{#[inline(always)]fn addr(&self) -> usize {self.addr as usize}#[inline(always)]fn maxsize(&self) -> usize {self.owner.size()}}impl<$( $lifetime ,)? D, const SIZE: usize> IoKnownSize for $impl<$( $lifetime ,)? D, SIZE>whereD: DriverObject,{const MIN_SIZE: usize = SIZE;}impl_vmap_io_capable!($impl, u8 $( , $lifetime )?);impl_vmap_io_capable!($impl, u16 $( , $lifetime )?);impl_vmap_io_capable!($impl, u32 $( , $lifetime )?);#[cfg(CONFIG_64BIT)]impl_vmap_io_capable!($impl, u64 $( , $lifetime )?);- };
+}
+/// An owned reference to a virtual mapping for a shmem-based GEM object in kernel address space. +/// +/// # Invariants +/// +/// - The size of `owner` is >= SIZE. +/// - The memory pointed to by addr remains valid at least until this object is dropped. +pub struct VMap<D: DriverObject, const SIZE: usize = 0> {
- addr: *mut c_void,
- owner: ARef<Object<D>>,
+}
+impl_vmap_common!(VMap);
+impl<D: DriverObject, const SIZE: usize> Clone for VMap<D, SIZE> {
- #[inline]
- fn clone(&self) -> Self {
// SAFETY: We have a successful vmap already, so this can't failunsafe { self.owner.owned_vmap().unwrap_unchecked() }- }
+}
+impl<'a, D: DriverObject, const SIZE: usize> From<VMapRef<'a, D, SIZE>> for VMap<D, SIZE> {
- #[inline]
- fn from(value: VMapRef<'a, D, SIZE>) -> Self {
let this = Self {addr: value.addr,owner: value.owner.into(),};mem::forget(value);this- }
+}
+// SAFETY: addr is guaranteed to be valid and accessible for the lifetime of VMap, ensuring its +// safe to send across threads. +unsafe impl<D: DriverObject, const SIZE: usize> Send for VMap<D, SIZE> {} +// SAFETY: addr is guaranteed to be valid and accessible for the lifetime of VMap, ensuring its +// safe to send across threads. +unsafe impl<D: DriverObject, const SIZE: usize> Sync for VMap<D, SIZE> {}
+/// A borrowed reference to a virtual mapping for a shmem-based GEM object in kernel address space. +pub struct VMapRef<'a, D: DriverObject, const SIZE: usize = 0> {
- addr: *mut c_void,
- owner: &'a Object<D>,
+}
+impl_vmap_common!(VMapRef, 'a);
+impl<'a, D: DriverObject, const SIZE: usize> Clone for VMapRef<'a, D, SIZE> {
- #[inline]
- fn clone(&self) -> Self {
// SAFETY: We have a successful vmap already, so this can't failunsafe { self.owner.vmap().unwrap_unchecked() }- }
+}
- /// An owned reference to a scatter-gather table of DMA address spans for a GEM shmem object. /// /// This object holds an owned reference to the underlying GEM shmem object, ensuring that the
@@ -298,3 +535,120 @@ fn deref(&self) -> &Self::Target { unsafe { self.sgt.as_ref() } } }
+#[kunit_tests(rust_drm_gem_shmem)] +mod tests {
- use super::*;
- use crate::{
drm,faux,page::PAGE_SIZE, //- };
- // The bare minimum needed to create a fake drm driver for kunit
- #[pin_data]
- struct KunitData {}
- struct KunitDriver;
- struct KunitFile;
- #[pin_data]
- struct KunitObject {}
- const INFO: drm::DriverInfo = drm::DriverInfo {
major: 0,minor: 0,patchlevel: 0,name: c"kunit",desc: c"Kunit",- };
- impl drm::file::DriverFile for KunitFile {
type Driver = KunitDriver;fn open(_dev: &drm::Device<KunitDriver>) -> Result<Pin<KBox<Self>>> {Ok(KBox::new(Self, GFP_KERNEL)?.into())}- }
- impl gem::DriverObject for KunitObject {
type Driver = KunitDriver;type Args = ();fn new(_dev: &drm::Device<KunitDriver>,_size: usize,_args: Self::Args,) -> impl PinInit<Self, Error> {try_pin_init!(KunitObject {})}- }
- #[vtable]
- impl drm::Driver for KunitDriver {
type Data = KunitData;type File = KunitFile;type Object = Object<KunitObject>;const INFO: drm::DriverInfo = INFO;const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[];- }
- fn create_drm_dev() -> Result<(faux::Registration, ARef<drm::Device<KunitDriver>>)> {
// Create a faux DRM device so we can test gem object creation.let data = try_pin_init!(KunitData {});let dev = faux::Registration::new(c"Kunit", None)?;let drm = drm::Device::<KunitDriver>::new(dev.as_ref(), data)?;
When applying "PATCH v5 0/4 Introduce DeviceContext", I got a compilation error: error[E0049]: associated type `Object` has 0 type parameters but its trait declaration has 1 type parameter --> rust/kernel/drm/gem/shmem.rs:610:20 | 610 | type Object = Object<KunitObject>; | ^ found 0 type parameters | ::: rust/kernel/drm/driver.rs:114:17 | 114 | type Object<Ctx: drm::DeviceContext>: AllocImpl; | --- expected 1 type parameter
error[E0599]: no function or associated item named `new` found for struct `drm::device::Devicedrm::gem::shmem::tests::KunitDriver` in the current scope --> rust/kernel/drm/gem/shmem.rs:620:47 | 620 | let drm = drm::Device::<KunitDriver>::new(dev.as_ref(), data)?; | ^^^ function or associated item not found in `drm::device::Devicedrm::gem::shmem::tests::KunitDriver` | ::: rust/kernel/drm/device.rs:244:1 | 244 | pub struct Device<T: drm::Driver, C: DeviceContext = Registered> { | ---------------------------------------------------------------- function or associated item `new` not found for this struct | = help: items from traits can only be used if the trait is implemented and in scope note: `drm::gem::DriverObject` defines an item `new`, perhaps you need to implement it --> rust/kernel/drm/gem/mod.rs:91:1 | 91 | pub trait DriverObject: Sync + Send + Sized { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors
Ok((dev, drm))- }
- #[test]
- fn compile_time_vmap_sizes() -> Result {
let (_dev, drm) = create_drm_dev()?;// Create a gem object to test withlet cfg_ = ObjectConfig::<KunitObject> {map_wc: false,parent_resv_obj: None,};let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, cfg_, ())?;// Try creating a normal vmapobj.vmap::<PAGE_SIZE>()?;// Try creating a vmap that's smaller then the size we specifiedobj.vmap::<{ PAGE_SIZE - 100 }>()?;// Make sure creating a vmap that's too large failsassert!(obj.vmap::<{ PAGE_SIZE + 200 }>().is_err());Ok(())- }
- #[test]
- fn vmap_io() -> Result {
let (_dev, drm) = create_drm_dev()?;// Create a gem object to test withlet cfg_ = ObjectConfig::<KunitObject> {map_wc: false,parent_resv_obj: None,};let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, cfg_, ())?;let vmap = obj.vmap::<PAGE_SIZE>()?;vmap.write8(0xDE, 0x0);assert_eq!(vmap.read8(0x0), 0xDE);vmap.write32(0xFFFFFFFF, 0x20);assert_eq!(vmap.read32(0x20), 0xFFFFFFFF);assert_eq!(vmap.read8(0x20), 0xFF);assert_eq!(vmap.read8(0x21), 0xFF);assert_eq!(vmap.read8(0x22), 0xFF);assert_eq!(vmap.read8(0x23), 0xFF);Ok(())- }
+}
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-r... [2] https://rust-for-linux.com/contributing#submit-checklist-addendum
(@Deborah: I assume you were testing this with Tyr built-in?)
@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: Opaquebindings::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)]
On Thu, Mar 26, 2026 at 2:15 AM Danilo Krummrich dakr@kernel.org 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-r... [2] https://rust-for-linux.com/contributing#submit-checklist-addendum
(@Deborah: I assume you were testing this with Tyr built-in?)
@Lyude, Alice, Miguel: Please have a look at what I came up with below.
It looks okay to me.
Alice
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-r... [2] https://rust-for-linux.com/contributing#submit-checklist-addendum
(@Deborah: I assume you were testing this with Tyr built-in?)
@Lyude, Alice, Miguel: Please have a look at what I came up with below.
Looks fine, asahi had the bool CONFIG_RUST_DRM_GEM_SHMEM_HELPER already in the asahi Kconfig so I never noticed that's missing. Same for configs which do not excercise gem shmem.
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>
Reviewed-by: Janne Grunau j@jananu.net
Janne
On Thu, Mar 26, 2026 at 2:15 AM Danilo Krummrich dakr@kernel.org wrote:
@Lyude, Alice, Miguel: Please have a look at what I came up with below.
Sounds fine to me, thanks!
- Regarding the enabled unstable feature: looks good -- it will go away this cycle anyway, so we may get a conflict, but that is fine.
- Regarding the `&raw`, that sounds also good. I see you did it similarly to how I did it for `bitmap.rs` -- thanks!
(And for future reference, the `bitmap.rs` one was 1.92 instead of 1.82 because that one was for unions, while this one is https://blog.rust-lang.org/2024/10/17/Rust-1.82.0/#safely-addressing-unsafe-...).
- Regarding the `expect(unused)` -- using `cfg_attr` is fine since it is a simple case, but if it becomes a big issue, then of course please feel free to use `allow` (you already know this, but in case it helps others: I wrote some considerations about this at https://docs.kernel.org/rust/coding-guidelines.html#lints).
Cheers, Miguel
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-r... [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 Rustconfig 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: Opaquebindings::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_objectlet 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)]
On Mon Mar 16, 2026 at 10:16 PM CET, Lyude Paul wrote:
Applied to drm-rust-next, thanks!
Asahi Lina (2): rust: drm: gem: shmem: Add DRM shmem helper abstraction
[ * 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 ]
Lyude Paul (5): rust: drm: gem: Add raw_dma_resv() function
[ Fix incorrect reference in safety comment. - Danilo ]
linaro-mm-sig@lists.linaro.org