[igt-dev] [PATCH i-g-t 12/16] Verify execbuf fails with stale PXP buffer after teardown

[Teres Alexis, Alan Previn]

On Thu, 2021-05-13 at 23:49 -0700, Alan Previn wrote:
> Add a subtest 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 -EIO error.
> 
> 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 <alan.previn.teres.alexis at intel.com>
> ---
>  tests/i915/gem_pxp.c | 44 ++++++++++++++++++++++++++++++++++++----
> ----
>  1 file changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
> index 1952e230..605b6524 100644
> --- a/tests/i915/gem_pxp.c
> +++ b/tests/i915/gem_pxp.c
> @@ -38,6 +38,7 @@ IGT_TEST_DESCRIPTION("Test PXP that manages
> protected content through arbitrated
>  /* test-configs for power-management triggered protected session
> teardown */
>  #define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
>  #define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
> +#define SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC 3
>  
>  /* Struct and defintions for power management. */
>  struct powermgt_data {
> @@ -688,10 +689,10 @@ static void test_protected_session_teardown(int
> i915, uint32_t test_cfg,
>  	uint32_t encrypted_pixels_b4[TSTSURF_SIZE/TSTSURF_BYTESPP];
>  	uint32_t encrypted_pixels_aft[TSTSURF_SIZE/TSTSURF_BYTESPP];
>  	int matched_after_keychange = 0, loop = 0;
> -	uint32_t ctx, fencebo;
> +	uint32_t ctx, ctx2, fencebo;
>  	struct intel_buf *fencebuf;
>  	struct buf_ops *bops;
> -	struct intel_bb *ibb;
> +	struct intel_bb *ibb, *ibb2;
>  
>  	switch (test_cfg) {
>  	case SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE:
> @@ -712,14 +713,19 @@ static void test_protected_session_teardown(int
> i915, uint32_t test_cfg,
>  		break;
>  
>  	case SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC:
> +	case SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC:
>  		ctx = create_protected_ctx(i915, true, true, true,
> false, 0);
>  		assert_ctx_protected_param(i915, ctx, true);
>  
> -		/* use normal buffers for testing for invalidation
> -		 * of protected contexts to ensure kernel is catching
> -		 * the invalidated context (not buffer)
> -		 */
> -		fencebo = alloc_and_fill_dest_buff(i915, false, 4096,
> 0);
> +		if (test_cfg ==
> SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC) {
> +			/* use normal buffers for testing for
> invalidation
> +			 * of protected contexts to ensure kernel is
> catching
> +			 * the invalidated context
> +			 */
> +			fencebo = alloc_and_fill_dest_buff(i915, false,
> 4096, 0);
> +		} else {
> +			fencebo = alloc_and_fill_dest_buff(i915, true,
> 4096, 0);
> +		}
>  
>  		ibb = intel_bb_create_with_context(i915, ctx, 4096);
>  		igt_assert(ibb);
> @@ -734,12 +740,31 @@ static void test_protected_session_teardown(int
> i915, uint32_t test_cfg,
>  		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf,
> 0);
>  		trigger_powermgt_suspend_cycle(i915, pm);
>  
> -		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf,
> -EACCES);
> +		if (test_cfg == SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC)
> {
> +			/* after teardown, alloc new assets for
> everything
> +			 * except the bo to ensure the kernel is
> catching
> +			 * the invalidated bo
> +			 */
> +			ctx2 = create_protected_ctx(i915, true, true,
> true, false, 0);
> +			assert_ctx_protected_param(i915, ctx2, true);
> +			ibb2 = intel_bb_create_with_context(i915, ctx2,
> 4096);
> +			igt_assert(ibb2);
> +
> +			intel_bb_detach_intel_buf(ibb, fencebuf);
> +			intel_bb_add_intel_buf(ibb2, fencebuf, true);
> +			gem_execbuf_flush_store_dw(i915, ibb2, ctx2,
> fencebuf, -ENOEXEC);
> +		} else {
> +			gem_execbuf_flush_store_dw(i915, ibb, ctx,
> fencebuf, -EACCES);
> +		}
>  
>  		intel_bb_destroy(ibb);
>  		intel_buf_destroy(fencebuf);
>  		gem_close(i915, fencebo);
>  		gem_context_destroy(i915, ctx);
> +		if (test_cfg == SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC)
> {
> +			intel_bb_destroy(ibb2);
> +			gem_context_destroy(i915, ctx2);
> +		}
>  		buf_ops_destroy(bops);
>  		break;
>  
> @@ -842,6 +867,9 @@ igt_main
>  		igt_subtest("reject-old-prot-context-execution-after-
> suspend-resume") {
>  			test_protected_session_teardown(i915,
> SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC, &pm);
>  		}
> +		igt_subtest("reject-old-prot-buffer-execution-after-
> suspend-resume") {
> +			test_protected_session_teardown(i915,
> SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC, &pm);
> 
Feedback from testing: combine this test together with
test_protected_session_teardown because the total execution time of
this IGT increases by ~15 seconds everytime we take a suspend-resume
(required for this test).



> +		}
>  	}
>  
>  	igt_fixture {