On 22/03/2019 09:21, Chris Wilson wrote:
Exercise basic creation and swapping between new address spaces.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
lib/Makefile.sources | 2 +
lib/i915/gem_vm.c | 130 ++++++++++++++++++++
lib/i915/gem_vm.h | 38 ++++++
lib/meson.build | 1 +
tests/Makefile.sources | 1 +
tests/i915/gem_vm_create.c | 236 +++++++++++++++++++++++++++++++++++++
tests/meson.build | 1 +
7 files changed, 409 insertions(+)
create mode 100644 lib/i915/gem_vm.c
create mode 100644 lib/i915/gem_vm.h
create mode 100644 tests/i915/gem_vm_create.c
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index e00347f94..a7074209a 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -13,6 +13,8 @@ lib_source_list = \
i915/gem_ring.c \
i915/gem_mman.c \
i915/gem_mman.h \
+ i915/gem_vm.c \
+ i915/gem_vm.h \
i915_3d.h \
i915_reg.h \
i915_pciids.h \
diff --git a/lib/i915/gem_vm.c b/lib/i915/gem_vm.c
new file mode 100644
index 000000000..e0d46d509
--- /dev/null
+++ b/lib/i915/gem_vm.c
@@ -0,0 +1,130 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <errno.h>
+#include <string.h>
+
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+
+#include "i915/gem_vm.h"
+
+/**
+ * SECTION:gem_vm
+ * @short_description: Helpers for dealing with address spaces (vm/GTT)
+ * @title: GEM Virtual Memory
+ *
+ * This helper library contains functions used for handling gem address
+ * spaces..
+ */
+
+/**
+ * gem_has_vm:
+ * @i915: open i915 drm file descriptor
+ *
+ * Returns: whether VM creation is supported or not.
+ */
+bool gem_has_vm(int i915)
+{
+ uint32_t vm_id = 0;
+
+ __gem_vm_create(i915, &vm_id);
+ if (vm_id)
+ gem_vm_destroy(i915, vm_id);
+
+ return vm_id;
+}
+
+/**
+ * gem_require_vm:
+ * @i915: open i915 drm file descriptor
+ *
+ * This helper will automatically skip the test on platforms where address
+ * space creation is not available.
+ */
+void gem_require_vm(int i915)
+{
+ igt_require(gem_has_vm(i915));
+}
+
+int __gem_vm_create(int i915, uint32_t *vm_id)
+{
+ struct drm_i915_gem_vm_control ctl = {};
+ int err = 0;
+
+ if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_VM_CREATE, &ctl) == 0) {
+ *vm_id = ctl.vm_id;
+ } else {
+ err = -errno;
+ igt_assume(err != 0);
+ }
+
+ errno = 0;
+ return err;
+}
+
+/**
+ * gem_vm_create:
+ * @i915: open i915 drm file descriptor
+ *
+ * This wraps the VM_CREATE ioctl, which is used to allocate a new
+ * vm_set_caching() this wrapper skips on
auto-complete? :)
+ * kernels and platforms where address space support is not available.
+ *
+ * Returns: The id of the allocated address space.
+ */
+uint32_t gem_vm_create(int i915)
+{
+ uint32_t vm_id;
+
+ igt_assert_eq(__gem_vm_create(i915, &vm_id), 0);
+ igt_assert(vm_id != 0);
+
+ return vm_id;
+}
+
+int __gem_vm_destroy(int i915, uint32_t vm_id)
+{
+ struct drm_i915_gem_vm_control ctl = { .vm_id = vm_id };
+ int err = 0;
+
+ if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_VM_DESTROY, &ctl)) {
+ err = -errno;
+ igt_assume(err);
+ }
+
+ errno = 0;
+ return err;
+}
+
+/**
+ * gem_vm_destroy:
+ * @i915: open i915 drm file descriptor
+ * @vm_id: i915 VM id
+ *
+ * This wraps the VM_DESTROY ioctl, which is used to free an address space.
+ */
+void gem_vm_destroy(int i915, uint32_t vm_id)
+{
+ igt_assert_eq(__gem_vm_destroy(i915, vm_id), 0);
+}
diff --git a/lib/i915/gem_vm.h b/lib/i915/gem_vm.h
new file mode 100644
index 000000000..27af899d4
--- /dev/null
+++ b/lib/i915/gem_vm.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef GEM_VM_H
+#define GEM_VM_H
+
+#include <stdint.h>
+
+bool gem_has_vm(int i915);
+void gem_require_vm(int i915);
+
+uint32_t gem_vm_create(int i915);
+int __gem_vm_create(int i915, uint32_t *vm_id);
+
+void gem_vm_destroy(int i915, uint32_t vm_id);
+int __gem_vm_destroy(int i915, uint32_t vm_id);
+
+#endif /* GEM_VM_H */
diff --git a/lib/meson.build b/lib/meson.build
index 89de06e69..f95922330 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -5,6 +5,7 @@ lib_sources = [
'i915/gem_submission.c',
'i915/gem_ring.c',
'i915/gem_mman.c',
+ 'i915/gem_vm.c',
'igt_color_encoding.c',
'igt_debugfs.c',
'igt_device.c',
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 71ccf00af..809b25612 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -21,6 +21,7 @@ TESTS_progs = \
drm_import_export \
drm_mm \
drm_read \
+ i915/gem_vm_create \
It looked like strange placement at first and then I opened the file and
got reminded of the horror.
kms_3d \
kms_addfb_basic \
kms_atomic \
diff --git a/tests/i915/gem_vm_create.c b/tests/i915/gem_vm_create.c
new file mode 100644
index 000000000..0264fa301
--- /dev/null
+++ b/tests/i915/gem_vm_create.c
@@ -0,0 +1,236 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include "igt_dummyload.h"
+#include "i915/gem_vm.h"
+
+static int __gem_vm_create_local(int i915, struct drm_i915_gem_vm_control *ctl)
Bikeshedding only, but local suffix scared me you are using local ioctl
definitions. On closer look, since functions are local, you could afford
to use names like vm_create/vm_destroy.
+{
+ int err = 0;
+ if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_VM_CREATE, ctl)) {
+ err = -errno;
+ igt_assume(err);
+ }
+ errno = 0;
+ return err;
+}
+
+static int __gem_vm_destroy_local(int i915, struct drm_i915_gem_vm_control *ctl)
+{
+ int err = 0;
+ if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_VM_DESTROY, ctl)) {
+ err = -errno;
+ igt_assume(err);
+ }
+ errno = 0;
+ return err;
+}
+
+static bool has_vm(int i915)
+{
+ struct drm_i915_gem_vm_control ctl = {};
+ int err;
+
+ err = __gem_vm_create_local(i915, &ctl);
+ switch (err) {
+ case -EINVAL: /* unknown ioctl */
+ case -ENODEV: /* !full-ppgtt */
+ return false;
+
+ case 0:
+ gem_vm_destroy(i915, ctl.vm_id);
+ return true;
+
+ default:
+ igt_fail_on_f(err, "Unknown response from VM_CREATE\n");
+ return false;
+ }
+}
+
+static void invalid_create(int i915)
+{
+ struct drm_i915_gem_vm_control ctl = {};
+ struct i915_user_extension ext = { .name = -1 };
+
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), 0);
+ gem_vm_destroy(i915, ctl.vm_id);
+
+ ctl.vm_id = 0xdeadbeef;
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), 0);
+ gem_vm_destroy(i915, ctl.vm_id);
+ ctl.vm_id = 0;
Oh we allow garbage in.. hm.. perhaps disallow that?
Otherwise both tests here are not invalid input/behaviour. First is also
covered by has_vm.
+
+ ctl.flags = -1;
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), -EINVAL);
+ ctl.flags = 0;
+
+ ctl.extensions = -1;
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), -EFAULT);
+ ctl.extensions = to_user_pointer(&ext);
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), -EINVAL);
+ ctl.extensions = 0;
Could add a loop rejection test as well, even though the underlying
implementation is shared.
+}
+
+static void invalid_destroy(int i915)
+{
+ struct drm_i915_gem_vm_control ctl = {};
+
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), -ENOENT);
Should we have id 0 be -EINVAL?
+
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), 0);
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), 0);
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), -ENOENT);
+
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), 0);
+ ctl.vm_id = ctl.vm_id + 1; /* XXX assume cyclic allocator */
I think this actually assumes, and correctly so, that the fd is clean
ie. only the created vm_id has been allocated. So comment needs to be
corrected I think.
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), -ENOENT);
+ ctl.vm_id = ctl.vm_id - 1;
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), 0);
+
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), 0);
+ ctl.flags = -1;
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), -EINVAL);
+ ctl.flags = 0;
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), 0);
Second create-destroy is redundant but okay.
+
+ igt_assert_eq(__gem_vm_create_local(i915, &ctl), 0);
+ ctl.extensions = -1;
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), -EINVAL);
+ ctl.extensions = 0;
+ igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), 0);
+}
+
+static uint32_t __batch_create(int i915, uint32_t offset)
+{
+ const uint32_t bbe = MI_BATCH_BUFFER_END;
+ uint32_t handle;
+
+ handle = gem_create(i915, ALIGN(offset + 4, 4096));
+ gem_write(i915, handle, offset, &bbe, sizeof(bbe));
+
+ return handle;
+}
+
+static uint32_t batch_create(int i915)
+{
+ return __batch_create(i915, 0);
+}
Oi! :D
+
+static void execbuf(int i915)
+{
+ struct drm_i915_gem_exec_object2 batch = {
+ .handle = batch_create(i915),
+ };
+ struct drm_i915_gem_execbuffer2 eb = {
+ .buffers_ptr = to_user_pointer(&batch),
+ .buffer_count = 1,
+ };
+ struct drm_i915_gem_context_param arg = {
+ .param = I915_CONTEXT_PARAM_VM,
+ };
+
+ /* First verify that we try to use "softpinning" by default */
+ batch.offset = 48 << 20;
Choice of 48 is a bit misleading, but okay.
+ gem_execbuf(i915, &eb);
+ igt_assert_eq_u64(batch.offset, 48 << 20);
+
+ arg.value = gem_vm_create(i915);
+ gem_context_set_param(i915, &arg);
+ gem_execbuf(i915, &eb);
+ igt_assert_eq_u64(batch.offset, 48 << 20);
Not sure what the offset check proves in this case? It's a new ppgtt, or
even if was old one. Seems it would never fail?
+ gem_vm_destroy(i915, arg.value);
+
+ arg.value = gem_vm_create(i915);
+ gem_context_set_param(i915, &arg);
+ batch.offset = 0;
+ gem_execbuf(i915, &eb);
+ igt_assert_eq_u64(batch.offset, 0);
+ gem_vm_destroy(i915, arg.value);
+
+ gem_sync(i915, batch.handle);
+ gem_close(i915, batch.handle);
+}
+
+static void async_destroy(int i915)
+{
+ struct drm_i915_gem_context_param arg = {
+ .ctx_id = gem_context_create(i915),
+ .value = gem_vm_create(i915),
+ .param = I915_CONTEXT_PARAM_VM,
+ };
+ igt_spin_t *spin[2];
+
+ spin[0] = igt_spin_batch_new(i915,
+ .ctx = arg.ctx_id,
+ .flags = IGT_SPIN_POLL_RUN);
+ igt_spin_busywait_until_running(spin[0]);
+
+ gem_context_set_param(i915, &arg);
+ spin[1] = __igt_spin_batch_new(i915, .ctx = arg.ctx_id);
+
+ igt_spin_batch_end(spin[0]);
+ gem_sync(i915, spin[0]->handle);
+
+ gem_vm_destroy(i915, arg.value);
+ gem_context_destroy(i915, arg.ctx_id);
+
+ igt_spin_batch_end(spin[1]);
+ gem_sync(i915, spin[1]->handle);
+
+ for (int i = 0; i < ARRAY_SIZE(spin); i++)
+ igt_spin_batch_free(i915, spin[i]);
+}
+
+igt_main
+{
+ int i915 = -1;
+
+ igt_fixture {
+ i915 = drm_open_driver(DRIVER_INTEL);
+ igt_require_gem(i915);
+ igt_require(has_vm(i915));
+ }
+
+ igt_subtest("invalid-create")
+ invalid_create(i915);
+
+ igt_subtest("invalid-destroy")
+ invalid_destroy(i915);
+
+ igt_subtest_group {
+ igt_fixture {
+ gem_context_require_param(i915, I915_CONTEXT_PARAM_VM);
+ }
+
+ igt_subtest("execbuf")
+ execbuf(i915);
+
+ igt_subtest("async-destroy")
+ async_destroy(i915);
+ }
+
+ igt_fixture {
+ close(i915);
+ }
+}
diff --git a/tests/meson.build b/tests/meson.build
index 9015f809e..949eefd6f 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -209,6 +209,7 @@ i915_progs = [
'gem_unfence_active_buffers',
'gem_unref_active_buffers',
'gem_userptr_blits',
+ 'gem_vm_create',
'gem_wait',
'gem_workarounds',
'gem_write_read_ring_switch',
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
