From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1794699814; b=VSzaOxuELunZIlUrnCfAPpPTZrfDQTQ4rXXDx8UAK+yz2dfP/vvejWIH9Y4cnbhCJNl4 U08C3/OUZ/2ylvFtwwHUjgk0XY10li3HeleRPfQ5tBW/Nt+px7Ls7J3Gk8wzTfH9vMaRX +1UnHFfamtYLPQK8DVBeRFWKK+kGBA9CQClcJxQgfQ+BVANc0/G+pP9gD3nejyANQAjxm mLdN3t7Rp7Ey3zuuBjiG2/AshGBcKFyn5NxKU5FCaVNzW5ZEJR9z4izUGmCAPXxJQe5ZI 2SkkAxUZuwzWpA2Db42vihr9Xmg7fl1qr4D3LuQiOhj8tWE4jrXYI09P22x1GDCCuRw== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1794699814; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-ID:X-Mailer: MIME-Version:Content-Transfer-Encoding; bh=MxgAUxz8lITz4H+mvvbKLXgHIWwjdp7/ARMhyf/aP2k=; b=5WRV+b3d3iEGvjbbVi4PDjhRcM9Mw1CYDEZ4KanG0QN6sdoHivVow+9ILMUb9ZzcPB/T hVZk56Gto5h9TvbPdSIJPZEQEzZVM6thPiuEScIcKjOr8J1HAQdDc9/OE4PVnjWUYQmlc 5fgwTNl+DXPKNJJZIvns7FbEXAgB1ub/NObdxe/g4g7MyVDOSGWx7nsn5mZIacqORw8pe uiAUMAvtOc+sNeJDgu7EIb1ajvyrBdEUS8wEY+CICuU3T/Acln/CcaMiGcbEqw0WckLKI LKodxphBXuliNxcdi53yB9YTCk7bjSYe8+IfJSlqW2WlwDq0aypmvfruAbW2CR871kg== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=S0cNU0XM; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 59DAF606A33B; Sat, 14 Nov 2026 23:43:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=S0cNU0XM Received: by smtp.kernel.org (Postfix) with ESMTPSA id EA3671F000C6; Sat, 14 Nov 2026 23:43:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1794699814; bh=MxgAUxz8lITz4H+mvvbKLXgHIWwjdp7/ARMhyf/aP2k=; h=From:To:Cc:Subject:Date; b=S0cNU0XMTNWkFYuMguUbH6TECNztbe39I8DWNsRF75NLOPbbknrJT2eKguKaTNcHp ibLbh5zBRE76xkPqKclnz5p8HDDskfYQvuk8r/YylSnF3fN4Xm5/BKewx50ETD1hB9 B+905+TQ+qWrO5vGxQDziFEuNuTUk6sYQIXJnKYFWgRds8aZi1XI0W5hqLFrmy8TgI slHcAePZyMxPNOPgOEeRMN+yqHk/gRdCP525aRtXqvzvtI3Ehs7JlATzwVOLC9SYbR LS9RgWLRplCLmI791pfaqyQlOS+6lnAu/hM+PPBWrqoXUJ5xl3QGfIrAL4ZfUWKB00 bnL7QqU9kpMCA== From: Danilo Krummrich To: dakr@kernel.org, acourbot@nvidia.com, aliceryhl@google.com, abdiel.janulgue@gmail.com, daniel.almeida@collabora.com, robin.murphy@arm.com, a.hindborg@kernel.org, ojeda@kernel.org, boqun@kernel.org, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, tmgross@umich.edu, tamird@kernel.org, work@onurozkan.dev Cc: driver-core@lists.linux.dev, nova-gpu@lists.linux.dev, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org Subject: [PATCH 1/2] rust: scatterlist: tie owned tables to the device's bound lifetime Date: Sun, 15 Nov 2026 00:42:53 +0100 Message-ID: <20261114234324.82387-1-dakr@kernel.org> X-Mailer: git-send-email 2.56.1 X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Add a lifetime parameter to Owned that ties owned scatter-gather tables to the device's bound scope, ensuring their DMA mappings are released before the device is unbound. The DMA mapping currently holds a pointer into Owned, requiring the table to stay at a fixed address and its cleanup to be ordered around the device-managed mapping. Let DmaMappedSgt own RawSGTable and borrow Device. Its destructor unmaps the entries before RawSGTable frees them, and Owned releases the backing pages last. This removes the internal pointer and makes the mapping constructor safe without a self-reference. Carry the device lifetime through Owned and its users. Remove pin initialization from Owned and SGTable, return tables by value, and update the callers accordingly. Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/firmware/radix3.rs | 29 +++-- rust/kernel/scatterlist.rs | 141 ++++++++--------------- samples/rust/rust_dma.rs | 11 +- 3 files changed, 62 insertions(+), 119 deletions(-) diff --git a/drivers/gpu/nova-core/firmware/radix3.rs b/drivers/gpu/nova-core/firmware/radix3.rs index 94235bd1209d..a3e43fc8ec86 100644 --- a/drivers/gpu/nova-core/firmware/radix3.rs +++ b/drivers/gpu/nova-core/firmware/radix3.rs @@ -35,14 +35,11 @@ #[pin_data] pub(crate) struct Radix3<'a> { /// The mapped data. - #[pin] - data: SGTable>>, + data: SGTable>>, /// Level 2: one entry per page of `data`. - #[pin] - level2: SGTable>>, + level2: SGTable>>, /// Level 1: one entry per page of `level2`. - #[pin] - level1: SGTable>>, + level1: SGTable>>, /// Level 0: one page, whose single entry is the DMA address of the first `level1` page. level0: Coherent<'a, [u64]>, /// Length of `data`, in bytes. @@ -59,14 +56,14 @@ pub(crate) fn new( pin_init::pin_init_scope(move || { Ok(try_pin_init!(Self { - data <- SGTable::new(dev, data, DataDirection::ToDevice, GFP_KERNEL), - level2 <- { - build_lvl(&data) - .map(|l2| SGTable::new(dev, l2, DataDirection::ToDevice, GFP_KERNEL))? + data: SGTable::new(dev, data, DataDirection::ToDevice, GFP_KERNEL)?, + level2: { + let l2 = build_lvl(data)?; + SGTable::new(dev, l2, DataDirection::ToDevice, GFP_KERNEL)? }, - level1 <- { - build_lvl(&level2) - .map(|l1| SGTable::new(dev, l1, DataDirection::ToDevice, GFP_KERNEL))? + level1: { + let l1 = build_lvl(level2)?; + SGTable::new(dev, l1, DataDirection::ToDevice, GFP_KERNEL)? }, level0: { // Allocate the level 0 page table as a device-visible DMA object, and map the @@ -81,7 +78,7 @@ pub(crate) fn new( let mut level0 = CoherentBox::<'_, [u64]>::zeroed_slice( dev, GSP_PAGE_SIZE / size_of::(), - GFP_KERNEL + GFP_KERNEL, )?; level0[0] = level1_entry_addr.to_le(); @@ -105,7 +102,7 @@ pub(crate) fn size(&self) -> usize { /// Returns the size of the level that maps `sg_table`: one `u64` entry per [`GSP_PAGE_SIZE`] /// page of each DMA-mapped region, rounded up to whole pages. -fn lvl_size(sg_table: &SGTable>>) -> usize { +fn lvl_size(sg_table: &SGTable>>) -> usize { let entries: usize = sg_table .iter() .map(|sg_entry| usize::from_safe_cast(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE)) @@ -122,7 +119,7 @@ fn lvl_size(sg_table: &SGTable>>) -> usize { /// /// - `ENOMEM` if the level cannot be allocated. /// - `EINVAL` if `sg_table` yields more entries than [`lvl_size`] sized the level for. -fn build_lvl(sg_table: &SGTable>>) -> Result> { +fn build_lvl(sg_table: &SGTable>>) -> Result> { let mut dst = VVec::::zeroed(lvl_size(sg_table), GFP_KERNEL).map_err(|_| ENOMEM)?; let mut entries = dst.chunks_exact_mut(size_of::()); diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs index 6379a8748117..93f431d7f548 100644 --- a/rust/kernel/scatterlist.rs +++ b/rust/kernel/scatterlist.rs @@ -19,7 +19,8 @@ //! The main entry point is the [`SGTable`] type, which represents a complete scatter-gather table. //! It can be either: //! -//! - An owned table ([`SGTable>`]), created from a Rust memory buffer (e.g., [`VVec`]). +//! - An owned table ([`SGTable>`]), created from a Rust memory buffer (e.g., +//! [`VVec`]) and tied to the [`Device`] lifetime. //! This type manages the allocation of the `struct sg_table`, the DMA mapping of the buffer, and //! the automatic cleanup of all resources. //! - A borrowed reference (&[`SGTable`]), which provides safe, read-only access to a table that was @@ -33,15 +34,13 @@ alloc::allocator::VmallocPageIter, bindings, device::{Bound, Device}, - devres::Devres, dma, error, page, prelude::*, - sync::aref::ARef, types::Opaque, // }; -use core::{ops::Deref, ptr::NonNull}; +use core::ops::Deref; /// A single entry in a scatter-gather list. /// @@ -123,9 +122,7 @@ unsafe impl Sync for Borrowed {} /// /// All [`SGTable`] variants can be iterated over the individual [`SGEntry`]s. #[repr(transparent)] -#[pin_data] pub struct SGTable { - #[pin] inner: T, } @@ -177,63 +174,41 @@ pub fn iter(&self) -> SGTableIter<'_> { /// /// # Invariants /// -/// - `sgt` is a valid pointer to a `struct sg_table` for the entire lifetime of the -/// [`DmaMappedSgt`]. /// - `sgt` is always DMA mapped. -struct DmaMappedSgt { - sgt: NonNull, - dev: ARef, +struct DmaMappedSgt<'a> { + dev: &'a Device, + sgt: RawSGTable, dir: dma::DataDirection, } -// SAFETY: `DmaMappedSgt` can be sent to any task. -unsafe impl Send for DmaMappedSgt {} - -// SAFETY: `DmaMappedSgt` has no interior mutability and can be accessed concurrently. -unsafe impl Sync for DmaMappedSgt {} - -impl DmaMappedSgt { - /// # Safety - /// - /// - `sgt` must be a valid pointer to a `struct sg_table` for the entire lifetime of the - /// returned [`DmaMappedSgt`]. - /// - The caller must guarantee that `sgt` remains DMA mapped for the entire lifetime of - /// [`DmaMappedSgt`]. - unsafe fn new( - sgt: NonNull, - dev: &Device, - dir: dma::DataDirection, - ) -> Result { +impl<'a> DmaMappedSgt<'a> { + /// Takes ownership of `sgt` and maps it for DMA. + fn new(dev: &'a Device, sgt: RawSGTable, dir: dma::DataDirection) -> Result { // SAFETY: // - `dev.as_raw()` is a valid pointer to a `struct device`, which is guaranteed to be // bound to a driver for the duration of this call. - // - `sgt` is a valid pointer to a `struct sg_table`. + // - `sgt.as_raw()` is a valid pointer to a `struct sg_table`. error::to_result(unsafe { - bindings::dma_map_sgtable(dev.as_raw(), sgt.as_ptr(), dir.into(), 0) + bindings::dma_map_sgtable(dev.as_raw(), sgt.as_raw(), dir.into(), 0) })?; - // INVARIANT: By the safety requirements of this function it is guaranteed that `sgt` is - // valid for the entire lifetime of this object instance. - Ok(Self { - sgt, - dev: dev.into(), - dir, - }) + // INVARIANT: `sgt` was successfully DMA mapped and is owned by this object until drop. + Ok(Self { dev, sgt, dir }) } } -impl Drop for DmaMappedSgt { +impl Drop for DmaMappedSgt<'_> { #[inline] fn drop(&mut self) { // SAFETY: // - `self.dev.as_raw()` is a pointer to a valid `struct device`. // - `self.dev` is the same device the mapping has been created for in `Self::new()`. - // - `self.sgt.as_ptr()` is a valid pointer to a `struct sg_table` by the type invariants - // of `Self`. + // - `self.sgt.as_raw()` is a valid pointer to a `struct sg_table` for the lifetime of + // `self`. // - `self.dir` is the same `dma::DataDirection` the mapping has been created with in // `Self::new()`. unsafe { - bindings::dma_unmap_sgtable(self.dev.as_raw(), self.sgt.as_ptr(), self.dir.into(), 0) + bindings::dma_unmap_sgtable(self.dev.as_raw(), self.sgt.as_raw(), self.dir.into(), 0) }; } } @@ -308,38 +283,27 @@ fn drop(&mut self) { /// /// - The backing memory pages. /// - The `struct sg_table` allocation (`sgt`). -/// - The DMA mapping, managed through a [`Devres`]-managed `DmaMappedSgt`. +/// - The DMA mapping, released before the table and backing pages are freed. /// /// Users interact with this type through the [`SGTable`] handle and do not need to manage /// [`Owned`] directly. -#[pin_data] -pub struct Owned

{ +pub struct Owned<'a, P> { // Note: The drop order is relevant; we first have to unmap the `struct sg_table`, then free the // `struct sg_table` and finally free the backing pages. - #[pin] - dma: Devres, - #[pin] - sgt: RawSGTable, + dma: DmaMappedSgt<'a>, _pages: P, } -// SAFETY: `Owned` can be sent to any task if `P` can be send to any task. -unsafe impl Send for Owned

{} - -// SAFETY: `Owned` has no interior mutability and can be accessed concurrently if `P` can be -// accessed concurrently. -unsafe impl Sync for Owned

{} - -impl

Owned

+impl<'a, P> Owned<'a, P> where - for<'a> P: page::AsPageIter = VmallocPageIter<'a>> + 'static, + for<'b> P: page::AsPageIter = VmallocPageIter<'b>> + 'static, { fn new( - dev: &Device, + dev: &'a Device, mut pages: P, dir: dma::DataDirection, flags: alloc::Flags, - ) -> Result + '_> { + ) -> Result { let page_iter = pages.page_iter(); let size = page_iter.size(); @@ -367,34 +331,22 @@ fn new( let max_segment = max_mapping_size.min(max_seg_size); - Ok(try_pin_init!(&this in Self { - // SAFETY: - // - `page_vec` is a `KVec` of valid `struct page *` obtained from `pages`. - // - The pages contained in `pages` remain valid for the entire lifetime of the - // `RawSGTable`. - sgt: unsafe { RawSGTable::new(&mut page_vec, size, max_segment, flags) }?, - dma <- { - // SAFETY: `this` is a valid pointer to uninitialized memory. - let sgt = unsafe { &raw mut (*this.as_ptr()).sgt }.cast(); - - // SAFETY: `sgt` is guaranteed to be non-null. - let sgt = unsafe { NonNull::new_unchecked(sgt) }; - - // SAFETY: - // - It is guaranteed that the object returned by `DmaMappedSgt::new` won't out-live - // `sgt`. - // - `sgt` is never DMA unmapped manually. - // - `sgt` is structurally pinned and therefore cannot move. - Devres::new(dev, unsafe { DmaMappedSgt::new(sgt, dev, dir) }) - }, + // SAFETY: + // - `page_vec` is a `KVec` of valid `struct page *` obtained from `pages`. + // - The pages contained in `pages` remain valid for the entire lifetime of the + // `RawSGTable`. + let sgt = unsafe { RawSGTable::new(&mut page_vec, size, max_segment, flags) }?; + + Ok(Self { + dma: DmaMappedSgt::new(dev, sgt, dir)?, _pages: pages, - })) + }) } } -impl

SGTable> +impl<'a, P> SGTable> where - for<'a> P: page::AsPageIter = VmallocPageIter<'a>> + 'static, + for<'b> P: page::AsPageIter = VmallocPageIter<'b>> + 'static, { /// Allocates a new scatter-gather table from the given pages and maps it for DMA. /// @@ -403,8 +355,7 @@ impl

SGTable> /// pages of `P`, and maps the table for DMA with the specified [`Device`] and /// [`dma::DataDirection`]. /// - /// The DMA mapping is managed through [`Devres`], ensuring that the DMA mapping is unmapped - /// once the associated [`Device`] is unbound, or when the [`SGTable`] is dropped. + /// The table is tied to the [`Device`] lifetime and unmaps the DMA mapping when dropped. /// /// # Parameters /// @@ -428,38 +379,38 @@ impl

SGTable> /// let size = 4 * page::PAGE_SIZE; /// let pages = VVec::::with_capacity(size, GFP_KERNEL)?; /// - /// let sgt = KBox::pin_init(SGTable::new( + /// let sgt = SGTable::new( /// dev, /// pages, /// dma::DataDirection::ToDevice, /// GFP_KERNEL, - /// ), GFP_KERNEL)?; + /// )?; /// /// Ok(()) /// } /// ``` pub fn new( - dev: &Device, + dev: &'a Device, pages: P, dir: dma::DataDirection, flags: alloc::Flags, - ) -> impl PinInit + '_ { - try_pin_init!(Self { - inner <- Owned::new(dev, pages, dir, flags)? + ) -> Result { + Ok(Self { + inner: Owned::new(dev, pages, dir, flags)?, }) } } -impl

Deref for SGTable> { +impl

Deref for SGTable> { type Target = SGTable; #[inline] fn deref(&self) -> &Self::Target { // SAFETY: - // - `self.inner.sgt.as_raw()` is a valid pointer to a `struct sg_table` for the entire + // - `self.inner.dma.sgt.as_raw()` is a valid pointer to a `struct sg_table` for the entire // lifetime of `self`. // - The backing `struct sg_table` is not modified for the entire lifetime of `self`. - unsafe { SGTable::from_raw(self.inner.sgt.as_raw()) } + unsafe { SGTable::from_raw(self.inner.dma.sgt.as_raw()) } } } @@ -467,7 +418,7 @@ mod private { pub trait Sealed {} impl Sealed for super::Borrowed {} - impl

Sealed for super::Owned

{} + impl

Sealed for super::Owned<'_, P> {} } /// An [`Iterator`] over the DMA mapped [`SGEntry`] items of an [`SGTable`]. diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs index ffb693544673..844e137cddc8 100644 --- a/samples/rust/rust_dma.rs +++ b/samples/rust/rust_dma.rs @@ -35,8 +35,7 @@ struct DmaSampleData<'bound> { pdev: &'bound pci::Device, ca: Coherent<'bound, [MyStruct]>, - #[pin] - sgt: SGTable>>, + sgt: SGTable>>, } const TEST_VALUES: [(u32, u32); 5] = [ @@ -96,13 +95,9 @@ fn probe<'bound>( let size = 4 * page::PAGE_SIZE; let pages = VVec::with_capacity(size, GFP_KERNEL)?; - let sgt = SGTable::new(pdev.as_ref(), pages, DataDirection::ToDevice, GFP_KERNEL); + let sgt = SGTable::new(pdev.as_ref(), pages, DataDirection::ToDevice, GFP_KERNEL)?; - Ok(try_pin_init!(Self::Data { - pdev, - ca, - sgt <- sgt, - })) + Ok(try_pin_init!(Self::Data { pdev, ca, sgt })) }) } } base-commit: 2268cf8e3d7b4192629e4beff4a413efe5501f11 -- 2.56.1