[Intel-gfx] [PATCH 1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf

[Intel-gfx] [PATCH 1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf

[Matthew Auld]

We need to iterate over the original entries here for the sg_table,
pulling out the struct page for each one, to be remapped. However
currently this incorrectly iterates over the final dma mapped entries,
which is likely just one gigantic sg entry if the iommu is enabled,
leading to us only mapping the first struct page (and any physically
contiguous pages following it), even if there is potentially lots more
data to follow.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7306
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 07eee1c09aaf..05ebbdfd3b3b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -40,13 +40,13 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme
 		goto err;
 	}
 
-	ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);
+	ret = sg_alloc_table(st, obj->mm.pages->orig_nents, GFP_KERNEL);
 	if (ret)
 		goto err_free;
 
 	src = obj->mm.pages->sgl;
 	dst = st->sgl;
-	for (i = 0; i < obj->mm.pages->nents; i++) {
+	for (i = 0; i < obj->mm.pages->orig_nents; i++) {
 		sg_set_page(dst, sg_page(src), src->length, 0);
 		dst = sg_next(dst);
 		src = sg_next(src);
-- 
2.37.3

[Intel-gfx] [PATCH 2/2] drm/i915/selftests: exercise GPU access from the importer

[Matthew Auld]

Using PAGE_SIZE here potentially hides issues so bump that to something
larger. This should also make it possible for iommu to coalesce entries
for us. With that in place verify we can write from the GPU using the
importers sg_table, followed by checking that our writes match when read
from the CPU side.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/7306
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/gem/selftests/i915_gem_dmabuf.c  | 37 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index f2f3cfad807b..e55b255f69f8 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -6,6 +6,7 @@
 
 #include "i915_drv.h"
 #include "i915_selftest.h"
+#include "gt/intel_migrate.h"
 
 #include "mock_dmabuf.h"
 #include "selftests/mock_gem_device.h"
@@ -148,13 +149,15 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
 	struct drm_gem_object *import;
 	struct dma_buf *dmabuf;
 	struct dma_buf_attachment *import_attach;
+	struct i915_request *rq;
 	struct sg_table *st;
+	u32 *vaddr;
 	long timeout;
-	int err;
+	int err, i;
 
 	force_different_devices = true;
 
-	obj = __i915_gem_object_create_user(i915, PAGE_SIZE,
+	obj = __i915_gem_object_create_user(i915, SZ_8M,
 					    regions, num_regions);
 	if (IS_ERR(obj)) {
 		pr_err("__i915_gem_object_create_user failed with err=%ld\n",
@@ -194,6 +197,19 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
 		goto out_import;
 	}
 
+	err = intel_context_migrate_clear(to_gt(i915)->migrate.context, NULL,
+					  import_obj->mm.pages->sgl,
+					  import_obj->cache_level,
+					  false,
+					  0xdeadbeaf, &rq);
+	if (rq) {
+		err = dma_resv_reserve_fences(obj->base.resv, 1);
+		if (!err)
+			dma_resv_add_fence(obj->base.resv, &rq->fence,
+					   DMA_RESV_USAGE_KERNEL);
+		i915_request_put(rq);
+	}
+
 	/*
 	 * If the exported object is not in system memory, something
 	 * weird is going on. TODO: When p2p is supported, this is no
@@ -206,6 +222,23 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
 
 	i915_gem_object_unlock(import_obj);
 
+	if (err)
+		goto out_import;
+
+	vaddr = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB);
+	if (IS_ERR(vaddr)) {
+		err = PTR_ERR(vaddr);
+		goto out_import;
+	}
+
+	for (i = 0; i < obj->base.size / sizeof(u32); i++) {
+		if (vaddr[i] != 0xdeadbeaf) {
+			pr_err("Data mismatch [%d]=%u\n", i, vaddr[i]);
+			err = -EINVAL;
+			goto out_import;
+		}
+	}
+
 	/* Now try a fake an importer */
 	import_attach = dma_buf_attach(dmabuf, obj->base.dev->dev);
 	if (IS_ERR(import_attach)) {
-- 
2.37.3

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf
URL   : https://patchwork.freedesktop.org/series/110229/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:180:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:186:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:188:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:192:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:237:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:239:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:55:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:57:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:58:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:15: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:73:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:75:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:76:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:77:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:79:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:80:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:93:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:95:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:96:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:97:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:99:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:21: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/instrumented-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:112:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:115:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:127:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:130:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:139:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:142:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:26:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:42:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:58:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:97:1: warning: unreplaced symbol 'return'

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7813 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf
URL   : https://patchwork.freedesktop.org/series/110229/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12311 -> Patchwork_110229v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_110229v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_110229v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/index.html

Participating hosts (43 -> 41)
------------------------------

  Missing    (2): fi-ctg-p8600 fi-icl-u2 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_110229v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@dmabuf:
    - fi-hsw-g3258:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-hsw-g3258/igt@i915_selftest@live@dmabuf.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-hsw-g3258/igt@i915_selftest@live@dmabuf.html
    - fi-hsw-4770:        [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-hsw-4770/igt@i915_selftest@live@dmabuf.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-hsw-4770/igt@i915_selftest@live@dmabuf.html
    - fi-ivb-3770:        [PASS][5] -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-ivb-3770/igt@i915_selftest@live@dmabuf.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-ivb-3770/igt@i915_selftest@live@dmabuf.html
    - fi-elk-e7500:       [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-elk-e7500/igt@i915_selftest@live@dmabuf.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-elk-e7500/igt@i915_selftest@live@dmabuf.html
    - fi-pnv-d510:        [PASS][9] -> [INCOMPLETE][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-pnv-d510/igt@i915_selftest@live@dmabuf.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-pnv-d510/igt@i915_selftest@live@dmabuf.html
    - fi-ilk-650:         [PASS][11] -> [INCOMPLETE][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-ilk-650/igt@i915_selftest@live@dmabuf.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-ilk-650/igt@i915_selftest@live@dmabuf.html
    - fi-blb-e6850:       [PASS][13] -> [INCOMPLETE][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-blb-e6850/igt@i915_selftest@live@dmabuf.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-blb-e6850/igt@i915_selftest@live@dmabuf.html
    - fi-snb-2520m:       [PASS][15] -> [INCOMPLETE][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-snb-2520m/igt@i915_selftest@live@dmabuf.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-snb-2520m/igt@i915_selftest@live@dmabuf.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-kbl-soraka:      [PASS][17] -> [INCOMPLETE][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-kbl-soraka/igt@i915_selftest@live@gem_contexts.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-kbl-soraka/igt@i915_selftest@live@gem_contexts.html

  
Known issues
------------

  Here are the changes found in Patchwork_110229v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [PASS][19] -> [DMESG-FAIL][20] ([i915#5334])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
    - fi-kbl-soraka:      [PASS][21] -> [DMESG-FAIL][22] ([i915#5334])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-bdw-5557u:       [PASS][23] -> [INCOMPLETE][24] ([i915#146])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/fi-bdw-5557u/igt@i915_suspend@basic-s3-without-i915.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-bdw-5557u/igt@i915_suspend@basic-s3-without-i915.html

  * igt@runner@aborted:
    - fi-ivb-3770:        NOTRUN -> [FAIL][25] ([fdo#109271] / [i915#4312])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-ivb-3770/igt@runner@aborted.html
    - fi-elk-e7500:       NOTRUN -> [FAIL][26] ([fdo#109271] / [i915#4312])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-elk-e7500/igt@runner@aborted.html
    - fi-pnv-d510:        NOTRUN -> [FAIL][27] ([fdo#109271] / [i915#4312])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-pnv-d510/igt@runner@aborted.html
    - fi-ilk-650:         NOTRUN -> [FAIL][28] ([fdo#109271] / [i915#4312])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-ilk-650/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][29] ([fdo#109271] / [i915#4312])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-blb-e6850/igt@runner@aborted.html
    - fi-snb-2520m:       NOTRUN -> [FAIL][30] ([i915#4312])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-snb-2520m/igt@runner@aborted.html
    - fi-hsw-g3258:       NOTRUN -> [FAIL][31] ([fdo#109271] / [i915#4312] / [i915#4991])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/fi-hsw-g3258/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@migrate:
    - {bat-dg2-11}:       [DMESG-WARN][32] -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12311/bat-dg2-11/igt@i915_selftest@live@migrate.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/bat-dg2-11/igt@i915_selftest@live@migrate.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#4991]: https://gitlab.freedesktop.org/drm/intel/issues/4991
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#7029]: https://gitlab.freedesktop.org/drm/intel/issues/7029


Build changes
-------------

  * Linux: CI_DRM_12311 -> Patchwork_110229v1

  CI-20190529: 20190529
  CI_DRM_12311: 7e39cc6b9ee11e867ec5bd4910b65cefa7c644cc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7028: 9e635a1c502970e7e6d64112d409392a2f01c688 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_110229v1: 7e39cc6b9ee11e867ec5bd4910b65cefa7c644cc @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

25f5255cdc49 drm/i915/selftests: exercise GPU access from the importer
459a8583583e drm/i915/dmabuf: fix sg_table handling in map_dma_buf

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110229v1/index.html

[-- Attachment #2: Type: text/html, Size: 9445 bytes --]

Re: [Intel-gfx] [PATCH 1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf

[Ruhl, Michael J]

>-----Original Message-----
>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>Matthew Auld
>Sent: Thursday, October 27, 2022 11:27 AM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH 1/2] drm/i915/dmabuf: fix sg_table handling in
>map_dma_buf
>
>We need to iterate over the original entries here for the sg_table,
>pulling out the struct page for each one, to be remapped. However
>currently this incorrectly iterates over the final dma mapped entries,
>which is likely just one gigantic sg entry if the iommu is enabled,
>leading to us only mapping the first struct page (and any physically
>contiguous pages following it), even if there is potentially lots more
>data to follow.
>
>Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7306
>Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>index 07eee1c09aaf..05ebbdfd3b3b 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>@@ -40,13 +40,13 @@ static struct sg_table *i915_gem_map_dma_buf(struct
>dma_buf_attachment *attachme
> 		goto err;
> 	}
>
>-	ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);
>+	ret = sg_alloc_table(st, obj->mm.pages->orig_nents, GFP_KERNEL);
> 	if (ret)
> 		goto err_free;
>
> 	src = obj->mm.pages->sgl;
> 	dst = st->sgl;
>-	for (i = 0; i < obj->mm.pages->nents; i++) {
>+	for (i = 0; i < obj->mm.pages->orig_nents; i++) {

This really should use the for_each_sg() macro.

I proposed a clean up patch a while back that looked like this:

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index e2cdc2640c08..ccc5d46aa749 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -7,6 +7,7 @@
 #include <linux/dma-buf.h>
 #include <linux/highmem.h>
 #include <linux/dma-resv.h>
+#include <linux/scatterlist.h>
 
 #include "gem/i915_gem_dmabuf.h"
 #include "i915_drv.h"
@@ -41,12 +42,10 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attach,
        if (ret)
                goto err_free;
 
-       src = obj->mm.pages->sgl;
        dst = sgt->sgl;
-       for (i = 0; i < obj->mm.pages->nents; i++) {
+       for_each_sg(obj->mm.pages->sgl, src, obj->mm.pages->nents, i) {
                sg_set_page(dst, sg_page(src), src->length, 0);
                dst = sg_next(dst);
-               src = sg_next(src);
        }

If you are updating the for loop, this might be a reasonable update as well.

Mike

> 		sg_set_page(dst, sg_page(src), src->length, 0);
> 		dst = sg_next(dst);
> 		src = sg_next(src);
>--
>2.37.3

[Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf (rev2)
URL   : https://patchwork.freedesktop.org/series/110229/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/110229/revisions/2/mbox/ not applied
Applying: drm/i915/dmabuf: fix sg_table handling in map_dma_buf
error: corrupt patch at line 26
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/dmabuf: fix sg_table handling in map_dma_buf
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [Intel-gfx] [PATCH 1/2] drm/i915/dmabuf: fix sg_table handling in map_dma_buf

[Matthew Auld]

On 28/10/2022 14:55, Ruhl, Michael J wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Matthew Auld
>> Sent: Thursday, October 27, 2022 11:27 AM
>> To: intel-gfx@lists.freedesktop.org
>> Subject: [Intel-gfx] [PATCH 1/2] drm/i915/dmabuf: fix sg_table handling in
>> map_dma_buf
>>
>> We need to iterate over the original entries here for the sg_table,
>> pulling out the struct page for each one, to be remapped. However
>> currently this incorrectly iterates over the final dma mapped entries,
>> which is likely just one gigantic sg entry if the iommu is enabled,
>> leading to us only mapping the first struct page (and any physically
>> contiguous pages following it), even if there is potentially lots more
>> data to follow.
>>
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7306
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> index 07eee1c09aaf..05ebbdfd3b3b 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> @@ -40,13 +40,13 @@ static struct sg_table *i915_gem_map_dma_buf(struct
>> dma_buf_attachment *attachme
>> 		goto err;
>> 	}
>>
>> -	ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);
>> +	ret = sg_alloc_table(st, obj->mm.pages->orig_nents, GFP_KERNEL);
>> 	if (ret)
>> 		goto err_free;
>>
>> 	src = obj->mm.pages->sgl;
>> 	dst = st->sgl;
>> -	for (i = 0; i < obj->mm.pages->nents; i++) {
>> +	for (i = 0; i < obj->mm.pages->orig_nents; i++) {
> 
> This really should use the for_each_sg() macro.
> 
> I proposed a clean up patch a while back that looked like this:
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> index e2cdc2640c08..ccc5d46aa749 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> @@ -7,6 +7,7 @@
>   #include <linux/dma-buf.h>
>   #include <linux/highmem.h>
>   #include <linux/dma-resv.h>
> +#include <linux/scatterlist.h>
>   
>   #include "gem/i915_gem_dmabuf.h"
>   #include "i915_drv.h"
> @@ -41,12 +42,10 @@ static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attach,
>          if (ret)
>                  goto err_free;
>   
> -       src = obj->mm.pages->sgl;
>          dst = sgt->sgl;
> -       for (i = 0; i < obj->mm.pages->nents; i++) {
> +       for_each_sg(obj->mm.pages->sgl, src, obj->mm.pages->nents, i) {
>                  sg_set_page(dst, sg_page(src), src->length, 0);
>                  dst = sg_next(dst);
> -               src = sg_next(src);
>          }
> 
> If you are updating the for loop, this might be a reasonable update as well.

Ok, but such cleanups should normally be a separate patch. I'll grab 
your series and bolt that onto this one.

> 
> Mike
> 
>> 		sg_set_page(dst, sg_page(src), src->length, 0);
>> 		dst = sg_next(dst);
>> 		src = sg_next(src);
>> --
>> 2.37.3
>