Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping

Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping

[Horia Geanta]

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

Device obligation to write into a DMA_FROM_DEVICE streaming DMA mapping

[Horia Geanta]

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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Robin Murphy]

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.

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

[Robin Murphy]

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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu