| Message ID | 20190322092155.1656-15-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,01/24] i915/gem_exec_latency: Measure the latency of context switching | expand |
On 22/03/2019 09:21, Chris Wilson wrote: > Exercise basic creation and swapping between new address spaces. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > 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
Quoting Tvrtko Ursulin (2019-03-26 11:21:13) > > On 22/03/2019 09:21, Chris Wilson wrote: > > +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? It's documented as an out parameter, so meh. > Otherwise both tests here are not invalid input/behaviour. First is also > covered by has_vm. First test is with valid input. Second test checks that we fill the vm_id with something recognisable by the kernel. > > + 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. Can't loop as there are no valid extensions (yet), so it gets rejected by -EINVAL for unrecognised name. > > +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? That would be inconsistent, so no. > > + 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. It was more what I was planning to do. But yes, the assumption is that only one id is valid. > > + 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. Redundant with what? > > + 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. Misleading? It's a spot has a good chance of being clear on every platform. It's almost as if I just copied this from when I was thinking about GGTT. > > + 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? Right, this is just an unassuming valid path, checking that we follow the batch offset around, and explodes if the kernel forgets to actually update the PD inside the context image before execution. Not that happened. > > + 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); > > +}
On 26/03/2019 11:37, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-03-26 11:21:13) >> >> On 22/03/2019 09:21, Chris Wilson wrote: >>> +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? > > It's documented as an out parameter, so meh. > >> Otherwise both tests here are not invalid input/behaviour. First is also >> covered by has_vm. > > First test is with valid input. Second test checks that we fill the > vm_id with something recognisable by the kernel. > >>> + 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. > > Can't loop as there are no valid extensions (yet), so it gets rejected by > -EINVAL for unrecognised name. > >>> +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? > > That would be inconsistent, so no. Ok. >>> + 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. > > It was more what I was planning to do. But yes, the assumption is that > only one id is valid. > >>> + 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. > > Redundant with what? Forget I said it. >>> + 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. > > Misleading? It's a spot has a good chance of being clear on every > platform. It's almost as if I just copied this from when I was thinking > about GGTT. What is special about 48Mib? Apart that reminds me of 48-bit ppgtt? :) Regards, Tvrtko >>> + 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? > > Right, this is just an unassuming valid path, checking that we follow > the batch offset around, and explodes if the kernel forgets to actually > update the PD inside the context image before execution. > > Not that happened. > >>> + 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); >>> +} >
Quoting Tvrtko Ursulin (2019-03-26 11:48:10) > > On 26/03/2019 11:37, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2019-03-26 11:21:13) > >> > >> On 22/03/2019 09:21, Chris Wilson wrote: > >>> +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? > > > > It's documented as an out parameter, so meh. > > > >> Otherwise both tests here are not invalid input/behaviour. First is also > >> covered by has_vm. > > > > First test is with valid input. Second test checks that we fill the > > vm_id with something recognisable by the kernel. > > > >>> + 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. > > > > Can't loop as there are no valid extensions (yet), so it gets rejected by > > -EINVAL for unrecognised name. > > > >>> +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? > > > > That would be inconsistent, so no. > > Ok. > > >>> + 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. > > > > It was more what I was planning to do. But yes, the assumption is that > > only one id is valid. > > > >>> + 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. > > > > Redundant with what? > > Forget I said it. > > >>> + 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. > > > > Misleading? It's a spot has a good chance of being clear on every > > platform. It's almost as if I just copied this from when I was thinking > > about GGTT. > > What is special about 48Mib? Apart that reminds me of 48-bit ppgtt? :) Smallest aperture is 64MiB. Assume around 10MiB get consumed by framebuffer and ringbuffers; leaving another 10MiB for a second framebuffer at some pointer causing some fragmentation, so call it 20MiB. So anything in first 30MiB or so could be allocated on an idle system after, so 48MiB seemed like a reasonable guess to avoid a pinned framebuffer. Being lazy is hard work. -Chris
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 + * 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 \ 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) +{ + 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; + + 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; +} + +static void invalid_destroy(int i915) +{ + struct drm_i915_gem_vm_control ctl = {}; + + igt_assert_eq(__gem_vm_destroy_local(i915, &ctl), -ENOENT); + + 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 */ + 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); + + 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); +} + +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; + 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); + 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',
Exercise basic creation and swapping between new address spaces. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- 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