Re: [PATCH 2/2] rust: scatterlist: honor the device's maximum segment size

["Alexandre Courbot" <acourbot@nvidia.com>]

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.

>
> Fixes: 05aa6fb1c21d ("rust: scatterlist: Add abstraction for sg_table")
> Signed-off-by: Matteo Kloiber <kernel@matt3o12.de>
> ---
>  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.