On Thu, 24 Mar 2022 16:52:31 +0000 Robin Murphy <robin.murphy@xxxxxxx> wrote: > > Creating a new mapping for the same buffer before unmapping the > > previous one does looks rather bogus. But it does not fit the > > pattern where revering the sync_single changes make the driver > > work again. > > OK, you made me look :) > > Now that it's obvious what to look for, I can only conclude that during > the stanza in ath_edma_get_buffers(), the device is still writing to the > buffer while ownership has been transferred to the CPU, and whatever got > written while ath9k_hw_process_rxdesc_edma() was running then gets wiped > out by the subsequent sync_for_device, which currently resets the > SWIOTLB slot to the state that sync_for_cpu copied out. By the letter of > the DMA API that's not allowed, but on the other hand I'm not sure if we > even have a good idiom for "I can't tell if the device has finished with > this buffer or not unless I look at it" :/ I agree with your analysis. Especially with the latter part (were you state that we don't have a good idiom for that use case). I believe, a stronger statement is also true: it is fundamentally impossible to accommodate use cases where the device and the cpu need concurrent access to a dma buffer, if the dma buffer isn't in dma coherent memory. If the dma buffer is in dma coherent memory, and we don't need swiotlb, then we don't need the dma_sync functionality. Specifically for swiotlb, if the swiotlb buffer is in dma coherent memory, the driver could peek the swiotlb buffer, but I have no idea if we can provide a remotely sane API for that. The point is the device would have peek not via a pointer to the original buffer, but get suitable pointer to the bounce buffer, which would be probably be considered valid, as long as the mapping is valid. Honestly IMHO quite ugly but I see no other way. Regards, Halil
