Re: [Intel-gfx] [RFC PATCH i-g-t 1/1] tests/gem_mmap_offset: Exercise mapping to userptr
On Friday, January 31, 2020 3:37:05 PM CET Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2020-01-31 13:12:34)
> > Creating a mapping to a userptr backed GEM object may cause a currently
> > unavoidable lockdep splat inside the i915 driver. Then, such operation
> > is expected to fail to prevent from that badness to happen.
> >
> > Add a respective subtest for each mapping type.
> >
> > Signed-off-by: Janusz Krzysztofik
> > Cc: Matthew Auld
> > Cc: Chris Wilson
> > ---
> > tests/i915/gem_mmap_offset.c | 55
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
> > index 7c4088cdf..a5f28328b 100644
> > --- a/tests/i915/gem_mmap_offset.c
> > +++ b/tests/i915/gem_mmap_offset.c
> > @@ -141,6 +141,36 @@ static void bad_extensions(int i915)
> > gem_close(i915, arg.handle);
> > }
> >
> > +static bool has_userptr(int i915)
> > +{
> > + uint32_t handle = 0;
> > + void *ptr;
> > +
> > + igt_assert_eq(posix_memalign(, 4096, 4096), 0);
> > + if (__gem_userptr(i915, ptr, 4096, 0, 0, ) == 0)
> > + gem_close(i915, handle);
> > + free(ptr);
> > +
> > + return handle;
> > +}
> > +
> > +static void userptr(int i915, uint64_t flags)
> > +{
> > + struct drm_i915_gem_mmap_offset arg = {
> > + .flags = flags,
> > + };
> > + void *ptr;
> > +
> > + igt_assert_eq(posix_memalign(, 4096, 4096), 0);
> > +
> > + gem_userptr(i915, ptr, 4096, 0, 0, );
> > +
> > + igt_assert_eq(mmap_offset_ioctl(i915, ), -EINVAL);
>
> Not quite. The only reason it doesn't work is because the implementation
> ties itself into knots, not that it is meant to not work. :|
Are you suggesting the test should fail if the IOCTL fails? That would be
fair, but how are we going to have that accepted by CI then, and merged?
Skipping also doesn't seem a good option to me.
I can add some more exhaustive examination should the IOCTL succeed, but that
won't help to make CI happy.
>From your long experience, what approach should we take?
Thanks,
Janusz
> -Chris
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH i-g-t 1/1] tests/gem_mmap_offset: Exercise mapping to userptr
Quoting Janusz Krzysztofik (2020-01-31 13:12:34)
> Creating a mapping to a userptr backed GEM object may cause a currently
> unavoidable lockdep splat inside the i915 driver. Then, such operation
> is expected to fail to prevent from that badness to happen.
>
> Add a respective subtest for each mapping type.
>
> Signed-off-by: Janusz Krzysztofik
> Cc: Matthew Auld
> Cc: Chris Wilson
> ---
> tests/i915/gem_mmap_offset.c | 55
> 1 file changed, 55 insertions(+)
>
> diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
> index 7c4088cdf..a5f28328b 100644
> --- a/tests/i915/gem_mmap_offset.c
> +++ b/tests/i915/gem_mmap_offset.c
> @@ -141,6 +141,36 @@ static void bad_extensions(int i915)
> gem_close(i915, arg.handle);
> }
>
> +static bool has_userptr(int i915)
> +{
> + uint32_t handle = 0;
> + void *ptr;
> +
> + igt_assert_eq(posix_memalign(, 4096, 4096), 0);
> + if (__gem_userptr(i915, ptr, 4096, 0, 0, ) == 0)
> + gem_close(i915, handle);
> + free(ptr);
> +
> + return handle;
> +}
> +
> +static void userptr(int i915, uint64_t flags)
> +{
> + struct drm_i915_gem_mmap_offset arg = {
> + .flags = flags,
> + };
> + void *ptr;
> +
> + igt_assert_eq(posix_memalign(, 4096, 4096), 0);
> +
> + gem_userptr(i915, ptr, 4096, 0, 0, );
> +
> + igt_assert_eq(mmap_offset_ioctl(i915, ), -EINVAL);
Not quite. The only reason it doesn't work is because the implementation
ties itself into knots, not that it is meant to not work. :|
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH i-g-t 1/1] tests/gem_mmap_offset: Exercise mapping to userptr
Creating a mapping to a userptr backed GEM object may cause a currently
unavoidable lockdep splat inside the i915 driver. Then, such operation
is expected to fail to prevent from that badness to happen.
Add a respective subtest for each mapping type.
Signed-off-by: Janusz Krzysztofik
Cc: Matthew Auld
Cc: Chris Wilson
---
tests/i915/gem_mmap_offset.c | 55
1 file changed, 55 insertions(+)
diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
index 7c4088cdf..a5f28328b 100644
--- a/tests/i915/gem_mmap_offset.c
+++ b/tests/i915/gem_mmap_offset.c
@@ -141,6 +141,36 @@ static void bad_extensions(int i915)
gem_close(i915, arg.handle);
}
+static bool has_userptr(int i915)
+{
+ uint32_t handle = 0;
+ void *ptr;
+
+ igt_assert_eq(posix_memalign(, 4096, 4096), 0);
+ if (__gem_userptr(i915, ptr, 4096, 0, 0, ) == 0)
+ gem_close(i915, handle);
+ free(ptr);
+
+ return handle;
+}
+
+static void userptr(int i915, uint64_t flags)
+{
+ struct drm_i915_gem_mmap_offset arg = {
+ .flags = flags,
+ };
+ void *ptr;
+
+ igt_assert_eq(posix_memalign(, 4096, 4096), 0);
+
+ gem_userptr(i915, ptr, 4096, 0, 0, );
+
+ igt_assert_eq(mmap_offset_ioctl(i915, ), -EINVAL);
+
+ gem_close(i915, arg.handle);
+ free(ptr);
+}
+
static void basic_uaf(int i915)
{
const uint32_t obj_size = 4096;
@@ -461,6 +491,31 @@ igt_main
igt_subtest_f("bad-extensions")
bad_extensions(i915);
+ igt_subtest_group {
+ igt_fixture
+ igt_require(has_userptr(i915));
+
+ for_each_mmap_offset_type(t) {
+ igt_describe_f("Verify %s mapping to userptr backed GEM
object will fail",
+ t->name);
+ igt_subtest_f("userptr-%s-mapping", t->name) {
+ switch (t->type) {
+ case I915_MMAP_OFFSET_GTT:
+ gem_require_mappable_ggtt(i915);
+ break;
+ case I915_MMAP_OFFSET_WC:
+ case I915_MMAP_OFFSET_UC:
+
igt_require(gem_mmap_offset__has_wc(i915));
+ break;
+ defalut:
+ break;
+ }
+
+ userptr(i915, t->type);
+ }
+ }
+ }
+
igt_describe("Check buffer object mapping persists after gem_close");
igt_subtest_f("basic-uaf")
basic_uaf(i915);
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
