From patchwork Tue Aug 15 22:57:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 9902589 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 229B66028A for ; Tue, 15 Aug 2017 22:58:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1457F2894E for ; Tue, 15 Aug 2017 22:58:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0911428950; Tue, 15 Aug 2017 22:58:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3EEF62894C for ; Tue, 15 Aug 2017 22:58:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 656E66E38C; Tue, 15 Aug 2017 22:57:51 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id DC6EF6E388 for ; Tue, 15 Aug 2017 22:57:48 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 8273860-1500050 for multiple; Tue, 15 Aug 2017 23:57:40 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Tue, 15 Aug 2017 23:57:41 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 15 Aug 2017 23:57:40 +0100 Message-Id: <20170815225740.383-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.13.3 X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH igt] igt/gem_exec_params: Delete the circularly defined tests X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP For the set of execbuf flags who by definition are based on whether or not the kernel supports that feature, the ultimate arbiter of whether or not the kernel accepts the flag is the kernel. The negative tests were second guessing the kernel and not checking behaviour. Indeed, the tests failed quite spectacularly to spot a 5 year old bug in engine selection. The dubious invalid-flag negative test remains. It will always be a catch-22, either the test is neutered before the kernel, or it will lag behind the kernel and fail. It doesn't actually tell us if we fluked out in failing the execbuf, but by testing all unknown flags it functions better at the role it was meant for (that all as of yet unused flags are not accepted). Signed-off-by: Chris Wilson Reviewed-by: Arkadiusz Hiler --- tests/gem_exec_params.c | 277 ++++++++---------------------------------------- 1 file changed, 42 insertions(+), 235 deletions(-) diff --git a/tests/gem_exec_params.c b/tests/gem_exec_params.c index ba6d67cb..e3432cba 100644 --- a/tests/gem_exec_params.c +++ b/tests/gem_exec_params.c @@ -25,54 +25,15 @@ * */ -#include "igt.h" -#include -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include "drm.h" +#include +#include "igt.h" -#define LOCAL_I915_EXEC_VEBOX (4<<0) -#define LOCAL_I915_EXEC_BSD_MASK (3<<13) -#define LOCAL_I915_EXEC_BSD_RING1 (1<<13) -#define LOCAL_I915_EXEC_BSD_RING2 (2<<13) -#define LOCAL_I915_EXEC_RESOURCE_STREAMER (1<<15) #define LOCAL_I915_EXEC_FENCE_IN (1 << 16) #define LOCAL_I915_EXEC_FENCE_OUT (1 << 17) #define LOCAL_I915_EXEC_BATCH_FIRST (1 << 18) -static bool has_ring(int fd, unsigned ring_exec_flags) -{ - switch (ring_exec_flags & I915_EXEC_RING_MASK) { - case 0: - case I915_EXEC_RENDER: - return true; - - case I915_EXEC_BSD: - if (ring_exec_flags & LOCAL_I915_EXEC_BSD_MASK) - return gem_has_bsd2(fd); - else - return gem_has_bsd(fd); - - case I915_EXEC_BLT: - return gem_has_blt(fd); - - case I915_EXEC_VEBOX: - return gem_has_vebox(fd); - } - - igt_assert_f(0, "invalid exec flag 0x%x\n", ring_exec_flags); - return false; -} - static bool has_exec_batch_first(int fd) { int val = -1; @@ -182,158 +143,30 @@ static void test_batch_first(int fd) struct drm_i915_gem_execbuffer2 execbuf; struct drm_i915_gem_exec_object2 gem_exec[1]; uint32_t batch[2] = {MI_BATCH_BUFFER_END}; -uint32_t handle, devid; +uint32_t devid; int fd; igt_main { - const struct intel_execution_engine *e; - igt_fixture { fd = drm_open_driver(DRIVER_INTEL); igt_require_gem(fd); devid = intel_get_drm_devid(fd); - handle = gem_create(fd, 4096); - gem_write(fd, handle, 0, batch, sizeof(batch)); - - gem_exec[0].handle = handle; - gem_exec[0].relocation_count = 0; - gem_exec[0].relocs_ptr = 0; - gem_exec[0].alignment = 0; - gem_exec[0].offset = 0; - gem_exec[0].flags = 0; - gem_exec[0].rsvd1 = 0; - gem_exec[0].rsvd2 = 0; + gem_exec[0].handle = gem_create(fd, 4096); + gem_write(fd, gem_exec[0].handle, 0, batch, sizeof(batch)); execbuf.buffers_ptr = to_user_pointer(gem_exec); execbuf.buffer_count = 1; - execbuf.batch_start_offset = 0; - execbuf.batch_len = 8; - execbuf.cliprects_ptr = 0; - execbuf.num_cliprects = 0; - execbuf.DR1 = 0; - execbuf.DR4 = 0; - execbuf.flags = 0; - i915_execbuffer2_set_context_id(execbuf, 0); - execbuf.rsvd2 = 0; - } - - igt_subtest("control") { - for (e = intel_execution_engines; e->name; e++) { - if (has_ring(fd, e->exec_id | e->flags)) { - execbuf.flags = e->exec_id | e->flags; - gem_execbuf(fd, &execbuf); - } - } - } - -#define RUN_FAIL(expected_errno) do { \ - igt_assert(drmIoctl(fd, \ - DRM_IOCTL_I915_GEM_EXECBUFFER2, \ - &execbuf) == -1); \ - igt_assert_eq(errno, expected_errno); \ - } while(0) - - igt_subtest("no-bsd") { - igt_require(!gem_has_bsd(fd)); - execbuf.flags = I915_EXEC_BSD; - RUN_FAIL(EINVAL); - } - igt_subtest("no-blt") { - igt_require(!gem_has_blt(fd)); - execbuf.flags = I915_EXEC_BLT; - RUN_FAIL(EINVAL); - } - igt_subtest("no-vebox") { - igt_require(!gem_has_vebox(fd)); - execbuf.flags = LOCAL_I915_EXEC_VEBOX; - RUN_FAIL(EINVAL); - } - igt_subtest("invalid-ring") { - execbuf.flags = I915_EXEC_RING_MASK; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-ring2") { - execbuf.flags = LOCAL_I915_EXEC_VEBOX+1; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd-ring") { - igt_require(gem_has_bsd2(fd)); - execbuf.flags = I915_EXEC_BSD | LOCAL_I915_EXEC_BSD_MASK; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd1-flag-on-render") { - execbuf.flags = I915_EXEC_RENDER | LOCAL_I915_EXEC_BSD_RING1; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd2-flag-on-render") { - execbuf.flags = I915_EXEC_RENDER | LOCAL_I915_EXEC_BSD_RING2; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd1-flag-on-blt") { - execbuf.flags = I915_EXEC_BLT | LOCAL_I915_EXEC_BSD_RING1; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd2-flag-on-blt") { - execbuf.flags = I915_EXEC_BLT | LOCAL_I915_EXEC_BSD_RING2; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd1-flag-on-vebox") { - igt_require(gem_has_vebox(fd)); - execbuf.flags = LOCAL_I915_EXEC_VEBOX | LOCAL_I915_EXEC_BSD_RING1; - RUN_FAIL(EINVAL); - } - - igt_subtest("invalid-bsd2-flag-on-vebox") { - igt_require(gem_has_vebox(fd)); - execbuf.flags = LOCAL_I915_EXEC_VEBOX | LOCAL_I915_EXEC_BSD_RING2; - RUN_FAIL(EINVAL); - } - - igt_subtest("rel-constants-invalid-ring") { - igt_require(gem_has_bsd(fd)); - execbuf.flags = I915_EXEC_BSD | I915_EXEC_CONSTANTS_ABSOLUTE; - RUN_FAIL(EINVAL); - } - - igt_subtest("rel-constants-invalid-rel-gen5") { - igt_require(intel_gen(devid) > 5); - execbuf.flags = I915_EXEC_RENDER | I915_EXEC_CONSTANTS_REL_SURFACE; - RUN_FAIL(EINVAL); - } - - igt_subtest("rel-constants-invalid") { - execbuf.flags = I915_EXEC_RENDER | (I915_EXEC_CONSTANTS_REL_SURFACE+(1<<6)); - RUN_FAIL(EINVAL); - } - - igt_subtest("sol-reset-invalid") { - igt_require(gem_has_bsd(fd)); - execbuf.flags = I915_EXEC_BSD | I915_EXEC_GEN7_SOL_RESET; - RUN_FAIL(EINVAL); - } - - igt_subtest("sol-reset-not-gen7") { - igt_require(intel_gen(devid) != 7); - execbuf.flags = I915_EXEC_RENDER | I915_EXEC_GEN7_SOL_RESET; - RUN_FAIL(EINVAL); } igt_subtest("secure-non-root") { igt_fork(child, 1) { igt_drop_root(); - execbuf.flags = I915_EXEC_RENDER | I915_EXEC_SECURE; - RUN_FAIL(EPERM); + execbuf.flags = I915_EXEC_SECURE; + igt_assert_eq(__gem_execbuf(fd, &execbuf), -EPERM); } igt_waitchildren(); @@ -341,12 +174,10 @@ igt_main igt_subtest("secure-non-master") { do_or_die(drmDropMaster(fd)); - execbuf.flags = I915_EXEC_RENDER | I915_EXEC_SECURE; - RUN_FAIL(EPERM); + execbuf.flags = I915_EXEC_SECURE; + igt_assert_eq(__gem_execbuf(fd, &execbuf), -EPERM); do_or_die(drmSetMaster(fd)); - igt_assert(drmIoctl(fd, - DRM_IOCTL_I915_GEM_EXECBUFFER2, - &execbuf) == 0); + gem_execbuf(fd, &execbuf); } /* HANDLE_LUT and NO_RELOC are already exercised by gem_exec_lut_handle, @@ -354,84 +185,60 @@ igt_main * gem_exec_fence, invalid usage of EXEC_FENCE_IN is tested below. */ igt_subtest("invalid-flag") { - /* NOTE: This test intentionally exercise the next available - * flag. Don't "fix" this testcase without adding the required - * tests for the new flag first. */ - execbuf.flags = I915_EXEC_RENDER | (LOCAL_I915_EXEC_BATCH_FIRST << 1); - RUN_FAIL(EINVAL); + /* Any individual unknown flag should result in an -EINVAL. + * Since there is a lag between the kernel advertising + * a new feature and us knowing about that feature, this + * test *will* fail during the interim. + */ + for (int bit = ffs(__I915_EXEC_UNKNOWN_FLAGS) - 1; + bit < 64; + bit++) { + int err; + + execbuf.flags = 1ull << bit; + err = __gem_execbuf(fd, &execbuf); + if (err == 0) { + igt_warn("Unknown flag (1ull << %d) accepted by execbuf\n", + bit); + } else { + igt_assert_eq(err, -EINVAL); + } + } + + /* Any combination of invalid flags should also be an error. + * But that will take too long to test them all, so we assume + * how the kernel will behave. + */ + execbuf.flags = __I915_EXEC_UNKNOWN_FLAGS; + igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL); } /* rsvd1 aka context id is already exercised by gem_ctx_bad_exec */ igt_subtest("cliprects-invalid") { - igt_require(intel_gen(devid) >= 5); execbuf.flags = 0; execbuf.num_cliprects = 1; - RUN_FAIL(EINVAL); + igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL); execbuf.num_cliprects = 0; - } - - igt_subtest("rs-invalid-on-bsd-ring") { - igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8); - execbuf.flags = I915_EXEC_BSD | LOCAL_I915_EXEC_RESOURCE_STREAMER; - RUN_FAIL(EINVAL); - } - - igt_subtest("rs-invalid-on-blt-ring") { - igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8); - execbuf.flags = I915_EXEC_BLT | LOCAL_I915_EXEC_RESOURCE_STREAMER; - RUN_FAIL(EINVAL); - } - - igt_subtest("rs-invalid-on-vebox-ring") { - igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8); - execbuf.flags = I915_EXEC_VEBOX | LOCAL_I915_EXEC_RESOURCE_STREAMER; - RUN_FAIL(EINVAL); - } - - igt_subtest("rs-invalid-gen") { - igt_require(!IS_HASWELL(devid) && intel_gen(devid) < 8); - execbuf.flags = I915_EXEC_RENDER | LOCAL_I915_EXEC_RESOURCE_STREAMER; - RUN_FAIL(EINVAL); + gem_execbuf(fd, &execbuf); } igt_subtest("invalid-fence-in") { igt_require(gem_has_exec_fence(fd)); execbuf.flags = LOCAL_I915_EXEC_FENCE_IN; execbuf.rsvd2 = -1; - RUN_FAIL(EINVAL); + igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL); execbuf.rsvd2 = fd; - RUN_FAIL(EINVAL); - } - - igt_subtest("rsvd2-dirt") { - igt_require(!gem_has_exec_fence(fd)); + igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL); execbuf.flags = 0; - execbuf.rsvd2 = 1; - RUN_FAIL(EINVAL); - execbuf.rsvd2 = 0; + gem_execbuf(fd, &execbuf); } igt_subtest("batch-first") test_batch_first(fd); -#define DIRT(name) \ - igt_subtest(#name "-dirt") { \ - execbuf.flags = 0; \ - execbuf.name = 1; \ - RUN_FAIL(EINVAL); \ - execbuf.name = 0; \ - } - - DIRT(cliprects_ptr); - DIRT(DR1); - DIRT(DR4); -#undef DIRT - -#undef RUN_FAIL - igt_fixture { - gem_close(fd, handle); + gem_close(fd, gem_exec[0].handle); close(fd); }