* [PATCH v6 00/10] drm/tyr: add VM and BO ioctl support
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
Add the VM and BO ioctls to the Tyr driver, aligning its userspace
interface with panthor.
The series is based on v2 of Danilo's lifetime-parameterized DRM File
private data series:
https://lore.kernel.org/all/20261107021427.2105374-1-dakr@kernel.org/
It also carries Sami Tolvanen's buffer cache-management patches from
https://gitlab.freedesktop.org/panfrost/linux sami/tyr/7.3/dev.
The panthor GEM and VM IGT tests pass on an RK3588S device. The group
and dev_query tests are not supported yet.
A branch with the full series and all prerequisites applied is
available at:
https://gitlab.freedesktop.org/alvinsun039/linux.git tyr-ioctls
Link: https://gitlab.freedesktop.org/panfrost/linux/-/merge_requests/64
Signed-off-by: Ke Sun <sunke@kylinos.cn>
---
Changes in v6:
- Rebase onto v2 of Danilo's lifetime-parameterized DRM File private
data series and use the Driver::File<'a> associated type instead of
ForLt.
- Drop the unrelated Firmware::new closure rework from the VM
specifications patch (Deborah).
- Merge the write-combine and WB_MMAP patches into one, with the full
should_map_wc() helper in a single place (Deborah).
- Accept DRM_PANTHOR_BO_WB_MMAP directly in the BO_CREATE patch,
reject NO_MMAP combined with WB_MMAP, and return EOPNOTSUPP for
unsupported requests such as an exclusive VM (Deborah).
- Pass the bound device from file.inner_with() to new_object() and
drop the unsafe as_bound() access (Deborah).
- Expose all three DmaAttr values as constants instead of providing
is_coherent() (Deborah).
- Validate the VM id before the empty-ops early return in VM_BIND and
reject sync objects before the size check, as in Panthor (Deborah).
- Require an exact ops stride for now, use from_safe_cast for the id
and size conversions, and add TODO notes for variable strides and
zerocopy FromBytes (Deborah).
- Link to v5: https://patch.msgid.link/20261027-tyr-ioctls-v5-0-59dfe98cb003@kylinos.cn
Changes in v5:
- Replace the mm-based task_size() (always returned 0) with Sami's
CurrentTask::task_size() implementation (Deborah, Alice).
- Use from_safe_cast() for the id conversions and annotate the AddError
fields so the VM pool patch compiles on its own (Deborah).
- Store a clone on pool conflicts and return the caller's VM instead of
expect()-ing (Deborah).
- Drop the redundant project() in PinnedDrop (Deborah).
- Fix the rustfmt issues found in the series (Deborah).
- Link to v4: https://patch.msgid.link/20261020-tyr-ioctls-v4-0-704de77da223@kylinos.cn
Changes in v4:
- Rework VmPool teardown: add() returns an AddError the caller owns,
a conflicting stored VM is killed with EBUSY, and remove() kills
before the ID is reused (Daniel, Deborah).
- Wrap dev_dma_attr in a DmaAttr type with dma_attr()/is_coherent()
and rename the patch accordingly (Danilo).
- Fetch the sg_table unconditionally in new_object() and drop the
conditional flush call (Daniel).
- Use dev_dbg!() instead of dev_err!() for user-input validation and
EOPNOTSUPP for the async VM_BIND path (Daniel).
- Drop the pr_err!() calls from the VM specification patch (Daniel).
- Link to v3: https://patch.msgid.link/20260929-tyr-ioctls-v3-0-26955fb111d5@kylinos.cn
Changes in v3:
- Drop the VmOwner wrapper and kill VMs explicitly on destroy and
file close (Deborah).
- Add Sami Tolvanen's buffer cache-management patches: coherent
devices map buffers cached, write-combine buffers flush at
creation, and BO_CREATE accepts WB_MMAP.
- Drop the new_dummy_object refactor; the dummy object stays a direct
Bo::new since new_object now fetches the sgtable for write-combine
buffers.
- Document the LargeSizeConstants trait in the sizes patch (Miguel).
- Misc commit message and comment cleanups.
- Link to v2: https://lore.kernel.org/r/20260908-tyr-ioctls-v2-0-88bea777df67@kylinos.cn
Changes in v2:
- Fold the VM pool into vm.rs and cap it at 32 VMs per file (Deborah).
- Add VmOwner so a VM is killed exactly once on drop, regardless of
remaining references (Daniel).
- Replace the VmSpec enum with new_for_fw()/new_for_user()
constructors (Daniel).
- Parse VM_BIND op types via VmBindOpType and use the kernel
Atomic<bool> for the unusable flag (Daniel).
- Use PAGE_SIZE for the dummy object size (Daniel).
- Rename BoData::create_flags() to flags() (Daniel).
- Report VA-split failures with dev_err! at the caller.
- Link to v1: https://lore.kernel.org/r/20260902-tyr-ioctls-v1-0-e0fdbf8bd108@kylinos.cn
---
Alvin Sun (7):
rust: sizes: add SZ_4G constant
rust: sync: arc: relax `ForeignOwnable` for `Arc<T>`
drm/tyr: add per-file VM pool
drm/tyr: add user and MCU VM specifications
drm/tyr: add BO creation and lookup helpers
drm/tyr: add VM-related ioctls
drm/tyr: add BO-related ioctls
Sami Tolvanen (3):
rust: task: add task_size()
rust: device: expose the device DMA attribute
drm/tyr: support write-combine BO mappings
drivers/gpu/drm/tyr/driver.rs | 25 ++-
drivers/gpu/drm/tyr/file.rs | 390 +++++++++++++++++++++++++++++++++++++++-
drivers/gpu/drm/tyr/fw.rs | 5 +-
drivers/gpu/drm/tyr/gem.rs | 79 +++++++-
drivers/gpu/drm/tyr/vm.rs | 355 ++++++++++++++++++++++++++++++++++--
rust/bindings/bindings_helper.h | 1 +
rust/helpers/task.c | 6 +
rust/kernel/device.rs | 27 +++
rust/kernel/sizes.rs | 12 ++
rust/kernel/sync/arc.rs | 12 +-
rust/kernel/task.rs | 12 ++
11 files changed, 883 insertions(+), 41 deletions(-)
---
base-commit: cef0cd809e564cca9ac824a369c995ab7e0d16f3
change-id: 20260901-tyr-ioctls-f10ef0dcbfa6
prerequisite-message-id: <20261107021427.2105374-1-dakr@kernel.org>
prerequisite-patch-id: e17775a2fb002d71edd479f6002dc3380b9b6bb4
prerequisite-patch-id: 388c5b871c09dbb6bad46c68be04813bcbdaadb6
prerequisite-patch-id: f31098f7dcd1b2d7843767f9ef778788f8773477
prerequisite-patch-id: b0ea30b4c39e5a43e53ceaf28bd8b39db5b26a76
prerequisite-patch-id: e0735b7cd0f226e702c0edc4ed889340f93d24cf
prerequisite-patch-id: 086eb00ec9c9c445c99bbb8d1a052b3fad56b5bd
prerequisite-patch-id: d186dd1af2f3ce256afd8c7b6ff749d74def5d1a
Best regards,
--
Ke Sun <sunke@kylinos.cn>
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v6 01/10] rust: sizes: add SZ_4G constant
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
SZ_4G is used by Tyr when splitting the GPU VA range into
user and kernel regions. Add a LargeSizeConstants trait, implemented
for u64 since SizeConstants only covers sizes that fit in a u32.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/sizes.rs | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3791af0aadc73..fd39cc954eaba 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -116,6 +116,7 @@ const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
const size_t RUST_CONST_HELPER_GENLMSG_DEFAULT_SIZE = GENLMSG_DEFAULT_SIZE;
+const unsigned long long RUST_CONST_HELPER_SZ_4G = SZ_4G;
const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
const gfp_t RUST_CONST_HELPER_GFP_KERNEL_ACCOUNT = GFP_KERNEL_ACCOUNT;
diff --git a/rust/kernel/sizes.rs b/rust/kernel/sizes.rs
index 188e00c2b8b46..760d26ebe6ff6 100644
--- a/rust/kernel/sizes.rs
+++ b/rust/kernel/sizes.rs
@@ -177,3 +177,15 @@ impl SizeConstants for $first {
}
define_sizes!(u32, u64, usize);
+
+/// Large size constants (≥ 4 GiB).
+///
+/// Only implemented for [`u64`].
+pub trait LargeSizeConstants {
+ /// `0x1_0000_0000`.
+ const SZ_4G: Self;
+}
+
+impl LargeSizeConstants for u64 {
+ const SZ_4G: Self = bindings::SZ_4G;
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 01/10] rust: sizes: add SZ_4G constant
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
SZ_4G is used by Tyr when splitting the GPU VA range into
user and kernel regions. Add a LargeSizeConstants trait, implemented
for u64 since SizeConstants only covers sizes that fit in a u32.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/sizes.rs | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3791af0aadc73..fd39cc954eaba 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -116,6 +116,7 @@ const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
const size_t RUST_CONST_HELPER_GENLMSG_DEFAULT_SIZE = GENLMSG_DEFAULT_SIZE;
+const unsigned long long RUST_CONST_HELPER_SZ_4G = SZ_4G;
const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
const gfp_t RUST_CONST_HELPER_GFP_KERNEL_ACCOUNT = GFP_KERNEL_ACCOUNT;
diff --git a/rust/kernel/sizes.rs b/rust/kernel/sizes.rs
index 188e00c2b8b46..760d26ebe6ff6 100644
--- a/rust/kernel/sizes.rs
+++ b/rust/kernel/sizes.rs
@@ -177,3 +177,15 @@ impl SizeConstants for $first {
}
define_sizes!(u32, u64, usize);
+
+/// Large size constants (≥ 4 GiB).
+///
+/// Only implemented for [`u64`].
+pub trait LargeSizeConstants {
+ /// `0x1_0000_0000`.
+ const SZ_4G: Self;
+}
+
+impl LargeSizeConstants for u64 {
+ const SZ_4G: Self = bindings::SZ_4G;
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 02/10] rust: task: add task_size()
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Sami Tolvanen <samitolvanen@google.com>
A DRM driver that lets userspace pick GPU virtual addresses needs the
address space size of the calling task, so it can offer a GPU range
that the CPU can map at the same addresses. TASK_SIZE_OF() is a macro,
so reading it from Rust needs a helper.
Add CurrentTask::task_size(). It is bound to the current task because
TASK_SIZE_OF() falls back to TASK_SIZE on architectures that do not
define it, and TASK_SIZE describes current.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
[ Add the <linux/mm.h> include, needed where TASK_SIZE is defined in
<asm/pgtable.h> (e.g. riscv). - Alvin ]
Co-developed-by: Alvin Sun <alvin.sun@linux.dev>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
rust/helpers/task.c | 6 ++++++
rust/kernel/task.rs | 12 ++++++++++++
2 files changed, 18 insertions(+)
diff --git a/rust/helpers/task.c b/rust/helpers/task.c
index b46b1433a67e8..b49bd40027154 100644
--- a/rust/helpers/task.c
+++ b/rust/helpers/task.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/kernel.h>
+#include <linux/mm.h>
#include <linux/sched/task.h>
__rust_helper void rust_helper_might_resched(void)
@@ -13,6 +14,11 @@ __rust_helper struct task_struct *rust_helper_get_current(void)
return current;
}
+__rust_helper unsigned long rust_helper_current_task_size(void)
+{
+ return TASK_SIZE_OF(current);
+}
+
__rust_helper void rust_helper_get_task_struct(struct task_struct *t)
{
get_task_struct(t);
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index a733816fd5dd4..1978f54fe5897 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -317,6 +317,18 @@ pub fn mm(&self) -> Option<&MmWithUser> {
Some(unsafe { MmWithUser::from_raw(mm) })
}
+ /// Returns the size of the userspace address space of the current task.
+ ///
+ /// This is the first address above the userspace range, so it is smaller
+ /// for a task running in compat mode than for a native one. See
+ /// `TASK_SIZE_OF()` in [`include/linux/sched.h`](srctree/include/linux/sched.h).
+ #[doc(alias = "TASK_SIZE_OF")]
+ #[inline]
+ pub fn task_size(&self) -> usize {
+ // SAFETY: FFI call without preconditions, valid in any context.
+ unsafe { bindings::current_task_size() as usize }
+ }
+
/// Access the pid namespace of the current task.
///
/// This function does not touch the refcount of the namespace or use RCU protection.
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 02/10] rust: task: add task_size()
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Sami Tolvanen <samitolvanen@google.com>
A DRM driver that lets userspace pick GPU virtual addresses needs the
address space size of the calling task, so it can offer a GPU range
that the CPU can map at the same addresses. TASK_SIZE_OF() is a macro,
so reading it from Rust needs a helper.
Add CurrentTask::task_size(). It is bound to the current task because
TASK_SIZE_OF() falls back to TASK_SIZE on architectures that do not
define it, and TASK_SIZE describes current.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
[ Add the <linux/mm.h> include, needed where TASK_SIZE is defined in
<asm/pgtable.h> (e.g. riscv). - Alvin ]
Co-developed-by: Alvin Sun <alvin.sun@linux.dev>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
rust/helpers/task.c | 6 ++++++
rust/kernel/task.rs | 12 ++++++++++++
2 files changed, 18 insertions(+)
diff --git a/rust/helpers/task.c b/rust/helpers/task.c
index b46b1433a67e8..b49bd40027154 100644
--- a/rust/helpers/task.c
+++ b/rust/helpers/task.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/kernel.h>
+#include <linux/mm.h>
#include <linux/sched/task.h>
__rust_helper void rust_helper_might_resched(void)
@@ -13,6 +14,11 @@ __rust_helper struct task_struct *rust_helper_get_current(void)
return current;
}
+__rust_helper unsigned long rust_helper_current_task_size(void)
+{
+ return TASK_SIZE_OF(current);
+}
+
__rust_helper void rust_helper_get_task_struct(struct task_struct *t)
{
get_task_struct(t);
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index a733816fd5dd4..1978f54fe5897 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -317,6 +317,18 @@ pub fn mm(&self) -> Option<&MmWithUser> {
Some(unsafe { MmWithUser::from_raw(mm) })
}
+ /// Returns the size of the userspace address space of the current task.
+ ///
+ /// This is the first address above the userspace range, so it is smaller
+ /// for a task running in compat mode than for a native one. See
+ /// `TASK_SIZE_OF()` in [`include/linux/sched.h`](srctree/include/linux/sched.h).
+ #[doc(alias = "TASK_SIZE_OF")]
+ #[inline]
+ pub fn task_size(&self) -> usize {
+ // SAFETY: FFI call without preconditions, valid in any context.
+ unsafe { bindings::current_task_size() as usize }
+ }
+
/// Access the pid namespace of the current task.
///
/// This function does not touch the refcount of the namespace or use RCU protection.
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 03/10] rust: sync: arc: relax `ForeignOwnable` for `Arc<T>`
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Drop the `'static` bound so that refcounted values borrowing from a
driver registration scope can be foreign-owned by the XArray
abstraction.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/sync/arc.rs | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 8ae0fe6f19ec0..9b582a815f888 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -363,11 +363,17 @@ pub fn into_unique_or_drop(this: Self) -> Option<Pin<UniqueArc<T>>> {
// SAFETY: The pointer returned by `into_foreign` was originally allocated as an
// `KBox<ArcInner<T>>`, so that type is what determines the alignment.
-unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
+unsafe impl<T> ForeignOwnable for Arc<T> {
const FOREIGN_ALIGN: usize = <KBox<ArcInner<T>> as ForeignOwnable>::FOREIGN_ALIGN;
- type Borrowed<'a> = ArcBorrow<'a, T>;
- type BorrowedMut<'a> = Self::Borrowed<'a>;
+ type Borrowed<'a>
+ = ArcBorrow<'a, T>
+ where
+ T: 'a;
+ type BorrowedMut<'a>
+ = Self::Borrowed<'a>
+ where
+ T: 'a;
fn into_foreign(self) -> *mut c_void {
ManuallyDrop::new(self).ptr.as_ptr().cast()
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 03/10] rust: sync: arc: relax `ForeignOwnable` for `Arc<T>`
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Drop the `'static` bound so that refcounted values borrowing from a
driver registration scope can be foreign-owned by the XArray
abstraction.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
---
rust/kernel/sync/arc.rs | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 8ae0fe6f19ec0..9b582a815f888 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -363,11 +363,17 @@ pub fn into_unique_or_drop(this: Self) -> Option<Pin<UniqueArc<T>>> {
// SAFETY: The pointer returned by `into_foreign` was originally allocated as an
// `KBox<ArcInner<T>>`, so that type is what determines the alignment.
-unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
+unsafe impl<T> ForeignOwnable for Arc<T> {
const FOREIGN_ALIGN: usize = <KBox<ArcInner<T>> as ForeignOwnable>::FOREIGN_ALIGN;
- type Borrowed<'a> = ArcBorrow<'a, T>;
- type BorrowedMut<'a> = Self::Borrowed<'a>;
+ type Borrowed<'a>
+ = ArcBorrow<'a, T>
+ where
+ T: 'a;
+ type BorrowedMut<'a>
+ = Self::Borrowed<'a>
+ where
+ T: 'a;
fn into_foreign(self) -> *mut c_void {
ManuallyDrop::new(self).ptr.as_ptr().cast()
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 04/10] drm/tyr: add per-file VM pool
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Userspace needs multiple independent GPU address spaces per file,
addressed by ID through the VM ioctls as in panthor. Store them in an
IdPool (capped at 32 for panthor parity) plus an XArray. On failure
the caller receives the VM for teardown through the returned error,
whether the un-stored clone or the original on an ID conflict, so an
aborted creation never leaks a VM. The pool kills every VM still
stored when the file closes.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/vm.rs | 140 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 1dd11b2b12532..6f5518652a03f 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -33,6 +33,7 @@
}, //
},
fmt,
+ id_pool::IdPool,
impl_flags,
io::PhysAddr,
iommu::pgtable::{
@@ -41,6 +42,7 @@
ARM64LPAES1, //
},
new_mutex,
+ num::casts::FromSafeCast,
prelude::*,
sizes::{
SZ_1G,
@@ -53,7 +55,11 @@
ArcBorrow,
Mutex, //
},
- uapi, //
+ uapi,
+ xarray::{
+ AllocKind,
+ XArray, //
+ }, //
};
use crate::{
@@ -947,3 +953,135 @@ fn pt_unmap(dev: &Device, pt: &IoPageTable<'_, ARM64LPAES1>, range: Range<u64>)
Ok(())
}
+
+/// Maximum number of VMs a single file may hold, matching panthor's
+/// `PANTHOR_MAX_VMS_PER_FILE`.
+const MAX_VMS_PER_FILE: usize = 32;
+
+/// Per-open-file pool of VMs.
+#[pin_data(PinnedDrop)]
+pub(crate) struct VmPool<'drm> {
+ /// Parent device, for error reporting.
+ dev: &'drm Device<Bound>,
+ #[pin]
+ ids: Mutex<IdPool>,
+ #[pin]
+ vms: XArray<Arc<Vm<'drm>>>,
+}
+
+/// Error returned when a VM cannot be added to the pool. The caller
+/// owns the teardown of the VM in [`AddError::vm`].
+pub(crate) struct AddError<'drm> {
+ #[expect(dead_code)]
+ pub(crate) error: Error,
+ #[expect(dead_code)]
+ pub(crate) vm: Arc<Vm<'drm>>,
+}
+
+impl<'drm> VmPool<'drm> {
+ /// Creates a new [`VmPool`].
+ #[expect(dead_code)]
+ pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
+ let ids = IdPool::new();
+ pin_init!(Self {
+ dev,
+ ids <- new_mutex!(ids),
+ vms <- XArray::new(AllocKind::Alloc),
+ })
+ }
+
+ /// Stores the VM and returns the allocated ID.
+ ///
+ /// On failure - ID space exhausted or a store failure - the VM is
+ /// returned in an [`AddError`] so the caller owns the teardown.
+ // TODO: allocate IDs with the XArray directly (once it grows range
+ // allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
+ #[expect(dead_code)]
+ pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
+ let id = (|| {
+ let mut ids = self.ids.lock();
+ let unused = ids.find_unused_id(1).ok_or(ENOSPC)?;
+ if unused.as_usize() > MAX_VMS_PER_FILE {
+ return Err(ENOSPC);
+ }
+ Ok(unused.acquire())
+ })();
+ let id = match id {
+ Ok(id) => id,
+ Err(error) => return Err(AddError { error, vm }),
+ };
+
+ let mut vms = self.vms.lock();
+ match vms.store(id, vm.clone(), GFP_KERNEL) {
+ Ok(prev_vm) => {
+ if let Some(prev_vm) = prev_vm {
+ // An existing VM at the freshly allocated slot means the
+ // IdPool and the XArray are out of sync: the stored VM is
+ // a pool entry, so the pool kills it and the caller's VM
+ // is returned for teardown.
+ drop(vms.remove(id));
+ drop(vms);
+ prev_vm.kill();
+ self.ids.lock().release_id(id);
+ dev_err!(
+ self.dev,
+ "IdPool/XArray out of sync at VM pool id {id} (stored VM conflict)"
+ );
+ return Err(AddError { error: EBUSY, vm });
+ }
+ Ok(id as u32)
+ }
+ Err(err) => {
+ // Drop the XArray spinlock before acquiring the `ids` mutex.
+ drop(vms);
+ // Release the pooled id; the VM never entered the pool, so it
+ // is returned to the caller for teardown.
+ self.ids.lock().release_id(id);
+ Err(AddError {
+ error: err.error,
+ vm: err.value,
+ })
+ }
+ }
+ }
+
+ /// Removes and kills the VM with the given ID.
+ #[expect(dead_code)]
+ pub(crate) fn remove(&self, id: u32) -> Result<()> {
+ let mut vms = self.vms.lock();
+ let vm = match vms.remove(usize::from_safe_cast(id)) {
+ Some(vm) => vm,
+ None => return Err(EINVAL),
+ };
+ // Drop the XArray spinlock before killing, since `kill()` may sleep.
+ drop(vms);
+ // Kill the VM before handing the id back to the pool, so a VM is
+ // never reusable while still alive.
+ vm.kill();
+ self.ids.lock().release_id(usize::from_safe_cast(id));
+ Ok(())
+ }
+
+ /// Gets a shared reference to the VM with the given ID.
+ #[expect(dead_code)]
+ pub(crate) fn get(&self, id: u32) -> Option<Arc<Vm<'drm>>> {
+ let vms = self.vms.lock();
+ let borrow = vms.get(usize::from_safe_cast(id))?;
+ Some(Arc::from(borrow))
+ }
+}
+
+#[pinned_drop]
+impl PinnedDrop for VmPool<'_> {
+ fn drop(self: Pin<&mut Self>) {
+ // Kill every VM left in the pool. The ID range is bounded by
+ // `MAX_VMS_PER_FILE`, so this loop is cheap and runs at file close.
+ for id in 1..=MAX_VMS_PER_FILE {
+ // Release the XArray lock guard before killing: `kill()` may sleep.
+ let vm = self.vms.lock().remove(id);
+ if let Some(vm) = vm {
+ vm.kill();
+ }
+ }
+ }
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 04/10] drm/tyr: add per-file VM pool
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Userspace needs multiple independent GPU address spaces per file,
addressed by ID through the VM ioctls as in panthor. Store them in an
IdPool (capped at 32 for panthor parity) plus an XArray. On failure
the caller receives the VM for teardown through the returned error,
whether the un-stored clone or the original on an ID conflict, so an
aborted creation never leaks a VM. The pool kills every VM still
stored when the file closes.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/vm.rs | 140 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 1dd11b2b12532..6f5518652a03f 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -33,6 +33,7 @@
}, //
},
fmt,
+ id_pool::IdPool,
impl_flags,
io::PhysAddr,
iommu::pgtable::{
@@ -41,6 +42,7 @@
ARM64LPAES1, //
},
new_mutex,
+ num::casts::FromSafeCast,
prelude::*,
sizes::{
SZ_1G,
@@ -53,7 +55,11 @@
ArcBorrow,
Mutex, //
},
- uapi, //
+ uapi,
+ xarray::{
+ AllocKind,
+ XArray, //
+ }, //
};
use crate::{
@@ -947,3 +953,135 @@ fn pt_unmap(dev: &Device, pt: &IoPageTable<'_, ARM64LPAES1>, range: Range<u64>)
Ok(())
}
+
+/// Maximum number of VMs a single file may hold, matching panthor's
+/// `PANTHOR_MAX_VMS_PER_FILE`.
+const MAX_VMS_PER_FILE: usize = 32;
+
+/// Per-open-file pool of VMs.
+#[pin_data(PinnedDrop)]
+pub(crate) struct VmPool<'drm> {
+ /// Parent device, for error reporting.
+ dev: &'drm Device<Bound>,
+ #[pin]
+ ids: Mutex<IdPool>,
+ #[pin]
+ vms: XArray<Arc<Vm<'drm>>>,
+}
+
+/// Error returned when a VM cannot be added to the pool. The caller
+/// owns the teardown of the VM in [`AddError::vm`].
+pub(crate) struct AddError<'drm> {
+ #[expect(dead_code)]
+ pub(crate) error: Error,
+ #[expect(dead_code)]
+ pub(crate) vm: Arc<Vm<'drm>>,
+}
+
+impl<'drm> VmPool<'drm> {
+ /// Creates a new [`VmPool`].
+ #[expect(dead_code)]
+ pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
+ let ids = IdPool::new();
+ pin_init!(Self {
+ dev,
+ ids <- new_mutex!(ids),
+ vms <- XArray::new(AllocKind::Alloc),
+ })
+ }
+
+ /// Stores the VM and returns the allocated ID.
+ ///
+ /// On failure - ID space exhausted or a store failure - the VM is
+ /// returned in an [`AddError`] so the caller owns the teardown.
+ // TODO: allocate IDs with the XArray directly (once it grows range
+ // allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
+ #[expect(dead_code)]
+ pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
+ let id = (|| {
+ let mut ids = self.ids.lock();
+ let unused = ids.find_unused_id(1).ok_or(ENOSPC)?;
+ if unused.as_usize() > MAX_VMS_PER_FILE {
+ return Err(ENOSPC);
+ }
+ Ok(unused.acquire())
+ })();
+ let id = match id {
+ Ok(id) => id,
+ Err(error) => return Err(AddError { error, vm }),
+ };
+
+ let mut vms = self.vms.lock();
+ match vms.store(id, vm.clone(), GFP_KERNEL) {
+ Ok(prev_vm) => {
+ if let Some(prev_vm) = prev_vm {
+ // An existing VM at the freshly allocated slot means the
+ // IdPool and the XArray are out of sync: the stored VM is
+ // a pool entry, so the pool kills it and the caller's VM
+ // is returned for teardown.
+ drop(vms.remove(id));
+ drop(vms);
+ prev_vm.kill();
+ self.ids.lock().release_id(id);
+ dev_err!(
+ self.dev,
+ "IdPool/XArray out of sync at VM pool id {id} (stored VM conflict)"
+ );
+ return Err(AddError { error: EBUSY, vm });
+ }
+ Ok(id as u32)
+ }
+ Err(err) => {
+ // Drop the XArray spinlock before acquiring the `ids` mutex.
+ drop(vms);
+ // Release the pooled id; the VM never entered the pool, so it
+ // is returned to the caller for teardown.
+ self.ids.lock().release_id(id);
+ Err(AddError {
+ error: err.error,
+ vm: err.value,
+ })
+ }
+ }
+ }
+
+ /// Removes and kills the VM with the given ID.
+ #[expect(dead_code)]
+ pub(crate) fn remove(&self, id: u32) -> Result<()> {
+ let mut vms = self.vms.lock();
+ let vm = match vms.remove(usize::from_safe_cast(id)) {
+ Some(vm) => vm,
+ None => return Err(EINVAL),
+ };
+ // Drop the XArray spinlock before killing, since `kill()` may sleep.
+ drop(vms);
+ // Kill the VM before handing the id back to the pool, so a VM is
+ // never reusable while still alive.
+ vm.kill();
+ self.ids.lock().release_id(usize::from_safe_cast(id));
+ Ok(())
+ }
+
+ /// Gets a shared reference to the VM with the given ID.
+ #[expect(dead_code)]
+ pub(crate) fn get(&self, id: u32) -> Option<Arc<Vm<'drm>>> {
+ let vms = self.vms.lock();
+ let borrow = vms.get(usize::from_safe_cast(id))?;
+ Some(Arc::from(borrow))
+ }
+}
+
+#[pinned_drop]
+impl PinnedDrop for VmPool<'_> {
+ fn drop(self: Pin<&mut Self>) {
+ // Kill every VM left in the pool. The ID range is bounded by
+ // `MAX_VMS_PER_FILE`, so this loop is cheap and runs at file close.
+ for id in 1..=MAX_VMS_PER_FILE {
+ // Release the XArray lock guard before killing: `kill()` may sleep.
+ let vm = self.vms.lock().remove(id);
+ if let Some(vm) = vm {
+ vm.kill();
+ }
+ }
+ }
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 04/10] drm/tyr: add per-file VM pool
2026-11-09 0:36 ` Ke Sun
(?)
@ 2026-11-09 0:48 ` sashiko-bot
2026-11-13 1:17 ` Deborah Brouwer
-1 siblings, 1 reply; 27+ messages in thread
From: sashiko-bot @ 2026-11-09 0:48 UTC (permalink / raw)
To: Ke Sun; +Cc: dri-devel, ojeda, gary
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/tyr: off-by-one IdPool capacity in VmPool::new() on
32-bit builds
--
commit 6b8d6be457daf3e5e905977c364059ce3d3249e4
Author: Alvin Sun <alvin.sun@linux.dev>
drm/tyr: add per-file VM pool
This commit introduces VmPool to manage up to 32 GPU address spaces per open
file using an IdPool and an XArray, tearing down remaining VMs when the pool
is dropped.
> diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> index 1dd11b2b12532..6f5518652a03f 100644
> --- a/drivers/gpu/drm/tyr/vm.rs
> +++ b/drivers/gpu/drm/tyr/vm.rs
[ ... ]
> @@ -947,3 +953,135 @@ fn pt_unmap(dev: &Device, pt: &IoPageTable<'_, ARM64LPAES1>, range: Range<u64>)
>
> Ok(())
> }
> +
> +/// Maximum number of VMs a single file may hold, matching panthor's
> +/// `PANTHOR_MAX_VMS_PER_FILE`.
> +const MAX_VMS_PER_FILE: usize = 32;
[ ... ]
> +impl<'drm> VmPool<'drm> {
> + /// Creates a new [`VmPool`].
> + #[expect(dead_code)]
> + pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
> + let ids = IdPool::new();
> + pin_init!(Self {
> + dev,
> + ids <- new_mutex!(ids),
> + vms <- XArray::new(AllocKind::Alloc),
> + })
> + }
> +
> + /// Stores the VM and returns the allocated ID.
> + ///
> + /// On failure - ID space exhausted or a store failure - the VM is
> + /// returned in an [`AddError`] so the caller owns the teardown.
> + // TODO: allocate IDs with the XArray directly (once it grows range
> + // allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
> + #[expect(dead_code)]
> + pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
> + let id = (|| {
> + let mut ids = self.ids.lock();
> + let unused = ids.find_unused_id(1).ok_or(ENOSPC)?;
> + if unused.as_usize() > MAX_VMS_PER_FILE {
> + return Err(ENOSPC);
> + }
> + Ok(unused.acquire())
> + })();
[Severity: Medium]
Can VmPool::add() fail with ENOSPC on the 32nd VM on 32-bit builds?
IdPool::new() creates an inline bitmap of length usize::BITS, which is 32 on
32-bit platforms (indices 0..=31). Because find_unused_id(1) skips bit 0,
allocating 32 VMs requires bit index 32, so only 31 VMs (IDs 1..=31) can be
allocated before find_unused_id(1) returns None. Searching from bit 0 and
adding 1 to form the 1-based VM ID (subtracting 1 on release_id()) would fit
all 32 VMs within indices 0..=31.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-tyr-ioctls-v6-0-b2896bf2b3c9@kylinos.cn?part=4
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6 04/10] drm/tyr: add per-file VM pool
2026-11-09 0:48 ` sashiko-bot
@ 2026-11-13 1:17 ` Deborah Brouwer
0 siblings, 0 replies; 27+ messages in thread
From: Deborah Brouwer @ 2026-11-13 1:17 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Ke Sun, dri-devel, ojeda, gary
On Mon, Nov 09, 2026 at 12:48:15AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] drm/tyr: off-by-one IdPool capacity in VmPool::new() on
> 32-bit builds
> --
>
> commit 6b8d6be457daf3e5e905977c364059ce3d3249e4
> Author: Alvin Sun <alvin.sun@linux.dev>
>
> drm/tyr: add per-file VM pool
>
> This commit introduces VmPool to manage up to 32 GPU address spaces per open
> file using an IdPool and an XArray, tearing down remaining VMs when the pool
> is dropped.
>
> > diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> > index 1dd11b2b12532..6f5518652a03f 100644
> > --- a/drivers/gpu/drm/tyr/vm.rs
> > +++ b/drivers/gpu/drm/tyr/vm.rs
>
> [ ... ]
>
> > @@ -947,3 +953,135 @@ fn pt_unmap(dev: &Device, pt: &IoPageTable<'_, ARM64LPAES1>, range: Range<u64>)
> >
> > Ok(())
> > }
> > +
> > +/// Maximum number of VMs a single file may hold, matching panthor's
> > +/// `PANTHOR_MAX_VMS_PER_FILE`.
> > +const MAX_VMS_PER_FILE: usize = 32;
>
> [ ... ]
>
> > +impl<'drm> VmPool<'drm> {
> > + /// Creates a new [`VmPool`].
> > + #[expect(dead_code)]
> > + pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
> > + let ids = IdPool::new();
> > + pin_init!(Self {
> > + dev,
> > + ids <- new_mutex!(ids),
> > + vms <- XArray::new(AllocKind::Alloc),
> > + })
> > + }
> > +
> > + /// Stores the VM and returns the allocated ID.
> > + ///
> > + /// On failure - ID space exhausted or a store failure - the VM is
> > + /// returned in an [`AddError`] so the caller owns the teardown.
> > + // TODO: allocate IDs with the XArray directly (once it grows range
> > + // allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
> > + #[expect(dead_code)]
> > + pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
> > + let id = (|| {
> > + let mut ids = self.ids.lock();
> > + let unused = ids.find_unused_id(1).ok_or(ENOSPC)?;
> > + if unused.as_usize() > MAX_VMS_PER_FILE {
> > + return Err(ENOSPC);
> > + }
> > + Ok(unused.acquire())
> > + })();
>
> [Severity: Medium]
> Can VmPool::add() fail with ENOSPC on the 32nd VM on 32-bit builds?
>
> IdPool::new() creates an inline bitmap of length usize::BITS, which is 32 on
> 32-bit platforms (indices 0..=31). Because find_unused_id(1) skips bit 0,
> allocating 32 VMs requires bit index 32, so only 31 VMs (IDs 1..=31) can be
> allocated before find_unused_id(1) returns None. Searching from bit 0 and
> adding 1 to form the 1-based VM ID (subtracting 1 on release_id()) would fit
> all 32 VMs within indices 0..=31.
Yes, this makes sense, on 32-bit builds the bitmap will stop at bit 31
and since we won't use bit 0, this gives us only 31 possible vm ids.
Instead of adding an off-by-one offset which seems like it could add
more confusion, what do you think of using the IdPool::with_capacity()
function? Something like:
let ids = IdPool::with_capacity(MAX_VMS_PER_FILE + 1, GFP_KERNEL)?;
We would need to adjust the error handlings since this
is fallible but it might be simpler overall.
Deborah
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261109-tyr-ioctls-v6-0-b2896bf2b3c9@kylinos.cn?part=4
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 05/10] drm/tyr: add user and MCU VM specifications
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
The MCU and user VMs need different VA layouts. Give each a dedicated
constructor: new_for_fw() builds the kernel-only 4G layout, while
new_for_user() splits the address space by task_size or a user-provided
size, rejecting oversized requests rather than clamping. The resulting
user range is what VM_CREATE reports back as user_va_range.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/fw.rs | 3 +-
drivers/gpu/drm/tyr/vm.rs | 157 +++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 143 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
index aea151ff1f934..196ffc96d9d99 100644
--- a/drivers/gpu/drm/tyr/fw.rs
+++ b/drivers/gpu/drm/tyr/fw.rs
@@ -51,7 +51,6 @@
KernelBoVaAlloc, //
},
gpu::GpuInfo,
-
mmu::Mmu,
regs::{
gpu_control::{
@@ -219,7 +218,7 @@ pub(crate) fn new(
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
) -> Result<Firmware<'drm>> {
- let vm = Vm::new(dev, ddev, mmu, gpu_info)?;
+ let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info)?;
vm.activate()?;
let result = (|| {
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 6f5518652a03f..44f5046037895 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -8,6 +8,7 @@
//! mapped into hardware address space (AS) slots for GPU execution.
use core::marker::PhantomData;
+use core::num::NonZeroU64;
use core::ops::Range;
use kernel::{
@@ -45,6 +46,8 @@
num::casts::FromSafeCast,
prelude::*,
sizes::{
+ LargeSizeConstants,
+ SizeConstants,
SZ_1G,
SZ_2M,
SZ_4K, //
@@ -160,6 +163,90 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
}
}
+/// User VA size request for a user VM.
+pub(crate) enum UserVaRequest {
+ /// Split based on `task_size()` and the GPU VA range.
+ Auto,
+ /// Caller-specified size; construction guarantees `> 0`.
+ Fixed(NonZeroU64),
+}
+
+impl UserVaRequest {
+ /// UAPI boundary normalization: `0` -> [`Auto`](Self::Auto).
+ #[expect(dead_code)]
+ pub(crate) fn from_uapi(v: u64) -> Self {
+ match NonZeroU64::new(v) {
+ Some(size) => Self::Fixed(size),
+ None => Self::Auto,
+ }
+ }
+}
+
+/// Final user/kernel VA layout for a VM.
+pub(crate) struct VmLayout {
+ /// Full GPU VA range covered by this VM.
+ pub(crate) full: Range<u64>,
+ /// User-accessible VA range. Empty for MCU VMs.
+ pub(crate) user: Range<u64>,
+}
+
+impl VmLayout {
+ /// Kernel VA range, reserved for future kernel object allocation.
+ #[expect(dead_code)]
+ pub(crate) fn kernel(&self) -> Range<u64> {
+ self.user.end..self.full.end
+ }
+
+ /// Compute a user/kernel split for a user VM from the full GPU VA range and
+ /// a user request.
+ pub(crate) fn compute(full: Range<u64>, req: UserVaRequest) -> Result<Self> {
+ // Minimum VA space reserved for kernel objects (heaps, ring buffers, ...).
+ const MIN_KERNEL_VA: u64 = u64::SZ_256M;
+
+ if full.end <= MIN_KERNEL_VA {
+ return Err(EINVAL);
+ }
+
+ let user_max = full.end - MIN_KERNEL_VA;
+
+ let user_end = match req {
+ UserVaRequest::Fixed(v) => {
+ let user_size = v.get();
+ if user_size > user_max {
+ return Err(EINVAL);
+ }
+ user_size
+ }
+ UserVaRequest::Auto => {
+ let task_size = current!().task_size() as u64;
+ let candidate = if task_size < full.end {
+ task_size
+ } else {
+ // If the range exceeds 4G, split it in two so CPU and
+ // GPU share the same addresses (SVM).
+ if full.end > u64::SZ_4G {
+ full.end / 2
+ } else {
+ user_max
+ }
+ };
+ candidate.min(user_max)
+ }
+ };
+
+ let delta = full.end - user_end;
+ // Pick a kernel VA range that's a power of two, to have a clear split.
+ let kernel_va_range = 1u64 << delta.ilog2();
+ let kernel_va_start = full.end - kernel_va_range;
+ let full_start = full.start;
+
+ Ok(Self {
+ full,
+ user: full_start..kernel_va_start,
+ })
+ }
+}
+
/// Arguments for a virtual memory map operation.
struct VmMapArgs<'drm> {
/// Access permissions and caching behavior for the mapping.
@@ -335,26 +422,64 @@ pub(crate) struct Vm<'drm> {
/// Non-core part of the GPUVM. Can be used for stuff that doesn't modify the
/// internal mapping tree, like GpuVm::obtain()
gpuvm: ARef<GpuVm<GpuVmData<'drm>>>,
- /// VA range for this VM.
- va_range: Range<u64>,
+ /// VA layout for this VM.
+ pub(crate) layout: VmLayout,
}
impl<'drm> Vm<'drm> {
- /// Creates a new GPU virtual address space.
+ /// Creates the MCU/firmware VM.
///
- /// The VM is initialized with a page table configured according to the GPU's
- /// address translation capabilities and registered with the GPUVM framework.
- pub(crate) fn new(
+ /// The MCU VM is entirely kernel-managed: it has no user-accessible range.
+ pub(crate) fn new_for_fw(
+ dev: &'drm Device<Bound>,
+ ddev: &TyrDrmDevice,
+ mmu: ArcBorrow<'_, Mmu<'drm>>,
+ gpu_info: &GpuInfo,
+ ) -> Result<Arc<Vm<'drm>>> {
+ // As in panthor: the CSF MCU is a Cortex-M7 and can only address 4G.
+ let layout = VmLayout {
+ full: 0..u64::SZ_4G,
+ user: 0..0u64,
+ };
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ }
+
+ /// Creates a user VM, splitting the GPU VA range per `user_va`.
+ #[expect(dead_code)]
+ pub(crate) fn new_for_user(
dev: &'drm Device<Bound>,
ddev: &TyrDrmDevice,
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
+ user_va: UserVaRequest,
+ ) -> Result<Arc<Vm<'drm>>> {
+ let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
+ let va_bits = mmu_features.va_bits().get();
+ let range = 0..(1u64 << va_bits);
+
+ let layout = VmLayout::compute(range.clone(), user_va).inspect_err(|_| {
+ dev_dbg!(
+ dev,
+ "Failed to split GPU VA range {:#x}..{:#x} into user and kernel regions\n",
+ range.start,
+ range.end
+ );
+ })?;
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ }
+
+ /// Initializes a VM with the given layout.
+ fn new_internal(
+ dev: &'drm Device<Bound>,
+ ddev: &TyrDrmDevice,
+ mmu: ArcBorrow<'_, Mmu<'drm>>,
+ gpu_info: &GpuInfo,
+ layout: VmLayout,
) -> Result<Arc<Vm<'drm>>> {
let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
let va_bits = mmu_features.va_bits().get();
let pa_bits = mmu_features.pa_bits().get();
- let range = 0..(1u64 << va_bits);
let reserve_range = 0..0u64;
// dummy_obj is used to initialize the GPUVM tree.
@@ -366,7 +491,7 @@ pub(crate) fn new(
c"Tyr::GpuVm",
ddev,
&*dummy_obj,
- range.clone(),
+ layout.full.clone(),
reserve_range,
GpuVmData::<'drm> {
_phantom: PhantomData::<&()>,
@@ -386,7 +511,7 @@ pub(crate) fn new(
mmu: mmu.into(),
gpuvm,
gpuvm_unique <- new_mutex!(gpuvm_unique),
- va_range: range,
+ layout,
}),
GFP_KERNEL,
)?;
@@ -417,10 +542,12 @@ fn deactivate(&self) -> Result {
/// Kills the VM by deactivating it and unmapping all regions.
pub(crate) fn kill(&self) {
- // TODO: Turn the VM into a state where it can't be used.
let _ = self.deactivate();
let _ = self
- .unmap_range(self.va_range.start, self.va_range.end - self.va_range.start)
+ .unmap_range(
+ self.layout.full.start,
+ self.layout.full.end - self.layout.full.start,
+ )
.inspect_err(|e| {
dev_err!(self.dev, "Failed to unmap range during deactivate: {:?}", e);
});
@@ -557,14 +684,14 @@ pub(crate) fn unmap_range(&self, va: u64, size: u64) -> Result {
let end = va.checked_add(size).ok_or(EINVAL)?;
- if va < self.va_range.start || end > self.va_range.end {
+ if va < self.layout.full.start || end > self.layout.full.end {
dev_err!(
self.dev,
"Unmap range {:#x}..{:#x} exceeds VM range {:#x}..{:#x}",
va,
end,
- self.va_range.start,
- self.va_range.end
+ self.layout.full.start,
+ self.layout.full.end
);
return Err(EINVAL);
}
@@ -574,7 +701,7 @@ pub(crate) fn unmap_range(&self, va: u64, size: u64) -> Result {
region: va..end,
};
- let full_vm = va == self.va_range.start && end == self.va_range.end;
+ let full_vm = va == self.layout.full.start && end == self.layout.full.end;
let mut resources = VmOpResources {
preallocated_gpuvas: if full_vm {
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 05/10] drm/tyr: add user and MCU VM specifications
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
The MCU and user VMs need different VA layouts. Give each a dedicated
constructor: new_for_fw() builds the kernel-only 4G layout, while
new_for_user() splits the address space by task_size or a user-provided
size, rejecting oversized requests rather than clamping. The resulting
user range is what VM_CREATE reports back as user_va_range.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/fw.rs | 3 +-
drivers/gpu/drm/tyr/vm.rs | 157 +++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 143 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
index aea151ff1f934..196ffc96d9d99 100644
--- a/drivers/gpu/drm/tyr/fw.rs
+++ b/drivers/gpu/drm/tyr/fw.rs
@@ -51,7 +51,6 @@
KernelBoVaAlloc, //
},
gpu::GpuInfo,
-
mmu::Mmu,
regs::{
gpu_control::{
@@ -219,7 +218,7 @@ pub(crate) fn new(
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
) -> Result<Firmware<'drm>> {
- let vm = Vm::new(dev, ddev, mmu, gpu_info)?;
+ let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info)?;
vm.activate()?;
let result = (|| {
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 6f5518652a03f..44f5046037895 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -8,6 +8,7 @@
//! mapped into hardware address space (AS) slots for GPU execution.
use core::marker::PhantomData;
+use core::num::NonZeroU64;
use core::ops::Range;
use kernel::{
@@ -45,6 +46,8 @@
num::casts::FromSafeCast,
prelude::*,
sizes::{
+ LargeSizeConstants,
+ SizeConstants,
SZ_1G,
SZ_2M,
SZ_4K, //
@@ -160,6 +163,90 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
}
}
+/// User VA size request for a user VM.
+pub(crate) enum UserVaRequest {
+ /// Split based on `task_size()` and the GPU VA range.
+ Auto,
+ /// Caller-specified size; construction guarantees `> 0`.
+ Fixed(NonZeroU64),
+}
+
+impl UserVaRequest {
+ /// UAPI boundary normalization: `0` -> [`Auto`](Self::Auto).
+ #[expect(dead_code)]
+ pub(crate) fn from_uapi(v: u64) -> Self {
+ match NonZeroU64::new(v) {
+ Some(size) => Self::Fixed(size),
+ None => Self::Auto,
+ }
+ }
+}
+
+/// Final user/kernel VA layout for a VM.
+pub(crate) struct VmLayout {
+ /// Full GPU VA range covered by this VM.
+ pub(crate) full: Range<u64>,
+ /// User-accessible VA range. Empty for MCU VMs.
+ pub(crate) user: Range<u64>,
+}
+
+impl VmLayout {
+ /// Kernel VA range, reserved for future kernel object allocation.
+ #[expect(dead_code)]
+ pub(crate) fn kernel(&self) -> Range<u64> {
+ self.user.end..self.full.end
+ }
+
+ /// Compute a user/kernel split for a user VM from the full GPU VA range and
+ /// a user request.
+ pub(crate) fn compute(full: Range<u64>, req: UserVaRequest) -> Result<Self> {
+ // Minimum VA space reserved for kernel objects (heaps, ring buffers, ...).
+ const MIN_KERNEL_VA: u64 = u64::SZ_256M;
+
+ if full.end <= MIN_KERNEL_VA {
+ return Err(EINVAL);
+ }
+
+ let user_max = full.end - MIN_KERNEL_VA;
+
+ let user_end = match req {
+ UserVaRequest::Fixed(v) => {
+ let user_size = v.get();
+ if user_size > user_max {
+ return Err(EINVAL);
+ }
+ user_size
+ }
+ UserVaRequest::Auto => {
+ let task_size = current!().task_size() as u64;
+ let candidate = if task_size < full.end {
+ task_size
+ } else {
+ // If the range exceeds 4G, split it in two so CPU and
+ // GPU share the same addresses (SVM).
+ if full.end > u64::SZ_4G {
+ full.end / 2
+ } else {
+ user_max
+ }
+ };
+ candidate.min(user_max)
+ }
+ };
+
+ let delta = full.end - user_end;
+ // Pick a kernel VA range that's a power of two, to have a clear split.
+ let kernel_va_range = 1u64 << delta.ilog2();
+ let kernel_va_start = full.end - kernel_va_range;
+ let full_start = full.start;
+
+ Ok(Self {
+ full,
+ user: full_start..kernel_va_start,
+ })
+ }
+}
+
/// Arguments for a virtual memory map operation.
struct VmMapArgs<'drm> {
/// Access permissions and caching behavior for the mapping.
@@ -335,26 +422,64 @@ pub(crate) struct Vm<'drm> {
/// Non-core part of the GPUVM. Can be used for stuff that doesn't modify the
/// internal mapping tree, like GpuVm::obtain()
gpuvm: ARef<GpuVm<GpuVmData<'drm>>>,
- /// VA range for this VM.
- va_range: Range<u64>,
+ /// VA layout for this VM.
+ pub(crate) layout: VmLayout,
}
impl<'drm> Vm<'drm> {
- /// Creates a new GPU virtual address space.
+ /// Creates the MCU/firmware VM.
///
- /// The VM is initialized with a page table configured according to the GPU's
- /// address translation capabilities and registered with the GPUVM framework.
- pub(crate) fn new(
+ /// The MCU VM is entirely kernel-managed: it has no user-accessible range.
+ pub(crate) fn new_for_fw(
+ dev: &'drm Device<Bound>,
+ ddev: &TyrDrmDevice,
+ mmu: ArcBorrow<'_, Mmu<'drm>>,
+ gpu_info: &GpuInfo,
+ ) -> Result<Arc<Vm<'drm>>> {
+ // As in panthor: the CSF MCU is a Cortex-M7 and can only address 4G.
+ let layout = VmLayout {
+ full: 0..u64::SZ_4G,
+ user: 0..0u64,
+ };
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ }
+
+ /// Creates a user VM, splitting the GPU VA range per `user_va`.
+ #[expect(dead_code)]
+ pub(crate) fn new_for_user(
dev: &'drm Device<Bound>,
ddev: &TyrDrmDevice,
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
+ user_va: UserVaRequest,
+ ) -> Result<Arc<Vm<'drm>>> {
+ let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
+ let va_bits = mmu_features.va_bits().get();
+ let range = 0..(1u64 << va_bits);
+
+ let layout = VmLayout::compute(range.clone(), user_va).inspect_err(|_| {
+ dev_dbg!(
+ dev,
+ "Failed to split GPU VA range {:#x}..{:#x} into user and kernel regions\n",
+ range.start,
+ range.end
+ );
+ })?;
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ }
+
+ /// Initializes a VM with the given layout.
+ fn new_internal(
+ dev: &'drm Device<Bound>,
+ ddev: &TyrDrmDevice,
+ mmu: ArcBorrow<'_, Mmu<'drm>>,
+ gpu_info: &GpuInfo,
+ layout: VmLayout,
) -> Result<Arc<Vm<'drm>>> {
let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
let va_bits = mmu_features.va_bits().get();
let pa_bits = mmu_features.pa_bits().get();
- let range = 0..(1u64 << va_bits);
let reserve_range = 0..0u64;
// dummy_obj is used to initialize the GPUVM tree.
@@ -366,7 +491,7 @@ pub(crate) fn new(
c"Tyr::GpuVm",
ddev,
&*dummy_obj,
- range.clone(),
+ layout.full.clone(),
reserve_range,
GpuVmData::<'drm> {
_phantom: PhantomData::<&()>,
@@ -386,7 +511,7 @@ pub(crate) fn new(
mmu: mmu.into(),
gpuvm,
gpuvm_unique <- new_mutex!(gpuvm_unique),
- va_range: range,
+ layout,
}),
GFP_KERNEL,
)?;
@@ -417,10 +542,12 @@ fn deactivate(&self) -> Result {
/// Kills the VM by deactivating it and unmapping all regions.
pub(crate) fn kill(&self) {
- // TODO: Turn the VM into a state where it can't be used.
let _ = self.deactivate();
let _ = self
- .unmap_range(self.va_range.start, self.va_range.end - self.va_range.start)
+ .unmap_range(
+ self.layout.full.start,
+ self.layout.full.end - self.layout.full.start,
+ )
.inspect_err(|e| {
dev_err!(self.dev, "Failed to unmap range during deactivate: {:?}", e);
});
@@ -557,14 +684,14 @@ pub(crate) fn unmap_range(&self, va: u64, size: u64) -> Result {
let end = va.checked_add(size).ok_or(EINVAL)?;
- if va < self.va_range.start || end > self.va_range.end {
+ if va < self.layout.full.start || end > self.layout.full.end {
dev_err!(
self.dev,
"Unmap range {:#x}..{:#x} exceeds VM range {:#x}..{:#x}",
va,
end,
- self.va_range.start,
- self.va_range.end
+ self.layout.full.start,
+ self.layout.full.end
);
return Err(EINVAL);
}
@@ -574,7 +701,7 @@ pub(crate) fn unmap_range(&self, va: u64, size: u64) -> Result {
region: va..end,
};
- let full_vm = va == self.va_range.start && end == self.va_range.end;
+ let full_vm = va == self.layout.full.start && end == self.layout.full.end;
let mut resources = VmOpResources {
preallocated_gpuvas: if full_vm {
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 06/10] drm/tyr: add BO creation and lookup helpers
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Add helpers for the BO ioctls: new_object() creates a GEM object with
the size aligned up to PAGE_SIZE, and lookup_handle() resolves a handle
for a DRM file.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
---
drivers/gpu/drm/tyr/gem.rs | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 3bf3787f5c3fd..0e0989a48678f 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -9,8 +9,10 @@
use kernel::{
drm::gem::{
self,
- shmem, //
+ shmem,
+ BaseObject, //
},
+ page::PAGE_SIZE,
prelude::*,
sync::{
aref::ARef,
@@ -23,6 +25,7 @@
TyrDrmDevice,
TyrDrmDriver, //
},
+ file::TyrDrmFile,
vm::{
Vm,
VmMapFlags, //
@@ -53,6 +56,32 @@ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Se
/// Type alias for Tyr GEM buffer objects.
pub(crate) type Bo = gem::shmem::Object<BoData>;
+/// Create a new GEM buffer object.
+#[expect(dead_code)]
+pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result<ARef<Bo>> {
+ if size == 0 {
+ return Err(EINVAL);
+ }
+
+ let aligned_size = size.checked_next_multiple_of(PAGE_SIZE).ok_or(EINVAL)?;
+
+ Bo::new(
+ ddev,
+ aligned_size,
+ shmem::ObjectConfig {
+ map_wc: true,
+ parent_resv_obj: None,
+ },
+ BoCreateArgs { flags },
+ )
+}
+
+/// Look up a GEM object by handle for a DRM file.
+#[expect(dead_code)]
+pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>> {
+ Bo::lookup_handle(file, handle)
+}
+
/// Creates a dummy GEM object to serve as the root of a GPUVM.
pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice) -> Result<ARef<Bo>> {
let bo = Bo::new(
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 06/10] drm/tyr: add BO creation and lookup helpers
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Add helpers for the BO ioctls: new_object() creates a GEM object with
the size aligned up to PAGE_SIZE, and lookup_handle() resolves a handle
for a DRM file.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
---
drivers/gpu/drm/tyr/gem.rs | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 3bf3787f5c3fd..0e0989a48678f 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -9,8 +9,10 @@
use kernel::{
drm::gem::{
self,
- shmem, //
+ shmem,
+ BaseObject, //
},
+ page::PAGE_SIZE,
prelude::*,
sync::{
aref::ARef,
@@ -23,6 +25,7 @@
TyrDrmDevice,
TyrDrmDriver, //
},
+ file::TyrDrmFile,
vm::{
Vm,
VmMapFlags, //
@@ -53,6 +56,32 @@ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Se
/// Type alias for Tyr GEM buffer objects.
pub(crate) type Bo = gem::shmem::Object<BoData>;
+/// Create a new GEM buffer object.
+#[expect(dead_code)]
+pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result<ARef<Bo>> {
+ if size == 0 {
+ return Err(EINVAL);
+ }
+
+ let aligned_size = size.checked_next_multiple_of(PAGE_SIZE).ok_or(EINVAL)?;
+
+ Bo::new(
+ ddev,
+ aligned_size,
+ shmem::ObjectConfig {
+ map_wc: true,
+ parent_resv_obj: None,
+ },
+ BoCreateArgs { flags },
+ )
+}
+
+/// Look up a GEM object by handle for a DRM file.
+#[expect(dead_code)]
+pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>> {
+ Bo::lookup_handle(file, handle)
+}
+
/// Creates a dummy GEM object to serve as the root of a GPUVM.
pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice) -> Result<ARef<Bo>> {
let bo = Bo::new(
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 07/10] drm/tyr: add VM-related ioctls
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Implement VM_CREATE, VM_DESTROY, VM_BIND and VM_GET_STATE. VM_CREATE
hands the new VM to the per-file pool and, on insertion failure, the
caller tears it down, so an aborted creation never leaks a VM.
VM_DESTROY removes the VM from the pool and kills it, even if in-flight
jobs still hold references (matching panthor: such jobs are expected
to fault). VM_BIND runs synchronously for now.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/driver.rs | 10 +-
drivers/gpu/drm/tyr/file.rs | 295 ++++++++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/tyr/gem.rs | 1 -
drivers/gpu/drm/tyr/vm.rs | 69 ++++++++--
4 files changed, 356 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 46caf7b336d93..ba03cc4c52539 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -73,6 +73,9 @@ pub(crate) struct TyrDrmRegistrationData<'drm> {
/// Firmware sections.
pub(crate) fw: Firmware<'drm>,
+ /// Memory management unit for address space slots.
+ pub(crate) mmu: Arc<Mmu<'drm>>,
+
#[pin]
clks: Mutex<Clocks>,
@@ -171,6 +174,7 @@ fn probe<'bound>(
pin_init!(TyrDrmRegistrationData {
pdev,
fw: firmware,
+ mmu,
clks <- new_mutex!(Clocks {
core: core_clk,
stacks: stacks_clk,
@@ -209,7 +213,7 @@ fn drop(self: Pin<&mut Self>) {}
impl drm::Driver for TyrDrmDriver {
type Data = ();
type RegistrationData<'drm> = TyrDrmRegistrationData<'drm>;
- type File<'a> = TyrDrmFileData;
+ type File<'a> = TyrDrmFileData<'a>;
type Object = Bo;
type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
@@ -218,6 +222,10 @@ impl drm::Driver for TyrDrmDriver {
kernel::declare_drm_ioctls! {
(PANTHOR_DEV_QUERY, drm_panthor_dev_query, ioctl::RENDER_ALLOW, TyrDrmFileData::dev_query),
+ (PANTHOR_VM_CREATE, drm_panthor_vm_create, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_create),
+ (PANTHOR_VM_DESTROY, drm_panthor_vm_destroy, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_destroy),
+ (PANTHOR_VM_BIND, drm_panthor_vm_bind, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_bind),
+ (PANTHOR_VM_GET_STATE, drm_panthor_vm_get_state, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_get_state),
}
}
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 933a365cb016e..49b22e7ee4179 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -3,37 +3,58 @@
use kernel::{
drm::{
self,
+ gem::BaseObject,
Registered, //
},
+ num::casts::FromSafeCast,
prelude::*,
+ sizes::SizeConstants,
+ transmute::FromBytes,
uaccess::UserSlice,
uapi, //
};
-use crate::driver::{
- TyrDrmDevice,
- TyrDrmDriver,
- TyrDrmRegistrationData, //
+use crate::{
+ driver::{
+ TyrDrmDevice,
+ TyrDrmDriver,
+ TyrDrmRegistrationData, //
+ },
+ vm::{
+ UserVaRequest,
+ Vm,
+ VmBindOpType,
+ VmMapFlags,
+ VmPool, //
+ }, //
};
#[pin_data]
-pub(crate) struct TyrDrmFileData {}
+pub(crate) struct TyrDrmFileData<'a> {
+ reg: &'a TyrDrmRegistrationData<'a>,
+
+ #[pin]
+ vm_pool: VmPool<'a>,
+}
/// Convenience type alias for our DRM `File` type.
pub(crate) type TyrDrmFile = drm::file::File<TyrDrmDriver>;
-impl drm::file::DriverFile<'_> for TyrDrmFileData {
+impl<'a> drm::file::DriverFile<'a> for TyrDrmFileData<'a> {
type Driver = TyrDrmDriver;
fn open(
_device: &TyrDrmDevice<Registered>,
- _reg_data: &TyrDrmRegistrationData<'_>,
+ reg_data: &'a TyrDrmRegistrationData<'a>,
) -> impl PinInit<Self, Error> {
- Ok(Self {})
+ try_pin_init!(Self {
+ reg: reg_data,
+ vm_pool <- VmPool::new(reg_data.pdev.as_ref()),
+ })
}
}
-impl TyrDrmFileData {
+impl TyrDrmFileData<'_> {
pub(crate) fn dev_query(
_ddev: &TyrDrmDevice<Registered>,
reg_data: &TyrDrmRegistrationData<'_>,
@@ -65,4 +86,260 @@ pub(crate) fn dev_query(
}
}
}
+
+ pub(crate) fn vm_create(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmcreate: &mut uapi::drm_panthor_vm_create,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if vmcreate.flags != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM create flags: {:#x}\n",
+ vmcreate.flags
+ );
+ return Err(EINVAL);
+ }
+
+ let ret: Result<u32, Error> = file.inner_with(|pfile| {
+ let vm = Vm::new_for_user(
+ pfile.reg.pdev.as_ref(),
+ ddev,
+ pfile.reg.mmu.as_arc_borrow(),
+ &pfile.reg.gpu_info,
+ UserVaRequest::from_uapi(vmcreate.user_va_range),
+ )?;
+ let user_va_range = vm.layout.user.end;
+ let id = pfile.vm_pool.add(vm).map_err(|e| {
+ e.vm.kill();
+ e.error
+ })?;
+ vmcreate.user_va_range = user_va_range;
+ vmcreate.id = id;
+
+ Ok(0)
+ });
+ ret
+ }
+
+ pub(crate) fn vm_destroy(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmdestroy: &mut uapi::drm_panthor_vm_destroy,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if vmdestroy.pad != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM destroy pad: {:#x}\n",
+ vmdestroy.pad
+ );
+ return Err(EINVAL);
+ }
+
+ let ret: Result<u32, Error> = file.inner_with(|pfile| {
+ pfile.vm_pool.remove(vmdestroy.id)?;
+ Ok(0)
+ });
+ ret
+ }
+
+ pub(crate) fn vm_bind(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmbind: &mut uapi::drm_panthor_vm_bind,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ let async_flag = uapi::drm_panthor_vm_bind_flags_DRM_PANTHOR_VM_BIND_ASYNC;
+
+ if vmbind.flags & !async_flag != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM_BIND flags: {:#x}\n",
+ vmbind.flags
+ );
+ return Err(EINVAL);
+ }
+
+ if vmbind.flags & async_flag != 0 {
+ dev_dbg!(ddev.as_ref(), "Async VM_BIND not supported\n");
+ return Err(EOPNOTSUPP);
+ }
+
+ let count = usize::from_safe_cast(vmbind.ops.count);
+ if count == 0 {
+ return file.inner_with(|pfile| {
+ pfile.vm_pool.get(vmbind.vm_id).ok_or(EINVAL)?;
+ Ok(0)
+ });
+ }
+
+ // Panthor allows larger strides; UserSliceReader::read needs an exact one.
+ if vmbind.ops.stride as usize != size_of::<VmBindOp>() {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM_BIND op stride {} (expected {})\n",
+ vmbind.ops.stride,
+ size_of::<VmBindOp>()
+ );
+ return Err(EINVAL);
+ }
+ // TODO: support variable strides with copy_struct_from_user()
+ // semantics: zero-pad shorter structs, reject non-zero trailing bytes.
+
+ let total_len = size_of::<VmBindOp>().checked_mul(count).ok_or_else(|| {
+ dev_dbg!(ddev.as_ref(), "VM_BIND ops length overflow\n");
+ EINVAL
+ })?;
+ let mut reader =
+ UserSlice::new(UserPtr::from_addr(vmbind.ops.array as usize), total_len).reader();
+ let mut ops = KVec::new();
+ for _ in 0..count {
+ ops.push(reader.read::<VmBindOp>()?, GFP_KERNEL)?;
+ }
+
+ let ret: Result<u32, Error> = file.inner_with(|pfile| {
+ let vm = pfile.vm_pool.get(vmbind.vm_id).ok_or_else(|| {
+ dev_dbg!(ddev.as_ref(), "Invalid VM_BIND vm_id: {}\n", vmbind.vm_id);
+ EINVAL
+ })?;
+
+ for (i, op) in ops.iter().enumerate() {
+ if let Err(e) = vm_bind_exec_op(&vm, file, op) {
+ dev_dbg!(ddev.as_ref(), "VM_BIND op {} failed: {:?}\n", i, e);
+ vmbind.ops.count = i as u32;
+ return Err(e);
+ }
+ }
+
+ Ok(0)
+ });
+ ret
+ }
+
+ pub(crate) fn vm_get_state(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmgetstate: &mut uapi::drm_panthor_vm_get_state,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ file.inner_with(|pfile| {
+ let vm = pfile.vm_pool.get(vmgetstate.vm_id).ok_or_else(|| {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM_GET_STATE vm_id: {}\n",
+ vmgetstate.vm_id
+ );
+ EINVAL
+ })?;
+ vmgetstate.state = if vm.is_unusable() {
+ uapi::drm_panthor_vm_state_DRM_PANTHOR_VM_STATE_UNUSABLE
+ } else {
+ uapi::drm_panthor_vm_state_DRM_PANTHOR_VM_STATE_USABLE
+ };
+ Ok(0)
+ })
+ }
+}
+
+fn vm_bind_exec_op(vm: &Vm<'_>, file: &TyrDrmFile, op: &VmBindOp) -> Result {
+ if op.syncs.count != 0 {
+ dev_dbg!(vm.dev(), "VM_BIND op syncs not supported\n");
+ return Err(EINVAL);
+ }
+
+ if op.size == 0 {
+ return Ok(());
+ }
+
+ let end = match op.va.checked_add(op.size) {
+ Some(end) => end,
+ None => {
+ dev_dbg!(vm.dev(), "VM_BIND op VA range overflow\n");
+ return Err(EINVAL);
+ }
+ };
+ if op.va < vm.layout.user.start || end > vm.layout.user.end {
+ dev_dbg!(
+ vm.dev(),
+ "VM_BIND op VA range {:#x}..{:#x} outside user range\n",
+ op.va,
+ end
+ );
+ return Err(EINVAL);
+ }
+
+ if (op.va | op.size | op.bo_offset) & (u64::SZ_4K - 1) != 0 {
+ dev_dbg!(vm.dev(), "VM_BIND op not GPU-page-aligned\n");
+ return Err(EINVAL);
+ }
+
+ match VmBindOpType::try_from(op.flags) {
+ Ok(VmBindOpType::Map) => {
+ // Once the VM is unusable only MAP ops are rejected; UNMAP
+ // stays available for cleanup (see the UAPI docs).
+ if vm.is_unusable() {
+ dev_dbg!(vm.dev(), "VM_BIND map op on unusable VM\n");
+ return Err(EINVAL);
+ }
+
+ let map_flags = match VmMapFlags::try_from(op.flags & !VmBindOpType::MASK) {
+ Ok(flags) => flags,
+ Err(_) => {
+ dev_dbg!(vm.dev(), "VM_BIND op invalid map flags {:#x}\n", op.flags);
+ return Err(EINVAL);
+ }
+ };
+ let bo = crate::gem::lookup_handle(file, op.bo_handle).map_err(|_| {
+ dev_dbg!(vm.dev(), "VM_BIND op invalid BO handle {}\n", op.bo_handle);
+ EINVAL
+ })?;
+ // Validate the BO window before mapping.
+ let bo_size = u64::from_safe_cast(bo.size());
+ if op.size > bo_size || op.bo_offset > bo_size - op.size {
+ dev_dbg!(vm.dev(), "VM_BIND op BO range out of bounds\n");
+ return Err(EINVAL);
+ }
+ vm.map_bo_range(&bo, op.bo_offset, op.size, op.va, map_flags)
+ }
+ Ok(VmBindOpType::Unmap) => {
+ // Unmap must not carry map-specific flags or BO references.
+ if op.flags & !VmBindOpType::MASK != 0 || op.bo_handle != 0 || op.bo_offset != 0 {
+ dev_dbg!(
+ vm.dev(),
+ "VM_BIND UNMAP carries flags/BO refs: flags={:#x} bo_handle={} bo_offset={}\n",
+ op.flags,
+ op.bo_handle,
+ op.bo_offset
+ );
+ return Err(EINVAL);
+ }
+ vm.unmap_range(op.va, op.size)
+ }
+ Err(_) => {
+ dev_dbg!(
+ vm.dev(),
+ "VM_BIND op type {:#x} not supported\n",
+ op.flags & VmBindOpType::MASK
+ );
+ Err(EINVAL)
+ }
+ }
+}
+
+#[repr(transparent)]
+struct VmBindOp(uapi::drm_panthor_vm_bind_op);
+
+impl core::ops::Deref for VmBindOp {
+ type Target = uapi::drm_panthor_vm_bind_op;
+
+ fn deref(&self) -> &Self::Target {
+ &self.0
+ }
}
+
+// TODO: use zerocopy FromBytes when these traits are derived for bindings.
+// SAFETY: `VmBindOp` contains only integers, so any bit pattern is valid;
+// the `#[repr(transparent)]` wrapper has the same layout as the UAPI struct.
+unsafe impl FromBytes for VmBindOp {}
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 0e0989a48678f..60f6363995a5a 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -77,7 +77,6 @@ pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result
}
/// Look up a GEM object by handle for a DRM file.
-#[expect(dead_code)]
pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>> {
Bo::lookup_handle(file, handle)
}
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 44f5046037895..11bc8e2c9d0be 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -54,6 +54,11 @@
},
sync::{
aref::ARef,
+ atomic::{
+ Acquire,
+ Atomic,
+ Release, //
+ },
Arc,
ArcBorrow,
Mutex, //
@@ -173,7 +178,6 @@ pub(crate) enum UserVaRequest {
impl UserVaRequest {
/// UAPI boundary normalization: `0` -> [`Auto`](Self::Auto).
- #[expect(dead_code)]
pub(crate) fn from_uapi(v: u64) -> Self {
match NonZeroU64::new(v) {
Some(size) => Self::Fixed(size),
@@ -182,6 +186,38 @@ pub(crate) fn from_uapi(v: u64) -> Self {
}
}
+/// Operation type, packed into the top nibble of
+/// `drm_panthor_vm_bind_op::flags`.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum VmBindOpType {
+ /// Map a BO range into the VM.
+ Map,
+ /// Unmap a VA range.
+ Unmap,
+}
+
+impl VmBindOpType {
+ /// Bits occupied by the op type in `drm_panthor_vm_bind_op::flags`.
+ pub(crate) const MASK: u32 =
+ uapi::drm_panthor_vm_bind_op_flags_DRM_PANTHOR_VM_BIND_OP_TYPE_MASK as u32;
+}
+
+impl TryFrom<u32> for VmBindOpType {
+ type Error = Error;
+
+ fn try_from(flags: u32) -> Result<Self, Self::Error> {
+ const MAP: u32 = uapi::drm_panthor_vm_bind_op_flags_DRM_PANTHOR_VM_BIND_OP_TYPE_MAP as u32;
+ const UNMAP: u32 =
+ uapi::drm_panthor_vm_bind_op_flags_DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP as u32;
+
+ match flags & Self::MASK {
+ MAP => Ok(Self::Map),
+ UNMAP => Ok(Self::Unmap),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
/// Final user/kernel VA layout for a VM.
pub(crate) struct VmLayout {
/// Full GPU VA range covered by this VM.
@@ -424,6 +460,8 @@ pub(crate) struct Vm<'drm> {
gpuvm: ARef<GpuVm<GpuVmData<'drm>>>,
/// VA layout for this VM.
pub(crate) layout: VmLayout,
+ /// Whether the VM is unusable.
+ unusable: Atomic<bool>,
}
impl<'drm> Vm<'drm> {
@@ -445,7 +483,6 @@ pub(crate) fn new_for_fw(
}
/// Creates a user VM, splitting the GPU VA range per `user_va`.
- #[expect(dead_code)]
pub(crate) fn new_for_user(
dev: &'drm Device<Bound>,
ddev: &TyrDrmDevice,
@@ -512,6 +549,7 @@ fn new_internal(
gpuvm,
gpuvm_unique <- new_mutex!(gpuvm_unique),
layout,
+ unusable: Atomic::new(false),
}),
GFP_KERNEL,
)?;
@@ -542,6 +580,7 @@ fn deactivate(&self) -> Result {
/// Kills the VM by deactivating it and unmapping all regions.
pub(crate) fn kill(&self) {
+ self.mark_unusable();
let _ = self.deactivate();
let _ = self
.unmap_range(
@@ -553,6 +592,15 @@ pub(crate) fn kill(&self) {
});
}
+ /// Marks the VM unusable.
+ fn mark_unusable(&self) {
+ self.unusable.store(true, Release);
+ }
+
+ pub(crate) fn is_unusable(&self) -> bool {
+ self.unusable.load(Acquire)
+ }
+
/// Executes a virtual memory operation.
///
/// This handles both map and unmap operations by coordinating between the
@@ -664,6 +712,17 @@ pub(crate) fn map_bo_range(
};
let result = {
let mut gpuvm_unique = self.gpuvm_unique.lock();
+ // Check under the GPUVM lock, which the kill path also takes to
+ // unmap, so this serializes against a concurrent teardown.
+ if self.is_unusable() {
+ dev_err!(
+ self.dev,
+ "Failed to map VA {:#x}..{:#x}: VM is unusable\n",
+ req.region.start,
+ req.region.end
+ );
+ return Err(EINVAL);
+ }
self.exec_op(gpuvm_unique.as_mut().get_mut(), req, &mut resources)
};
// We flush the defer cleanup list now. Things will be different in
@@ -1099,15 +1158,12 @@ pub(crate) struct VmPool<'drm> {
/// Error returned when a VM cannot be added to the pool. The caller
/// owns the teardown of the VM in [`AddError::vm`].
pub(crate) struct AddError<'drm> {
- #[expect(dead_code)]
pub(crate) error: Error,
- #[expect(dead_code)]
pub(crate) vm: Arc<Vm<'drm>>,
}
impl<'drm> VmPool<'drm> {
/// Creates a new [`VmPool`].
- #[expect(dead_code)]
pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
let ids = IdPool::new();
pin_init!(Self {
@@ -1123,7 +1179,6 @@ pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
/// returned in an [`AddError`] so the caller owns the teardown.
// TODO: allocate IDs with the XArray directly (once it grows range
// allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
- #[expect(dead_code)]
pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
let id = (|| {
let mut ids = self.ids.lock();
@@ -1173,7 +1228,6 @@ pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
}
/// Removes and kills the VM with the given ID.
- #[expect(dead_code)]
pub(crate) fn remove(&self, id: u32) -> Result<()> {
let mut vms = self.vms.lock();
let vm = match vms.remove(usize::from_safe_cast(id)) {
@@ -1190,7 +1244,6 @@ pub(crate) fn remove(&self, id: u32) -> Result<()> {
}
/// Gets a shared reference to the VM with the given ID.
- #[expect(dead_code)]
pub(crate) fn get(&self, id: u32) -> Option<Arc<Vm<'drm>>> {
let vms = self.vms.lock();
let borrow = vms.get(usize::from_safe_cast(id))?;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 07/10] drm/tyr: add VM-related ioctls
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Implement VM_CREATE, VM_DESTROY, VM_BIND and VM_GET_STATE. VM_CREATE
hands the new VM to the per-file pool and, on insertion failure, the
caller tears it down, so an aborted creation never leaks a VM.
VM_DESTROY removes the VM from the pool and kills it, even if in-flight
jobs still hold references (matching panthor: such jobs are expected
to fault). VM_BIND runs synchronously for now.
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/driver.rs | 10 +-
drivers/gpu/drm/tyr/file.rs | 295 ++++++++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/tyr/gem.rs | 1 -
drivers/gpu/drm/tyr/vm.rs | 69 ++++++++--
4 files changed, 356 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 46caf7b336d93..ba03cc4c52539 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -73,6 +73,9 @@ pub(crate) struct TyrDrmRegistrationData<'drm> {
/// Firmware sections.
pub(crate) fw: Firmware<'drm>,
+ /// Memory management unit for address space slots.
+ pub(crate) mmu: Arc<Mmu<'drm>>,
+
#[pin]
clks: Mutex<Clocks>,
@@ -171,6 +174,7 @@ fn probe<'bound>(
pin_init!(TyrDrmRegistrationData {
pdev,
fw: firmware,
+ mmu,
clks <- new_mutex!(Clocks {
core: core_clk,
stacks: stacks_clk,
@@ -209,7 +213,7 @@ fn drop(self: Pin<&mut Self>) {}
impl drm::Driver for TyrDrmDriver {
type Data = ();
type RegistrationData<'drm> = TyrDrmRegistrationData<'drm>;
- type File<'a> = TyrDrmFileData;
+ type File<'a> = TyrDrmFileData<'a>;
type Object = Bo;
type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
@@ -218,6 +222,10 @@ impl drm::Driver for TyrDrmDriver {
kernel::declare_drm_ioctls! {
(PANTHOR_DEV_QUERY, drm_panthor_dev_query, ioctl::RENDER_ALLOW, TyrDrmFileData::dev_query),
+ (PANTHOR_VM_CREATE, drm_panthor_vm_create, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_create),
+ (PANTHOR_VM_DESTROY, drm_panthor_vm_destroy, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_destroy),
+ (PANTHOR_VM_BIND, drm_panthor_vm_bind, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_bind),
+ (PANTHOR_VM_GET_STATE, drm_panthor_vm_get_state, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_get_state),
}
}
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 933a365cb016e..49b22e7ee4179 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -3,37 +3,58 @@
use kernel::{
drm::{
self,
+ gem::BaseObject,
Registered, //
},
+ num::casts::FromSafeCast,
prelude::*,
+ sizes::SizeConstants,
+ transmute::FromBytes,
uaccess::UserSlice,
uapi, //
};
-use crate::driver::{
- TyrDrmDevice,
- TyrDrmDriver,
- TyrDrmRegistrationData, //
+use crate::{
+ driver::{
+ TyrDrmDevice,
+ TyrDrmDriver,
+ TyrDrmRegistrationData, //
+ },
+ vm::{
+ UserVaRequest,
+ Vm,
+ VmBindOpType,
+ VmMapFlags,
+ VmPool, //
+ }, //
};
#[pin_data]
-pub(crate) struct TyrDrmFileData {}
+pub(crate) struct TyrDrmFileData<'a> {
+ reg: &'a TyrDrmRegistrationData<'a>,
+
+ #[pin]
+ vm_pool: VmPool<'a>,
+}
/// Convenience type alias for our DRM `File` type.
pub(crate) type TyrDrmFile = drm::file::File<TyrDrmDriver>;
-impl drm::file::DriverFile<'_> for TyrDrmFileData {
+impl<'a> drm::file::DriverFile<'a> for TyrDrmFileData<'a> {
type Driver = TyrDrmDriver;
fn open(
_device: &TyrDrmDevice<Registered>,
- _reg_data: &TyrDrmRegistrationData<'_>,
+ reg_data: &'a TyrDrmRegistrationData<'a>,
) -> impl PinInit<Self, Error> {
- Ok(Self {})
+ try_pin_init!(Self {
+ reg: reg_data,
+ vm_pool <- VmPool::new(reg_data.pdev.as_ref()),
+ })
}
}
-impl TyrDrmFileData {
+impl TyrDrmFileData<'_> {
pub(crate) fn dev_query(
_ddev: &TyrDrmDevice<Registered>,
reg_data: &TyrDrmRegistrationData<'_>,
@@ -65,4 +86,260 @@ pub(crate) fn dev_query(
}
}
}
+
+ pub(crate) fn vm_create(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmcreate: &mut uapi::drm_panthor_vm_create,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if vmcreate.flags != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM create flags: {:#x}\n",
+ vmcreate.flags
+ );
+ return Err(EINVAL);
+ }
+
+ let ret: Result<u32, Error> = file.inner_with(|pfile| {
+ let vm = Vm::new_for_user(
+ pfile.reg.pdev.as_ref(),
+ ddev,
+ pfile.reg.mmu.as_arc_borrow(),
+ &pfile.reg.gpu_info,
+ UserVaRequest::from_uapi(vmcreate.user_va_range),
+ )?;
+ let user_va_range = vm.layout.user.end;
+ let id = pfile.vm_pool.add(vm).map_err(|e| {
+ e.vm.kill();
+ e.error
+ })?;
+ vmcreate.user_va_range = user_va_range;
+ vmcreate.id = id;
+
+ Ok(0)
+ });
+ ret
+ }
+
+ pub(crate) fn vm_destroy(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmdestroy: &mut uapi::drm_panthor_vm_destroy,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if vmdestroy.pad != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM destroy pad: {:#x}\n",
+ vmdestroy.pad
+ );
+ return Err(EINVAL);
+ }
+
+ let ret: Result<u32, Error> = file.inner_with(|pfile| {
+ pfile.vm_pool.remove(vmdestroy.id)?;
+ Ok(0)
+ });
+ ret
+ }
+
+ pub(crate) fn vm_bind(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmbind: &mut uapi::drm_panthor_vm_bind,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ let async_flag = uapi::drm_panthor_vm_bind_flags_DRM_PANTHOR_VM_BIND_ASYNC;
+
+ if vmbind.flags & !async_flag != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM_BIND flags: {:#x}\n",
+ vmbind.flags
+ );
+ return Err(EINVAL);
+ }
+
+ if vmbind.flags & async_flag != 0 {
+ dev_dbg!(ddev.as_ref(), "Async VM_BIND not supported\n");
+ return Err(EOPNOTSUPP);
+ }
+
+ let count = usize::from_safe_cast(vmbind.ops.count);
+ if count == 0 {
+ return file.inner_with(|pfile| {
+ pfile.vm_pool.get(vmbind.vm_id).ok_or(EINVAL)?;
+ Ok(0)
+ });
+ }
+
+ // Panthor allows larger strides; UserSliceReader::read needs an exact one.
+ if vmbind.ops.stride as usize != size_of::<VmBindOp>() {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM_BIND op stride {} (expected {})\n",
+ vmbind.ops.stride,
+ size_of::<VmBindOp>()
+ );
+ return Err(EINVAL);
+ }
+ // TODO: support variable strides with copy_struct_from_user()
+ // semantics: zero-pad shorter structs, reject non-zero trailing bytes.
+
+ let total_len = size_of::<VmBindOp>().checked_mul(count).ok_or_else(|| {
+ dev_dbg!(ddev.as_ref(), "VM_BIND ops length overflow\n");
+ EINVAL
+ })?;
+ let mut reader =
+ UserSlice::new(UserPtr::from_addr(vmbind.ops.array as usize), total_len).reader();
+ let mut ops = KVec::new();
+ for _ in 0..count {
+ ops.push(reader.read::<VmBindOp>()?, GFP_KERNEL)?;
+ }
+
+ let ret: Result<u32, Error> = file.inner_with(|pfile| {
+ let vm = pfile.vm_pool.get(vmbind.vm_id).ok_or_else(|| {
+ dev_dbg!(ddev.as_ref(), "Invalid VM_BIND vm_id: {}\n", vmbind.vm_id);
+ EINVAL
+ })?;
+
+ for (i, op) in ops.iter().enumerate() {
+ if let Err(e) = vm_bind_exec_op(&vm, file, op) {
+ dev_dbg!(ddev.as_ref(), "VM_BIND op {} failed: {:?}\n", i, e);
+ vmbind.ops.count = i as u32;
+ return Err(e);
+ }
+ }
+
+ Ok(0)
+ });
+ ret
+ }
+
+ pub(crate) fn vm_get_state(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ vmgetstate: &mut uapi::drm_panthor_vm_get_state,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ file.inner_with(|pfile| {
+ let vm = pfile.vm_pool.get(vmgetstate.vm_id).ok_or_else(|| {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid VM_GET_STATE vm_id: {}\n",
+ vmgetstate.vm_id
+ );
+ EINVAL
+ })?;
+ vmgetstate.state = if vm.is_unusable() {
+ uapi::drm_panthor_vm_state_DRM_PANTHOR_VM_STATE_UNUSABLE
+ } else {
+ uapi::drm_panthor_vm_state_DRM_PANTHOR_VM_STATE_USABLE
+ };
+ Ok(0)
+ })
+ }
+}
+
+fn vm_bind_exec_op(vm: &Vm<'_>, file: &TyrDrmFile, op: &VmBindOp) -> Result {
+ if op.syncs.count != 0 {
+ dev_dbg!(vm.dev(), "VM_BIND op syncs not supported\n");
+ return Err(EINVAL);
+ }
+
+ if op.size == 0 {
+ return Ok(());
+ }
+
+ let end = match op.va.checked_add(op.size) {
+ Some(end) => end,
+ None => {
+ dev_dbg!(vm.dev(), "VM_BIND op VA range overflow\n");
+ return Err(EINVAL);
+ }
+ };
+ if op.va < vm.layout.user.start || end > vm.layout.user.end {
+ dev_dbg!(
+ vm.dev(),
+ "VM_BIND op VA range {:#x}..{:#x} outside user range\n",
+ op.va,
+ end
+ );
+ return Err(EINVAL);
+ }
+
+ if (op.va | op.size | op.bo_offset) & (u64::SZ_4K - 1) != 0 {
+ dev_dbg!(vm.dev(), "VM_BIND op not GPU-page-aligned\n");
+ return Err(EINVAL);
+ }
+
+ match VmBindOpType::try_from(op.flags) {
+ Ok(VmBindOpType::Map) => {
+ // Once the VM is unusable only MAP ops are rejected; UNMAP
+ // stays available for cleanup (see the UAPI docs).
+ if vm.is_unusable() {
+ dev_dbg!(vm.dev(), "VM_BIND map op on unusable VM\n");
+ return Err(EINVAL);
+ }
+
+ let map_flags = match VmMapFlags::try_from(op.flags & !VmBindOpType::MASK) {
+ Ok(flags) => flags,
+ Err(_) => {
+ dev_dbg!(vm.dev(), "VM_BIND op invalid map flags {:#x}\n", op.flags);
+ return Err(EINVAL);
+ }
+ };
+ let bo = crate::gem::lookup_handle(file, op.bo_handle).map_err(|_| {
+ dev_dbg!(vm.dev(), "VM_BIND op invalid BO handle {}\n", op.bo_handle);
+ EINVAL
+ })?;
+ // Validate the BO window before mapping.
+ let bo_size = u64::from_safe_cast(bo.size());
+ if op.size > bo_size || op.bo_offset > bo_size - op.size {
+ dev_dbg!(vm.dev(), "VM_BIND op BO range out of bounds\n");
+ return Err(EINVAL);
+ }
+ vm.map_bo_range(&bo, op.bo_offset, op.size, op.va, map_flags)
+ }
+ Ok(VmBindOpType::Unmap) => {
+ // Unmap must not carry map-specific flags or BO references.
+ if op.flags & !VmBindOpType::MASK != 0 || op.bo_handle != 0 || op.bo_offset != 0 {
+ dev_dbg!(
+ vm.dev(),
+ "VM_BIND UNMAP carries flags/BO refs: flags={:#x} bo_handle={} bo_offset={}\n",
+ op.flags,
+ op.bo_handle,
+ op.bo_offset
+ );
+ return Err(EINVAL);
+ }
+ vm.unmap_range(op.va, op.size)
+ }
+ Err(_) => {
+ dev_dbg!(
+ vm.dev(),
+ "VM_BIND op type {:#x} not supported\n",
+ op.flags & VmBindOpType::MASK
+ );
+ Err(EINVAL)
+ }
+ }
+}
+
+#[repr(transparent)]
+struct VmBindOp(uapi::drm_panthor_vm_bind_op);
+
+impl core::ops::Deref for VmBindOp {
+ type Target = uapi::drm_panthor_vm_bind_op;
+
+ fn deref(&self) -> &Self::Target {
+ &self.0
+ }
}
+
+// TODO: use zerocopy FromBytes when these traits are derived for bindings.
+// SAFETY: `VmBindOp` contains only integers, so any bit pattern is valid;
+// the `#[repr(transparent)]` wrapper has the same layout as the UAPI struct.
+unsafe impl FromBytes for VmBindOp {}
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 0e0989a48678f..60f6363995a5a 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -77,7 +77,6 @@ pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result
}
/// Look up a GEM object by handle for a DRM file.
-#[expect(dead_code)]
pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>> {
Bo::lookup_handle(file, handle)
}
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 44f5046037895..11bc8e2c9d0be 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -54,6 +54,11 @@
},
sync::{
aref::ARef,
+ atomic::{
+ Acquire,
+ Atomic,
+ Release, //
+ },
Arc,
ArcBorrow,
Mutex, //
@@ -173,7 +178,6 @@ pub(crate) enum UserVaRequest {
impl UserVaRequest {
/// UAPI boundary normalization: `0` -> [`Auto`](Self::Auto).
- #[expect(dead_code)]
pub(crate) fn from_uapi(v: u64) -> Self {
match NonZeroU64::new(v) {
Some(size) => Self::Fixed(size),
@@ -182,6 +186,38 @@ pub(crate) fn from_uapi(v: u64) -> Self {
}
}
+/// Operation type, packed into the top nibble of
+/// `drm_panthor_vm_bind_op::flags`.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum VmBindOpType {
+ /// Map a BO range into the VM.
+ Map,
+ /// Unmap a VA range.
+ Unmap,
+}
+
+impl VmBindOpType {
+ /// Bits occupied by the op type in `drm_panthor_vm_bind_op::flags`.
+ pub(crate) const MASK: u32 =
+ uapi::drm_panthor_vm_bind_op_flags_DRM_PANTHOR_VM_BIND_OP_TYPE_MASK as u32;
+}
+
+impl TryFrom<u32> for VmBindOpType {
+ type Error = Error;
+
+ fn try_from(flags: u32) -> Result<Self, Self::Error> {
+ const MAP: u32 = uapi::drm_panthor_vm_bind_op_flags_DRM_PANTHOR_VM_BIND_OP_TYPE_MAP as u32;
+ const UNMAP: u32 =
+ uapi::drm_panthor_vm_bind_op_flags_DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP as u32;
+
+ match flags & Self::MASK {
+ MAP => Ok(Self::Map),
+ UNMAP => Ok(Self::Unmap),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
/// Final user/kernel VA layout for a VM.
pub(crate) struct VmLayout {
/// Full GPU VA range covered by this VM.
@@ -424,6 +460,8 @@ pub(crate) struct Vm<'drm> {
gpuvm: ARef<GpuVm<GpuVmData<'drm>>>,
/// VA layout for this VM.
pub(crate) layout: VmLayout,
+ /// Whether the VM is unusable.
+ unusable: Atomic<bool>,
}
impl<'drm> Vm<'drm> {
@@ -445,7 +483,6 @@ pub(crate) fn new_for_fw(
}
/// Creates a user VM, splitting the GPU VA range per `user_va`.
- #[expect(dead_code)]
pub(crate) fn new_for_user(
dev: &'drm Device<Bound>,
ddev: &TyrDrmDevice,
@@ -512,6 +549,7 @@ fn new_internal(
gpuvm,
gpuvm_unique <- new_mutex!(gpuvm_unique),
layout,
+ unusable: Atomic::new(false),
}),
GFP_KERNEL,
)?;
@@ -542,6 +580,7 @@ fn deactivate(&self) -> Result {
/// Kills the VM by deactivating it and unmapping all regions.
pub(crate) fn kill(&self) {
+ self.mark_unusable();
let _ = self.deactivate();
let _ = self
.unmap_range(
@@ -553,6 +592,15 @@ pub(crate) fn kill(&self) {
});
}
+ /// Marks the VM unusable.
+ fn mark_unusable(&self) {
+ self.unusable.store(true, Release);
+ }
+
+ pub(crate) fn is_unusable(&self) -> bool {
+ self.unusable.load(Acquire)
+ }
+
/// Executes a virtual memory operation.
///
/// This handles both map and unmap operations by coordinating between the
@@ -664,6 +712,17 @@ pub(crate) fn map_bo_range(
};
let result = {
let mut gpuvm_unique = self.gpuvm_unique.lock();
+ // Check under the GPUVM lock, which the kill path also takes to
+ // unmap, so this serializes against a concurrent teardown.
+ if self.is_unusable() {
+ dev_err!(
+ self.dev,
+ "Failed to map VA {:#x}..{:#x}: VM is unusable\n",
+ req.region.start,
+ req.region.end
+ );
+ return Err(EINVAL);
+ }
self.exec_op(gpuvm_unique.as_mut().get_mut(), req, &mut resources)
};
// We flush the defer cleanup list now. Things will be different in
@@ -1099,15 +1158,12 @@ pub(crate) struct VmPool<'drm> {
/// Error returned when a VM cannot be added to the pool. The caller
/// owns the teardown of the VM in [`AddError::vm`].
pub(crate) struct AddError<'drm> {
- #[expect(dead_code)]
pub(crate) error: Error,
- #[expect(dead_code)]
pub(crate) vm: Arc<Vm<'drm>>,
}
impl<'drm> VmPool<'drm> {
/// Creates a new [`VmPool`].
- #[expect(dead_code)]
pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
let ids = IdPool::new();
pin_init!(Self {
@@ -1123,7 +1179,6 @@ pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
/// returned in an [`AddError`] so the caller owns the teardown.
// TODO: allocate IDs with the XArray directly (once it grows range
// allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
- #[expect(dead_code)]
pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
let id = (|| {
let mut ids = self.ids.lock();
@@ -1173,7 +1228,6 @@ pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
}
/// Removes and kills the VM with the given ID.
- #[expect(dead_code)]
pub(crate) fn remove(&self, id: u32) -> Result<()> {
let mut vms = self.vms.lock();
let vm = match vms.remove(usize::from_safe_cast(id)) {
@@ -1190,7 +1244,6 @@ pub(crate) fn remove(&self, id: u32) -> Result<()> {
}
/// Gets a shared reference to the VM with the given ID.
- #[expect(dead_code)]
pub(crate) fn get(&self, id: u32) -> Option<Arc<Vm<'drm>>> {
let vms = self.vms.lock();
let borrow = vms.get(usize::from_safe_cast(id))?;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 07/10] drm/tyr: add VM-related ioctls
2026-11-09 0:36 ` Ke Sun
(?)
@ 2026-11-09 1:01 ` sashiko-bot
-1 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-11-09 1:01 UTC (permalink / raw)
To: Ke Sun; +Cc: dri-devel, ojeda, gary
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] drm/tyr: resource leak in Vm::map_bo_range() when VM is
unusable
- [Medium] drm/tyr: partial 2 MiB block unmap in
GpuVmData::sm_step_remap() fails
- [Low] drm/tyr: inline IdPool in VmPool::new() caps 32-bit VM IDs at 31
--
commit 30d6021aeaa9e7d9a97a8dfc27efff7e1fbee34d
Author: Alvin Sun <alvin.sun@linux.dev>
drm/tyr: add VM-related ioctls
This commit implements the PANTHOR_VM_CREATE, PANTHOR_VM_DESTROY,
PANTHOR_VM_BIND, and PANTHOR_VM_GET_STATE ioctls for the Tyr driver, wiring
them up to per-file VM pool management and synchronous VM bind operations.
> diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
> index 933a365cb016e..49b22e7ee4179 100644
> --- a/drivers/gpu/drm/tyr/file.rs
> +++ b/drivers/gpu/drm/tyr/file.rs
[ ... ]
> +fn vm_bind_exec_op(vm: &Vm<'_>, file: &TyrDrmFile, op: &VmBindOp) -> Result {
[ ... ]
> + if (op.va | op.size | op.bo_offset) & (u64::SZ_4K - 1) != 0 {
> + dev_dbg!(vm.dev(), "VM_BIND op not GPU-page-aligned\n");
> + return Err(EINVAL);
> + }
> +
> + match VmBindOpType::try_from(op.flags) {
> + Ok(VmBindOpType::Map) => {
[ ... ]
> + vm.map_bo_range(&bo, op.bo_offset, op.size, op.va, map_flags)
> + }
> + Ok(VmBindOpType::Unmap) => {
> + // Unmap must not carry map-specific flags or BO references.
> + if op.flags & !VmBindOpType::MASK != 0 || op.bo_handle != 0 || op.bo_offset != 0 {
> + dev_dbg!(
> + vm.dev(),
> + "VM_BIND UNMAP carries flags/BO refs: flags={:#x} bo_handle={} bo_offset={}\n",
> + op.flags,
> + op.bo_handle,
> + op.bo_offset
> + );
> + return Err(EINVAL);
> + }
> + vm.unmap_range(op.va, op.size)
> + }
[Severity: Medium]
Can a 4 KiB-granularity partial unmap or overlapping remap here split a 2 MiB
block mapping and trigger WARN_ONCE in __arm_lpae_unmap()?
VmAsData::new() enables SZ_4K | SZ_2M in pgsize_bitmap, and pt_map() installs
2 MiB leaf block PTEs whenever both IOVA and physical address are 2 MiB-aligned
and at least 2 MiB remains. When userspace issues a partial unmap or
overlapping map inside that 2 MiB block, GpuVmData::sm_step_remap() calls
pt_unmap() directly on the unaligned sub-range:
drivers/gpu/drm/tyr/vm.rs:GpuVmData::sm_step_remap() {
...
if unmap_length > 0 {
let region = unmap_start..(unmap_start + unmap_length);
pt_unmap(context.dev, context.pt, region.clone()).inspect_err(|e| {
...
}
For a sub-block range, pt_unmap() calls IoPageTable::unmap_pages() with
pgsize = SZ_4K against the 2 MiB block PTE, which hits
WARN_ONCE(true, "Unmap of a partial large IOPTE is not allowed") in
__arm_lpae_unmap() and returns 0, failing VM_BIND with EINVAL (whereas C
panthor aligns the unmap range to 2 MiB boundaries and remaps the retained
4 KiB pages of op.prev and op.next).
> diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> index 44f5046037895..11bc8e2c9d0be 100644
> --- a/drivers/gpu/drm/tyr/vm.rs
> +++ b/drivers/gpu/drm/tyr/vm.rs
[ ... ]
> @@ -664,6 +712,17 @@ pub(crate) fn map_bo_range(
> };
> let result = {
> let mut gpuvm_unique = self.gpuvm_unique.lock();
> + // Check under the GPUVM lock, which the kill path also takes to
> + // unmap, so this serializes against a concurrent teardown.
> + if self.is_unusable() {
> + dev_err!(
> + self.dev,
> + "Failed to map VA {:#x}..{:#x}: VM is unusable\n",
> + req.region.start,
> + req.region.end
> + );
> + return Err(EINVAL);
> + }
> self.exec_op(gpuvm_unique.as_mut().get_mut(), req, &mut resources)
> };
> // We flush the defer cleanup list now. Things will be different in
[Severity: Medium]
Does returning early here leak the GpuVmBo, the GEM BO, and the GpuVm when a
concurrent PANTHOR_VM_DESTROY kills the VM between self.gpuvm.obtain() and
self.gpuvm_unique.lock()?
Earlier in Vm::map_bo_range(), req.op_type holds the ARef<GpuVmBo> returned by
self.gpuvm.obtain(bo, ()), which takes references on both the GpuVm and the
BO:
drivers/gpu/drm/tyr/vm.rs:Vm::map_bo_range() {
...
let req = VmOpRequest {
op_type: VmOpType::Map(VmMapArgs {
vm_bo: self.gpuvm.obtain(bo, ())?,
flags,
bo_offset,
}),
region: va..va_end,
};
...
}
When this early return unwinds req, dropping GpuVmBo calls
drm_gpuvm_bo_put_deferred(), which only queues the GpuVmBo onto gpuvm->bo_defer
while skipping self.gpuvm.deferred_cleanup() at the bottom of the function.
Because Vm::kill() has already finished its own unmap_range() before releasing
gpuvm_unique and the VM has been removed from VmPool, deferred_cleanup() will
never run again on this VM, permanently leaking the GpuVmBo, the BO's backing
pages, and the GpuVm. Dropping req andgpuvm_unique before calling
self.gpuvm.deferred_cleanup() (or deferring self.gpuvm.obtain() until after
the check) would avoid this.
> @@ -1099,15 +1158,12 @@ pub(crate) struct VmPool<'drm> {
> /// Error returned when a VM cannot be added to the pool. The caller
> /// owns the teardown of the VM in [`AddError::vm`].
> pub(crate) struct AddError<'drm> {
> - #[expect(dead_code)]
> pub(crate) error: Error,
> - #[expect(dead_code)]
> pub(crate) vm: Arc<Vm<'drm>>,
> }
>
> impl<'drm> VmPool<'drm> {
> /// Creates a new [`VmPool`].
> - #[expect(dead_code)]
> pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
> let ids = IdPool::new();
> pin_init!(Self {
> @@ -1123,7 +1179,6 @@ pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
> /// returned in an [`AddError`] so the caller owns the teardown.
> // TODO: allocate IDs with the XArray directly (once it grows range
> // allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
> - #[expect(dead_code)]
> pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
> let id = (|| {
> let mut ids = self.ids.lock();
[Severity: Low]
On 32-bit architectures (ARM is enabled in Kconfig), does IdPool::new() limit
each file to 31 VMs instead of MAX_VMS_PER_FILE (32)?
IdPool::new() creates an inline bitmap of usize::BITS bits (indices 0..=31 on
32-bit), and VmPool::add() calls ids.find_unused_id(1) without growing the
pool:
drivers/gpu/drm/tyr/vm.rs:VmPool::add() {
...
let mut ids = self.ids.lock();
let unused = ids.find_unused_id(1).ok_or(ENOSPC)?;
if unused.as_usize() > MAX_VMS_PER_FILE {
return Err(ENOSPC);
}
...
}
Because index 0 is skipped and index 32 is outside the 32-bit inline bitmap,
creating the 32nd VM on a file fails with ENOSPC.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-tyr-ioctls-v6-0-b2896bf2b3c9@kylinos.cn?part=7
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 08/10] rust: device: expose the device DMA attribute
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Sami Tolvanen <samitolvanen@google.com>
A driver may need to know whether its device is DMA-coherent, but
rust/kernel exposes no accessor for the attribute today.
Add a transparent DmaAttr wrapper around device_get_dma_attr(), with
the three attribute values (unset, non-coherent, coherent) exposed as
constants, so drivers can compare against the exact attribute they
care about.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Co-developed-by: Alvin Sun <alvin.sun@linux.dev>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
rust/kernel/device.rs | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 2291d85b6849e..c6381d47f51e8 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -260,6 +260,33 @@ pub unsafe fn drvdata_borrow<T>(&self) -> Pin<&T> {
}
}
+/// The DMA attribute of a device, as reported by firmware. A transparent
+/// wrapper around the kernel's `enum dev_dma_attr`, covering all its states.
+#[repr(transparent)]
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub struct DmaAttr(bindings::dev_dma_attr);
+
+impl DmaAttr {
+ /// The device has no reported DMA attributes.
+ pub const NOT_SUPPORTED: Self = Self(bindings::dev_dma_attr_DEV_DMA_NOT_SUPPORTED);
+
+ /// The device is not DMA-coherent.
+ pub const NON_COHERENT: Self = Self(bindings::dev_dma_attr_DEV_DMA_NON_COHERENT);
+
+ /// The device is DMA-coherent.
+ pub const COHERENT: Self = Self(bindings::dev_dma_attr_DEV_DMA_COHERENT);
+}
+
+impl Device<Bound> {
+ /// Returns the DMA attribute of the device as reported by firmware.
+ #[inline]
+ pub fn dma_attr(&self) -> DmaAttr {
+ // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer
+ // to a `struct device`.
+ unsafe { DmaAttr(bindings::device_get_dma_attr(self.as_raw())) }
+ }
+}
+
impl<Ctx: DeviceContext> Device<Ctx> {
/// Obtain the raw `struct device *`.
pub(crate) fn as_raw(&self) -> *mut bindings::device {
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 08/10] rust: device: expose the device DMA attribute
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Sami Tolvanen <samitolvanen@google.com>
A driver may need to know whether its device is DMA-coherent, but
rust/kernel exposes no accessor for the attribute today.
Add a transparent DmaAttr wrapper around device_get_dma_attr(), with
the three attribute values (unset, non-coherent, coherent) exposed as
constants, so drivers can compare against the exact attribute they
care about.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Co-developed-by: Alvin Sun <alvin.sun@linux.dev>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
rust/kernel/device.rs | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 2291d85b6849e..c6381d47f51e8 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -260,6 +260,33 @@ pub unsafe fn drvdata_borrow<T>(&self) -> Pin<&T> {
}
}
+/// The DMA attribute of a device, as reported by firmware. A transparent
+/// wrapper around the kernel's `enum dev_dma_attr`, covering all its states.
+#[repr(transparent)]
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub struct DmaAttr(bindings::dev_dma_attr);
+
+impl DmaAttr {
+ /// The device has no reported DMA attributes.
+ pub const NOT_SUPPORTED: Self = Self(bindings::dev_dma_attr_DEV_DMA_NOT_SUPPORTED);
+
+ /// The device is not DMA-coherent.
+ pub const NON_COHERENT: Self = Self(bindings::dev_dma_attr_DEV_DMA_NON_COHERENT);
+
+ /// The device is DMA-coherent.
+ pub const COHERENT: Self = Self(bindings::dev_dma_attr_DEV_DMA_COHERENT);
+}
+
+impl Device<Bound> {
+ /// Returns the DMA attribute of the device as reported by firmware.
+ #[inline]
+ pub fn dma_attr(&self) -> DmaAttr {
+ // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer
+ // to a `struct device`.
+ unsafe { DmaAttr(bindings::device_get_dma_attr(self.as_raw())) }
+ }
+}
+
impl<Ctx: DeviceContext> Device<Ctx> {
/// Obtain the raw `struct device *`.
pub(crate) fn as_raw(&self) -> *mut bindings::device {
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 09/10] drm/tyr: support write-combine BO mappings
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Sami Tolvanen <samitolvanen@google.com>
Tyr maps shmem-backed buffers write-combine, but on a coherent device
they are mapped write-back. Cache the coherence at probe and thread it
through to every shmem::ObjectConfig::map_wc site.
Userspace can opt out with DRM_PANTHOR_BO_WB_MMAP, which maps the
buffer cached instead.
The shmem layer zero-inits pages through a cached mapping, and those
stale lines can land over later uncached writes, so fetch the sg_table
at creation to flush them via dma_map_sgtable() before anyone accesses
them uncached. Taking the bound device keeps the unsafe
as_bound() call out of BO creation.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Co-developed-by: Alvin Sun <alvin.sun@linux.dev>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/driver.rs | 13 ++++++++-
drivers/gpu/drm/tyr/file.rs | 1 +
drivers/gpu/drm/tyr/fw.rs | 4 ++-
drivers/gpu/drm/tyr/gem.rs | 61 ++++++++++++++++++++++++++++++++++---------
drivers/gpu/drm/tyr/vm.rs | 9 ++++---
5 files changed, 71 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index ba03cc4c52539..6869ab345d8fd 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -9,7 +9,8 @@
Bound,
Core,
Device,
- DeviceContext, //
+ DeviceContext,
+ DmaAttr, //
},
dma::{
Device as DmaDevice,
@@ -87,6 +88,12 @@ pub(crate) struct TyrDrmRegistrationData<'drm> {
/// GPU information read from hardware during probe.
pub(crate) gpu_info: GpuInfo,
+
+ /// Whether the device is reported as DMA-coherent by firmware.
+ ///
+ /// Cached at probe via `device_get_dma_attr()`. Drives the BO
+ /// cacheability policy in `crate::gem::should_map_wc`.
+ pub(crate) coherent: bool,
}
fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
@@ -151,6 +158,8 @@ fn probe<'bound>(
// other threads of execution.
unsafe { pdev.dma_set_mask_and_coherent(DmaMask::try_new(pa_bits)?)? };
+ let coherent = pdev.as_ref().dma_attr() == DmaAttr::COHERENT;
+
let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, Ok(()))?;
let mmu = Mmu::new(pdev.as_ref(), iomem.as_arc_borrow(), &gpu_info)?;
@@ -161,6 +170,7 @@ fn probe<'bound>(
&unreg_dev,
mmu.as_arc_borrow(),
&gpu_info,
+ coherent,
)?;
firmware.boot()?;
@@ -186,6 +196,7 @@ fn probe<'bound>(
}),
iomem,
gpu_info,
+ coherent,
}),
)},
_: { dev_dbg!(pdev, "Tyr initialized correctly.") },
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 49b22e7ee4179..b982e8db2c4c1 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -109,6 +109,7 @@ pub(crate) fn vm_create(
pfile.reg.mmu.as_arc_borrow(),
&pfile.reg.gpu_info,
UserVaRequest::from_uapi(vmcreate.user_va_range),
+ pfile.reg.coherent,
)?;
let user_va_range = vm.layout.user.end;
let id = pfile.vm_pool.add(vm).map_err(|e| {
diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
index 196ffc96d9d99..1c77bf109a9be 100644
--- a/drivers/gpu/drm/tyr/fw.rs
+++ b/drivers/gpu/drm/tyr/fw.rs
@@ -217,8 +217,9 @@ pub(crate) fn new(
ddev: &TyrDrmDevice,
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
+ coherent: bool,
) -> Result<Firmware<'drm>> {
- let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info)?;
+ let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info, coherent)?;
vm.activate()?;
let result = (|| {
@@ -235,6 +236,7 @@ pub(crate) fn new(
size,
KernelBoVaAlloc::Explicit(va),
parsed.vm_map_flags,
+ coherent,
)?;
let section_start = parsed.data_range.start as usize;
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 60f6363995a5a..55e2d67b96d98 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -7,6 +7,10 @@
use core::ops::Range;
use kernel::{
+ device::{
+ Bound,
+ Device, //
+ },
drm::gem::{
self,
shmem,
@@ -17,7 +21,8 @@
sync::{
aref::ARef,
Arc, //
- }, //
+ },
+ uapi, //
};
use crate::{
@@ -38,6 +43,14 @@ pub(crate) struct BoData {
flags: u32,
}
+impl BoData {
+ /// Returns the flags the BO was created with.
+ #[expect(dead_code)]
+ pub(crate) fn flags(&self) -> u32 {
+ self.flags
+ }
+}
+
/// Provides a way to pass arguments when creating BoData
/// as required by the gem::DriverObject trait.
pub(crate) struct BoCreateArgs {
@@ -56,24 +69,48 @@ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Se
/// Type alias for Tyr GEM buffer objects.
pub(crate) type Bo = gem::shmem::Object<BoData>;
+/// Returns whether a BO should be mapped write-combine.
+pub(crate) fn should_map_wc(coherent: bool, flags: u32) -> bool {
+ if coherent {
+ return false;
+ }
+
+ if flags & uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_WB_MMAP != 0 {
+ return false;
+ }
+
+ true
+}
+
/// Create a new GEM buffer object.
#[expect(dead_code)]
-pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result<ARef<Bo>> {
+pub(crate) fn new_object(
+ ddev: &TyrDrmDevice,
+ pdev: &Device<Bound>,
+ size: usize,
+ flags: u32,
+ coherent: bool,
+) -> Result<ARef<Bo>> {
if size == 0 {
return Err(EINVAL);
}
let aligned_size = size.checked_next_multiple_of(PAGE_SIZE).ok_or(EINVAL)?;
- Bo::new(
+ let map_wc = should_map_wc(coherent, flags);
+ let bo = Bo::new(
ddev,
aligned_size,
shmem::ObjectConfig {
- map_wc: true,
+ map_wc,
parent_resv_obj: None,
},
BoCreateArgs { flags },
- )
+ )?;
+
+ bo.sg_table(pdev)?;
+
+ Ok(bo)
}
/// Look up a GEM object by handle for a DRM file.
@@ -82,18 +119,17 @@ pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>>
}
/// Creates a dummy GEM object to serve as the root of a GPUVM.
-pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice) -> Result<ARef<Bo>> {
- let bo = Bo::new(
+pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice, coherent: bool) -> Result<ARef<Bo>> {
+ // FIXME: use a Rust resv-object abstraction once available, rather than a real BO.
+ Bo::new(
ddev,
4096,
shmem::ObjectConfig {
- map_wc: true,
+ map_wc: should_map_wc(coherent, 0),
parent_resv_obj: None,
},
BoCreateArgs { flags: 0 },
- )?;
-
- Ok(bo)
+ )
}
/// Specifies how to choose a GPU virtual address for a [`KernelBo`].
@@ -131,6 +167,7 @@ pub(crate) fn new(
size: u64,
va_alloc: KernelBoVaAlloc,
flags: VmMapFlags,
+ coherent: bool,
) -> Result<Self> {
if size == 0 {
dev_err!(vm.dev(), "Cannot create KernelBo with size 0");
@@ -146,7 +183,7 @@ pub(crate) fn new(
ddev,
bo_size,
shmem::ObjectConfig {
- map_wc: true,
+ map_wc: should_map_wc(coherent, 0),
parent_resv_obj: None,
},
BoCreateArgs { flags: 0 },
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 11bc8e2c9d0be..6d312c3b2ccd1 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -473,13 +473,14 @@ pub(crate) fn new_for_fw(
ddev: &TyrDrmDevice,
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
+ coherent: bool,
) -> Result<Arc<Vm<'drm>>> {
// As in panthor: the CSF MCU is a Cortex-M7 and can only address 4G.
let layout = VmLayout {
full: 0..u64::SZ_4G,
user: 0..0u64,
};
- Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout, coherent)
}
/// Creates a user VM, splitting the GPU VA range per `user_va`.
@@ -489,6 +490,7 @@ pub(crate) fn new_for_user(
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
user_va: UserVaRequest,
+ coherent: bool,
) -> Result<Arc<Vm<'drm>>> {
let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
let va_bits = mmu_features.va_bits().get();
@@ -502,7 +504,7 @@ pub(crate) fn new_for_user(
range.end
);
})?;
- Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout, coherent)
}
/// Initializes a VM with the given layout.
@@ -512,6 +514,7 @@ fn new_internal(
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
layout: VmLayout,
+ coherent: bool,
) -> Result<Arc<Vm<'drm>>> {
let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
let va_bits = mmu_features.va_bits().get();
@@ -520,7 +523,7 @@ fn new_internal(
let reserve_range = 0..0u64;
// dummy_obj is used to initialize the GPUVM tree.
- let dummy_obj = gem::new_dummy_object(ddev).inspect_err(|e| {
+ let dummy_obj = gem::new_dummy_object(ddev, coherent).inspect_err(|e| {
dev_err!(dev, "Failed to create dummy GEM object: {:?}", e);
})?;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 09/10] drm/tyr: support write-combine BO mappings
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Sami Tolvanen <samitolvanen@google.com>
Tyr maps shmem-backed buffers write-combine, but on a coherent device
they are mapped write-back. Cache the coherence at probe and thread it
through to every shmem::ObjectConfig::map_wc site.
Userspace can opt out with DRM_PANTHOR_BO_WB_MMAP, which maps the
buffer cached instead.
The shmem layer zero-inits pages through a cached mapping, and those
stale lines can land over later uncached writes, so fetch the sg_table
at creation to flush them via dma_map_sgtable() before anyone accesses
them uncached. Taking the bound device keeps the unsafe
as_bound() call out of BO creation.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Co-developed-by: Alvin Sun <alvin.sun@linux.dev>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/driver.rs | 13 ++++++++-
drivers/gpu/drm/tyr/file.rs | 1 +
drivers/gpu/drm/tyr/fw.rs | 4 ++-
drivers/gpu/drm/tyr/gem.rs | 61 ++++++++++++++++++++++++++++++++++---------
drivers/gpu/drm/tyr/vm.rs | 9 ++++---
5 files changed, 71 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index ba03cc4c52539..6869ab345d8fd 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -9,7 +9,8 @@
Bound,
Core,
Device,
- DeviceContext, //
+ DeviceContext,
+ DmaAttr, //
},
dma::{
Device as DmaDevice,
@@ -87,6 +88,12 @@ pub(crate) struct TyrDrmRegistrationData<'drm> {
/// GPU information read from hardware during probe.
pub(crate) gpu_info: GpuInfo,
+
+ /// Whether the device is reported as DMA-coherent by firmware.
+ ///
+ /// Cached at probe via `device_get_dma_attr()`. Drives the BO
+ /// cacheability policy in `crate::gem::should_map_wc`.
+ pub(crate) coherent: bool,
}
fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
@@ -151,6 +158,8 @@ fn probe<'bound>(
// other threads of execution.
unsafe { pdev.dma_set_mask_and_coherent(DmaMask::try_new(pa_bits)?)? };
+ let coherent = pdev.as_ref().dma_attr() == DmaAttr::COHERENT;
+
let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, Ok(()))?;
let mmu = Mmu::new(pdev.as_ref(), iomem.as_arc_borrow(), &gpu_info)?;
@@ -161,6 +170,7 @@ fn probe<'bound>(
&unreg_dev,
mmu.as_arc_borrow(),
&gpu_info,
+ coherent,
)?;
firmware.boot()?;
@@ -186,6 +196,7 @@ fn probe<'bound>(
}),
iomem,
gpu_info,
+ coherent,
}),
)},
_: { dev_dbg!(pdev, "Tyr initialized correctly.") },
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 49b22e7ee4179..b982e8db2c4c1 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -109,6 +109,7 @@ pub(crate) fn vm_create(
pfile.reg.mmu.as_arc_borrow(),
&pfile.reg.gpu_info,
UserVaRequest::from_uapi(vmcreate.user_va_range),
+ pfile.reg.coherent,
)?;
let user_va_range = vm.layout.user.end;
let id = pfile.vm_pool.add(vm).map_err(|e| {
diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
index 196ffc96d9d99..1c77bf109a9be 100644
--- a/drivers/gpu/drm/tyr/fw.rs
+++ b/drivers/gpu/drm/tyr/fw.rs
@@ -217,8 +217,9 @@ pub(crate) fn new(
ddev: &TyrDrmDevice,
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
+ coherent: bool,
) -> Result<Firmware<'drm>> {
- let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info)?;
+ let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info, coherent)?;
vm.activate()?;
let result = (|| {
@@ -235,6 +236,7 @@ pub(crate) fn new(
size,
KernelBoVaAlloc::Explicit(va),
parsed.vm_map_flags,
+ coherent,
)?;
let section_start = parsed.data_range.start as usize;
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 60f6363995a5a..55e2d67b96d98 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -7,6 +7,10 @@
use core::ops::Range;
use kernel::{
+ device::{
+ Bound,
+ Device, //
+ },
drm::gem::{
self,
shmem,
@@ -17,7 +21,8 @@
sync::{
aref::ARef,
Arc, //
- }, //
+ },
+ uapi, //
};
use crate::{
@@ -38,6 +43,14 @@ pub(crate) struct BoData {
flags: u32,
}
+impl BoData {
+ /// Returns the flags the BO was created with.
+ #[expect(dead_code)]
+ pub(crate) fn flags(&self) -> u32 {
+ self.flags
+ }
+}
+
/// Provides a way to pass arguments when creating BoData
/// as required by the gem::DriverObject trait.
pub(crate) struct BoCreateArgs {
@@ -56,24 +69,48 @@ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Se
/// Type alias for Tyr GEM buffer objects.
pub(crate) type Bo = gem::shmem::Object<BoData>;
+/// Returns whether a BO should be mapped write-combine.
+pub(crate) fn should_map_wc(coherent: bool, flags: u32) -> bool {
+ if coherent {
+ return false;
+ }
+
+ if flags & uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_WB_MMAP != 0 {
+ return false;
+ }
+
+ true
+}
+
/// Create a new GEM buffer object.
#[expect(dead_code)]
-pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result<ARef<Bo>> {
+pub(crate) fn new_object(
+ ddev: &TyrDrmDevice,
+ pdev: &Device<Bound>,
+ size: usize,
+ flags: u32,
+ coherent: bool,
+) -> Result<ARef<Bo>> {
if size == 0 {
return Err(EINVAL);
}
let aligned_size = size.checked_next_multiple_of(PAGE_SIZE).ok_or(EINVAL)?;
- Bo::new(
+ let map_wc = should_map_wc(coherent, flags);
+ let bo = Bo::new(
ddev,
aligned_size,
shmem::ObjectConfig {
- map_wc: true,
+ map_wc,
parent_resv_obj: None,
},
BoCreateArgs { flags },
- )
+ )?;
+
+ bo.sg_table(pdev)?;
+
+ Ok(bo)
}
/// Look up a GEM object by handle for a DRM file.
@@ -82,18 +119,17 @@ pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>>
}
/// Creates a dummy GEM object to serve as the root of a GPUVM.
-pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice) -> Result<ARef<Bo>> {
- let bo = Bo::new(
+pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice, coherent: bool) -> Result<ARef<Bo>> {
+ // FIXME: use a Rust resv-object abstraction once available, rather than a real BO.
+ Bo::new(
ddev,
4096,
shmem::ObjectConfig {
- map_wc: true,
+ map_wc: should_map_wc(coherent, 0),
parent_resv_obj: None,
},
BoCreateArgs { flags: 0 },
- )?;
-
- Ok(bo)
+ )
}
/// Specifies how to choose a GPU virtual address for a [`KernelBo`].
@@ -131,6 +167,7 @@ pub(crate) fn new(
size: u64,
va_alloc: KernelBoVaAlloc,
flags: VmMapFlags,
+ coherent: bool,
) -> Result<Self> {
if size == 0 {
dev_err!(vm.dev(), "Cannot create KernelBo with size 0");
@@ -146,7 +183,7 @@ pub(crate) fn new(
ddev,
bo_size,
shmem::ObjectConfig {
- map_wc: true,
+ map_wc: should_map_wc(coherent, 0),
parent_resv_obj: None,
},
BoCreateArgs { flags: 0 },
diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 11bc8e2c9d0be..6d312c3b2ccd1 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -473,13 +473,14 @@ pub(crate) fn new_for_fw(
ddev: &TyrDrmDevice,
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
+ coherent: bool,
) -> Result<Arc<Vm<'drm>>> {
// As in panthor: the CSF MCU is a Cortex-M7 and can only address 4G.
let layout = VmLayout {
full: 0..u64::SZ_4G,
user: 0..0u64,
};
- Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout, coherent)
}
/// Creates a user VM, splitting the GPU VA range per `user_va`.
@@ -489,6 +490,7 @@ pub(crate) fn new_for_user(
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
user_va: UserVaRequest,
+ coherent: bool,
) -> Result<Arc<Vm<'drm>>> {
let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
let va_bits = mmu_features.va_bits().get();
@@ -502,7 +504,7 @@ pub(crate) fn new_for_user(
range.end
);
})?;
- Self::new_internal(dev, ddev, mmu, gpu_info, layout)
+ Self::new_internal(dev, ddev, mmu, gpu_info, layout, coherent)
}
/// Initializes a VM with the given layout.
@@ -512,6 +514,7 @@ fn new_internal(
mmu: ArcBorrow<'_, Mmu<'drm>>,
gpu_info: &GpuInfo,
layout: VmLayout,
+ coherent: bool,
) -> Result<Arc<Vm<'drm>>> {
let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
let va_bits = mmu_features.va_bits().get();
@@ -520,7 +523,7 @@ fn new_internal(
let reserve_range = 0..0u64;
// dummy_obj is used to initialize the GPUVM tree.
- let dummy_obj = gem::new_dummy_object(ddev).inspect_err(|e| {
+ let dummy_obj = gem::new_dummy_object(ddev, coherent).inspect_err(|e| {
dev_err!(dev, "Failed to create dummy GEM object: {:?}", e);
})?;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 09/10] drm/tyr: support write-combine BO mappings
2026-11-09 0:36 ` Ke Sun
(?)
@ 2026-11-09 1:10 ` sashiko-bot
-1 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-11-09 1:10 UTC (permalink / raw)
To: Ke Sun; +Cc: dri-devel, ojeda, gary
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Critical] drm/tyr: invalid downcast in lookup_handle() for PRIME or
dumb GEM BOs
- [High] drm/tyr: cached BO mappings on coherent devices without GPU
coherency
- [Medium] drm/tyr: unconditional sg_table() call in new_object() when
map_wc is false
--
commit 6624593ccd32e8779801a0fe4fb33fa8b31a7440
Author: Sami Tolvanen <samitolvanen@google.com>
drm/tyr: support write-combine BO mappings
This commit caches device DMA coherency at probe time and threads it to
GEM shmem object creation to configure write-combine vs write-back CPU
mappings, while calling sg_table() at BO creation to flush cached zero-init
pages.
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index ba03cc4c52539..6869ab345d8fd 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
[ ... ]
> @@ -151,6 +158,8 @@ fn probe<'bound>(
> // other threads of execution.
> unsafe { pdev.dma_set_mask_and_coherent(DmaMask::try_new(pa_bits)?)? };
>
> + let coherent = pdev.as_ref().dma_attr() == DmaAttr::COHERENT;
> +
> let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, Ok(()))?;
>
> let mmu = Mmu::new(pdev.as_ref(), iomem.as_arc_borrow(), &gpu_info)?;
> @@ -161,6 +170,7 @@ fn probe<'bound>(
> &unreg_dev,
> mmu.as_arc_borrow(),
> &gpu_info,
> + coherent,
> )?;
[Severity: High]
Does setting coherent from dma_attr() break MCU firmware boot and BO
mappings on platforms whose Device Tree or ACPI tables mark the GPU
DMA-coherent?
When coherent is true, gem::should_map_wc() returns false, so firmware
KernelBo sections and user BOs are mapped write-back cached (PAGE_KERNEL) on
the CPU, and dma_map_sgtable() performs no cache maintenance because the
device is marked coherent. However, Tyr does not yet enable GPU or MMU
coherency: GpuInfo::new() hardcodes selected_coherency to
DRM_PANTHOR_GPU_COHERENCY_NONE, gpu::l2_power_on() never programs
COHERENCY_ENABLE, VmAsData::new() sets coherent_walk to false, and
MEMATTR::attribute_from_mair() always encodes Coherency::MidgardInnerDomain:
drivers/gpu/drm/tyr/regs.rs:MEMATTR::attribute_from_mair() {
...
// TODO: Add a `coherent` parameter like panthor's mair_to_memattr().
// For now, assume a non-coherent system and always encode write-back
// memory with MidgardInnerDomain coherency.
Self::try_new_memattr(
UseRecipe::AttributeMode,
Coherency::MidgardInnerDomain,
...
}
As a result, CPU writes to the cached vmap in Firmware::init_section_mem()
can remain in dirty CPU cache lines while the non-coherent MCU and GPU read
stale DRAM.
> diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
> index 60f6363995a5a..55e2d67b96d98 100644
> --- a/drivers/gpu/drm/tyr/gem.rs
> +++ b/drivers/gpu/drm/tyr/gem.rs
[ ... ]
> @@ -38,6 +43,14 @@ pub(crate) struct BoData {
> flags: u32,
> }
>
> +impl BoData {
> + /// Returns the flags the BO was created with.
> + #[expect(dead_code)]
> + pub(crate) fn flags(&self) -> u32 {
> + self.flags
> + }
> +}
[Severity: Critical]
Can looking up a GEM handle via gem::lookup_handle() and accessing Bo fields
such as BoData::flags() or Object::sg_table() read and write out of bounds on
PRIME-imported or dumb GEM objects?
In rust/kernel/drm/gem/shmem.rs, Object::ALLOC_OPS leaves gem_create_object
as None while wiring gem_prime_import_sg_table and dumb_create to the C
drm_gem_shmem helpers:
rust/kernel/drm/gem/shmem.rs:Object::ALLOC_OPS {
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,
};
}
When userspace creates a handle via DRM_IOCTL_PRIME_FD_TO_HANDLE or
DRM_IOCTL_MODE_CREATE_DUMB, __drm_gem_shmem_create() sees a NULL
gem_create_object callback and allocates only sizeof(struct
drm_gem_shmem_object) bytes without the trailing Rust gem::shmem::Object<T>
fields (parent_resv_obj, sgt_res, sgt_lock, and inner: BoData). Passing
that handle to DRM_IOCTL_PANTHOR_VM_BIND (or a subsequent BO ioctl) resolves
it through Bo::lookup_handle(), which uncheckedly casts the bare struct
drm_gem_shmem_object pointer to &gem::shmem::Object<BoData> and accesses
sgt_lock, sgt_res, or inner.flags past the end of the slab allocation.
> @@ -56,24 +69,48 @@ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Se
[ ... ]
> /// Create a new GEM buffer object.
> #[expect(dead_code)]
> -pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result<ARef<Bo>> {
> +pub(crate) fn new_object(
> + ddev: &TyrDrmDevice,
> + pdev: &Device<Bound>,
> + size: usize,
> + flags: u32,
> + coherent: bool,
> +) -> Result<ARef<Bo>> {
> if size == 0 {
> return Err(EINVAL);
> }
>
> let aligned_size = size.checked_next_multiple_of(PAGE_SIZE).ok_or(EINVAL)?;
>
> - Bo::new(
> + let map_wc = should_map_wc(coherent, flags);
> + let bo = Bo::new(
> ddev,
> aligned_size,
> shmem::ObjectConfig {
> - map_wc: true,
> + map_wc,
> parent_resv_obj: None,
> },
> BoCreateArgs { flags },
> - )
> + )?;
> +
> + bo.sg_table(pdev)?;
> +
> + Ok(bo)
> }
[Severity: Medium]
Should bo.sg_table(pdev) only be called when map_wc is true?
Object::sg_table() caches its result in a SetOnce (self.sgt_res). On a
non-coherent device when userspace sets DRM_PANTHOR_BO_WB_MMAP (map_wc is
false), calling bo.sg_table(pdev) at BO creation immediately pins the pages
and consumes the one-time dma_map_sgtable() cache flush before userspace
mmaps and writes to the cached buffer. When the BO is later bound via
DRM_IOCTL_PANTHOR_VM_BIND, GpuVmData::sm_step_map() -> bo.sg_table() hits the
already-populated SetOnce and skips dma_map_sgtable(), leaving CPU writes
made before VM_BIND unflushed in the CPU cache.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-tyr-ioctls-v6-0-b2896bf2b3c9@kylinos.cn?part=9
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 10/10] drm/tyr: add BO-related ioctls
2026-11-09 0:36 ` Ke Sun
@ 2026-11-09 0:36 ` Ke Sun
-1 siblings, 0 replies; 27+ messages in thread
From: Ke Sun via B4 Relay @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Implement BO_CREATE and BO_MMAP_OFFSET so userspace can allocate GPU
buffers and obtain a DRM mmap offset for the generic mmap path.
BO_CREATE page-aligns the requested size and accepts NO_MMAP or WB_MMAP,
which are mutually exclusive; an exclusive VM returns EOPNOTSUPP.
BO_MMAP_OFFSET rejects NO_MMAP objects, which must never be CPU-mapped.
Co-developed-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/driver.rs | 2 +
drivers/gpu/drm/tyr/file.rs | 94 +++++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/tyr/gem.rs | 2 -
3 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 6869ab345d8fd..8632e6e820c97 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -237,6 +237,8 @@ impl drm::Driver for TyrDrmDriver {
(PANTHOR_VM_DESTROY, drm_panthor_vm_destroy, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_destroy),
(PANTHOR_VM_BIND, drm_panthor_vm_bind, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_bind),
(PANTHOR_VM_GET_STATE, drm_panthor_vm_get_state, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_get_state),
+ (PANTHOR_BO_CREATE, drm_panthor_bo_create, ioctl::RENDER_ALLOW, TyrDrmFileData::bo_create),
+ (PANTHOR_BO_MMAP_OFFSET, drm_panthor_bo_mmap_offset, ioctl::RENDER_ALLOW, TyrDrmFileData::bo_mmap_offset),
}
}
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index b982e8db2c4c1..f99bb2992d87a 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -242,6 +242,100 @@ pub(crate) fn vm_get_state(
Ok(0)
})
}
+
+ pub(crate) fn bo_create(
+ ddev: &TyrDrmDevice<Registered>,
+ reg_data: &TyrDrmRegistrationData<'_>,
+ bocreate: &mut uapi::drm_panthor_bo_create,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if bocreate.size == 0 || bocreate.pad != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid BO_CREATE params: size={}, pad={}\n",
+ bocreate.size,
+ bocreate.pad
+ );
+ return Err(EINVAL);
+ }
+
+ let no_mmap = uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_NO_MMAP;
+ let wb_mmap = uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_WB_MMAP;
+ let valid_flags = no_mmap | wb_mmap;
+
+ if bocreate.flags & !valid_flags != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid BO_CREATE flags: {:#x}\n",
+ bocreate.flags
+ );
+ return Err(EINVAL);
+ }
+ if bocreate.flags & no_mmap != 0 && bocreate.flags & wb_mmap != 0 {
+ dev_dbg!(ddev.as_ref(), "BO_CREATE: NO_MMAP conflicts with WB_MMAP\n");
+ return Err(EINVAL);
+ }
+
+ if bocreate.exclusive_vm_id != 0 {
+ dev_dbg!(ddev.as_ref(), "BO_CREATE: exclusive_vm_id not supported\n");
+ return Err(EOPNOTSUPP);
+ }
+
+ let size = usize::try_from(bocreate.size).map_err(|_| {
+ dev_dbg!(
+ ddev.as_ref(),
+ "BO_CREATE size {:#x} too large\n",
+ bocreate.size
+ );
+ EINVAL
+ })?;
+ let bo = file.inner_with(|pfile| {
+ crate::gem::new_object(
+ ddev,
+ pfile.reg.pdev.as_ref(),
+ size,
+ bocreate.flags,
+ reg_data.coherent,
+ )
+ })?;
+ bocreate.handle = bo.create_handle(file)?;
+ bocreate.size = u64::from_safe_cast(bo.size());
+
+ Ok(0)
+ }
+
+ pub(crate) fn bo_mmap_offset(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ bommap: &mut uapi::drm_panthor_bo_mmap_offset,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if bommap.pad != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "BO mmap offset pad not zero: {}\n",
+ bommap.pad
+ );
+ return Err(EINVAL);
+ }
+
+ let bo = crate::gem::lookup_handle(file, bommap.handle).inspect_err(|_| {
+ dev_dbg!(ddev.as_ref(), "Invalid BO mmap handle: {}\n", bommap.handle);
+ })?;
+ if bo.flags() & uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_NO_MMAP != 0 {
+ dev_dbg!(ddev.as_ref(), "BO mmap offset on NO_MMAP object\n");
+ return Err(EPERM);
+ }
+ bommap.offset = bo.create_mmap_offset().inspect_err(|_| {
+ dev_err!(
+ ddev.as_ref(),
+ "Failed to create mmap offset for handle {}\n",
+ bommap.handle
+ );
+ })?;
+
+ Ok(0)
+ }
}
fn vm_bind_exec_op(vm: &Vm<'_>, file: &TyrDrmFile, op: &VmBindOp) -> Result {
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 55e2d67b96d98..d2ef121c8b1b6 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -45,7 +45,6 @@ pub(crate) struct BoData {
impl BoData {
/// Returns the flags the BO was created with.
- #[expect(dead_code)]
pub(crate) fn flags(&self) -> u32 {
self.flags
}
@@ -83,7 +82,6 @@ pub(crate) fn should_map_wc(coherent: bool, flags: u32) -> bool {
}
/// Create a new GEM buffer object.
-#[expect(dead_code)]
pub(crate) fn new_object(
ddev: &TyrDrmDevice,
pdev: &Device<Bound>,
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v6 10/10] drm/tyr: add BO-related ioctls
@ 2026-11-09 0:36 ` Ke Sun
0 siblings, 0 replies; 27+ messages in thread
From: Ke Sun @ 2026-11-09 0:36 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Lyude Paul, David Airlie,
Simona Vetter, Greg Kroah-Hartman, Rafael J. Wysocki,
Johan Hovold
Cc: Sami Tolvanen, rust-for-linux, dri-devel, driver-core, Ke Sun,
Alvin Sun
From: Alvin Sun <alvin.sun@linux.dev>
Implement BO_CREATE and BO_MMAP_OFFSET so userspace can allocate GPU
buffers and obtain a DRM mmap offset for the generic mmap path.
BO_CREATE page-aligns the requested size and accepts NO_MMAP or WB_MMAP,
which are mutually exclusive; an exclusive VM returns EOPNOTSUPP.
BO_MMAP_OFFSET rejects NO_MMAP objects, which must never be CPU-mapped.
Co-developed-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
---
drivers/gpu/drm/tyr/driver.rs | 2 +
drivers/gpu/drm/tyr/file.rs | 94 +++++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/tyr/gem.rs | 2 -
3 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 6869ab345d8fd..8632e6e820c97 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -237,6 +237,8 @@ impl drm::Driver for TyrDrmDriver {
(PANTHOR_VM_DESTROY, drm_panthor_vm_destroy, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_destroy),
(PANTHOR_VM_BIND, drm_panthor_vm_bind, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_bind),
(PANTHOR_VM_GET_STATE, drm_panthor_vm_get_state, ioctl::RENDER_ALLOW, TyrDrmFileData::vm_get_state),
+ (PANTHOR_BO_CREATE, drm_panthor_bo_create, ioctl::RENDER_ALLOW, TyrDrmFileData::bo_create),
+ (PANTHOR_BO_MMAP_OFFSET, drm_panthor_bo_mmap_offset, ioctl::RENDER_ALLOW, TyrDrmFileData::bo_mmap_offset),
}
}
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index b982e8db2c4c1..f99bb2992d87a 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -242,6 +242,100 @@ pub(crate) fn vm_get_state(
Ok(0)
})
}
+
+ pub(crate) fn bo_create(
+ ddev: &TyrDrmDevice<Registered>,
+ reg_data: &TyrDrmRegistrationData<'_>,
+ bocreate: &mut uapi::drm_panthor_bo_create,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if bocreate.size == 0 || bocreate.pad != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid BO_CREATE params: size={}, pad={}\n",
+ bocreate.size,
+ bocreate.pad
+ );
+ return Err(EINVAL);
+ }
+
+ let no_mmap = uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_NO_MMAP;
+ let wb_mmap = uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_WB_MMAP;
+ let valid_flags = no_mmap | wb_mmap;
+
+ if bocreate.flags & !valid_flags != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "Invalid BO_CREATE flags: {:#x}\n",
+ bocreate.flags
+ );
+ return Err(EINVAL);
+ }
+ if bocreate.flags & no_mmap != 0 && bocreate.flags & wb_mmap != 0 {
+ dev_dbg!(ddev.as_ref(), "BO_CREATE: NO_MMAP conflicts with WB_MMAP\n");
+ return Err(EINVAL);
+ }
+
+ if bocreate.exclusive_vm_id != 0 {
+ dev_dbg!(ddev.as_ref(), "BO_CREATE: exclusive_vm_id not supported\n");
+ return Err(EOPNOTSUPP);
+ }
+
+ let size = usize::try_from(bocreate.size).map_err(|_| {
+ dev_dbg!(
+ ddev.as_ref(),
+ "BO_CREATE size {:#x} too large\n",
+ bocreate.size
+ );
+ EINVAL
+ })?;
+ let bo = file.inner_with(|pfile| {
+ crate::gem::new_object(
+ ddev,
+ pfile.reg.pdev.as_ref(),
+ size,
+ bocreate.flags,
+ reg_data.coherent,
+ )
+ })?;
+ bocreate.handle = bo.create_handle(file)?;
+ bocreate.size = u64::from_safe_cast(bo.size());
+
+ Ok(0)
+ }
+
+ pub(crate) fn bo_mmap_offset(
+ ddev: &TyrDrmDevice<Registered>,
+ _reg_data: &TyrDrmRegistrationData<'_>,
+ bommap: &mut uapi::drm_panthor_bo_mmap_offset,
+ file: &TyrDrmFile,
+ ) -> Result<u32> {
+ if bommap.pad != 0 {
+ dev_dbg!(
+ ddev.as_ref(),
+ "BO mmap offset pad not zero: {}\n",
+ bommap.pad
+ );
+ return Err(EINVAL);
+ }
+
+ let bo = crate::gem::lookup_handle(file, bommap.handle).inspect_err(|_| {
+ dev_dbg!(ddev.as_ref(), "Invalid BO mmap handle: {}\n", bommap.handle);
+ })?;
+ if bo.flags() & uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_NO_MMAP != 0 {
+ dev_dbg!(ddev.as_ref(), "BO mmap offset on NO_MMAP object\n");
+ return Err(EPERM);
+ }
+ bommap.offset = bo.create_mmap_offset().inspect_err(|_| {
+ dev_err!(
+ ddev.as_ref(),
+ "Failed to create mmap offset for handle {}\n",
+ bommap.handle
+ );
+ })?;
+
+ Ok(0)
+ }
}
fn vm_bind_exec_op(vm: &Vm<'_>, file: &TyrDrmFile, op: &VmBindOp) -> Result {
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 55e2d67b96d98..d2ef121c8b1b6 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -45,7 +45,6 @@ pub(crate) struct BoData {
impl BoData {
/// Returns the flags the BO was created with.
- #[expect(dead_code)]
pub(crate) fn flags(&self) -> u32 {
self.flags
}
@@ -83,7 +82,6 @@ pub(crate) fn should_map_wc(coherent: bool, flags: u32) -> bool {
}
/// Create a new GEM buffer object.
-#[expect(dead_code)]
pub(crate) fn new_object(
ddev: &TyrDrmDevice,
pdev: &Device<Bound>,
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6 10/10] drm/tyr: add BO-related ioctls
2026-11-09 0:36 ` Ke Sun
(?)
@ 2026-11-09 1:05 ` sashiko-bot
-1 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-11-09 1:05 UTC (permalink / raw)
To: Ke Sun; +Cc: dri-devel, ojeda, gary
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] drm/tyr: slab-out-of-bounds access in bo_mmap_offset() on
imported or dumb BOs
- [Medium] drm/tyr: NO_MMAP buffer objects remain mappable via mmap()
and PRIME
--
commit 31c4893f869db54a6627810d48273b6077efa3af
Author: Alvin Sun <alvin.sun@linux.dev>
drm/tyr: add BO-related ioctls
This commit implements the PANTHOR_BO_CREATE and PANTHOR_BO_MMAP_OFFSET
ioctls for allocating GEM buffer objects and retrieving their mmap offsets.
> diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
> index b982e8db2c4c1..f99bb2992d87a 100644
> --- a/drivers/gpu/drm/tyr/file.rs
> +++ b/drivers/gpu/drm/tyr/file.rs
[ ... ]
> + pub(crate) fn bo_mmap_offset(
> + ddev: &TyrDrmDevice<Registered>,
> + _reg_data: &TyrDrmRegistrationData<'_>,
> + bommap: &mut uapi::drm_panthor_bo_mmap_offset,
> + file: &TyrDrmFile,
> + ) -> Result<u32> {
> + if bommap.pad != 0 {
> + dev_dbg!(
> + ddev.as_ref(),
> + "BO mmap offset pad not zero: {}\n",
> + bommap.pad
> + );
> + return Err(EINVAL);
> + }
> +
> + let bo = crate::gem::lookup_handle(file, bommap.handle).inspect_err(|_| {
> + dev_dbg!(ddev.as_ref(), "Invalid BO mmap handle: {}\n", bommap.handle);
> + })?;
[Severity: High]
Can passing a PRIME-imported or dumb-buffer handle to bo_mmap_offset() read
past the end of the GEM object allocation when bo.flags() is called below?
rust/kernel/drm/gem/shmem.rs:Object::ALLOC_OPS {
...
gem_create_object: None,
...
gem_prime_import_sg_table: Some(bindings::drm_gem_shmem_prime_import_sg_table),
dumb_create: Some(bindings::drm_gem_shmem_dumb_create),
...
}
Because gem_create_object is None while gem_prime_import_sg_table and
dumb_create are wired up, DRM_IOCTL_PRIME_FD_TO_HANDLE (or
DRM_IOCTL_MODE_CREATE_DUMB) calls __drm_gem_shmem_create() and allocates a
bare struct drm_gem_shmem_object rather than a gem::shmem::Object<BoData>.
lookup_handle() casts that pointer to gem::shmem::Object<BoData>, so calling
bo.flags() reads self.inner.flags out of bounds in the slab allocation.
> + if bo.flags() & uapi::drm_panthor_bo_flags_DRM_PANTHOR_BO_NO_MMAP != 0 {
> + dev_dbg!(ddev.as_ref(), "BO mmap offset on NO_MMAP object\n");
> + return Err(EPERM);
> + }
[Severity: Medium]
Can userspace still CPU-map a DRM_PANTHOR_BO_NO_MMAP buffer object despite
this check?
In bo_create(), new_object() calls __drm_gem_shmem_init(), which allocates
the object's VMA offset via drm_gem_create_mmap_offset(), and
bo.create_handle() authorizes that offset for the file via
drm_vma_node_allow(). Because gem::shmem::Object<BoData>::VTABLE sets mmap
directly to drm_gem_shmem_object_mmap() without inspecting BoData::flags,
userspace can export a DRM_PANTHOR_BO_NO_MMAP handle with
DRM_IOCTL_PRIME_HANDLE_TO_FD and mmap() the returned dma-buf fd (or mmap()
the DRM fd directly using the pre-allocated VMA offset).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-tyr-ioctls-v6-0-b2896bf2b3c9@kylinos.cn?part=10
^ permalink raw reply [flat|nested] 27+ messages in thread