[PATCH i-g-t 4/7] tests/intel/xe_pxp: Add PXP object and queue creation tests
Teres Alexis, Alan Previn
alan.previn.teres.alexis at intel.com
Wed Jan 15 06:17:41 UTC 2025
everything looks good.. except only one issue and one nit.
that said, to speed things up, here is a conditional RB pending that missing memory free in query_pxp_status.
(considering everything else appears correct and concise).
Reviewed-by: Alan Previn <alan.previn.teres.alexis at intel.com>
...alan
On Wed, 2024-12-11 at 16:18 -0800, Daniele Ceraolo Spurio wrote:
> PXP support introduces new SET_PROPERTY extensions to both BOs and
> exec_queues to mark them as being used for PXP workloads, so we need to
> test both correct and incorrect usage of those new interfaces.
>
> Since this is the first usage of extensions for BO creation, the
> common BO code has been update to support the extra parameter.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
> lib/xe/xe_ioctl.c | 14 +--
> lib/xe/xe_ioctl.h | 2 +-
> tests/intel/xe_mmap.c | 1 +
> tests/intel/xe_pxp.c | 201 ++++++++++++++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 5 files changed, 213 insertions(+), 6 deletions(-)
> create mode 100644 tests/intel/xe_pxp.c
>
> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
> index 6d8388918..01ab7c758 100644
> --- a/lib/xe/xe_ioctl.c
> +++ b/lib/xe/xe_ioctl.c
>
alan:snip
> diff --git a/tests/intel/xe_mmap.c b/tests/intel/xe_mmap.c
> index d818cc2f8..9f72cf2da 100644
> --- a/tests/intel/xe_mmap.c
> +++ b/tests/intel/xe_mmap.c
alan:snip
> diff --git a/tests/intel/xe_pxp.c b/tests/intel/xe_pxp.c
> new file mode 100644
> index 000000000..cfe118a1a
> --- /dev/null
> +++ b/tests/intel/xe_pxp.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "igt.h"
> +#include "xe_drm.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
> +
> +IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
> +/* Note: PXP = "Protected Xe Path" */
> +
> +/**
> + * TEST: Test PXP functionality
> + * Category: Content protection
> + * Mega feature: PXP
> + * Sub-category: PXP tests
> + * Functionality: Execution of protected content
> + * Test category: functionality test
> + */
> +
> +static int __pxp_bo_create(int fd, uint32_t vm, uint64_t size,
> + uint32_t session_type, uint32_t *handle)
> +{
> + struct drm_xe_ext_set_property ext = {
> + .base.next_extension = 0,
> + .base.name = DRM_XE_GEM_CREATE_EXTENSION_SET_PROPERTY,
> + .property = DRM_XE_GEM_CREATE_SET_PROPERTY_PXP_TYPE,
> + .value = session_type,
> + };
> + int ret = 0;
> +
> + if (__xe_bo_create(fd, vm, size, system_memory(fd), 0, &ext, handle)) {
> + ret = -errno;
> + errno = 0;
> + }
> +
> + return ret;
> +}
> +
> +static int __create_pxp_rcs_queue(int fd, uint32_t vm,
> + uint32_t session_type,
> + uint32_t *q)
> +{
> + struct drm_xe_engine_class_instance inst = {
> + .engine_class = DRM_XE_ENGINE_CLASS_RENDER,
> + };
> + struct drm_xe_ext_set_property ext = { 0 };
> + uint64_t ext_ptr = to_user_pointer(&ext);
> +
> + ext.base.next_extension = 0,
> + ext.base.name = DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY,
> + ext.property = DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE,
> + ext.value = session_type;
> +
> + return __xe_exec_queue_create(fd, vm, 1, 1, &inst, ext_ptr, q);
> +}
> +
> +static int query_pxp_status(int fd)
> +{
> + struct drm_xe_query_pxp_status *pxp_query;
> + struct drm_xe_device_query query = {
> + .extensions = 0,
> + .query = DRM_XE_DEVICE_QUERY_PXP_STATUS,
> + .size = 0,
> + .data = 0,
> + };
> + int status;
> +
> + if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
> + return -errno;
> +
> + pxp_query = malloc(query.size);
> + igt_assert(pxp_query);
> + memset(pxp_query, 0, query.size);
> +
> + query.data = to_user_pointer(pxp_query);
> +
> + if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
alan: forgot to free pxp_query before return
> + return -errno;
> +
> + status = pxp_query->status;
> + free(pxp_query);
> +
> + return status;
> +}
> +
> +static bool is_pxp_hw_supported(int fd)
> +{
> + int pxp_status;
> + int i = 0;
> +
> + /* PXP init completes after driver init, so we might have to wait for it */
> + while (i++ < 50) {
> + pxp_status = query_pxp_status(fd);
> +
> + /* -EINVAL means the PXP interface is not available */
> + igt_require(pxp_status != -EINVAL);
> +
> + /* -ENODEV means PXP not supported or disabled */
> + if (pxp_status == -ENODEV)
> + return false;
> +
> + /* status 1 means pxp is ready */
> + if (pxp_status == 1)
> + return true;
> +
> + /*
> + * 0 means init still in progress, any other remaining state
> + * is an error
> + */
> + igt_assert_eq(pxp_status, 0);
> +
> + usleep(50*1000);
> + }
> +
> + igt_assert_f(0, "PXP failed to initialize within the timeout\n");
> + return false;
> +}
> +
> +/**
> + * SUBTEST: pxp-bo-alloc
> + * Description: Verify PXP bo allocation works as expected
> + */
> +static void test_pxp_bo_alloc(int fd, bool pxp_supported)
> +{
> + uint32_t bo;
> + int ret;
> +
> + /* BO creation with DRM_XE_PXP_TYPE_NONE must always succeed */
> + ret = __pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_NONE, &bo);
> + igt_assert_eq(ret, 0);
> + gem_close(fd, bo);
> +
> + /* BO creation with DRM_XE_PXP_TYPE_HWDRM must only succeed if PXP is supported */
> + ret = __pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_HWDRM, &bo);
> + igt_assert_eq(ret, pxp_supported ? 0 : -ENODEV);
> + if (!ret)
> + gem_close(fd, bo);
> +
> + /* BO creation with an invalid type must always fail */
> + ret = __pxp_bo_create(fd, 0, 4096, 0xFF, &bo);
> + igt_assert_eq(ret, -EINVAL);
> +}
> +
> +/**
> + * SUBTEST: pxp-queue-alloc
> + * Description: Verify PXP exec queue creation works as expected
> + */
> +static void test_pxp_queue_creation(int fd, bool pxp_supported)
> +{
> + uint32_t q;
> + uint32_t vm;
> + int ret;
> +
> + vm = xe_vm_create(fd, 0, 0);
> +
> + /* queue creation with DRM_XE_PXP_TYPE_NONE must always succeed */
> + ret = __create_pxp_rcs_queue(fd, vm, DRM_XE_PXP_TYPE_NONE, &q);
> + igt_assert_eq(ret, 0);
> + xe_exec_queue_destroy(fd, q);
> +
> + /* queue creation with DRM_XE_PXP_TYPE_HWDRM must only succeed if PXP is supported */
> + ret = __create_pxp_rcs_queue(fd, vm, DRM_XE_PXP_TYPE_HWDRM, &q);
> + igt_assert_eq(ret, pxp_supported ? 0 : -ENODEV);
> + if (!ret)
> + xe_exec_queue_destroy(fd, q);
> +
> + /* queue creation with an invalid type must always fail */
> + ret = __create_pxp_rcs_queue(fd, vm, 0xFF, &q);
> + igt_assert_eq(ret, -EINVAL);
> +
> + xe_vm_destroy(fd, vm);
> +}
> +
> +igt_main
> +{
> + int xe_fd = -1;
> + bool pxp_supported = true;
> +
> + igt_fixture {
> + xe_fd = drm_open_driver(DRIVER_XE);
> + igt_require(xe_fd);
alan: nit: i think u can skip this check? (looking at implementation of drm_open_driver, test will skip if there's no
fd?)
> + igt_require(xe_has_engine_class(xe_fd, DRM_XE_ENGINE_CLASS_RENDER));
> + pxp_supported = is_pxp_hw_supported(xe_fd);
> + }
> +
> + igt_subtest_group {
> + igt_describe("Verify PXP allocations work as expected");
> + igt_subtest("pxp-bo-alloc")
> + test_pxp_bo_alloc(xe_fd, pxp_supported);
> +
> + igt_subtest("pxp-queue-alloc")
> + test_pxp_queue_creation(xe_fd, pxp_supported);
> + }
> +
> + igt_fixture {
> + close(xe_fd);
> + }
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 2724c7a9a..5904d9523 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -310,6 +310,7 @@ intel_xe_progs = [
> 'xe_pm',
> 'xe_pm_residency',
> 'xe_prime_self_import',
> + 'xe_pxp',
> 'xe_query',
> 'xe_render_copy',
> 'xe_vm',
> --
> 2.43.0
>
More information about the igt-dev
mailing list