> On 23 Oct 2025, at 18:22, Lyude Paul <[email protected]> wrote: > > Currently we expose the ability to retrieve an SGTable for an shmem gem > object using gem::shmem::Object::<T>::sg_table(). However, this only gives > us a borrowed reference. This being said - retrieving an SGTable is a > fallible operation, and as such it's reasonable that a driver may want to > hold onto an SGTable for longer then a reference would allow in order to > avoid having to deal with fallibility every time they want to access the > SGTable. One such driver with this usecase is the Asahi driver. > > So to support this, let's introduce shmem::SGTable - which both holds a > pointer to the SGTable and a reference to its respective GEM object in > order to keep the GEM object alive for as long as the shmem::SGTable. The > type can be used identically to a normal SGTable. > > Signed-off-by: Lyude Paul <[email protected]> > > --- > V3: > * Rename OwnedSGTable to shmem::SGTable. Since the current version of the > SGTable abstractions now has a `Owned` and `Borrowed` variant, I think > renaming this to shmem::SGTable makes things less confusing. > We do however, keep the name of owned_sg_table() as-is. > V4: > * Clarify safety comments for SGTable to explain why the object is > thread-safe. > * Rename from SGTableRef to SGTable > > rust/kernel/drm/gem/shmem.rs | 50 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs > index 45b95d60a3ec7..21ccb6c1824be 100644 > --- a/rust/kernel/drm/gem/shmem.rs > +++ b/rust/kernel/drm/gem/shmem.rs > @@ -173,6 +173,25 @@ pub fn sg_table(&self) -> Result<&scatterlist::SGTable> { > // pointer to a scatterlist > Ok(unsafe { scatterlist::SGTable::from_raw(sgt) }) > } > + > + /// Creates (if necessary) and returns an owned reference to a > scatter-gather table of DMA pages > + /// for this object. > + /// > + /// This is the same as [`sg_table`](Self::sg_table), except that it > instead returns an > + /// [`shmem::SGTable`] which holds a reference to the associated gem > object, instead of a > + /// reference to an [`scatterlist::SGTable`]. > + /// > + /// This will pin the object in memory. > + /// > + /// [`shmem::SGTable`]: SGTable > + pub fn owned_sg_table(&self) -> Result<SGTable<T>> { > + Ok(SGTable { > + sgt: self.sg_table()?.into(), > + // INVARIANT: We take an owned refcount to `self` here, ensuring > that `sgt` remains > + // valid for as long as this `SGTable`. > + _owner: self.into(), > + }) > + } > } > > impl<T: DriverObject> Deref for Object<T> { > @@ -223,3 +242,34 @@ impl<T: DriverObject> driver::AllocImpl for Object<T> { > dumb_map_offset: None, > }; > } > + > +/// An owned reference to a scatter-gather table of DMA address spans for a > GEM shmem object. > +/// > +/// This object holds an owned reference to the underlying GEM shmem object, > ensuring that the > +/// [`scatterlist::SGTable`] referenced by this type remains valid for the > lifetime of this object. > +/// > +/// # Invariants > +/// > +/// - `sgt` is kept alive by `_owner`, ensuring it remains valid for as long > as `Self`. > +/// - `sgt` corresponds to the owned object in `_owner`. > +/// - This object is only exposed in situations where we know the underlying > `SGTable` will not be > +/// modified for the lifetime of this object. Thus, it is safe to > send/access this type across > +/// threads. > +pub struct SGTable<T: DriverObject> { > + sgt: NonNull<scatterlist::SGTable>, > + _owner: ARef<Object<T>>, > +} > + > +// SAFETY: This object is thread-safe via our type invariants. > +unsafe impl<T: DriverObject> Send for SGTable<T> {} > +// SAFETY: This object is thread-safe via our type invariants. > +unsafe impl<T: DriverObject> Sync for SGTable<T> {} > + > +impl<T: DriverObject> Deref for SGTable<T> { > + type Target = scatterlist::SGTable; > + > + fn deref(&self) -> &Self::Target { > + // SAFETY: Creating an immutable reference to this is safe via our > type invariants. > + unsafe { self.sgt.as_ref() } > + } > +} > -- > 2.51.0 > Same comment as last patch. — Daniel
