Page MenuHomeFreeBSD

Introduce support for DMA coherent ARM platforms
ClosedPublic

Authored by mw_semihalf.com on Jun 14 2017, 10:45 PM.

Details

Summary
  • Inherit BUS_DMA_COHERENT flag from parent buses
  • Use cacheable memory attributes on dma coherent platform
  • Disable cache synchronization on coherent platform

Changes are based on ARMv8 busdma code and commit r299683.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ian edited edge metadata.Jun 18 2017, 11:10 PM

Is coherent IO a platform-wide thing? These changes imply that there could be a system with a mix of tags, some describing coherent memory and some not. But the code wouldn't actually work right in such a situation... bounce buffers are not allocated from special memory, but the code avoids cache ops on bounce buffers if the tag coherent flag is set. For mixed tags on the same platform to work, bounce buffers would have to be allocated from different pools depending on the tag flags.

sys/arm/arm/busdma_machdep-v6.c
764 ↗(On Diff #29626)

This confused me for a while. Once I figured it out, I realized the problem is mostly that the comment is now a bit misleading. I think I'd make 2 comments now...

/* For coherent memory, set the map flag that disables sync ops. */       
if (flags & BUS_DMA_COHERENT)
    map->flags |= DMAMAP_COHERENT;
	​
/* 
 * Choose a busdma buffer allocator based on memory type flags.  If the tag's
 * COHERENT flag is set, that means normal memory is already coherent, use
 * the normal allocator.
 */
​if ((flags & BUS_DMA_COHERENT) &&
​    ((dmat->flags & BUS_DMA_COHERENT) == 0)) {

Yes, coherent IO is a platform-wide feature.
I added two comments to the code as you suggested. First one is different because DMAMAP_COHERENT flag does not disable sync ops. It only show that the buffer must be coherent.

ian added a comment.Jun 19 2017, 2:18 PM

Yes, coherent IO is a platform-wide feature.
I added two comments to the code as you suggested. First one is different because DMAMAP_COHERENT flag does not disable sync ops. It only show that the buffer must be coherent.

The DMAMAP_COHERENT flag does three things:

  • It indicates that an allocated buffer must be returned to the coherent pool.
  • It indicates that cache sync ops are not needed (except a dsb()).
  • It indicates that bouncing is not needed due to cache misalignment, because there will be no sync to cause a partial cacheline flush.

You are right, I will change first comment.

Correct comments in the code, pointed by @ian

ian accepted this revision.Jun 21 2017, 1:31 PM
This revision is now accepted and ready to land.Jun 21 2017, 1:31 PM
This revision was automatically updated to reflect the committed changes.