[PATCH 1/1] drm/i915/dmabuf: avoid global wbinvd on dma-buf import

From: Krishna Prabhakaran <[email protected]>

When i915 needs to make an imported dma-buf coherent for GPU access on
non-LLC platforms, or for objects that bypass LLC, it currently calls
wbinvd_on_all_cpus(). get_pages() runs whenever an imported buffer is
pinned, so this triggers a whole-cache write-back and invalidate,
broadcast by IPI to every CPU, on every execbuf submission involving an
imported dma-buf. That stalls the entire machine for milliseconds and
starves latency-sensitive work on unrelated cores (e.g. USB isochronous
audio serviced on the VMM's main thread).

A dma-buf sg_table is not guaranteed to be backed by struct pages, and
the importer has no way to tell, so drm_clflush_sg() cannot be used
here. Instead vmap the buffer and flush that virtual range with
drm_clflush_virt_range(): x86 uses PIPT caches, so flushing one virtual
alias evicts the cache lines for every alias of the same physical
pages. This provides the required coherency, locally and without an IPI.

Fall back to wbinvd only when the buffer cannot be vmapped or is backed
by I/O memory, where there is no CPU-side range to clflush. The
dma_resv lock required by dma_buf_vmap() is already held here via the
imported object.

Fixes: a035154da45d ("drm/i915/dmabuf: add paranoid flush-on-acquire")
Signed-off-by: Krishna Prabhakaran <[email protected]>
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 26 +++++++++++++++-------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index b43d34c7d641..1c1f3dc9c212 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -10,6 +10,8 @@
 
 #include <asm/smp.h>
 
+#include <drm/drm_cache.h>
+
 #include "gem/i915_gem_dmabuf.h"
 #include "i915_drv.h"
 #include "i915_gem_object.h"
@@ -249,16 +251,24 @@ static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
 	 * DG1 is special here since it still snoops transactions even with
 	 * CACHE_NONE. This is not the case with other HAS_SNOOP platforms. We
 	 * might need to revisit this as we add new discrete platforms.
-	 *
-	 * XXX: Consider doing a vmap flush or something, where possible.
-	 * Currently we just do a heavy handed wbinvd_on_all_cpus() here since
-	 * the underlying sg_table might not even point to struct pages, so we
-	 * can't just call drm_clflush_sg or similar, like we do elsewhere in
-	 * the driver.
 	 */
 	if (i915_gem_object_can_bypass_llc(obj) ||
-	    (!HAS_LLC(i915) && !IS_DG1(i915)))
-		wbinvd_on_all_cpus();
+	    (!HAS_LLC(i915) && !IS_DG1(i915))) {
+		struct dma_buf *dma_buf = obj->base.import_attach->dmabuf;
+		struct iosys_map map;
+		int vmap_ret;
+
+		/* We already hold the dma_resv lock via the imported obj. */
+		vmap_ret = dma_buf_vmap(dma_buf, &map);
+		if (!vmap_ret && !map.is_iomem) {
+			drm_clflush_virt_range(map.vaddr, obj->base.size);
+			dma_buf_vunmap(dma_buf, &map);
+		} else {
+			if (!vmap_ret)
+				dma_buf_vunmap(dma_buf, &map);
+			wbinvd_on_all_cpus();
+		}
+	}
 
 	__i915_gem_object_set_pages(obj, sgt);
 
-- 
2.43.0