ASYNC_TX: async_xor mapping fix

The destination address may be present in the source list, so we should map the addresses from the source list first.
Otherwise, if page corresponding to destination is not marked as write-
through (with regards to CPU cache), then mapping it with DMA_FROM_DEVICE may lead to data loss, and finally to an incorrect result of calculations.

Quoted 6 lines · Yuri Tikhonov

The destination address may be present in the source list, so we should map the addresses from the source list first.
Otherwise, if page corresponding to destination is not marked as write-
through (with regards to CPU cache), then mapping it with DMA_FROM_DEVICE may lead to data loss, and finally to an incorrect result of calculations.

Thanks Yuri. I think we should avoid mapping the destination twice altogether, and for simplicity just always map it bidirectionally.

Quoted 8 lines · Dan Williams

Quoted 6 lines · Yuri Tikhonov

The destination address may be present in the source list, so we should map the addresses from the source list first.
Otherwise, if page corresponding to destination is not marked as write-
through (with regards to CPU cache), then mapping it with DMA_FROM_DEVICE may lead to data loss, and finally to an incorrect result of calculations.

Quoted 2 lines

Thanks Yuri. I think we should avoid mapping the destination twice altogether, and for simplicity just always map it bidirectionally.

Yuri, Saeed, can I get your acked-by's for the following 2.6.28 patch:

Thanks,
Dan

async_xor: dma_map destination DMA_BIDIRECTIONAL

From: Dan Williams <[email protected]>

Mapping the destination multiple times is a misuse of the dma-api. Since the destination may be reused as a source, ensure that it is only mapped once and that it is mapped bidirectionally. This appears to add ugliness on the unmap side in that it always reads back the destination address from the descriptor, but gcc can determine that dma_unmap is a nop and not emit the code that calculates its arguments.

Cc <[email protected]>

Cc Saeed Bishara <[email protected]>

Reported-by Yuri Tikhonov <[email protected]>

Signed-off-by Dan Williams <[email protected]>

---
 crypto/async_tx/async_xor.c |   11 +++++++++--
 drivers/dma/iop-adma.c      |   16 +++++++++++++---
 drivers/dma/mv_xor.c        |   15 ++++++++++++---
 3 files changed, 34 insertions(+), 8 deletions(-)

crypto/async_tx/async_xor.c +9 -2

index c029d3e..a6faa90 100644
@@ -53,10 +53,17 @@ do_async_xor(struct dma_chan *chan, struct page *dest, struct page **src_list,
        int xor_src_cnt;
        dma_addr_t dma_dest;
 
-       dma_dest = dma_map_page(dma->dev, dest, offset, len, DMA_FROM_DEVICE);
-       for (i = 0; i < src_cnt; i++)
+       /* map the dest bidrectional in case it is re-used as a source */
+       dma_dest = dma_map_page(dma->dev, dest, offset, len, DMA_BIDIRECTIONAL);
+       for (i = 0; i < src_cnt; i++) {
+               /* only map the dest once */
+               if (src_list[i] == dest) {
+                       dma_src[i] = dma_dest;
+                       continue;
+               }
                dma_src[i] = dma_map_page(dma->dev, src_list[i], offset,
                                          len, DMA_TO_DEVICE);
+       }
 
        while (src_cnt) {
                async_flags = flags;

drivers/dma/iop-adma.c +13 -3

index c7a9306..6be3172 100644
@@ -85,18 +85,28 @@ iop_adma_run_tx_complete_actions(struct iop_adma_desc_slot *desc,
                        enum dma_ctrl_flags flags = desc->async_tx.flags;
                        u32 src_cnt;
                        dma_addr_t addr;
+                       dma_addr_t dest;
 
+                       src_cnt = unmap->unmap_src_cnt;
+                       dest = iop_desc_get_dest_addr(unmap, iop_chan);
                        if (!(flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
-                               addr =
 iop_desc_get_dest_addr(unmap, iop_chan);
-                               dma_unmap_page(dev, addr, len, DMA_FROM_DEVICE);
+                               enum dma_data_direction dir;
+
+                               if (src_cnt > 1) /* is xor? */
+                                       dir = DMA_BIDIRECTIONAL;
+                               else
+                                       dir = DMA_FROM_DEVICE;
+
+                               dma_unmap_page(dev, dest, len, dir);
                        }
 
                        if (!(flags & DMA_COMPL_SKIP_SRC_UNMAP)) {
-                               src_cnt = unmap->unmap_src_cnt;
                                while (src_cnt--) {
                                        addr = iop_desc_get_src_addr(unmap,
                                                                    iop_chan,
                                                                    src_cnt);
+                                       if (addr == dest)
+                                               continue;
                                        dma_unmap_page(dev, addr, len,
                                                       DMA_TO_DEVICE);

                                }

drivers/dma/mv_xor.c +10 -2

index 0328da0..bcda174 100644
@@ -311,17 +311,26 @@ mv_xor_run_tx_complete_actions(struct mv_xor_desc_slot *desc,
                        enum dma_ctrl_flags flags = desc->async_tx.flags;
                        u32 src_cnt;
                        dma_addr_t addr;
+                       dma_addr_t dest;
 
+                       src_cnt = unmap->unmap_src_cnt;
+                       dest = mv_desc_get_dest_addr(unmap);
                        if (!(flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
-                               addr = mv_desc_get_dest_addr(unmap);
-                               dma_unmap_page(dev, addr, len, DMA_FROM_DEVICE);
+                               enum dma_data_direction dir;
+
+                               if (src_cnt > 1) /* is xor ? */
+                                       dir = DMA_BIDIRECTIONAL;
+                               else
+                                       dir = DMA_FROM_DEVICE;
+                               dma_unmap_page(dev, dest, len, dir);
                        }