>-----Original Message-----
>From: Auld, Matthew <matthew.a...@intel.com>
>Sent: Friday, October 28, 2022 11:50 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Landwerlin, Lionel G <lionel.g.landwer...@intel.com>; Tvrtko Ursulin
><tvrtko.ursu...@linux.intel.com>; Ville Syrjälä
><ville.syrj...@linux.intel.com>;
>Ruhl, Michael J <michael.j.r...@intel.com>
>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.a...@intel.com>
>Cc: Lionel Landwerlin <lionel.g.landwer...@intel.com>
>Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
>Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
>Cc: Michael J. Ruhl <michael.j.r...@intel.com>
>---
> .../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.r...@intel.com>
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
