From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C81CA42EEA3 for ; Fri, 29 May 2026 18:37:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780079859; cv=none; b=jnLpmB0hbYEQJw6oUQLacBGvl06B7jdyZLTs/0z31gWj3nLcwm/M+nV6d/0oj/ZJ9HvgfEdklVhp+oeoG2gWtr54RWWkhxvj/EIjRdp4nbr89Z1VbY0L368ktLsET/BzsftckDv+hTSAzeLqCbYAOWWprvTLnwvNo/cKLsbgesI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780079859; c=relaxed/simple; bh=4cjwe87q/t7s3dmQsDYYrUXGLi6ANKnSrefxeksGMpo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:content-type; b=tfd96hvaBvxgu+cotx6LQ4PK2pjXpfLswmQzOpDLGDwqokwWtlXdStEkp2lELfKKPEkdhZPIZqqazFf8DHAENh4Kkp6D1/4MWwFUXsqf+rHeZrgaH8TU4U1hNQvRLtC/kWY7hmwsMCfQf8yLJYzl6HE7nGjysqQ3KvWtSTD1Rvs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=J/k47Mrn; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="J/k47Mrn" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1780079856; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FW4muBq04eUEQ5DPUv/MIH2aGLX2hNQSawY7hFA7cfc=; b=J/k47Mrnsh9fpSGhgAmC/hNm0+WWHEdmlwAHs+nPVonnBMEMN5Nll14xrdv1LqM7mUyGpT IUmvsWholb/uyXtGdXftPBCzVGBzYJ3uxJovkdLJMbg0hVQRkxNJSdIxkozTrc9MVMq21L 680QTt/KHDUc5Iy7oqsVas8hGsGgRlc= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-297-kMqJ3efKP_6-3Cyqrp-ceg-1; Fri, 29 May 2026 14:37:34 -0400 X-MC-Unique: kMqJ3efKP_6-3Cyqrp-ceg-1 X-Mimecast-MFC-AGG-ID: kMqJ3efKP_6-3Cyqrp-ceg_1780079851 Received: from mx-prod-int-10.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-10.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.95]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 25A85195608B; Fri, 29 May 2026 18:37:31 +0000 (UTC) Received: from GoldenWind.redhat.com (unknown [10.22.89.101]) by mx-prod-int-10.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 0DCF21776; Fri, 29 May 2026 18:37:27 +0000 (UTC) From: Lyude Paul To: dri-devel@lists.freedesktop.org, rust-for-linux@vger.kernel.org, nouveau@lists.freedesktop.org Cc: Alexandre Courbot , Gary Guo , =?UTF-8?q?Christian=20K=C3=B6nig?= , driver-core@lists.linux.dev, Miguel Ojeda , Maarten Lankhorst , Alice Ryhl , Simona Vetter , linux-kernel@vger.kernel.org, Sumit Semwal , linux-media@vger.kernel.org, "Rafael J . Wysocki" , Thomas Zimmermann , Maxime Ripard , David Airlie , Benno Lossin , linaro-mm-sig@lists.linaro.org, Danilo Krummrich , Mukesh Kumar Chaurasiya , Asahi Lina , Daniel Almeida , Lyude Paul , Greg Kroah-Hartman Subject: [PATCH v15 6/6] rust: drm: gem: Introduce shmem::Object::sg_table() Date: Fri, 29 May 2026 14:34:04 -0400 Message-ID: <20260529183702.677677-7-lyude@redhat.com> In-Reply-To: <20260529183702.677677-1-lyude@redhat.com> References: <20260529183702.677677-1-lyude@redhat.com> Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.6 on 10.30.177.95 X-Mimecast-MFC-PROC-ID: wKVJUvOfgtsz-GgeDBf1D7YUZJ8haNxBvMHejvGDngs_1780079851 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit content-type: text/plain; charset="US-ASCII"; x-default=true 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 --- 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 V10: * Use Devres in order to ensure that SGTables are revocable, and are unmapped on driver-unbind. V11: * s/create_sg_table()/get_sg_table() * Get rid of extraneous `ret = ` in shmem::Object::get_sg_table() V12: * Actually move sgt_res in this patch and not the next one V13: * Use DmaResvGuard suggestion from Alexander * Use Alexander's (much better) solution for get_sg_table() * Use SetOnce instead of UnsafeCell * s/SGTableRef/SGTableMap * Fix typo in SGTableMap documentation * Create fallible constructor for SGTableMap * Don't reuse dma_resv lock for protecting Object contents, just use Mutex + SetOnce * Drop use of drm_gem_shmem_get_pages_sgt_locked(), since we don't need to hold the dma_resv lock ourselves for anything but this function. * Check that the device we receive in the bounds for sg_table() and owned_sg_table() that said Device is in fact, the correct device. * Remove redundant docs in owned_sg_table(), just point it back to sg_table(). * Implement Deborah's suggestion to fix double-free in free_callback() * Restore original order of Object * Fix doc typo for SGTableMap V14: * Use new InitOnce container over the Mutex/SetOnce horror show we had before. * Start using LazyInit container for storing Devres for sgt unmap * Add some kunit tests for sg_table (not sure why I didn't do this before) using some of the boilerplate code leftover from the vmap bindings * Get rid of the owned SGTable variant for now, we'll add it back in a future patch if people actually need it. * Use new LazyInit container from me to get rid of the horrid Mutex> mess. * Add the best we can do for unit tests w/r/t SGTable at the moment rust/kernel/drm/gem/shmem.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 494e0d0d8d0d6..5655c2a1ae8fb 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -664,4 +664,28 @@ fn vmap_io() -> Result { Ok(()) } + + // TODO: I would love to actually test the success paths of sg_table(), but that would require + // also implementing dummy dma_ops so that trying to create a mapping doesn't explode. So, leave + // that for someone else. + + // Ensures that passing the wrong device to sg_table() fails as we expect, and also ensure it + // skips initializing `sgt_res` since we could otherwise create `sgt_res` with the wrong device + // bound to it. + #[test] + fn fail_sg_table_on_wrong_dev() -> Result { + let (_dev, drm) = create_drm_dev()?; + let wrong_dev = faux::Registration::new(c"EvilKunit", None)?; + + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + + assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); + + // If sgt_res was not initialized mistakenly with the wrong device, this should still fail. + assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); + + // TODO: Someday, we should test that creating an sg_table here still succeeds. + + Ok(()) + } } -- 2.54.0