Re: [PATCH 2/2] rust: scatterlist: honor the device's maximum segment size
From: "Alexandre Courbot" <acourbot@nvidia.com>
Date: 2026-09-07 02:43:56
Also in:
dri-devel, driver-core, lkml, rust-for-linux
On Tue Sep 1, 2026 at 8:32 AM JST, Matteo Kloiber wrote:
SGTable::new() caps segment length at dma_max_mapping_size() only, which limits the DMA mapping path (e.g. swiotlb), not the device itself. The per-device limit from dma_set_max_seg_size() is ignored, so contiguous page segments can be longer than the declared max segment size, potentially causing problems for future drivers that use this abstraction. nova-core declares an unlimited segment size, so this does not change its behavior.
I guess what this last paragraph wants to state is that no user is affected by this patch? There is another subtle user though: the Rust DMA sample. In any case, this patch without patch 1 wouldn't break either of those (only waste a bit more memory in SG entries with nova-core), so maybe we can skip it.
quoted hunk ↗ jump to hunk
Fixes: 05aa6fb1c21d ("rust: scatterlist: Add abstraction for sg_table") Signed-off-by: Matteo Kloiber <redacted> --- rust/helpers/dma.c | 5 +++++ rust/kernel/scatterlist.rs | 12 ++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-)diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c index 9fbeb507b08c..ff8f24dae9df 100644 --- a/rust/helpers/dma.c +++ b/rust/helpers/dma.c@@ -49,3 +49,8 @@ __rust_helper void rust_helper_dma_set_max_seg_size(struct device *dev, { dma_set_max_seg_size(dev, size); } + +__rust_helper unsigned int rust_helper_dma_get_max_seg_size(struct device *dev) +{ + return dma_get_max_seg_size(dev); +}diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs index b83c468b5c63..d677dcbe7aac 100644 --- a/rust/kernel/scatterlist.rs +++ b/rust/kernel/scatterlist.rs@@ -350,15 +350,23 @@ fn new( page_vec.push(page.as_ptr(), flags)?; } + // Cap segments at both the DMA mapping-path limit and the device's declared + // max segment size. + // // `dma_max_mapping_size` returns `size_t`, but `sg_alloc_table_from_pages_segment()` takes // an `unsigned int`. // // SAFETY: `dev.as_raw()` is a valid pointer to a `struct device`. - let max_segment = match unsafe { bindings::dma_max_mapping_size(dev.as_raw()) } { + let max_mapping = match unsafe { bindings::dma_max_mapping_size(dev.as_raw()) } { 0 => u32::MAX, - max_segment => u32::try_from(max_segment).unwrap_or(u32::MAX), + max_mapping => u32::try_from(max_mapping).unwrap_or(u32::MAX),
nit: if we rename this variable, let's name it `max_mapping_size` for consistency.
};
+ // SAFETY: `dev.as_raw()` is a valid pointer to a `struct device`.
+ let max_seg_size = unsafe { bindings::dma_get_max_seg_size(dev.as_raw()) };
+
+ let max_segment = max_mapping.min(max_seg_size);This looks like the correct thing to do indeed. Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> (the following is a related thought, not a request for this patch) It also means that without patch 1, nova-core would split the firmware into hundreds of 64KB SG entries, which is not breaking but still something we want to avoid. The correct fix is to make sure that `dma_set_max_seg_size` is called by the driver, and while we are at it we also want every driver to call `dma_set_mask_and_coherent`. Ideally we would use the type system to make sure that both functions are called before any DMA operation can take place (using a safe interface), but I'm not quite sure yet how we can do this.