Re: [igt-dev] [PATCH i-g-t] tests/gem_create: Test unaligned BO size update

Chris Wilson
2019-03-26T16:48:52Z

Quoting Michał Winiarski (2019-03-26 16:32:56)
> The driver is supposed to update the size when it's trying to outsmart
> userspace. Let's test it, and tweak the __gem_create in order to control
> its input.
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> lib/ioctl_wrappers.c | 19 ++++++++----------
> lib/ioctl_wrappers.h | 2 +-
> tests/i915/gem_create.c | 35 +++++++++++++++++++++++++---------
> tests/i915/gem_fd_exhaustion.c | 11 ++++++-----
> 4 files changed, 41 insertions(+), 26 deletions(-)
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index a66eb4bc..bb387a27 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -541,21 +541,16 @@ uint32_t gem_create_stolen(int fd, uint64_t size)
> return create.handle;
> }
>
> -int __gem_create(int fd, uint64_t size, uint32_t *handle)
> +int __gem_create(int fd, struct drm_i915_gem_create *create)
> {
> - struct drm_i915_gem_create create = {
> - .size = size,
> - };
> int err = 0;
>
> - if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create) == 0) {
> - *handle = create.handle;
> - } else {
> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, create) != 0) {
> err = -errno;
> igt_assume(err != 0);
> }
> -
> errno = 0;
> +
> return err;
> }
>
> @@ -571,11 +566,13 @@ int __gem_create(int fd, uint64_t size, uint32_t *handle)
> */
> uint32_t gem_create(int fd, uint64_t size)
> {
> - uint32_t handle;
> + struct drm_i915_gem_create create = {
> + .size = size,
> + };
>
> - igt_assert_eq(__gem_create(fd, size, &handle), 0);
> + igt_assert_eq(__gem_create(fd, &create), 0);
>
> - return handle;
> + return create.handle;
> }
>
> /**
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index ad93daff..8202374f 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -77,7 +77,7 @@ void gem_sync(int fd, uint32_t handle);
> bool gem_create__has_stolen_support(int fd);
> uint32_t __gem_create_stolen(int fd, uint64_t size);
> uint32_t gem_create_stolen(int fd, uint64_t size);
> -int __gem_create(int fd, uint64_t size, uint32_t *handle);
> +int __gem_create(int fd, struct drm_i915_gem_create *create);
> uint32_t gem_create(int fd, uint64_t size);
> void gem_execbuf_wr(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
> int __gem_execbuf_wr(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
> diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
> index 2a861ca8..057816a5 100644
> --- a/tests/i915/gem_create.c
> +++ b/tests/i915/gem_create.c
> @@ -71,7 +71,7 @@ struct local_i915_gem_create_v2 {
> uint32_t pad;
> #define I915_CREATE_PLACEMENT_STOLEN (1<<0)
> uint32_t flags;
> -} create;
> +} create_v2;

So recently we started being more thorough in having a local ioctl
wrapper for the ioctl-specific test cases, so that we don't have to
contort the library functions too much.

I think a local

static int create_ioctl(int i915, struct drm_i915_gem_create *create)
{
int err = 0;

if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CREATE, create)) {
err = -errno;
igt_assume(err != 0);
}

errno = 0;
return err;
}

is my recommended practice.
-Chris