A potential issue in ARM DMA for cortex processors

[Ramakrishna Rao Desetti <ramakrishnadtv%gmail.com@localhost>]

Hi,

I would like to discuss what seems to be a bug in ARM DMA for cortex based
processors.

When DMA buffer syncing needs to be done, ultimately
_bus_dmamap_sync_segment() is
called. Consider the following code in this function:

#ifdef CPU_CORTEX
    /*
     * Cortex CPUs can do speculative loads so we need to clean the cache
     * after a DMA read to deal with any speculatively loaded cache lines.
     * Since these can't be dirty, we can just invalidate them and don't
     * have to worry about having to write back their contents.
     */
    case BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE:
        STAT_INCR(sync_postreadwrite);
        arm_dmb();
        cpu_dcache_inv_range(va, len);
        cpu_sdcache_inv_range(va, pa, len);
        break;
    case BUS_DMASYNC_POSTREAD:
        STAT_INCR(sync_postread);
        arm_dmb();
        cpu_dcache_inv_range(va, len);
        cpu_sdcache_inv_range(va, pa, len);
        break;
#endif

For BUS_DMASYNC_POSTREAD, the above code invalidates the data caches. The
comment explains why writeback of the contents is not required. The comment
is correct that writeback of the contents is not required for speculative
loads because they can't be dirty.

But it misses another possibility. That is misalignment of length of the
buffer to be synced. Note that cache invalidation or write-back can be
done only in multiples of cache line sizes (i.e, arm_dcache_align). If "len"
above is misaligned, then writeback of the contents would be required in those
cases. Otherwise, it may lead to memory corruption.

In fact, it appears this case should be handled in exactly the same manner
this function handles for BUS_DMASYNC_PREREAD case. It takes care of
misalignment.

Could you please let me know your comments?

Thanks and regards,
Ramakrishna DTV.