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

[Marek Szyprowski <m.szyprowski@samsung.com>]

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@gmail.com> 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().


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland