rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects
@ 2025-05-21 20:29 Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock Lyude Paul
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Sumit Semwal,
	Christian König,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

This is the next version of the shmem backed GEM objects series
originally from Asahi, previously posted by Daniel Almeida. Along with
bindings for shmem backed GEM objects, it also adds a few features that
various users like Tyr and Asahi are interested in:

* The ability to pass custom arguments to new GEM objects (needed by
  Tyr)
* OpaqueObject (to enable the use of custom private GEM objects, which I
  believe asahi wanted)

And replaces some of the hand-rolled API bindings (sg_table mainly) with
some of the WIP patch series for adding kernel-wide bindings. It also
addresses the comments from the code review of the last version of this
patch series.

Currently doesn't apply on an upstream branch, but should very soon as
all of the dependencies in this series are on a mailing list already.

The current branch this can be applied on top of is here:
  https://gitlab.freedesktop.org/lyudess/linux/-/commits/rust%2Fgem-shmem-base

Which is based on top of nova/nova-next with the following patch series
applied:
  * My (hopefully final) gem bindings cleanup:
    https://lkml.org/lkml/2025/5/20/1541
  * Benno's derive Zeroable series:
    https://lkml.org/lkml/2025/5/20/1446
  * Abdiel's sg_table series:
    https://lwn.net/Articles/1020986/
    Also, there is one FIXES patch on top of Abdiel's work to fix some
    iterator bugs. These fixes have already been mentioned on the
    mailing list and should not be needed for their V2 version

Asahi Lina (3):
  rust: helpers: Add bindings/wrappers for dma_resv_lock
  rust: drm: gem: shmem: Add DRM shmem helper abstraction
  rust: drm: gem: shmem: Add share_dma_resv to ObjectConfig

Lyude Paul (9):
  rust: drm: gem: Add raw_dma_resv() function
  drm/gem/shmem: Extract drm_gem_shmem_init() from
    drm_gem_shmem_create()
  drm/gem/shmem: Extract drm_gem_shmem_release() from
    drm_gem_shmem_free()
  rust: gem: Introduce BaseDriverObject::Args
  rust: drm: gem: Add OpaqueObject
  rust: drm: gem: Introduce OwnedSGTable
  rust: Add dma_buf stub bindings
  rust: drm: gem: Add export() callback
  rust: drm: gem: Add BaseObject::prime_export()

 drivers/gpu/drm/drm_gem_shmem_helper.c |  98 +++++--
 drivers/gpu/drm/nova/gem.rs            |   6 +-
 include/drm/drm_gem_shmem_helper.h     |   2 +
 rust/bindings/bindings_helper.h        |   4 +
 rust/helpers/dma-resv.c                |  13 +
 rust/helpers/drm.c                     |  48 +++-
 rust/helpers/helpers.c                 |   1 +
 rust/kernel/dma_buf.rs                 |  39 +++
 rust/kernel/drm/gem/mod.rs             | 187 ++++++++++++-
 rust/kernel/drm/gem/shmem.rs           | 370 +++++++++++++++++++++++++
 rust/kernel/lib.rs                     |   1 +
 11 files changed, 727 insertions(+), 42 deletions(-)
 create mode 100644 rust/helpers/dma-resv.c
 create mode 100644 rust/kernel/dma_buf.rs
 create mode 100644 rust/kernel/drm/gem/shmem.rs

-- 
2.49.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-22  8:44   ` Christian König
  2025-05-21 20:29 ` [PATCH v2 02/12] rust: drm: gem: Add raw_dma_resv() function Lyude Paul
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Asahi Lina, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Sumit Semwal,
	Christian König, Greg Kroah-Hartman, Viresh Kumar,
	Wedson Almeida Filho, Daniel Almeida,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

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>
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 31369b7b23884..409e9a595e051 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -18,6 +18,7 @@
 #include <linux/blkdev.h>
 #include <linux/cpumask.h>
 #include <linux/cred.h>
+#include <linux/dma-resv.h>
 #include <linux/device/faux.h>
 #include <linux/dma-mapping.h>
 #include <linux/dma-direction.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 20a4ee59acd89..3ba1652899c2b 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -15,6 +15,7 @@
 #include "cpumask.c"
 #include "cred.c"
 #include "device.c"
+#include "dma-resv.c"
 #include "drm.c"
 #include "err.c"
 #include "fs.c"
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 02/12] rust: drm: gem: Add raw_dma_resv() function
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 03/12] drm/gem/shmem: Extract drm_gem_shmem_init() from drm_gem_shmem_create() Lyude Paul
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Sumit Semwal,
	Christian König, Daniel Almeida, Asahi Lina,
	Alyssa Rosenzweig,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

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>
---
 rust/kernel/drm/gem/mod.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 992e098d0a3e2..1165417b22df6 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -186,6 +186,17 @@ fn create_mmap_offset(&self) -> Result<u64> {
 
 impl<T: IntoGEMObject> BaseObject for T {}
 
+/// Crate-private base operations shared by all GEM object classes.
+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
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 03/12] drm/gem/shmem: Extract drm_gem_shmem_init() from drm_gem_shmem_create()
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 02/12] rust: drm: gem: Add raw_dma_resv() function Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 04/12] drm/gem/shmem: Extract drm_gem_shmem_release() from drm_gem_shmem_free() Lyude Paul
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich

With gem objects in rust, the most ideal way for us to be able to handle
gem shmem object creation is to be able to handle the memory allocation of
a gem object ourselves - and then have the DRM gem shmem helpers initialize
the object we've allocated afterwards. So, let's spit out
drm_gem_shmem_init() from drm_gem_shmem_create() to allow for doing this.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 75 +++++++++++++++++---------
 include/drm/drm_gem_shmem_helper.h     |  1 +
 2 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index a4144e0ef67ef..f3e88fb579b52 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -48,28 +48,12 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
 	.vm_ops = &drm_gem_shmem_vm_ops,
 };
 
-static struct drm_gem_shmem_object *
-__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
-		       struct vfsmount *gemfs)
+static int __drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object *shmem,
+				size_t size, bool private, struct vfsmount *gemfs)
 {
-	struct drm_gem_shmem_object *shmem;
-	struct drm_gem_object *obj;
+	struct drm_gem_object *obj = &shmem->base;
 	int ret = 0;
 
-	size = PAGE_ALIGN(size);
-
-	if (dev->driver->gem_create_object) {
-		obj = dev->driver->gem_create_object(dev, size);
-		if (IS_ERR(obj))
-			return ERR_CAST(obj);
-		shmem = to_drm_gem_shmem_obj(obj);
-	} else {
-		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
-		if (!shmem)
-			return ERR_PTR(-ENOMEM);
-		obj = &shmem->base;
-	}
-
 	if (!obj->funcs)
 		obj->funcs = &drm_gem_shmem_funcs;
 
@@ -81,7 +65,7 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
 	}
 	if (ret) {
 		drm_gem_private_object_fini(obj);
-		goto err_free;
+		return ret;
 	}
 
 	ret = drm_gem_create_mmap_offset(obj);
@@ -102,14 +86,55 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
 				     __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
 	}
 
-	return shmem;
-
+	return 0;
 err_release:
 	drm_gem_object_release(obj);
-err_free:
-	kfree(obj);
+	return ret;
+}
 
-	return ERR_PTR(ret);
+/**
+ * drm_gem_shmem_init - Initialize an allocated object.
+ * @dev: DRM device
+ * @obj: The allocated shmem GEM object.
+ *
+ * Returns:
+ * 0 on success, or a negative error code on failure.
+ */
+int drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object *shmem, size_t size)
+{
+	return __drm_gem_shmem_init(dev, shmem, size, false, NULL);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_init);
+
+static struct drm_gem_shmem_object *
+__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
+		       struct vfsmount *gemfs)
+{
+	struct drm_gem_shmem_object *shmem;
+	struct drm_gem_object *obj;
+	int ret = 0;
+
+	size = PAGE_ALIGN(size);
+
+	if (dev->driver->gem_create_object) {
+		obj = dev->driver->gem_create_object(dev, size);
+		if (IS_ERR(obj))
+			return ERR_CAST(obj);
+		shmem = to_drm_gem_shmem_obj(obj);
+	} else {
+		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+		if (!shmem)
+			return ERR_PTR(-ENOMEM);
+		obj = &shmem->base;
+	}
+
+	ret = __drm_gem_shmem_init(dev, shmem, size, private, gemfs);
+	if (ret) {
+		kfree(obj);
+		return ERR_PTR(ret);
+	}
+
+	return shmem;
 }
 /**
  * drm_gem_shmem_create - Allocate an object with the given size
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index cef5a6b5a4d63..ea0785d1700a5 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -96,6 +96,7 @@ struct drm_gem_shmem_object {
 #define to_drm_gem_shmem_obj(obj) \
 	container_of(obj, struct drm_gem_shmem_object, base)
 
+int drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object *shmem, size_t size);
 struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size);
 struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device *dev,
 							   size_t size,
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 04/12] drm/gem/shmem: Extract drm_gem_shmem_release() from drm_gem_shmem_free()
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (2 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 03/12] drm/gem/shmem: Extract drm_gem_shmem_init() from drm_gem_shmem_create() Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 05/12] rust: gem: Introduce BaseDriverObject::Args Lyude Paul
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich

At the moment the way that freeing gem shmem objects is not ideal for rust
bindings. drm_gem_shmem_free() releases all of the associated memory with a
gem shmem object with kfree(), which means that for us to correctly release
a gem shmem object in rust we have to manually drop all of the contents of
our gem object structure in-place by hand before finally calling
drm_gem_shmem_free() to release the shmem resources and the allocation for
the gem object.

Since the only reason this is an issue is because of drm_gem_shmem_free()
calling kfree(), we can fix this by splitting drm_gem_shmem_free() out into
itself and drm_gem_shmem_release(), where drm_gem_shmem_release() releases
the various gem shmem resources without freeing the structure itself. With
this, we can safely re-acquire the KBox for the gem object's memory
allocation and let rust handle cleaning up all of the other struct members
automatically.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 23 ++++++++++++++++++-----
 include/drm/drm_gem_shmem_helper.h     |  1 +
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index f3e88fb579b52..42652be37ba77 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -175,13 +175,13 @@ struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device *de
 EXPORT_SYMBOL_GPL(drm_gem_shmem_create_with_mnt);
 
 /**
- * drm_gem_shmem_free - Free resources associated with a shmem GEM object
- * @shmem: shmem GEM object to free
+ * drm_gem_shmem_release - Release resources associated with a shmem GEM object.
+ * @shmem: shmem GEM object
  *
- * This function cleans up the GEM object state and frees the memory used to
- * store the object itself.
+ * This function cleans up the GEM object state, but does not free the memory used to store the
+ * object itself. This function is meant to be a dedicated helper for the Rust GEM bindings.
  */
-void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
+void drm_gem_shmem_release(struct drm_gem_shmem_object *shmem)
 {
 	struct drm_gem_object *obj = &shmem->base;
 
@@ -207,6 +207,19 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
 	}
 
 	drm_gem_object_release(obj);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_release);
+
+/**
+ * drm_gem_shmem_free - Free resources associated with a shmem GEM object
+ * @shmem: shmem GEM object to free
+ *
+ * This function cleans up the GEM object state and frees the memory used to
+ * store the object itself.
+ */
+void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
+{
+	drm_gem_shmem_release(shmem);
 	kfree(shmem);
 }
 EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index ea0785d1700a5..e009022b0bb48 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -101,6 +101,7 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t
 struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device *dev,
 							   size_t size,
 							   struct vfsmount *gemfs);
+void drm_gem_shmem_release(struct drm_gem_shmem_object *shmem);
 void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem);
 
 void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 05/12] rust: gem: Introduce BaseDriverObject::Args
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (3 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 04/12] drm/gem/shmem: Extract drm_gem_shmem_release() from drm_gem_shmem_free() Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 06/12] rust: drm: gem: Add OpaqueObject Lyude Paul
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Daniel Almeida, Asahi Lina, Alyssa Rosenzweig,
	open list:DRM DRIVER FOR NVIDIA GPUS [RUST]

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>
---
 drivers/gpu/drm/nova/gem.rs |  5 +++--
 rust/kernel/drm/gem/mod.rs  | 17 ++++++++++++++---
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index a3024922f0d90..f2f23320110dd 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::BaseDriverObject for NovaObject {
     type Driver = NovaDriver;
     type Object = gem::Object<Self>;
+    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 {})
     }
 }
@@ -34,7 +35,7 @@ pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self
             return Err(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/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 1165417b22df6..f0029e6989ed6 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -29,8 +29,15 @@ pub trait BaseDriverObject: Sync + Send + Sized {
     /// The GEM object type that will be passed to various callbacks.
     type Object: AllocImpl;
 
+    /// The data type to use for passing arguments to [`BaseDriverObject::new`].
+    type Args;
+
     /// Create a new driver data object for a GEM object of a given size.
-    fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>;
+    fn new(
+        dev: &drm::Device<Self::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::Object, _file: &DriverFile<Self>) -> Result {
@@ -232,11 +239,15 @@ impl<T: BaseDriverObject> 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),
                 // INVARIANT: The drm subsystem guarantees that the `struct drm_device` will live
                 // as long as the GEM object lives.
                 dev: dev.into(),
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 06/12] rust: drm: gem: Add OpaqueObject
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (4 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 05/12] rust: gem: Introduce BaseDriverObject::Args Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 07/12] rust: drm: gem: shmem: Add DRM shmem helper abstraction Lyude Paul
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Daniel Almeida, Asahi Lina,
	Alyssa Rosenzweig

In the future, we want to have the ability for a driver to have private gem
objects - or use gem objects across ffi boundaries that don't use our
driver's GEM object implementation. So, let's take some inspiration from
the KMS bindings I've been working on and introduce an OpaqueObject type.

This type can be used identically to a normal gem object, with the
exception that the private-data layout of the object is not known.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/drm/gem/mod.rs | 61 +++++++++++++++++++++++++++++++++++---
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index f0029e6989ed6..aeec847b80aa7 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -6,13 +6,13 @@
 
 use crate::{
     alloc::flags::*,
-    bindings, drm,
+    bindings, drm::{self, private::Sealed},
     drm::driver::{AllocImpl, AllocOps},
     error::{to_result, Result},
     prelude::*,
     types::{ARef, AlwaysRefCounted, Opaque},
 };
-use core::{ops::Deref, ptr::NonNull};
+use core::{ops::Deref, ptr::NonNull, marker::PhantomData};
 
 /// A type alias for retrieving a [`Driver`]s [`DriverFile`] implementation from its
 /// [`DriverObject`] implementation.
@@ -21,6 +21,26 @@
 /// [`DriverFile`]: drm::file::DriverFile
 pub type DriverFile<T> = drm::File<<<T as BaseDriverObject>::Driver as drm::Driver>::File>;
 
+/// A helper macro for implementing AsRef<OpaqueObject<…>>
+macro_rules! impl_as_opaque {
+    ($type:ty where $tparam:ident : $tparam_trait:ident) => {
+        impl<D, $tparam> core::convert::AsRef<kernel::drm::gem::OpaqueObject<D>> for $type
+        where
+            D: kernel::drm::driver::Driver,
+            Self: kernel::drm::gem::BaseDriverObject<Driver = D>,
+            Self: kernel::drm::gem::IntoGEMObject,
+            $tparam: $tparam_trait
+        {
+            fn as_ref(&self) -> &kernel::drm::gem::OpaqueObject<D> {
+                // SAFETY: This cast is safe via our type invariant.
+                unsafe { &*((self.as_raw().cast_const()).cast()) }
+            }
+        }
+    };
+}
+
+pub(crate) use impl_as_opaque;
+
 /// GEM object functions, which must be implemented by drivers.
 pub trait BaseDriverObject: Sync + Send + Sized {
     /// Parent `Driver` for this object.
@@ -49,7 +69,7 @@ fn close(_obj: &Self::Object, _file: &DriverFile<Self>) {}
 }
 
 /// Trait that represents a GEM object subtype
-pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
+pub trait IntoGEMObject: Sized + Sealed + AlwaysRefCounted {
     /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as
     /// this owning object is valid.
     fn as_raw(&self) -> *mut bindings::drm_gem_object;
@@ -297,7 +317,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
     }
 }
 
-impl<T: BaseDriverObject> super::private::Sealed for Object<T> {}
+impl<T: BaseDriverObject> Sealed for Object<T> {}
 
 impl<T: BaseDriverObject> Deref for Object<T> {
     type Target = T;
@@ -321,6 +341,39 @@ impl<T: BaseDriverObject> AllocImpl for Object<T> {
     };
 }
 
+impl_as_opaque!(Object<T> where T: BaseDriverObject);
+
+/// A GEM object whose private-data layout is not known.
+///
+/// Not all GEM objects are created equal, and subsequently drivers may occasionally need to deal
+/// with situations where they are working with a GEM object but have no knowledge of its
+/// private-data layout.
+///
+/// It may be used just like a normal [`Object`], with the exception that it cannot access
+/// driver-private data.
+///
+/// # Invariant
+///
+/// Via `#[repr(transparent)]`, this type is guaranteed to have an identical data layout to
+/// `struct drm_gem_object`.
+#[repr(transparent)]
+pub struct OpaqueObject<T: drm::Driver>(Opaque<bindings::drm_gem_object>, PhantomData<T>);
+
+impl<T: drm::Driver> IntoGEMObject for OpaqueObject<T> {
+    unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self {
+        // SAFETY:
+        // - This cast is safe via our type invariant.
+        // - `self_ptr` is guaranteed to be a valid pointer to a gem object by our safety contract.
+        unsafe { &*self_ptr.cast::<Self>().cast_const() }
+    }
+
+    fn as_raw(&self) -> *mut bindings::drm_gem_object {
+        self.0.get()
+    }
+}
+
+impl<D: drm::Driver> Sealed for OpaqueObject<D> {}
+
 pub(super) const fn create_fops() -> bindings::file_operations {
     // SAFETY: As by the type invariant, it is safe to initialize `bindings::file_operations`
     // zeroed.
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 07/12] rust: drm: gem: shmem: Add DRM shmem helper abstraction
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (5 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 06/12] rust: drm: gem: Add OpaqueObject Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 08/12] rust: drm: gem: shmem: Add share_dma_resv to ObjectConfig Lyude Paul
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Asahi Lina, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, David Airlie, Simona Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Greg Kroah-Hartman, Wedson Almeida Filho, Daniel Almeida,
	Alyssa Rosenzweig

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>
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.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/bindings/bindings_helper.h |   3 +
 rust/helpers/drm.c              |  48 ++++-
 rust/kernel/drm/gem/mod.rs      |   2 +
 rust/kernel/drm/gem/shmem.rs    | 338 ++++++++++++++++++++++++++++++++
 4 files changed, 390 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/drm/gem/shmem.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 409e9a595e051..3974cb1a252d3 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -10,6 +10,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>
@@ -27,6 +28,8 @@
 #include <linux/file.h>
 #include <linux/firmware.h>
 #include <linux/fs.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 450b406c6f273..a4e997d0b4732 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
@@ -20,4 +21,49 @@ __u64 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
+void rust_helper_drm_gem_shmem_object_free(struct drm_gem_object *obj)
+{
+	return drm_gem_shmem_object_free(obj);
+}
+
+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);
+}
+
+int rust_helper_drm_gem_shmem_object_pin(struct drm_gem_object *obj)
+{
+	return drm_gem_shmem_object_pin(obj);
+}
+
+void rust_helper_drm_gem_shmem_object_unpin(struct drm_gem_object *obj)
+{
+	drm_gem_shmem_object_unpin(obj);
+}
+
+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);
+}
+
+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);
+}
+
+void rust_helper_drm_gem_shmem_object_vunmap(struct drm_gem_object *obj,
+                                              struct iosys_map *map)
+{
+	drm_gem_shmem_object_vunmap(obj, map);
+}
+
+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 aeec847b80aa7..6d87e75690d2f 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/linux/drm/drm_gem.h`](srctree/include/linux/drm/drm_gem.h)
+#[cfg(CONFIG_DRM_GEM_SHMEM_HELPER = "y")]
+pub mod shmem;
 
 use crate::{
     alloc::flags::*,
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
new file mode 100644
index 0000000000000..57ec2538a905e
--- /dev/null
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -0,0 +1,338 @@
+// 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::{
+    drm::{
+        device,
+        driver,
+        gem,
+        private::Sealed
+    },
+    error::{from_err_ptr, to_result},
+    prelude::*,
+    types::{ARef, Opaque},
+    container_of,
+    scatterlist,
+};
+use core::{
+    mem::MaybeUninit,
+    ops::{Deref, DerefMut},
+    ptr::{addr_of_mut, NonNull},
+    slice,
+};
+use gem::{
+    BaseObject,
+    BaseObjectPrivate,
+    BaseDriverObject,
+    OpaqueObject,
+    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 {
+    /// Whether to set the write-combine map flag.
+    pub map_wc: bool,
+}
+
+/// A shmem-backed GEM object.
+///
+/// # Invariants
+///
+/// The DRM core ensures that `dev` will remain valid for as long as the object.
+#[repr(C)]
+#[pin_data]
+pub struct Object<T: BaseDriverObject> {
+    #[pin]
+    obj: Opaque<bindings::drm_gem_shmem_object>,
+    dev: NonNull<device::Device<T::Driver>>,
+    #[pin]
+    inner: T,
+}
+
+super::impl_as_opaque!(Object<T> where T: BaseDriverObject);
+
+impl<T: BaseDriverObject> 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_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,
+        args: T::Args,
+    ) -> Result<ARef<Self>> {
+        let new: Pin<KBox<Self>> = KBox::try_pin_init(
+            try_pin_init!(Self {
+                obj <- pin_init::zeroed(),
+                dev: NonNull::from(dev),
+                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_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 mut obj = unsafe { ARef::from_raw(NonNull::new_unchecked(new)) };
+
+        // 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_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: We are guaranteed that `dev` is valid for as long as this object is valid by our
+        // type invariants
+        unsafe { self.dev.as_ref() }
+    }
+
+    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.cast_mut()) };
+
+        // 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!(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 a scatter-gather table of DMA pages for this object.
+    ///
+    /// This will pin the object in memory.
+    pub fn sg_table(&self) -> Result<SGTable<T>> {
+        // 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_shmem()) })?;
+
+        Ok(SGTable {
+            // SAFETY: We checked that `sgt` is not an error pointer, so it must be a valid pointer
+            // to a scatterlist.
+            sgt: NonNull::from(unsafe { scatterlist::SGTable::as_ref(sgt) }),
+            // INVARIANT: We take an owned refcount to `self` here, ensuring that `sgt` remains
+            // valid for as long as this `OwnedSGTable`.
+            _owner: self.into()
+        })
+    }
+
+    /// Creates and returns a virtual kernel memory mapping for this object.
+    pub fn vmap(&self) -> Result<VMap<T>> {
+        let mut map: MaybeUninit<bindings::iosys_map> = MaybeUninit::uninit();
+
+        // SAFETY:
+        // - drm_gem_shmem_vmap can be called with the DMA reservation lock held
+        // - Our ARef is proof that `obj` is safe to deref
+        to_result(unsafe {
+            // TODO: see top of file
+            bindings::dma_resv_lock(self.raw_dma_resv(), core::ptr::null_mut());
+            let ret = bindings::drm_gem_shmem_vmap(self.as_shmem(), map.as_mut_ptr());
+            bindings::dma_resv_unlock(self.raw_dma_resv());
+            ret
+        })?;
+
+        // SAFETY: if drm_gem_shmem_vmap did not fail, map is initialized now
+        let map = unsafe { map.assume_init() };
+
+        Ok(VMap {
+            map,
+            owner: self.into(),
+        })
+    }
+}
+
+impl<T: BaseDriverObject> Deref for Object<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
+
+impl<T: BaseDriverObject> DerefMut for Object<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.inner
+    }
+}
+
+impl<T: BaseDriverObject> Sealed for Object<T> {}
+
+impl<T: BaseDriverObject> gem::IntoGEMObject for Object<T> {
+    fn as_raw(&self) -> *mut bindings::drm_gem_object {
+        // SAFETY: Our immutable reference is proof that this is afe to dereference
+        unsafe { addr_of_mut!((*self.obj.get()).base) }
+    }
+
+    unsafe fn as_ref<'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 = container_of!(obj, bindings::drm_gem_shmem_object, base);
+
+            &*container_of!(obj, Object<T>, obj)
+        }
+    }
+}
+
+impl<T: BaseDriverObject> 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,
+    };
+}
+
+/// A virtual mapping for a shmem-backed GEM object in kernel address space.
+pub struct VMap<T: BaseDriverObject> {
+    map: bindings::iosys_map,
+    owner: ARef<Object<T>>,
+}
+
+impl<T: BaseDriverObject> VMap<T> {
+    /// Returns a const raw pointer to the start of the mapping.
+    pub fn as_ptr(&self) -> *const core::ffi::c_void {
+        // SAFETY: The shmem helpers always return non-iomem maps
+        unsafe { self.map.__bindgen_anon_1.vaddr }
+    }
+
+    /// Returns a mutable raw pointer to the start of the mapping.
+    pub fn as_mut_ptr(&mut self) -> *mut core::ffi::c_void {
+        // SAFETY: The shmem helpers always return non-iomem maps
+        unsafe { self.map.__bindgen_anon_1.vaddr }
+    }
+
+    /// Returns a byte slice view of the mapping.
+    pub fn as_slice(&self) -> &[u8] {
+        // SAFETY: The vmap maps valid memory up to the owner size
+        unsafe { slice::from_raw_parts(self.as_ptr() as *const u8, self.owner.size()) }
+    }
+
+    /// Returns mutable a byte slice view of the mapping.
+    pub fn as_mut_slice(&mut self) -> &mut [u8] {
+        // SAFETY: The vmap maps valid memory up to the owner size
+        unsafe { slice::from_raw_parts_mut(self.as_mut_ptr() as *mut u8, self.owner.size()) }
+    }
+
+    /// Borrows a reference to the object that owns this virtual mapping.
+    pub fn owner(&self) -> &ARef<Object<T>> {
+        &self.owner
+    }
+}
+
+impl<T: BaseDriverObject> Drop for VMap<T> {
+    fn drop(&mut self) {
+        // 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 {
+            let resv = self.owner.raw_dma_resv();
+
+            // TODO: see top of file
+            bindings::dma_resv_lock(resv, core::ptr::null_mut());
+            bindings::drm_gem_shmem_vunmap(self.owner.as_shmem(), &mut self.map);
+            bindings::dma_resv_unlock(resv);
+        }
+    }
+}
+
+/// SAFETY: `iosys_map` objects are safe to send across threads.
+unsafe impl<T: BaseDriverObject> Send for VMap<T> {}
+/// SAFETY: `iosys_map` objects are safe to send across threads.
+unsafe impl<T: BaseDriverObject> Sync for VMap<T> {}
+
+/// An owned scatter-gather table of DMA address spans for a GEM shmem 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.
+pub struct OwnedSGTable<T: BaseDriverObject> {
+    sgt: NonNull<SGTable>,
+    _owner: ARef<Object<T>>,
+}
+
+// SAFETY: This object is only exposed in situations where we know the underlying `SGTable` will not
+// be modified for the lifetime of this object.
+unsafe impl<T: BaseDriverObject> Send for OwnedSGTable<T> {}
+// SAFETY: This object is only exposed in situations where we know the underlying `SGTable` will not
+// be modified for the lifetime of this object.
+unsafe impl<T: BaseDriverObject> Sync for OwnedSGTable<T> {}
+
+impl<T: BaseDriverObject> Deref for OwnedSGTable<T> {
+    type Target = SGTable;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: Creating an immutable reference to this is safe via our type invariants.
+        unsafe { self.sgt.as_ref() }
+    }
+}
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 08/12] rust: drm: gem: shmem: Add share_dma_resv to ObjectConfig
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (6 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 07/12] rust: drm: gem: shmem: Add DRM shmem helper abstraction Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 09/12] rust: drm: gem: Introduce OwnedSGTable Lyude Paul
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Asahi Lina, David Airlie, Simona Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida

From: Asahi Lina <lina@asahilina.net>

Allow a GEM object to share another object's DMA reservation, for use
with drm_gpuvm. To keep memory safety, we hold a reference to the GEM
object owning the resv, and drop it when the child object is freed.

Signed-off-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/drm/gem/shmem.rs | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 57ec2538a905e..c38fca715429e 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -41,9 +41,14 @@
 /// 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 {
+pub struct ObjectConfig<'a, T: BaseDriverObject> {
     /// 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 OpaqueObject<T::Driver>>,
 }
 
 /// A shmem-backed GEM object.
@@ -57,6 +62,8 @@ pub struct Object<T: BaseDriverObject> {
     #[pin]
     obj: Opaque<bindings::drm_gem_shmem_object>,
     dev: NonNull<device::Device<T::Driver>>,
+    // Parent object that owns this object's DMA reservation object
+    parent_resv_obj: Option<ARef<OpaqueObject<T::Driver>>>,
     #[pin]
     inner: T,
 }
@@ -96,13 +103,14 @@ fn as_shmem(&self) -> *mut bindings::drm_gem_shmem_object {
     pub fn new(
         dev: &device::Device<T::Driver>,
         size: usize,
-        config: ObjectConfig,
+        config: ObjectConfig<'_, T>,
         args: T::Args,
     ) -> Result<ARef<Self>> {
         let new: Pin<KBox<Self>> = KBox::try_pin_init(
             try_pin_init!(Self {
                 obj <- pin_init::zeroed(),
                 dev: NonNull::from(dev),
+                parent_resv_obj: config.parent_resv_obj.map(|p| p.into()),
                 inner <- T::new(dev, size, args),
             }),
             GFP_KERNEL
@@ -118,7 +126,14 @@ pub fn new(
         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 mut obj = unsafe { ARef::from_raw(NonNull::new_unchecked(new)) };
+        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.
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 09/12] rust: drm: gem: Introduce OwnedSGTable
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (7 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 08/12] rust: drm: gem: shmem: Add share_dma_resv to ObjectConfig Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 10/12] rust: Add dma_buf stub bindings Lyude Paul
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Asahi Lina,
	Daniel Almeida

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 OwnedSGTable - 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 OwnedSGTable. The
type can be used identically to a normal SGTable.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/drm/gem/shmem.rs | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index c38fca715429e..bff038df93334 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -20,7 +20,7 @@
     prelude::*,
     types::{ARef, Opaque},
     container_of,
-    scatterlist,
+    scatterlist::SGTable,
 };
 use core::{
     mem::MaybeUninit,
@@ -172,23 +172,36 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
         let _ = unsafe { KBox::from_raw(this) };
     }
 
-    /// Creates (if necessary) and returns a scatter-gather table of DMA pages for this object.
+    /// 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.
-    pub fn sg_table(&self) -> Result<SGTable<T>> {
+    #[inline]
+    pub fn sg_table(&self) -> Result<&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_shmem()) })?;
 
-        Ok(SGTable {
-            // SAFETY: We checked that `sgt` is not an error pointer, so it must be a valid pointer
-            // to a scatterlist.
-            sgt: NonNull::from(unsafe { scatterlist::SGTable::as_ref(sgt) }),
+        // SAFETY: We checked above that `sgt` is not an error pointer, so it must be a valid
+        // pointer to a scatterlist
+        Ok(unsafe { SGTable::as_ref(sgt) })
+    }
+
+    /// Creates (if necessary) and returns an owned scatter-gather table of DMA pages for this
+    /// object.
+    ///
+    /// This is the same as [`sg_table`](Self::sg_table), except that it instead returns a
+    /// [`OwnedSGTable`] which holds a reference to the associated gem object.
+    ///
+    /// This will pin the object in memory.
+    pub fn owned_sg_table(&self) -> Result<OwnedSGTable<T>> {
+        Ok(OwnedSGTable {
+            sgt: self.sg_table()?.into(),
             // INVARIANT: We take an owned refcount to `self` here, ensuring that `sgt` remains
             // valid for as long as this `OwnedSGTable`.
-            _owner: self.into()
+            _owner: self.into(),
         })
     }
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 10/12] rust: Add dma_buf stub bindings
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (8 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 09/12] rust: drm: gem: Introduce OwnedSGTable Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 11/12] rust: drm: gem: Add export() callback Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 12/12] rust: drm: gem: Add BaseObject::prime_export() Lyude Paul
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Sumit Semwal,
	Christian König, Greg Kroah-Hartman, Wedson Almeida Filho,
	Xiangfei Ding,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

In order to implement the gem export callback, we need a type to represent
struct dma_buf. So - this commit introduces a set of stub bindings for
dma_buf. These bindings provide a ref-counted DmaBuf object, but don't
currently implement any functionality for using the DmaBuf.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/dma_buf.rs | 39 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs     |  1 +
 2 files changed, 40 insertions(+)
 create mode 100644 rust/kernel/dma_buf.rs

diff --git a/rust/kernel/dma_buf.rs b/rust/kernel/dma_buf.rs
new file mode 100644
index 0000000000000..318518ff0b28f
--- /dev/null
+++ b/rust/kernel/dma_buf.rs
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! DMA buffer API
+//!
+//! C header: [`include/linux/dma-buf.h`](srctree/include/linux/dma-buf.h)
+
+use bindings;
+use kernel::types::*;
+
+/// A DMA buffer object.
+///
+/// # Invariants
+///
+/// The data layout of this type is equivalent to that of `struct dma_buf`.
+#[repr(transparent)]
+pub struct DmaBuf(Opaque<bindings::dma_buf>);
+
+// SAFETY: `struct dma_buf` is thread-safe
+unsafe impl Send for DmaBuf {}
+// SAFETY: `struct dma_buf` is thread-safe
+unsafe impl Sync for DmaBuf {}
+
+impl DmaBuf {
+    /// Convert from a `*mut bindings::dma_buf` to a [`DmaBuf`].
+    ///
+    /// # Safety
+    ///
+    /// The caller guarantees that `self_ptr` points to a valid initialized `struct dma_buf` for the
+    /// duration of the lifetime of `'a`, and promises to not violate rust's data aliasing rules
+    /// using the reference provided by this function.
+    pub(crate) unsafe fn as_ref<'a>(self_ptr: *mut bindings::dma_buf) -> &'a Self {
+        // SAFETY: Our data layout is equivalent to `dma_buf` .
+        unsafe { &*self_ptr.cast() }
+    }
+
+    pub(crate) fn as_raw(&self) -> *mut bindings::dma_buf {
+        self.0.get()
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index cbacedfe54434..02d4311bb080c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -49,6 +49,7 @@
 pub mod device_id;
 pub mod devres;
 pub mod dma;
+pub mod dma_buf;
 pub mod driver;
 #[cfg(CONFIG_DRM = "y")]
 pub mod drm;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 11/12] rust: drm: gem: Add export() callback
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (9 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 10/12] rust: Add dma_buf stub bindings Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  2025-05-21 20:29 ` [PATCH v2 12/12] rust: drm: gem: Add BaseObject::prime_export() Lyude Paul
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Sumit Semwal, Christian König, Daniel Almeida, Asahi Lina,
	Alyssa Rosenzweig, open list:DRM DRIVER FOR NVIDIA GPUS [RUST],
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

This introduces an optional export() callback for GEM objects, which is
used to implement the drm_gem_object_funcs->export function.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nova/gem.rs  |  1 +
 rust/kernel/drm/gem/mod.rs   | 73 +++++++++++++++++++++++++++++++++++-
 rust/kernel/drm/gem/shmem.rs |  6 ++-
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index f2f23320110dd..6cc7a65a50fae 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -16,6 +16,7 @@
 #[pin_data]
 pub(crate) struct NovaObject {}
 
+#[vtable]
 impl gem::BaseDriverObject for NovaObject {
     type Driver = NovaDriver;
     type Object = gem::Object<Self>;
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 6d87e75690d2f..ef697f323e52c 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -10,7 +10,8 @@
     alloc::flags::*,
     bindings, drm::{self, private::Sealed},
     drm::driver::{AllocImpl, AllocOps},
-    error::{to_result, Result},
+    dma_buf,
+    error::{to_result, from_err_ptr, Result},
     prelude::*,
     types::{ARef, AlwaysRefCounted, Opaque},
 };
@@ -44,6 +45,7 @@ fn as_ref(&self) -> &kernel::drm::gem::OpaqueObject<D> {
 pub(crate) use impl_as_opaque;
 
 /// GEM object functions, which must be implemented by drivers.
+#[vtable]
 pub trait BaseDriverObject: Sync + Send + Sized {
     /// Parent `Driver` for this object.
     type Driver: drm::Driver;
@@ -68,6 +70,11 @@ fn open(_obj: &Self::Object, _file: &DriverFile<Self>) -> Result {
 
     /// Close a handle to an existing object, associated with a File.
     fn close(_obj: &Self::Object, _file: &DriverFile<Self>) {}
+
+    /// Optional handle for exporting a gem object.
+    fn export(_obj: &Self::Object, _flags: u32) -> Result<DmaBuf<Self::Object>> {
+        unimplemented!()
+    }
 }
 
 /// Trait that represents a GEM object subtype
@@ -137,6 +144,21 @@ extern "C" fn close_callback<T: BaseDriverObject>(
     T::close(obj, file);
 }
 
+extern "C" fn export_callback<T: BaseDriverObject>(
+    raw_obj: *mut bindings::drm_gem_object,
+    flags: i32,
+) -> *mut bindings::dma_buf {
+    // SAFETY: `export_callback` is specified in the AllocOps structure for `Object<T>`, ensuring
+    // that `raw_obj` is contained within a `Object<T>`.
+    let obj = unsafe { T::Object::as_ref(raw_obj) };
+
+    match T::export(obj, flags as _) {
+        // DRM takes a hold of the reference
+        Ok(buf) => buf.into_raw(),
+        Err(e) => e.to_ptr(),
+    }
+}
+
 impl<T: BaseDriverObject> IntoGEMObject for Object<T> {
     fn as_raw(&self) -> *mut bindings::drm_gem_object {
         self.obj.get()
@@ -247,7 +269,11 @@ impl<T: BaseDriverObject> Object<T> {
         open: Some(open_callback::<T>),
         close: Some(close_callback::<T>),
         print_info: None,
-        export: None,
+        export: if T::HAS_EXPORT {
+            Some(export_callback::<T>)
+        } else {
+            None
+        },
         pin: None,
         unpin: None,
         get_sg_table: None,
@@ -376,6 +402,49 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object {
 
 impl<D: drm::Driver> Sealed for OpaqueObject<D> {}
 
+/// A [`dma_buf::DmaBuf`] which has been exported from a GEM object.
+///
+/// The [`dma_buf::DmaBuf`] will be released when this type is dropped.
+///
+/// # Invariants
+///
+/// - `self.0` points to a valid initialized [`dma_buf::DmaBuf`] for the lifetime of this object.
+/// - The GEM object from which this [`dma_buf::DmaBuf`] was exported from is guaranteed to be of
+///   type `T`.
+pub struct DmaBuf<T: IntoGEMObject>(NonNull<dma_buf::DmaBuf>, PhantomData<T>);
+
+impl<T: IntoGEMObject> Deref for DmaBuf<T> {
+    type Target = dma_buf::DmaBuf;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: This pointer is guaranteed to be valid by our type invariants.
+        unsafe { self.0.as_ref() }
+    }
+}
+
+impl<T: IntoGEMObject> Drop for DmaBuf<T> {
+    #[inline]
+    fn drop(&mut self) {
+        // SAFETY:
+        // - `dma_buf::DmaBuf` is guaranteed to have an identical layout to `struct dma_buf`
+        //   by its type invariants.
+        // - We hold the last reference to this `DmaBuf`, making it safe to destroy.
+        unsafe { bindings::drm_gem_dmabuf_release(self.0.cast().as_ptr()) }
+    }
+}
+
+impl<T: IntoGEMObject> DmaBuf<T> {
+    /// Leak the reference for this [`DmaBuf`] and return a raw pointer to it.
+    #[inline]
+    pub(crate) fn into_raw(self) -> *mut bindings::dma_buf {
+        let dma_ptr = self.as_raw();
+
+        core::mem::forget(self);
+        dma_ptr
+    }
+}
+
 pub(super) const fn create_fops() -> bindings::file_operations {
     // SAFETY: As by the type invariant, it is safe to initialize `bindings::file_operations`
     // zeroed.
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index bff038df93334..799f15c38cc36 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -77,7 +77,11 @@ impl<T: BaseDriverObject> Object<T> {
         open: Some(super::open_callback::<T>),
         close: Some(super::close_callback::<T>),
         print_info: Some(bindings::drm_gem_shmem_object_print_info),
-        export: None,
+        export: if T::HAS_EXPORT {
+            Some(super::export_callback::<T>)
+        } else {
+            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),
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 12/12] rust: drm: gem: Add BaseObject::prime_export()
  2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
                   ` (10 preceding siblings ...)
  2025-05-21 20:29 ` [PATCH v2 11/12] rust: drm: gem: Add export() callback Lyude Paul
@ 2025-05-21 20:29 ` Lyude Paul
  11 siblings, 0 replies; 15+ messages in thread
From: Lyude Paul @ 2025-05-21 20:29 UTC (permalink / raw)
  To: dri-devel, rust-for-linux, linux-kernel
  Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Sumit Semwal,
	Christian König, Daniel Almeida, Asahi Lina,
	Alyssa Rosenzweig,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

We just added an export() callback that GEM objects can implement, but
without any way of actually exporting a DmaBuf<T>. So let's add one by
introducing bindings for drm_gem_prime_export().

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/drm/gem/mod.rs | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index ef697f323e52c..ee74b9bdb7a1f 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -225,6 +225,29 @@ fn lookup_handle<D, F, O>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>
         Ok(unsafe { ARef::from_raw(obj.into()) })
     }
 
+    /// Export a [`DmaBuf`] for this GEM object using the DRM prime helper library.
+    ///
+    /// `flags` should be a set of flags from [`fs::file::flags`](kernel::fs::file::flags).
+    fn prime_export(&self, flags: u32) -> Result<DmaBuf<Self>> {
+        // SAFETY:
+        // - `as_raw()` always returns a valid pointer to a `drm_gem_object`.
+        // - `drm_gem_prime_export()` returns either an error pointer, or a valid pointer to an
+        //   initialized `dma_buf` on success.
+        let dma_ptr = from_err_ptr(unsafe {
+            bindings::drm_gem_prime_export(self.as_raw(), flags as _)
+        })?;
+
+        // SAFETY:
+        // - We checked that dma_ptr is not an error, so it must point to an initialized dma_buf
+        // - We used drm_gem_prime_export(), so `dma_ptr` will remain valid until a call to
+        //   `drm_gem_prime_release()` which we don't call here.
+        let dma_buf = unsafe { dma_buf::DmaBuf::as_ref(dma_ptr) };
+
+        // INVARIANT: We used drm_gem_prime_export() to create this dma_buf, fulfilling the
+        // invariant that this dma_buf came from a GEM object of type `Self`.
+        Ok(DmaBuf(dma_buf.into(), PhantomData))
+    }
+
     /// Creates an mmap offset to map the object from userspace.
     fn create_mmap_offset(&self) -> Result<u64> {
         // SAFETY: The arguments are valid per the type invariant.
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock
  2025-05-21 20:29 ` [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock Lyude Paul
@ 2025-05-22  8:44   ` Christian König
  2025-05-22 15:03     ` Daniel Almeida
  0 siblings, 1 reply; 15+ messages in thread
From: Christian König @ 2025-05-22  8:44 UTC (permalink / raw)
  To: Lyude Paul, dri-devel, rust-for-linux, linux-kernel
  Cc: Asahi Lina, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Sumit Semwal, Greg Kroah-Hartman,
	Viresh Kumar, Wedson Almeida Filho, Daniel Almeida,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

On 5/21/25 22:29, Lyude Paul wrote:
> 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.

Looks good in general, but my question is if it wouldn't be better to export the higher level drm_exec component instead?

The drm_exec component implements the necessary loop if you want to lock multiple GEM objects at the same time. As well as makes sure that those GEM objects can't be released while working with them.

Regtards,
Christian.

> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> 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 31369b7b23884..409e9a595e051 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -18,6 +18,7 @@
>  #include <linux/blkdev.h>
>  #include <linux/cpumask.h>
>  #include <linux/cred.h>
> +#include <linux/dma-resv.h>
>  #include <linux/device/faux.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dma-direction.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 20a4ee59acd89..3ba1652899c2b 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -15,6 +15,7 @@
>  #include "cpumask.c"
>  #include "cred.c"
>  #include "device.c"
> +#include "dma-resv.c"
>  #include "drm.c"
>  #include "err.c"
>  #include "fs.c"


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock
  2025-05-22  8:44   ` Christian König
@ 2025-05-22 15:03     ` Daniel Almeida
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Almeida @ 2025-05-22 15:03 UTC (permalink / raw)
  To: Christian König
  Cc: Lyude Paul, dri-devel, rust-for-linux, linux-kernel, Asahi Lina,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Sumit Semwal, Greg Kroah-Hartman,
	Viresh Kumar, Wedson Almeida Filho,
	open list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b,
	moderated list:DMA BUFFER SHARING FRAMEWORK:Keyword:bdma_(?:buf|fence|resv)b

Hi Christian

> On 22 May 2025, at 05:44, Christian König <christian.koenig@amd.com> wrote:
> 
> On 5/21/25 22:29, Lyude Paul wrote:
>> 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.
> 
> Looks good in general, but my question is if it wouldn't be better to export the higher level drm_exec component instead?
> 
> The drm_exec component implements the necessary loop if you want to lock multiple GEM objects at the same time. As well as makes sure that those GEM objects can't be released while working with them.
> 
> Regtards,
> Christian.
> 

I guess Danilo is the right person to ask, but IIRC the plan was to expose the
exec logic as part of the GPUVM abstraction, which is currently work in
progress.

— Daniel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-05-22 15:04 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-21 20:29 [PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects Lyude Paul
2025-05-21 20:29 ` [PATCH v2 01/12] rust: helpers: Add bindings/wrappers for dma_resv_lock Lyude Paul
2025-05-22  8:44   ` Christian König
2025-05-22 15:03     ` Daniel Almeida
2025-05-21 20:29 ` [PATCH v2 02/12] rust: drm: gem: Add raw_dma_resv() function Lyude Paul
2025-05-21 20:29 ` [PATCH v2 03/12] drm/gem/shmem: Extract drm_gem_shmem_init() from drm_gem_shmem_create() Lyude Paul
2025-05-21 20:29 ` [PATCH v2 04/12] drm/gem/shmem: Extract drm_gem_shmem_release() from drm_gem_shmem_free() Lyude Paul
2025-05-21 20:29 ` [PATCH v2 05/12] rust: gem: Introduce BaseDriverObject::Args Lyude Paul
2025-05-21 20:29 ` [PATCH v2 06/12] rust: drm: gem: Add OpaqueObject Lyude Paul
2025-05-21 20:29 ` [PATCH v2 07/12] rust: drm: gem: shmem: Add DRM shmem helper abstraction Lyude Paul
2025-05-21 20:29 ` [PATCH v2 08/12] rust: drm: gem: shmem: Add share_dma_resv to ObjectConfig Lyude Paul
2025-05-21 20:29 ` [PATCH v2 09/12] rust: drm: gem: Introduce OwnedSGTable Lyude Paul
2025-05-21 20:29 ` [PATCH v2 10/12] rust: Add dma_buf stub bindings Lyude Paul
2025-05-21 20:29 ` [PATCH v2 11/12] rust: drm: gem: Add export() callback Lyude Paul
2025-05-21 20:29 ` [PATCH v2 12/12] rust: drm: gem: Add BaseObject::prime_export() Lyude Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).