From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 792FD6E7FE for ; Thu, 3 Jun 2021 21:41:39 +0000 (UTC) Date: Thu, 3 Jun 2021 17:41:32 -0400 From: Rodrigo Vivi Message-ID: References: <20210518103344.2264397-1-alan.previn.teres.alexis@intel.com> <20210518103344.2264397-14-alan.previn.teres.alexis@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210518103344.2264397-14-alan.previn.teres.alexis@intel.com> Subject: Re: [igt-dev] [PATCH i-g-t 13/17] Verify execbuf fails with stale PXP buffer after teardown List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Alan Previn Cc: igt-dev@lists.freedesktop.org List-ID: On Tue, May 18, 2021 at 03:33:40AM -0700, Alan Previn wrote: > Add a check to verify that reusing a stale protected buffer > in a gem_execbuff (with a protected context) after a teardown > (triggered by suspend-resume cycle) shall fail with -ENOEXEC error. > Reuse the existing subtest function to with separate assets > to minimize the number of suspend-resume cycles we take. > > NOTE: The end-to-end architecture requirement includes that > any break in the links of the PXP sessions needs to trigger a > full teardown and the application needs to be made aware of that > allowing it to re-establish the end-to-end pipeline of buffers, > contexts and renders again if it chooses to. This stricter > behavior targets only contexts created with PXP enabled. > > Signed-off-by: Alan Previn Reviewed-by: Rodrigo Vivi > --- > tests/i915/gem_pxp.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c > index f1ef0a24..b4f369ae 100644 > --- a/tests/i915/gem_pxp.c > +++ b/tests/i915/gem_pxp.c > @@ -760,23 +760,45 @@ static void free_exec_assets(int i915, struct simple_exec_assets *data) > static void test_pxp_pwrcycle_staleasset_execution(int i915, struct powermgt_data *pm) > { > int ret; > - struct simple_exec_assets data = {0}; > + struct simple_exec_assets data[2] = {{0}, {0}}; > + uint32_t ctx2; > + struct intel_bb *ibb2; > > /* > - * Use normal buffers for testing for invalidation > + * For data[0]: Use normal buffers for testing for invalidation > * of protected contexts to ensure kernel is catching > * the invalidated context (not buffer) > */ > - prepare_exec_assets(i915, &data, true, false); > - ret = gem_execbuf_flush_store_dw(i915, data.ibb, data.ctx, data.fencebuf); > + prepare_exec_assets(i915, &data[0], true, false); > + ret = gem_execbuf_flush_store_dw(i915, data[0].ibb, data[0].ctx, data[0].fencebuf); > + igt_assert(ret == 0); > + > + prepare_exec_assets(i915, &data[1], true, true); > + ret = gem_execbuf_flush_store_dw(i915, data[1].ibb, data[1].ctx, data[1].fencebuf); > igt_assert(ret == 0); > > trigger_powermgt_suspend_cycle(i915, pm); > > - ret = gem_execbuf_flush_store_dw(i915, data.ibb, data.ctx, data.fencebuf); > + ret = gem_execbuf_flush_store_dw(i915, data[0].ibb, data[0].ctx, data[0].fencebuf); > igt_assert_f((ret == -EACCES), "Executing stale pxp context didn't fail with -EACCES\n"); > > - free_exec_assets(i915, &data); > + /* > + * For data[1]: after teardown, alloc new assets for context but > + * reuse the bo to ensure the kernel is catching the > + * invalidated bo (not context) > + */ > + ret = create_ctx_with_params(i915, true, true, true, false, &ctx2); > + igt_assert_eq(ret, 0); > + igt_assert_eq(get_ctx_protected_param(i915, ctx2), 1); > + ibb2 = intel_bb_create_with_context(i915, ctx2, 4096); > + igt_assert(ibb2); > + intel_bb_detach_intel_buf(data[1].ibb, data[1].fencebuf); > + intel_bb_add_intel_buf(ibb2, data[1].fencebuf, true); > + ret = gem_execbuf_flush_store_dw(i915, ibb2, ctx2, data[1].fencebuf); > + igt_assert_f((ret == -ENOEXEC), "Executing stale pxp buffer didn't fail with -ENOEXEC\n"); > + > + free_exec_assets(i915, &data[0]); > + free_exec_assets(i915, &data[1]); > } > > igt_main > -- > 2.25.1 > > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev