Re: Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping

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

On 21/05/2019 18:14, Horia Geanta wrote:
> Hi,
> 
> Is it mandatory for a device to write data in an area DMA mapped DMA_FROM_DEVICE?
> Can't the device just "ignore" that mapping - i.e. not write anything - and
> driver should expect original data to be found in that location (since it was
> not touched / written to by the device)?
> [Let's leave cache coherency aside, and consider "original data" to be in RAM.]
> 
> I am asking this since I am seeing what seems to be an inconsistent behavior /
> semantics between cases when swiotlb bouncing is used and when it's not.
> 
> Specifically, the context is:
> 1. driver prepares a scatterlist with several entries and performs a
> dma_map_sg() with direction FROM_DEVICE
> 2. device decides there's no need to write into the buffer pointed by first
> scatterlist entry and skips it (writing into subsequent buffers)
> 3. driver is notified the device finished processing and dma unmaps the scatterlist
> 
> When swiotlb bounce is used, the buffer pointed to by first scatterlist entry is
> corrupted. That's because swiotlb implementation expects the device to write
> something into that buffer, however the device logic is "whatever was previously
> in that buffer should be used" (2. above).
> 
> For FROM_DEVICE direction:
> -swiotlb_tbl_map_single() does not copy data from original location to swiotlb
> 	if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> 	    (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
> 		swiotlb_bounce(orig_addr, tlb_addr, size, DMA_TO_DEVICE);
> -swiotlb_tbl_unmap_single() copies data from swiotlb to original location
> 	if (orig_addr != INVALID_PHYS_ADDR &&
> 	    !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> 	    ((dir == DMA_FROM_DEVICE) || (dir == DMA_BIDIRECTIONAL)))
> 		swiotlb_bounce(orig_addr, tlb_addr, size, DMA_FROM_DEVICE);
> and when device did not write anything (as in current situation), it overwrites
> original data with zeros
> 
> In case swiotlb bounce is not used and device does not write into the
> FROM_DEVICE streaming DMA maping, the original data is available.
> 
> Could you please clarify whether:
> -I am missing something obvious OR
> -the DMA API documentation should be updated - to mandate for device writes into
> FROM_DEVICE mappings) OR
> -the swiotlb implementation should be updated - to copy data from original
> location irrespective of DMA mapping direction?

Hmm, that certainly feels like a bug in SWIOTLB - it seems reasonable in 
principle for a device to only partially update a mapped buffer before a 
sync/unmap, so I'd say it probably should be filling the bounce buffer 
with the original data at the start, regardless of direction.

Robin.