Re: [EXTERNAL] Re: [PATCH 1/1] swiotlb: add a KUnit test suite

[Robin Murphy <robin.murphy@arm.com>]

On 2024-04-03 3:19 pm, Linu Cherian wrote:
[...]
>>> Should we not try this on a buffer that is mapped with DMA_FROM_DEVICE ?
>>
>> I'm afraid I don't follow.
>>
>> AFAICT the direction is a property of the sync operation. In fact,
>> swiotlb_tbl_map_single() does not even use its direction parameter at all.
>> Removing that parameter is already on my TODO list of cleanups.
>>
> 
> Okay. Got it.
> 
>> swiotlb_map() uses its direction parameter only to perform the initial arch
>> sync if DMA is non-coherent.
>>
>> OTOH I may be missing some high-level logical concepts which do not
>> correspond to any actual code in the swiotlb implementation, so my use is still
>> wrong.
>>
> 
> Just thought that the keeping the DMA direction consistent for the map and sync would be more aligned to typical use case.
> For example, a buffer used for transmit in case of networking. OTOH, since the API by itself doesn't have such constraints on the direction parameter, may be it makes sense to test those scenarios.

Right, SWIOTLB exists to serve the DMA API, so it makes more sense to me 
to test it in the context of valid DMA API usage than to make up 
scenarios that aren't representative of real-world usage. The direction 
is a property of the whole DMA mapping itself, and thus must be passed 
consistently for every operation on a given mapping.

Yes, there is some internal trickery once we get down to the level of 
calling swiotlb_bounce() itself, but if we're driving the tests through 
the higher-level public interface then I'd prefer to see that used as 
expected. Given the whole partial-write-transparency business, the most 
significant effect of direction should just be that of DMA_TO_DEVICE 
skipping the copy-out for unmap and sync_for_cpu, so for the sake of 
coverage you may as well just use DMA_BIDIRECTIONAL everywhere.

Thanks,
Robin.