LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping
@ 2019-05-21 17:14 Horia Geanta
  2019-05-21 17:27 ` Robin Murphy
  0 siblings, 1 reply; 2+ messages in thread
From: Horia Geanta @ 2019-05-21 17:14 UTC (permalink / raw)
  To: Christoph Hellwig, Marek Szyprowski, Robin Murphy, Konrad Rzeszutek Wilk
  Cc: iommu, dl-linux-imx, linux-kernel

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?

Thanks,
Horia

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping
  2019-05-21 17:14 Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping Horia Geanta
@ 2019-05-21 17:27 ` Robin Murphy
  0 siblings, 0 replies; 2+ messages in thread
From: Robin Murphy @ 2019-05-21 17:27 UTC (permalink / raw)
  To: Horia Geanta, Christoph Hellwig, Marek Szyprowski, Konrad Rzeszutek Wilk
  Cc: iommu, dl-linux-imx, linux-kernel

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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-05-21 17:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-21 17:14 Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping Horia Geanta
2019-05-21 17:27 ` Robin Murphy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).