On 30/03/2017 18:25, Chris Wilson wrote:
On Thu, Mar 30, 2017 at 05:58:09PM +0100, Tvrtko Ursulin wrote:From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Stolen never materialized so convert this test into checking that the garbage in padding remains legal. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> --- tests/gem_create.c | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/tests/gem_create.c b/tests/gem_create.c index 21df13f7b655..0dad06c784c5 100644 --- a/tests/gem_create.c +++ b/tests/gem_create.c @@ -27,8 +27,7 @@ /** @file gem_create.c * - * This is a test for the extended and old gem_create ioctl, that - * includes allocation of object from stolen memory and shmem. + * This is a test for the gem_create ioctl. * * The goal is to simply ensure that basics work and invalid input * combinations are rejected. @@ -62,36 +61,22 @@ IGT_TEST_DESCRIPTION("This is a test for the extended & old gem_create ioctl," " that includes allocation of object from stolen memory" " and shmem."); -#define CLEAR(s) memset(&s, 0, sizeof(s)) #define PAGE_SIZE 4096 -struct local_i915_gem_create_v2 { - uint64_t size; - uint32_t handle; - uint32_t pad; -#define I915_CREATE_PLACEMENT_STOLEN (1<<0) - uint32_t flags; -} create; - -#define LOCAL_IOCTL_I915_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_CREATE, struct local_i915_gem_create_v2) - -static void invalid_flag_test(int fd) +/* + * Verify that historical omission of checking for garbage in the padding + * field still holds. + */ +static void test_pad_garbage(int fd) { + struct drm_i915_gem_create create = { }; int ret; - gem_require_stolen_support(fd); - create.handle = 0; create.size = PAGE_SIZE; - create.flags = ~I915_CREATE_PLACEMENT_STOLEN; - ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, &create); - - igt_assert(ret <= 0); - - create.flags = ~0; - ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, &create); - - igt_assert(ret <= 0); + create.pad = 1; + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create); + igt_assert_eq(ret, 0); } static void invalid_size_test(int fd) @@ -134,8 +119,8 @@ igt_main fd = drm_open_driver(DRIVER_INTEL); } - igt_subtest("stolen-invalid-flag") - invalid_flag_test(fd); + igt_subtest("basic-pad-garbage") + test_pad_garbage(fd);Not basic though. I just dislike having negative tests that we intend to break be part of basic. (I dislike negative tests in general as they are restrictive and limit creativity, a false limitation in terms of ABI.)
My understanding is that we can never break this now. I mean that we have to allow userspace putting garbage in the padding field forever now.
I don't fully understand right now then how createv2 implementation was suggesting to re-purpose half of the padding for flags at the moment as well.
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
