Generic ATA driver DMA coherency issues

[Marius Strobl]

On Wed, May 27, 2009 at 12:53:06PM +0200, Piotr Zicik wrote:
> While working on bring-up SATA on ARM SoC I had issues with DMA memory 
> coherency. After tracing, I have found problem in generic ATA code behaviour.
> 
> The ATA generic driver allocates 64k DMA workspace memory which can be 
> optionally used by ATA chipset drivers. Currently only ata-ahci, ata-marvell, 
> ata-promise and ata-siliconimage relies on this feature. In general, the 
> drivers use preallocated DMA workspace to hold small request descriptors, 
> consumed by hardware.
> 
> All of these drivers do not synchronize workspace with bus_dmamap_sync(). They 
> assumes that DMA workspace is coherent. However ATA driver neither enforces 
> coherency by using BUS_DMA_COHERENT flag nor synchronizes the workspace with 
> bus_dmamap_sync().
> 
> I have fixed my problem by adding BUS_DMA_COHERENT flag to workspace 
> allocation code:
> 
> diff --git a/sys/dev/ata/ata-dma.c b/sys/dev/ata/ata-dma.c
> index ab4c3c8..71ee41b 100644
> --- a/sys/dev/ata/ata-dma.c
> +++ b/sys/dev/ata/ata-dma.c
> @@ -95,8 +95,8 @@ ata_dmainit(device_t dev)
>                            0, NULL, NULL, &ch->dma.work_tag))
>         goto error;
> 
> -    if (bus_dmamem_alloc(ch->dma.work_tag, (void **)&ch->dma.work, 0,
> -                        &ch->dma.work_map))
> +    if (bus_dmamem_alloc(ch->dma.work_tag, (void **)&ch->dma.work,
> +                        BUS_DMA_COHERENT, &ch->dma.work_map))
>         goto error;
> 
>      if (bus_dmamap_load(ch->dma.work_tag, ch->dma.work_map, ch->dma.work,
> 
> The question is about multiplatform impact of this change. Especially I am 
> curious how everything works without any problems on i386.
> 

Well, bus_dma.9 documents BUS_DMA_COHERENT to not remove the
requirement of using bus_dmamap_sync(), so this would only be
a quick hack. The NetBSD version of the man page states the
intent of that flag even more clearly.

Marius