From patchwork Tue Nov 5 15:37:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janusz Krzysztofik X-Patchwork-Id: 11228187 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 49D7613BD for ; Tue, 5 Nov 2019 15:38:23 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 32327217F5 for ; Tue, 5 Nov 2019 15:38:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 32327217F5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AF8DA6EAB5; Tue, 5 Nov 2019 15:38:22 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 059396EAB5; Tue, 5 Nov 2019 15:38:21 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Nov 2019 07:38:21 -0800 X-IronPort-AV: E=Sophos;i="5.68,271,1569308400"; d="scan'208";a="195855256" Received: from jkrzyszt-desk.igk.intel.com ([172.22.244.17]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Nov 2019 07:38:17 -0800 From: Janusz Krzysztofik To: Chris Wilson , Vanshidhar Konda , Joonas Lahtinen Date: Tue, 5 Nov 2019 16:37:42 +0100 Message-Id: <20191105153742.1879-1-janusz.krzysztofik@linux.intel.com> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH i-g-t v5] tests/gem_exec_reloc: Don't filter out invalid addresses X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable addresses for !ppgtt") introduced filtering of shared GTT addresses possibly in use. Unfortunately, that filtering doesn't distinguish between addresses actually in use and otherwise invalid softpin offsets. If for example incorrect GTT alignment is assumed while softpin offsets are calculated, a half of those offsets to be tested may be incorrectly detected as reserved and silently skipped instead of reported as a problem. That can significantly distort the intended test pattern. Filter out unavailable addresses only if reported as reserved. v2: Skip unavailable addresses only when not running on full PPGTT. v3: Replace the not on full PPGTT requirement for skipping with error code checking. v4: Silently skip only those offsets which have been explicitly reported as overlapping with shared GTT reserved space, not simply all which raise failures other than -EINVAL (Chris), - as an implementation of moving the probe out of line so it's not easily confused with the central point of the test (Chris), use object validation library helper just introduced. v5: Detach from the series (rejected by Joonas) and submit once more as an independent (though related) standalone patch, - don't use relocations for anything new (Joonas), - don't depend on such quirk behavior as to kernel validating parameters in certain order (Joonas), - it's not straightforward that we are just checking the offset unless I check the code (of the helper) (Vanshi); the above three changes effectively revert most of v4 changes, leaving only that mentioned as the first one under v4 above, - use 'in use' or 'reserved' wording instead of 'occupied by other users' in commit description, they better correspond to the original commit being fixed as well as the comment proposed by Chris. Signed-off-by: Janusz Krzysztofik Cc: Chris Wilson Cc: Vanshidhar Konda Cc: Joonas Lahtinen --- tests/i915/gem_exec_reloc.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c index fdd9661d..238fa88f 100644 --- a/tests/i915/gem_exec_reloc.c +++ b/tests/i915/gem_exec_reloc.c @@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags) uint64_t gtt_size = gem_aperture_size(fd); const uint32_t bbe = MI_BATCH_BUFFER_END; igt_spin_t *spin = NULL; - int count, n; + int count, n, err; igt_require(gem_has_softpin(fd)); @@ -542,8 +542,12 @@ static void basic_range(int fd, unsigned flags) gem_write(fd, obj[n].handle, 0, &bbe, sizeof(bbe)); execbuf.buffers_ptr = to_user_pointer(&obj[n]); execbuf.buffer_count = 1; - if (__gem_execbuf(fd, &execbuf)) + err = __gem_execbuf(fd, &execbuf); + if (err) { + /* Iff using a shared GTT, some of it may be reserved */ + igt_assert_eq(err, -ENOSPC); continue; + } igt_debug("obj[%d] handle=%d, address=%llx\n", n, obj[n].handle, (long long)obj[n].offset); @@ -562,8 +566,12 @@ static void basic_range(int fd, unsigned flags) gem_write(fd, obj[n].handle, 0, &bbe, sizeof(bbe)); execbuf.buffers_ptr = to_user_pointer(&obj[n]); execbuf.buffer_count = 1; - if (__gem_execbuf(fd, &execbuf)) + err = __gem_execbuf(fd, &execbuf); + if (err) { + /* Iff using a shared GTT, some of it may be reserved */ + igt_assert_eq(err, -ENOSPC); continue; + } igt_debug("obj[%d] handle=%d, address=%llx\n", n, obj[n].handle, (long long)obj[n].offset);