| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <9C5177DB-CEBA-4DD5-8E93-DB39CB1F2079@collabora.com>
Date: Thu, 22 Jan 2026 19:58:34 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Danilo Krummrich <dakr@...nel.org>,
Boris Brezillon <boris.brezillon@...labora.com>,
Janne Grunau <j@...nau.net>,
Matthew Brost <matthew.brost@...el.com>,
Thomas Hellström <thomas.hellstrom@...ux.intel.com>,
Lyude Paul <lyude@...hat.com>,
Asahi Lina <lina+kernel@...hilina.net>,
dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map()
Hi Alice,
> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> Finally also add the operation for creating new mappings. Mapping
> operations need extra data in the context since they involve a vm_bo
> coming from the outside.
>
> Co-developed-by: Asahi Lina <lina+kernel@...hilina.net>
> Signed-off-by: Asahi Lina <lina+kernel@...hilina.net>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
> rust/kernel/drm/gpuvm/mod.rs | 9 ++-
> rust/kernel/drm/gpuvm/sm_ops.rs | 154 ++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 157 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> index 165a25666ccc3d62e59b73483d4eedff044423e9..557c0d629eec912a97fc4ef18495d5bf0807db0a 100644
> --- a/rust/kernel/drm/gpuvm/mod.rs
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -93,7 +93,7 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
> vm_bo_alloc: GpuVmBo::<T>::ALLOC_FN,
> vm_bo_free: GpuVmBo::<T>::FREE_FN,
> vm_bo_validate: None,
> - sm_step_map: None,
> + sm_step_map: Some(Self::sm_step_map),
> sm_step_unmap: Some(Self::sm_step_unmap),
> sm_step_remap: Some(Self::sm_step_remap),
> }
> @@ -248,6 +248,13 @@ pub trait DriverGpuVm: Sized {
> /// The private data passed to callbacks.
> type SmContext<'ctx>;
>
> + /// Indicates that a new mapping should be created.
> + fn sm_step_map<'op, 'ctx>(
> + &mut self,
> + op: OpMap<'op, Self>,
> + context: &mut Self::SmContext<'ctx>,
> + ) -> Result<OpMapped<'op, Self>, Error>;
> +
> /// Indicates that an existing mapping should be removed.
> fn sm_step_unmap<'op, 'ctx>(
> &mut self,
> diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs
> index 3c29d10d63f0b0a1976c714a86d486948ba81a15..5f3c5d3918147a6962e5658443c343835baa10b8 100644
> --- a/rust/kernel/drm/gpuvm/sm_ops.rs
> +++ b/rust/kernel/drm/gpuvm/sm_ops.rs
> @@ -8,6 +8,100 @@ struct SmData<'a, 'ctx, T: DriverGpuVm> {
> user_context: &'a mut T::SmContext<'ctx>,
> }
>
> +#[repr(C)]
> +struct SmMapData<'a, 'ctx, T: DriverGpuVm> {
> + sm_data: SmData<'a, 'ctx, T>,
> + vm_bo: GpuVmBoResident<T>,
> +}
> +
> +/// The argument for [`GpuVmCore::sm_map`].
> +pub struct OpMapRequest<'a, 'ctx, T: DriverGpuVm> {
> + /// Address in GPU virtual address space.
> + pub addr: u64,
> + /// Length of mapping to create.
> + pub range: u64,
> + /// Offset in GEM object.
> + pub offset: u64,
I’d rename this gem_offset. A bit vague/confusing otherwise.
> + /// The GEM object to map.
> + pub vm_bo: GpuVmBoResident<T>,
> + /// The user-provided context type.
> + pub context: &'a mut T::SmContext<'ctx>,
> +}
> +
> +impl<'a, 'ctx, T: DriverGpuVm> OpMapRequest<'a, 'ctx, T> {
> + fn raw_request(&self) -> bindings::drm_gpuvm_map_req {
> + bindings::drm_gpuvm_map_req {
> + map: bindings::drm_gpuva_op_map {
> + va: bindings::drm_gpuva_op_map__bindgen_ty_1 {
> + addr: self.addr,
> + range: self.range,
> + },
> + gem: bindings::drm_gpuva_op_map__bindgen_ty_2 {
> + offset: self.offset,
> + obj: self.vm_bo.obj().as_raw(),
> + },
> + },
> + }
> + }
> +}
> +
> +/// Represents an `sm_step_map` operation that has not yet been completed.
> +pub struct OpMap<'op, T: DriverGpuVm> {
> + op: &'op bindings::drm_gpuva_op_map,
> + // Since these abstractions are designed for immediate mode, the VM BO needs to be
> + // pre-allocated, so we always have it available when we reach this point.
> + vm_bo: &'op GpuVmBo<T>,
> + _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> +impl<'op, T: DriverGpuVm> OpMap<'op, T> {
> + /// The base address of the new mapping.
> + pub fn addr(&self) -> u64 {
> + self.op.va.addr
> + }
> +
> + /// The length of the new mapping.
> + pub fn length(&self) -> u64 {
> + self.op.va.range
> + }
> +
> + /// The offset within the [`drm_gem_object`](crate::gem::Object).
> + pub fn gem_offset(&self) -> u64 {
> + self.op.gem.offset
> + }
> +
> + /// The [`drm_gem_object`](crate::gem::Object) to map.
> + pub fn obj(&self) -> &T::Object {
> + // SAFETY: The `obj` pointer is guaranteed to be valid.
> + unsafe { <T::Object as IntoGEMObject>::from_raw(self.op.gem.obj) }
> + }
> +
> + /// The [`GpuVmBo`] that the new VA will be associated with.
> + pub fn vm_bo(&self) -> &GpuVmBo<T> {
> + self.vm_bo
> + }
> +
> + /// Use the pre-allocated VA to carry out this map operation.
> + pub fn insert(self, va: GpuVaAlloc<T>, va_data: impl PinInit<T::VaData>) -> OpMapped<'op, T> {
> + let va = va.prepare(va_data);
> + // SAFETY: By the type invariants we may access the interval tree.
> + unsafe { bindings::drm_gpuva_map(self.vm_bo.gpuvm().as_raw(), va, self.op) };
> +
> + let _gpuva_guard = self.vm_bo().lock_gpuva();
> + // SAFETY: The va is prepared for insertion, and we hold the GEM lock.
> + unsafe { bindings::drm_gpuva_link(va, self.vm_bo.as_raw()) };
> +
> + OpMapped {
> + _invariant: self._invariant,
> + }
> + }
> +}
> +
> +/// Represents a completed [`OpMap`] operation.
> +pub struct OpMapped<'op, T> {
> + _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> /// Represents an `sm_step_unmap` operation that has not yet been completed.
> pub struct OpUnmap<'op, T: DriverGpuVm> {
> op: &'op bindings::drm_gpuva_op_unmap,
> @@ -205,6 +299,30 @@ pub struct OpRemapped<'op, T> {
> }
>
> impl<T: DriverGpuVm> GpuVmCore<T> {
> + /// Create a mapping, removing or remapping anything that overlaps.
> + ///
> + /// Internally calls the [`DriverGpuVm`] callbacks similar to [`Self::sm_unmap`], except that
> + /// the [`DriverGpuVm::sm_step_map`] is called once to create the requested mapping.
> + #[inline]
> + pub fn sm_map(&mut self, req: OpMapRequest<'_, '_, T>) -> Result {
> + let gpuvm = self.as_raw();
> + let raw_req = req.raw_request();
> + let mut p = SmMapData {
> + sm_data: SmData {
> + gpuvm: self,
> + user_context: req.context,
> + },
> + vm_bo: req.vm_bo,
> + };
> + // SAFETY:
> + // * raw_request() creates a valid request.
> + // * The private data is valid to be interpreted as both SmData and SmMapData since the
> + // first field of SmMapData is SmData.
> + to_result(unsafe {
> + bindings::drm_gpuvm_sm_map(gpuvm, (&raw mut p).cast(), &raw const raw_req)
> + })
> + }
> +
> /// Remove any mappings in the given region.
> ///
> /// Internally calls [`DriverGpuVm::sm_step_unmap`] for ranges entirely contained within the
> @@ -218,19 +336,45 @@ pub fn sm_unmap(&mut self, addr: u64, length: u64, context: &mut T::SmContext<'_
> };
> // SAFETY:
> // * raw_request() creates a valid request.
> - // * The private data is valid to be interpreted as SmData.
> + // * The private data is a valid SmData.
> to_result(unsafe { bindings::drm_gpuvm_sm_unmap(gpuvm, (&raw mut p).cast(), addr, length) })
> }
> }
>
> impl<T: DriverGpuVm> GpuVm<T> {
> /// # Safety
> - /// Must be called from `sm_unmap` with a pointer to `SmData`.
> + /// Must be called from `sm_map` with a pointer to `SmMapData`.
> + pub(super) unsafe extern "C" fn sm_step_map(
> + op: *mut bindings::drm_gpuva_op,
> + p: *mut c_void,
> + ) -> c_int {
> + // SAFETY: If we reach `sm_step_map` then we were called from `sm_map` which always passes
> + // an `SmMapData` as private data.
> + let p = unsafe { &mut *p.cast::<SmMapData<'_, '_, T>>() };
> + let op = OpMap {
> + // SAFETY: sm_step_map is called with a map operation.
> + op: unsafe { &(*op).__bindgen_anon_1.map },
> + vm_bo: &p.vm_bo,
> + _invariant: PhantomData,
> + };
> + match p
> + .sm_data
> + .gpuvm
> + .data()
> + .sm_step_map(op, p.sm_data.user_context)
> + {
> + Ok(OpMapped { .. }) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + /// # Safety
> + /// Must be called from `sm_map` or `sm_unmap` with a pointer to `SmMapData` or `SmData`.
> pub(super) unsafe extern "C" fn sm_step_unmap(
> op: *mut bindings::drm_gpuva_op,
> p: *mut c_void,
> ) -> c_int {
> - // SAFETY: The caller provides a pointer to `SmData`.
> + // SAFETY: The caller provides a pointer that can be treated as `SmData`.
> let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
> let op = OpUnmap {
> // SAFETY: sm_step_unmap is called with an unmap operation.
> @@ -244,12 +388,12 @@ impl<T: DriverGpuVm> GpuVm<T> {
> }
>
> /// # Safety
> - /// Must be called from `sm_unmap` with a pointer to `SmData`.
> + /// Must be called from `sm_map` or `sm_unmap` with a pointer to `SmMapData` or `SmData`.
> pub(super) unsafe extern "C" fn sm_step_remap(
> op: *mut bindings::drm_gpuva_op,
> p: *mut c_void,
> ) -> c_int {
> - // SAFETY: The caller provides a pointer to `SmData`.
> + // SAFETY: The caller provides a pointer that can be treated as `SmData`.
> let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
> let op = OpRemap {
> // SAFETY: sm_step_remap is called with a remap operation.
>
> --
> 2.52.0.457.g6b5491de43-goog
>
Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
Powered by blists - more mailing lists