[igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc
Michal Wajdeczko
michal.wajdeczko at intel.com
Fri May 14 09:00:29 UTC 2021
On 14.05.2021 08:49, Alan Previn wrote:
> Test PXP capability support as well as the allocation of protected
> buffers and protected contexts.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
> ---
> tests/i915/gem_pxp.c | 366 +++++++++++++++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 2 files changed, 367 insertions(+)
> create mode 100644 tests/i915/gem_pxp.c
>
> diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
> new file mode 100644
> index 00000000..6340daae
> --- /dev/null
> +++ b/tests/i915/gem_pxp.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include <sys/ioctl.h>
> +
> +#include "igt.h"
> +#include "i915/gem.h"
> +
> +IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
> +/* Note: PXP = "Protected Xe Path" */
> +
> +/* test-configs for protected buffer creation*/
> +#define BO_ALLOC_NO_HW_SUPPORT 1
> +#define BO_ALLOC_PROTECT_ON 2
> +#define BO_ALLOC_PROTECT_OFF 3
> +
> +/* test-configs for protected context creation*/
> +#define CTX_ALLOC_NO_HW_SUPPORT 1
> +#define CTX_ALLOC_RECOVER_OFF_PROTECT_OFF 2
> +#define CTX_ALLOC_RECOVER_OFF_PROTECT_ON 3
> +#define CTX_ALLOC_RECOVER_ON_PROTECT_OFF 4
> +#define CTX_ALLOC_RECOVER_ON_PROTECT_ON 5
> +#define CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON 6
> +#define CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF 7
> +#define CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID 8
> +#define CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID 9
why not use enum instead?
> +
> +static bool is_pxp_hw_supported(int i915)
> +{
> + uint32_t devid = intel_get_drm_devid(i915);
> + const struct intel_device_info *devinfo = intel_get_device_info(devid);
can intel_get_device_info really return NULL ?
> +
> + if (!devinfo) {
> + igt_info("No device info found - assume no PXP support.\n");
> + return false;
> + }
> + if (devinfo->is_tigerlake || devinfo->is_rocketlake ||
> + devinfo->is_alderlake_s) {
> + return true;
shouldn't IS_TIGERLAKE and similar be used instead ?
> + }
> +
> + return false;
> +}
> +
> +static uint32_t create_protected_bo(int i915, uint32_t size, bool protected_is_true,
> + int expected_return)
> +{
> + int ret;
> +
> + struct drm_i915_gem_create_ext_protected_content protected_ext = {
> + .base = { .name = I915_GEM_CREATE_EXT_PROTECTED_CONTENT },
> + .flags = 0,
> + };
> +
> + struct drm_i915_gem_create_ext create_ext = {
> + .size = size,
> + .extensions = 0,
> + };
> +
> + if (protected_is_true)
> + create_ext.extensions = (uintptr_t)&protected_ext;
hmm, if call to "create_protected" can be used to create "unprotected"
bo, then maybe function shall be named as "create_bo_ext" ?
> +
> + ret = ioctl(i915, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
> + igt_assert_eq(ret, expected_return);
hmm, usually such asserts are done at caller's level
so maybe this function shall have signature like:
int create_protected_bo(int i915, uint32_t size, uint32_t *bo_out)
> +
> + return create_ext.handle;
> +}
> +
> +static void test_create_protected_buffer(int i915, uint32_t test_cfg)
> +{
> + uint32_t bo;
> +
> + switch (test_cfg) {
> + case BO_ALLOC_NO_HW_SUPPORT:
> + bo = create_protected_bo(i915, 4096, false, 0);
igt_assert(bo) ?
> + gem_close(i915, bo);
> + bo = create_protected_bo(i915, 4096, true, -ENODEV);
> + igt_assert_eq(bo, 0);
> + break;
> +
> + case BO_ALLOC_PROTECT_OFF:
> + bo = create_protected_bo(i915, 4096, false, 0);
> + gem_close(i915, bo);
> + break;
> +
> + case BO_ALLOC_PROTECT_ON:
> + bo = create_protected_bo(i915, 4096, true, 0);
> + gem_close(i915, bo);
> + break;
> +
> + default:
> + igt_info("Skipping unknown buffer test_cfg = %d\n", test_cfg);
> + break;
> + }
> +}
> +
> +static int create_ext_ioctl(int i915, struct drm_i915_gem_context_create_ext *arg)
I guess you should rather export this function from lib, not copy it
> +{
> + int err;
> +
> + err = 0;
> + if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> + err = -errno;
> + igt_assume(err);
> + }
> +
> + errno = 0;
> + return err;
> +}
> +
> +static uint32_t create_protected_ctx(int i915, bool with_protected_param, bool protected_is_true,
> + bool with_recoverable_param, bool recoverable_is_true,
> + int expected_return)
> +{
> + int ret;
> +
> + struct drm_i915_gem_context_create_ext_setparam p_prot = {
> + .base = {
> + .name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> + .next_extension = 0,
> + },
> + .param = {
> + .param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
> + .value = 0,
> + }
> + };
> + struct drm_i915_gem_context_create_ext_setparam p_norecover = {
> + .base = {
> + .name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> + .next_extension = 0,
> + },
> + .param = {
> + .param = I915_CONTEXT_PARAM_RECOVERABLE,
> + .value = 0,
> + }
> + };
> + struct drm_i915_gem_context_create_ext create = {
> + .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> + .extensions = 0,
> + };
> +
> + p_prot.param.value = protected_is_true;
> + p_norecover.param.value = recoverable_is_true;
> +
> + if (with_protected_param && with_recoverable_param) {
> + create.extensions = to_user_pointer(&(p_norecover.base));
> + p_norecover.base.next_extension = to_user_pointer(&(p_prot.base));
> + } else if (!with_protected_param && with_recoverable_param) {
> + create.extensions = to_user_pointer(&(p_norecover.base));
> + p_norecover.base.next_extension = 0;
> + } else if (with_protected_param && !with_recoverable_param) {
> + create.extensions = to_user_pointer(&(p_prot.base));
> + p_prot.base.next_extension = 0;
> + } else if (!with_protected_param && !with_recoverable_param) {
> + create.flags = 0;
> + }
> +
> + ret = create_ext_ioctl(i915, &create);
> + igt_assert_eq(ret, expected_return);
> +
> + return create.ctx_id;
> +}
> +
> +#define CHANGE_PARAM_PROTECTED 0x0001
> +#define CHANGE_PARAM_RECOVERY 0x0002
> +static void modify_ctx_protected_param(int i915, uint32_t ctx_id, uint32_t change_param_mask,
> + bool param_value, int expected_return)
> +{
> + int ret;
> +
> + struct drm_i915_gem_context_param ctx_param = {
> + .ctx_id = ctx_id,
> + .param = 0,
> + .value = 0,
> + };
> +
> + if (change_param_mask == CHANGE_PARAM_PROTECTED) {
> + ctx_param.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT;
> + ctx_param.value = (int)param_value;
> + } else if (change_param_mask == CHANGE_PARAM_RECOVERY) {
> + ctx_param.param = I915_CONTEXT_PARAM_RECOVERABLE;
> + ctx_param.value = (int)param_value;
> + } else {
> + return;
> + }
> +
> + ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, &ctx_param);
> +
> + igt_assert_eq(ret, expected_return);
> +}
> +
> +static void assert_ctx_protected_param(int i915, uint32_t ctx_id, bool is_protected)
> +{
> + int ret;
> +
> + struct drm_i915_gem_context_param ctx_param = {
> + .ctx_id = ctx_id,
> + .param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
> + };
> +
> + ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &ctx_param);
> + igt_assert_eq(ret, 0);
> + igt_assert_eq(ctx_param.value, (int)is_protected);
> +}
> +
> +static void assert_ctx_recovery_param(int i915, uint32_t ctx_id, bool is_recoverable)
> +{
> + int ret;
> +
> + struct drm_i915_gem_context_param ctx_param = {
> + .ctx_id = ctx_id,
> + .param = I915_CONTEXT_PARAM_RECOVERABLE,
> + };
> +
> + ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &ctx_param);
> + igt_assert_eq(ret, 0);
> + igt_assert_eq(ctx_param.value, (int)is_recoverable);
> +}
> +
> +static void test_create_protected_context(int i915, uint32_t test_cfg)
> +{
> + uint32_t ctx;
> +
> + switch (test_cfg) {
> + case CTX_ALLOC_NO_HW_SUPPORT:
> + ctx = create_protected_ctx(i915, true, true, true, false, -ENODEV);
> + igt_assert_eq(ctx, 0);
> + case CTX_ALLOC_RECOVER_OFF_PROTECT_OFF:
> + ctx = create_protected_ctx(i915, true, false, true, false, 0);
> + assert_ctx_protected_param(i915, ctx, false);
> + assert_ctx_recovery_param(i915, ctx, false);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + case CTX_ALLOC_RECOVER_OFF_PROTECT_ON:
> + ctx = create_protected_ctx(i915, true, true, true, false, 0);
> + assert_ctx_protected_param(i915, ctx, true);
> + assert_ctx_recovery_param(i915, ctx, false);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + case CTX_ALLOC_RECOVER_ON_PROTECT_OFF:
> + ctx = create_protected_ctx(i915, true, false, true, true, 0);
> + assert_ctx_protected_param(i915, ctx, false);
> + assert_ctx_recovery_param(i915, ctx, true);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + case CTX_ALLOC_RECOVER_ON_PROTECT_ON:
> + ctx = create_protected_ctx(i915, true, true, true, true, -EPERM);
> + igt_assert_eq(ctx, 0);
> + ctx = create_protected_ctx(i915, true, true, false, false, -EPERM);
> + igt_assert_eq(ctx, 0);
> + break;
> +
> + case CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON:
> + ctx = create_protected_ctx(i915, true, true, true, false, 0);
> + assert_ctx_protected_param(i915, ctx, true);
> + assert_ctx_recovery_param(i915, ctx, false);
> + modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, true, -EPERM);
> + assert_ctx_recovery_param(i915, ctx, false);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + case CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF:
> + ctx = create_protected_ctx(i915, true, true, true, false, 0);
> + assert_ctx_protected_param(i915, ctx, true);
> + assert_ctx_recovery_param(i915, ctx, false);
> + modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, false, -EPERM);
> + assert_ctx_protected_param(i915, ctx, true);
> + assert_ctx_recovery_param(i915, ctx, false);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + case CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID:
> + ctx = create_protected_ctx(i915, true, true, true, false, 0);
> + assert_ctx_protected_param(i915, ctx, true);
> + assert_ctx_recovery_param(i915, ctx, false);
> + modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, true, -EPERM);
> + modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, false, -EPERM);
> + assert_ctx_protected_param(i915, ctx, true);
> + assert_ctx_recovery_param(i915, ctx, false);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + case CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID:
> + ctx = create_protected_ctx(i915, false, false, false, false, 0);
> + assert_ctx_protected_param(i915, ctx, false);
> + assert_ctx_recovery_param(i915, ctx, true);
> + modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, false, 0);
> + modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, true, -EPERM);
> + assert_ctx_protected_param(i915, ctx, false);
> + assert_ctx_recovery_param(i915, ctx, false);
> + gem_context_destroy(i915, ctx);
> + break;
> +
> + default:
> + igt_info("Skipping unknown context test_cfg = %d\n", test_cfg);
> + break;
as there is nothing common between all these test configs, why bother
with yet another dispatcher function with this giant switch statement?
note that igt_main is dispatching tests anyway...
IMHO better to use set of dedicated functions:
test_ctx_no_hw_support()
test_ctx_alloc_recover_off_protect_of()
...
> + }
> +}
> +
> +igt_main
> +{
> + int i915 = -1;
> + bool pxp_supported = false;
> +
> + igt_fixture
> + {
> + i915 = drm_open_driver(DRIVER_INTEL);
> + igt_require(i915);
> + igt_require_gem(i915);
> + pxp_supported = is_pxp_hw_supported(i915);
> + }
> +
> + igt_subtest_group {
> + igt_fixture {
> + igt_require((pxp_supported == 0));
> + }
> +
> + igt_describe("Verify protected buffer on unsupported hw:");
> + igt_subtest("hw-rejects-pxp-buffer")
> + test_create_protected_buffer(i915, BO_ALLOC_NO_HW_SUPPORT);
> + igt_describe("Verify protected context on unsupported hw:");
> + igt_subtest("hw-rejects-pxp-context")
> + test_create_protected_context(i915, CTX_ALLOC_NO_HW_SUPPORT);
> + }
> +
> + igt_subtest_group {
> + igt_fixture {
> + igt_require(pxp_supported);
> + }
> +
> + igt_describe("Verify protected buffer on supported hw:");
> + igt_subtest("create-regular-buffer")
> + test_create_protected_buffer(i915, BO_ALLOC_PROTECT_OFF);
> + igt_subtest("create-protected-buffer")
> + test_create_protected_buffer(i915, BO_ALLOC_PROTECT_ON);
> +
> + igt_describe("Verify protected context on supported hw:");
> + igt_subtest("create-regular-context-1")
> + test_create_protected_context(i915, CTX_ALLOC_RECOVER_OFF_PROTECT_OFF);
> + igt_subtest("create-regular-context-2")
> + test_create_protected_context(i915, CTX_ALLOC_RECOVER_ON_PROTECT_OFF);
> + igt_subtest("fail-invalid-protected-context")
> + test_create_protected_context(i915, CTX_ALLOC_RECOVER_ON_PROTECT_ON);
> + igt_subtest("create-valid-protected-context")
> + test_create_protected_context(i915, CTX_ALLOC_RECOVER_OFF_PROTECT_ON);
> +
> + igt_describe("Verify protected context integrity:");
> + igt_subtest("reject-modify-context-protection-on")
> + test_create_protected_context(i915, CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID);
> + igt_subtest("reject-modify-context-protection-off-1")
> + test_create_protected_context(i915, CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON);
> + igt_subtest("reject-modify-context-protection-off-2")
> + test_create_protected_context(i915, CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF);
> + igt_subtest("reject-modify-context-protection-off-3")
> + test_create_protected_context(i915, CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID);
> + }
> +
> + igt_fixture {
> + close(i915);
> + }
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 19cc4ebe..e0bff4d1 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -187,6 +187,7 @@ i915_progs = [
> 'gem_pread_after_blit',
> 'gem_pwrite',
> 'gem_pwrite_snooped',
> + 'gem_pxp',
> 'gem_read_read_speed',
> 'gem_readwrite',
> 'gem_reg_read',
>
More information about the igt-dev
mailing list