Re: [RFC PATCH 0/2] scatterlist rust bindings

[Abdiel Janulgue]

On 14/05/2025 15:12, Marek Szyprowski wrote:
> On 14.05.2025 09:00, Abdiel Janulgue wrote:
> > On 12/05/2025 14:19, Daniel Almeida wrote:
> > > Hi Abdiel,
> > >
> > > > On 12 May 2025, at 06:53, Abdiel Janulgue
> > > > <abdiel.janulgue@xxxxxxxxx> wrote:
> > > >
> > > > Hi,
> > > >
> > > > Here are the scatterlist bindings that has been brewing for a while
> > > > in my
> > > > local tree while working with Nova code. The bindings are used
> > > > mostly to
> > > > build the radix3 table from the GSP firmware which is loaded via dma.
> > > > This interface can be used on top of existing kernel scatterlist
> > > > objects
> > > > or to allocate a new one from scratch.
> > > >
> > > > Some questions still need to be resolved, which mostly come from
> > > > the DeviceSGTable::dma_map() function. Primarily, what if you call
> > > > bindings::dma_map_sgtable() on an already mapped sg_table? From my
> > >
> > > Perhaps we should introduce a type for buffers which are known to be
> > > mapped. Then
> > > we can simply not offer the option to map for that type.
> > >
> > > > experiments it doesn't seem to do anything and no indication is
> > > > returned if
> > > > the call succeeded or not. Should we save the "mapping info" to a list
> > > > everytime we call DeviceSGTable::dma_map more than once?
> > >
> > > What mapping info are you referring to?
> > Basically the dma_data_direction enum and possibly `Device`, if we
> > decouple SGTable from the device. So this approach would mean that
> > every-time SGTable::dma_map() is called, unique mapping object(s)
> > would be created, and which would get unmapped later on the destructor:
> >
> > struct SgtDmaMap {
> >      dev: ARef<Device>,
> >      dir: DmaDataDirection,
> > }
> >
> > impl SgtDmaMap {
> >      /// Creates a new mapping object
> >      fn new(dev: &Device, dir: DmaDataDirection) -> Self {
> >          Self { dev: dev.into(), dir, }
> >      }
> > }
> > ...
> > ...
> >
> > impl SGTable {
> >      pub fn dma_map(dev: &Device, dir: DmaDataDirection) ->
> > Result<SgtDmaMap>
> >
> > But I'm not sure if there is any point to that as the C
> > `dma_map_sgtable()` doesn't seem to care anyway (I could be wrong with
> > this) if the sg_table gets mapped more than once?
>
>
> Standard DMA-mapping C api doesn't have the notion of the object,
> although in case of sgtable structure, one might add some flags might
> there. Originally the sgtable based helpers were just trivial wrappers
> for dma_sync_sg_*() and dma_unmap_sg() ensuring proper parameters (and
> avoiding the confusion which nents to pass).
>
> It is generally assumed that caller uses the DMA API properly and there
> are no checks for double dma_map calls. It is only correct to call
> dma_map_sgtable() for the same sgtable structure after earlier call to
> dma_unmap_sgtable().

Thanks for the clarification! I think this double mapping issue can be 
solved by the suggestion presented by Alexander Courbot.

/Abdiel
> Best regards