Re: [PATCH 1/2] rust: add initial scatterlist bindings

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, May 30, 2025 at 11:02:02PM +0900, Alexandre Courbot wrote:
> You would have a trait for providing the pages and their range:
> 
>     /// Provides a list of pages that can be used to build a `SGTable`.
>     trait SGTablePages {
>         /// Returns an iterator to the pages providing the backing memory of `self`.
>         fn pages_iter<'a>(&'a self) -> impl Iterator<Item = &'a bindings::page>;
>         /// Returns the effective range of the mapping.
>         fn range(&self) -> Range<usize>;
>     }
>
> The `SGTable` becomes something like:
> 
>     struct SGTable<P: SGTablePages, T: MapState>
>     {
>         table: Opaque<bindings::sg_table>,
>         pages: P,
>         _s: PhantomData<T>,
>     }

At this point it isn't exactly a sgtable anymore, it is some rust
specific way to get a dma mapped scatterlist. Most of the actual ways
to use a sgtable's cpu side would become unavailable for safety
reasons.

That seems fine to me, and is what I was suggesting when I said not to
expose set_page at all.

But I would maybe lean into it a bit more, why have the type state at
all anymore if the flow is SGTablePages -> SgTable -> Dma Mapped?
There isn't really a reason to expose the CPU populated but not yet
mapped state to the user at all. They can't do anything with it. Just
directly create the DMA mapped scatterlist and only expose the DMA
list through the rust API in a single step.

So much simpler to understand and doesn't leak the bad decisions of
the scatterlist design.

Certainly the initial uses of scatterlist don't need to ever know
about or touch the CPU side of the scatterlist, and it would be great
if Rust could stay that way..

Jason