On Fri Jun 5, 2026 at 4:24 AM JST, Lyude Paul wrote:
In order to do this, we need to be careful to ensure that any interface we expose for scatterlists ensures that any mappings created from one are destroyed on driver-unbind. To do this, we introduce a Devres resource into shmem::Object that we use in order to ensure that we release any SGTable mappings on driver-unbind.
There's some other slightly unfortunate caveats of this:
- Drivers don't have explicit control at the moment over when unmapping happens (which is exactly the same as the C side atm, so it might not be a problem).
- We can't just return `SGTableMap` to the user through an Arc to attempt to fix the last caveat - because that implies the gem object would need to hold a reference count to the scatterlist mapping, which just leaves us with the same problem.
Signed-off-by: Lyude Paul lyude@redhat.com
I really like how simplified `sg_table` has become!
Reviewed-by: Alexandre Courbot acourbot@nvidia.com
With the customary final nits below.
<...>
- /// Creates (if necessary) and returns an immutable reference to a scatter-gather table of DMA
- /// pages for this object.
- ///
- /// This will pin the object in memory. It is expected that `dev` should be a pointer to the
- /// same [`device::Device`] which `self` belongs to, otherwise this function will return
- /// `Err(EINVAL)`.
- pub fn sg_table<'a>(
&'a self,dev: &'a device::Device<Bound>,- ) -> Result<&'a scatterlist::SGTable> {
if dev.as_raw() != self.dev().as_ref().as_raw() {return Err(EINVAL);}let sgt_res = 'out: {// Fast path: sgt_res is already initializedif let Some(sgt_res) = self.sgt_res.as_ref() {break 'out sgt_res;}// Slow path: Grab the lock and see if we need to initialize sgt_res.let _guard = self.sgt_lock.lock();// If someone initialized it while we were waiting, we can exit early.if let Some(sgt_res) = self.sgt_res.as_ref() {break 'out sgt_res;}// If not, finish initializing and return.self.sgt_res.populate(Devres::new(dev, SGTableMap::new(self))?);
Maybe add a comment explaining that `populate` cannot return `false`, as its invocation it protected by the mutex? This helps understanding that the following unsafe block is ok.
// SAFETY: We just populated sgt_res above.unsafe { self.sgt_res.as_ref().unwrap_unchecked() }};Ok(sgt_res.access(dev)?)- }
} impl<T: DriverObject, C: DeviceContext> Deref for Object<T, C> { @@ -474,6 +545,63 @@ impl<D, R, C, const SIZE: usize> IoKnownSize for VMap<D, R, C, SIZE> #[cfg(CONFIG_64BIT)] impl_vmap_io_capable!(VMap, u64); +/// A reference to a GEM object that is known to have a mapped [`SGTable`]. +/// +/// This is used by the Rust bindings with [`Devres`] in order to ensure that mappings for SGTables +/// on GEM shmem objects are revoked on driver-unbind. +/// +/// # Invariants +/// +/// - `self.obj` always points to a valid GEM object. +/// - This object is proof that `self.obj.owner.sgt` has an initialized and valid +/// [`scatterlist::SGTable`].
The SGTable is not in `owner.sgt` anymore.
+pub struct SGTableMap<T: DriverObject, C: DeviceContext> {
- obj: NonNull<Object<T, C>>,
+}
+impl<T: DriverObject, C: DeviceContext> Deref for SGTableMap<T, C> {
- type Target = scatterlist::SGTable;
- fn deref(&self) -> &Self::Target {
// SAFETY:// - The NonNull is guaranteed to be valid via our type invariants.// - The sgt field is guaranteed to be initialized and valid via our type invariants.unsafe { scatterlist::SGTable::from_raw((*self.obj.as_ref().as_raw_shmem()).sgt) }- }
+}
+impl<T: DriverObject, C: DeviceContext> Drop for SGTableMap<T, C> {
- fn drop(&mut self) {
// SAFETY: `obj` is always valid via our type invariantslet obj = unsafe { self.obj.as_ref() };let _lock = DmaResvGuard::new(obj);// SAFETY: We acquired the lock needed for calling this function aboveunsafe { bindings::__drm_gem_shmem_free_sgt_locked(obj.as_raw_shmem()) };- }
+}
+impl<T: DriverObject, C: DeviceContext> SGTableMap<T, C> {
- fn new(obj: &Object<T, C>) -> impl Init<Self, Error> {
// INVARIANT:// - We call drm_gem_shmem_get_pages_sgt_locked below and check whether or not it
s/drm_gem_shmem_get_pages_sgt_locked/drm_gem_shmem_get_pages_sgt.