On Mon, Oct 24, 2016 at 11:09 AM, Konrad Rzeszutek Wilk
[off-list ref] wrote:

On Mon, Oct 24, 2016 at 08:04:37AM -0400, Alexander Duyck wrote:

quoted

As a first step to making DMA_ATTR_SKIP_CPU_SYNC apply to architectures
beyond just ARM I need to make it so that the swiotlb will respect the
flag.  In order to do that I also need to update the swiotlb-xen since it
heavily makes use of the functionality.

Cc: Konrad Rzeszutek Wilk <redacted>
Signed-off-by: Alexander Duyck <redacted>
---
 drivers/xen/swiotlb-xen.c |   40 ++++++++++++++++++++++----------------
 include/linux/swiotlb.h   |    6 ++++--
 lib/swiotlb.c             |   48 +++++++++++++++++++++++++++------------------
 3 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 87e6035..cf047d8 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c

@@ -405,7 +405,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
       */
      trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);

-     map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir);
+     map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir,
+                                  attrs);
      if (map == SWIOTLB_MAP_ERROR)
              return DMA_ERROR_CODE;

@@ -416,11 +417,13 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
      /*
       * Ensure that the address returned is DMA'ble
       */
-     if (!dma_capable(dev, dev_addr, size)) {
-             swiotlb_tbl_unmap_single(dev, map, size, dir);
-             dev_addr = 0;
-     }
-     return dev_addr;
+     if (dma_capable(dev, dev_addr, size))
+             return dev_addr;
+
+     swiotlb_tbl_unmap_single(dev, map, size, dir,
+                              attrs | DMA_ATTR_SKIP_CPU_SYNC);
+
+     return DMA_ERROR_CODE;

Why? This change (re-ordering the code - and returning DMA_ERROR_CODE instead
of 0) does not have anything to do with the title.

If you really feel strongly about it - then please send it as a seperate patch.

Okay I can do that.  This was mostly just to clean up the formatting
because I was over 80 characters when I added the attribute.  Changing
the return value to DMA_ERROR_CODE from 0 was based on the fact that
earlier in the function that is the value you return if there is a
mapping error.

quoted

 }
 EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);

@@ -444,7 +447,7 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,

      /* NOTE: We use dev_addr here, not paddr! */
      if (is_xen_swiotlb_buffer(dev_addr)) {
-             swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
+             swiotlb_tbl_unmap_single(hwdev, paddr, size, dir, attrs);
              return;
      }

@@ -557,16 +560,9 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
                                                               start_dma_addr,
                                                               sg_phys(sg),
                                                               sg->length,
-                                                              dir);
-                     if (map == SWIOTLB_MAP_ERROR) {
-                             dev_warn(hwdev, "swiotlb buffer is full\n");
-                             /* Don't panic here, we expect map_sg users
-                                to do proper error handling. */
-                             xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
-                                                        attrs);
-                             sg_dma_len(sgl) = 0;
-                             return 0;
-                     }
+                                                              dir, attrs);
+                     if (map == SWIOTLB_MAP_ERROR)
+                             goto map_error;
                      xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT),
                                              dev_addr,
                                              map & ~PAGE_MASK,

@@ -589,6 +585,16 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
              sg_dma_len(sg) = sg->length;
      }
      return nelems;
+map_error:
+     dev_warn(hwdev, "swiotlb buffer is full\n");
+     /*
+      * Don't panic here, we expect map_sg users
+      * to do proper error handling.
+      */
+     xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
+                                attrs | DMA_ATTR_SKIP_CPU_SYNC);
+     sg_dma_len(sgl) = 0;
+     return 0;
 }

This too. Why can't that be part of the existing code that was there?

Once again it was a formatting thing.  I was indented too far and
adding the attribute pushed me over 80 characters so I broke it out to
a label to avoid the problem.

- Alex