>-----Original Message----- >From: Auld, Matthew <matthew.auld@xxxxxxxxx> >Sent: Friday, October 28, 2022 11:50 AM >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Landwerlin, Lionel G <lionel.g.landwerlin@xxxxxxxxx>; Tvrtko Ursulin ><tvrtko.ursulin@xxxxxxxxxxxxxxx>; Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>; >Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx> >Subject: [PATCH v2 2/4] drm/i915/selftests: exercise GPU access from the >importer > >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. > >v2: Switch over to igt_gpu_fill_dw(), which looks to be more widely >supported than the migrate stuff (at least OOTB). > >References: https://gitlab.freedesktop.org/drm/intel/-/issues/7306 >Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx> >Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> >Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> >Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >Cc: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx> >--- > .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 79 >++++++++++++++++++- > 1 file changed, 78 insertions(+), 1 deletion(-) > >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..e57f9390076c 100644 >--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c >+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c >@@ -6,8 +6,12 @@ > > #include "i915_drv.h" > #include "i915_selftest.h" >+#include "gem/i915_gem_context.h" > >+#include "mock_context.h" > #include "mock_dmabuf.h" >+#include "igt_gem_utils.h" >+#include "selftests/mock_drm.h" > #include "selftests/mock_gem_device.h" > > static int igt_dmabuf_export(void *arg) >@@ -140,6 +144,75 @@ static int >igt_dmabuf_import_same_driver_lmem(void *arg) > return err; > } > >+static int verify_access(struct drm_i915_private *i915, >+ struct drm_i915_gem_object *native_obj, >+ struct drm_i915_gem_object *import_obj) >+{ >+ struct i915_gem_engines_iter it; >+ struct i915_gem_context *ctx; >+ struct intel_context *ce; >+ struct i915_vma *vma; >+ struct file *file; >+ u32 *vaddr; >+ int err = 0, i; >+ >+ file = mock_file(i915); >+ if (IS_ERR(file)) >+ return PTR_ERR(file); >+ >+ ctx = live_context(i915, file); >+ if (IS_ERR(ctx)) { >+ err = PTR_ERR(ctx); >+ goto out_file; >+ } >+ >+ for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { >+ if (intel_engine_can_store_dword(ce->engine)) >+ break; >+ } >+ i915_gem_context_unlock_engines(ctx); >+ if (!ce) >+ goto out_file; >+ >+ vma = i915_vma_instance(import_obj, ce->vm, NULL); >+ if (IS_ERR(vma)) { >+ err = PTR_ERR(vma); >+ goto out_file; >+ } >+ >+ err = i915_vma_pin(vma, 0, 0, PIN_USER); >+ if (err) >+ goto out_file; >+ >+ err = igt_gpu_fill_dw(ce, vma, 0, >+ vma->size >> PAGE_SHIFT, 0xdeadbeaf); >+ i915_vma_unpin(vma); >+ if (err) >+ goto out_file; >+ >+ err = i915_gem_object_wait(import_obj, 0, >MAX_SCHEDULE_TIMEOUT); >+ if (err) >+ goto out_file; >+ >+ vaddr = i915_gem_object_pin_map_unlocked(native_obj, >I915_MAP_WB); >+ if (IS_ERR(vaddr)) { >+ err = PTR_ERR(vaddr); >+ goto out_file; >+ } >+ >+ for (i = 0; i < native_obj->base.size / sizeof(u32); i += PAGE_SIZE / >sizeof(u32)) { >+ if (vaddr[i] != 0xdeadbeaf) { >+ pr_err("Data mismatch [%d]=%u\n", i, vaddr[i]); >+ err = -EINVAL; >+ goto out_file; >+ } Not sure what timing issues are related to this test, but this loop could have some impact (takes a long time, assuming the object is LMEM). Would checking the beginning, middle and end of each page be any less beneficial than the current check? Not a suggested change, but just a thought to ponder if the timing becomes an issue... This test looks reasonable to me. Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx> M >+ } >+ >+out_file: >+ fput(file); >+ return err; >+} >+ > static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915, > struct intel_memory_region >**regions, > unsigned int num_regions) >@@ -154,7 +227,7 @@ static int igt_dmabuf_import_same_driver(struct >drm_i915_private *i915, > > 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", >@@ -206,6 +279,10 @@ static int igt_dmabuf_import_same_driver(struct >drm_i915_private *i915, > > i915_gem_object_unlock(import_obj); > >+ err = verify_access(i915, obj, import_obj); >+ if (err) >+ 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
