Re: [PATCH v6 7/7] drm/xe/tests: Add KUnit tests for VF provisioning error handling

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 8/12/2026 2:43 PM, Satyanarayana K V P wrote:
> VF relies on the PF to provide a valid hardware configuration via GuC
> KLV responses. In unlikely event of PF malfunction or misconfiguration,
> a VF may receive incomplete, zero, or out-of-range values for its
> submission contexts, doorbells, VRAM or GGTT assignment.
> 
> Add KUnit test cases that use the xe_guc_mmio_send_recv() stub to inject
> bad KLV responses and verify that VF can survive without crashing for
> the invalid configuration data received.
> 
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> V5 -> V6:
> - Fixed review comments (Michal W).
> 
> V4 -> V5:
> - Added alignment tests for vram, GGTT base and size (Michal W).
> - Separated stubs for ctx, db, vram and GGTT (Michal W).
> - Added new test cases for xe_guc_mmio_send_recv() (Michal W).
> 
> V3 -> V4:
> - Changed stub function from guc_action_query_single_klv32() and
> guc_action_query_single_klv64() to xe_guc_mmio_send_recv() (Michal W).
> - Fixed review comments (Michal W).
> 
> V2 -> V3:
> - Renamed the test names. (Michal W).
> - Fixed review comments (Michal W).
> 
> V1 -> V2:
> - Renamed the test file (Michal W).
> - Fixed review comments (Michal W).
> ---
>  .../gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c   | 484 ++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.c           |   4 +
>  2 files changed, 488 insertions(+)
>  create mode 100644 drivers/gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c
> 
> diff --git a/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c b/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c
> new file mode 100644
> index 000000000000..8cb7f3cebb31
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c
> @@ -0,0 +1,484 @@
> +// SPDX-License-Identifier: GPL-2.0 AND MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <kunit/static_stub.h>
> +#include <kunit/test.h>
> +
> +#include "regs/xe_guc_regs.h"
> +#include "xe_device.h"
> +#include "xe_kunit_helpers.h"
> +#include "xe_pci_test.h"
> +#include "xe_guc.h"
> +
> +#define TEST_VRAM		SZ_8G
> +#define TEST_GGTT_SIZE		SZ_2G
> +#define TEST_CTXS		1024
> +#define TEST_DBS		128
> +#define TEST_GGTT_START	0xa0a00000ull
> +
> +struct config {
> +	u32 orig;
> +	u32 query;
> +	int expected_ret;
> +	int ret;
> +	const char *name;
> +} vf_ctx_testcase[] = {
> +	{
> +		.orig = TEST_CTXS,		.query = TEST_CTXS,
> +		.expected_ret = 0,		.name = "same",
> +	},
> +	{
> +		.orig = 0,			.query = 0,
> +		.expected_ret = -ENODATA,	.name = "none",
> +	},
> +	{
> +		.orig = 0,			.query = GUC_ID_MAX + 1,
> +		.expected_ret = -EPROTO,	.name = "overflow",
> +	},
> +	{
> +		.orig = TEST_CTXS,		.query = 0,
> +		.expected_ret = -EREMCHG,	.name = "lost",
> +	},
> +	{
> +		.orig = TEST_CTXS,		.query = TEST_CTXS / 2,
> +		.expected_ret = -EREMCHG,	.name = "reduced",
> +	},
> +	{
> +		.orig = TEST_CTXS,		.query = TEST_CTXS * 2,
> +		.expected_ret = -EREMCHG,	.name = "increased",
> +	},
> +	{
> +		.expected_ret = -EPROTO,	.ret = -EPROTO,
> +		.name = "proto_error",
> +	},
> +	{
> +		.expected_ret = -EIO,		.ret = -EIO,
> +		.name = "IO error",
> +	},
> +}, vf_db_testcase[] = {
> +	{
> +		.orig = TEST_DBS,		.query = TEST_DBS,
> +		.expected_ret = 0,		.name = "same",
> +	},
> +	{
> +		.orig = 0,			.query = 0,
> +		.expected_ret = 0,		.name = "none",
> +	},
> +	{
> +		.orig = TEST_DBS,		.query = GUC_NUM_DOORBELLS + 1,
> +		.expected_ret = -EPROTO,	.name = "overflow",
> +	},
> +	{
> +		.orig = TEST_DBS,		.query = 0,
> +		.expected_ret = -EREMCHG,	.name = "lost",
> +	},
> +	{
> +		.orig = TEST_DBS,		.query = TEST_DBS / 2,
> +		.expected_ret = -EREMCHG,	.name = "reduced",
> +	},
> +	{
> +		.orig = TEST_DBS,		.query = GUC_NUM_DOORBELLS - 1,
> +		.expected_ret = -EREMCHG,	.name = "increased",
> +	},
> +	{
> +		.expected_ret = -EPROTO,        .ret = -EPROTO,
> +		.name = "proto_error"
> +	},
> +	{
> +		.expected_ret = -EIO,		.ret = -EIO,
> +		.name = "IO error",
> +	},
> +};
> +
> +struct config_vram {
> +	u64 orig;
> +	u64 query;
> +	int expected_ret;
> +	int ret;
> +	const char *name;
> +} vf_vram_testcase[] = {
> +	{
> +		.orig = TEST_VRAM,		.query = TEST_VRAM,
> +		.expected_ret = 0,		.name = "same",
> +	},
> +	{
> +		.orig = 0,			.query = 0,
> +		.expected_ret = -ENODATA,	.name = "none",
> +	},
> +	{
> +		.orig = TEST_VRAM / 2,		.query = TEST_VRAM + SZ_1G,
> +		.expected_ret = -EREMCHG,	.name = "overflow",
> +	},
> +	{
> +		.orig = TEST_VRAM / 2,		.query = 0,
> +		.expected_ret = -EREMCHG,	.name = "lost",
> +	},
> +	{
> +		.orig = TEST_VRAM / 2,		.query = TEST_VRAM / 4,
> +		.expected_ret = -EREMCHG,	.name = "reduced",
> +	},
> +	{
> +		.orig = TEST_VRAM,		.query = TEST_VRAM * 2,
> +		.expected_ret = -EREMCHG,	.name = "increased",
> +	},
> +	{
> +		.orig = TEST_VRAM - SZ_1M,	.query = TEST_VRAM - SZ_1M,
> +		.expected_ret = -EINVAL,	.name = "unaligned",
> +	},
> +	{
> +		.expected_ret = -EPROTO,        .ret = -EPROTO,
> +		.name = "proto_error"
> +	},
> +	{
> +		.expected_ret = -EIO,		.ret = -EIO,
> +		.name = "IO error",
> +	},
> +};
> +
> +struct config_ggtt {
> +	u64 start_orig;
> +	u64 start_query;
> +	u64 size_orig;
> +	u64 size_query;
> +	int expected_ret;
> +	int ret;
> +	int flags;
> +	const char *name;
> +} vf_ggtt_testcase[] = {

static const ?

as a bonus you likely will not need to initialize some members to 0

> +	{
> +		.start_orig = TEST_GGTT_START,	.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE,	.size_query = TEST_GGTT_SIZE,
> +		.expected_ret = 0,		.flags = 0,
> +		.name = "same",
> +	},
> +	{
> +		.start_orig = 0,		.start_query = 0,
> +		.size_orig = 0,			.size_query = 0,
> +		.expected_ret = -ENODATA,	.flags = 0,
> +		.name = "none",
> +	},

we need all those negative test cases also for the initial query (orig = 0)

> +	{
> +		.start_orig = TEST_GGTT_START,	.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE,	.size_query = TEST_GGTT_SIZE + SZ_1G,
> +		.expected_ret = -EREMCHG,	.flags = 0,
> +		.name = "overflow",
> +	},
> +	{
> +		.start_orig = TEST_GGTT_START,	.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE,	.size_query = 0,
> +		.expected_ret = -ENODATA,	.flags = 0,
> +		.name = "lost",
> +	},
> +	{
> +		.start_orig = TEST_GGTT_START,	.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE,	.size_query = TEST_GGTT_SIZE - SZ_1M,
> +		.expected_ret = -EREMCHG,	.flags = 0,
> +		.name = "reduced",
> +	},
> +	{
> +		.start_orig = TEST_GGTT_START,	.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE,	.size_query = TEST_GGTT_SIZE + SZ_1M,
> +		.expected_ret = -EREMCHG,	.flags = 0,
> +		.name = "increased",
> +	},
> +	{
> +		.start_orig = TEST_GGTT_START,		.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE - SZ_2K,	.size_query = TEST_GGTT_SIZE - SZ_2K,

it shouldn't be possible to have unaligned original GGTT size (size_orig)

> +		.expected_ret = -EINVAL,		.flags = 0,
> +		.name = "unaligned_size_4K",
> +
> +	},
> +	{
> +		.start_orig = TEST_GGTT_START,		.start_query = TEST_GGTT_START,
> +		.size_orig = TEST_GGTT_SIZE - SZ_8K,	.size_query = TEST_GGTT_SIZE - SZ_8K,
> +		.expected_ret = -EINVAL,		.flags = XE_VRAM_FLAGS_NEED64K,
> +		.name = "unaligned_size_64K",
> +	},

ditto

> +	{
> +		.start_orig = TEST_GGTT_START - SZ_2K,	.start_query = TEST_GGTT_START - SZ_2K,
> +		.size_orig = TEST_GGTT_SIZE,		.size_query = TEST_GGTT_SIZE,
> +		.expected_ret = -EINVAL,		.flags = 0,
> +		.name = "unaligned_base_4k",
> +	},

same here, we should never store invalid GGTT start
> +	{
> +		.start_orig = TEST_GGTT_START - SZ_8K,	.start_query = TEST_GGTT_START - SZ_8K,
> +		.size_orig = TEST_GGTT_SIZE,		.size_query = TEST_GGTT_SIZE,
> +		.expected_ret = -EINVAL,		.flags = XE_VRAM_FLAGS_NEED64K,
> +		.name = "unaligned_base_64k",
> +	},
> +	{
> +		.expected_ret = -EPROTO,        .ret = -EPROTO,
> +		.name = "proto_error"
> +	},
> +	{
> +		.expected_ret = -EIO,		.ret = -EIO,
> +		.name = "IO error",
> +	},
> +};
> +
> +struct config_mmio {
> +	u32 response_length;
> +	u32 response_value;
> +	int expected_ret;
> +	int ret;
> +	const char *name;
> +} guc_mmio_resp_testcase[] = {
> +	{
> +		.response_length = sizeof(u32) / sizeof(u32),
> +		.response_value = TEST_CTXS,	.expected_ret = 0,	.name = "same",
> +	},
> +	{
> +		.response_length = 0,		.response_value = 0,
> +		.expected_ret = -ENODATA,	.name = "none",
> +	},
> +	{
> +		.response_length = sizeof(u64) / sizeof(u32),
> +		.response_value = TEST_CTXS,	.expected_ret = -EOVERFLOW,
> +		.name = "overflow",
> +	},
> +	{
> +		.response_length = sizeof(u32) / sizeof(u32),
> +		.response_value = 0,		.expected_ret = -EREMCHG,
> +		.name = "lost",
> +	},
> +	{
> +		.response_length = 0xffff,	.response_value = 0xffff,
> +		.expected_ret = -EOVERFLOW,	.name = "invalid",
> +	},
> +	{
> +		.expected_ret = -EPROTO,	.ret = -EPROTO,
> +		.name = "proto_error",
> +	},
> +	{
> +		.expected_ret = -EIO,		.ret = -EIO,
> +		.name = "IO error",
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM_DESC(ctx_testcase, vf_ctx_testcase, name);
> +KUNIT_ARRAY_PARAM_DESC(db_testcase, vf_db_testcase, name);
> +KUNIT_ARRAY_PARAM_DESC(vram_testcase, vf_vram_testcase, name);
> +KUNIT_ARRAY_PARAM_DESC(ggtt_testcase, vf_ggtt_testcase, name);
> +KUNIT_ARRAY_PARAM_DESC(guc_mmio_resp, guc_mmio_resp_testcase, name);

hmm, I'm wondering if maybe keeping config struct + KNIT_ARRAY helper
closer to the actual test case code will be easier to view/maintain ?

> +
> +static int xe_guc_mmio_send_recv_stub_ctx(struct xe_guc *guc, const u32 *request,
> +					  u32 len, u32  *response_buf)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	const struct config *c = test->param_value;
> +
> +	if (c->ret)
> +		return c->ret;
> +
> +	KUNIT_ASSERT_EQ(test, FIELD_GET(GUC_HXG_REQUEST_MSG_0_ACTION, request[0]),
> +			GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV);
> +	KUNIT_ASSERT_EQ(test,
> +			FIELD_GET(VF2GUC_QUERY_SINGLE_KLV_REQUEST_MSG_1_KEY, request[1]),
> +			GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY);
> +
> +	response_buf[0] = sizeof(u32) / sizeof(u32);
> +	response_buf[1] = c->query;
> +
> +	return 0;
> +}
> +
> +static int xe_guc_mmio_send_recv_stub_db(struct xe_guc *guc, const u32 *request,
> +					 u32 len, u32  *response_buf)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	const struct config *c = test->param_value;
> +
> +	if (c->ret)
> +		return c->ret;
> +
> +	KUNIT_ASSERT_EQ(test, FIELD_GET(GUC_HXG_REQUEST_MSG_0_ACTION, request[0]),
> +			GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV);
> +	KUNIT_ASSERT_EQ(test,
> +			FIELD_GET(VF2GUC_QUERY_SINGLE_KLV_REQUEST_MSG_1_KEY, request[1]),
> +			GUC_KLV_VF_CFG_NUM_DOORBELLS_KEY);
> +
> +	response_buf[0] = sizeof(u32) / sizeof(u32);
> +	response_buf[1] = c->query;
> +
> +	return 0;
> +}
> +
> +static int xe_guc_mmio_send_recv_stub_vram(struct xe_guc *guc, const u32 *request,
> +					   u32 len, u32  *response_buf)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	const struct config_vram *c = test->param_value;
> +
> +	if (c->ret)
> +		return c->ret;
> +
> +	KUNIT_ASSERT_EQ(test, FIELD_GET(GUC_HXG_REQUEST_MSG_0_ACTION, request[0]),
> +			GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV);
> +	KUNIT_ASSERT_EQ(test,
> +			FIELD_GET(VF2GUC_QUERY_SINGLE_KLV_REQUEST_MSG_1_KEY, request[1]),
> +			GUC_KLV_VF_CFG_LMEM_SIZE_KEY);
> +
> +	response_buf[0] = sizeof(u64) / sizeof(u32);
> +	response_buf[1] = lower_32_bits(c->query);
> +	response_buf[2] = upper_32_bits(c->query);
> +
> +	return 0;
> +}
> +
> +static int xe_guc_mmio_send_recv_stub_ggtt(struct xe_guc *guc, const u32 *request,
> +					   u32 len, u32  *response_buf)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	const struct config_ggtt *c = test->param_value;
> +	u32 key;
> +
> +	if (c->ret)
> +		return c->ret;
> +
> +	KUNIT_ASSERT_EQ(test, FIELD_GET(GUC_HXG_REQUEST_MSG_0_ACTION, request[0]),
> +			GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV);
> +	key = FIELD_GET(VF2GUC_QUERY_SINGLE_KLV_REQUEST_MSG_1_KEY, request[1]);
> +	KUNIT_ASSERT_TRUE(test,
> +			  key == GUC_KLV_VF_CFG_GGTT_START_KEY ||
> +			  key == GUC_KLV_VF_CFG_GGTT_SIZE_KEY);
> +
> +	response_buf[0] = sizeof(u64) / sizeof(u32);
> +	if (key == GUC_KLV_VF_CFG_GGTT_START_KEY) {
> +		response_buf[1] = lower_32_bits(c->start_query);
> +		response_buf[2] = upper_32_bits(c->start_query);
> +	} else {

else if (key == START)

> +		response_buf[1] = lower_32_bits(c->size_query);
> +		response_buf[2] = upper_32_bits(c->size_query);
> +	}		
else
	KUNIT_FAIL

> +
> +	return 0;
> +}
> +
> +static int xe_guc_mmio_send_recv_stub(struct xe_guc *guc, const u32 *request,
> +				      u32 len, u32  *response_buf)
> +{
> +	struct kunit *test = kunit_get_current_test();
> +	const struct config_mmio *c = test->param_value;
> +
> +	if (c->ret)
> +		return c->ret;
> +
> +	KUNIT_ASSERT_EQ(test, FIELD_GET(GUC_HXG_REQUEST_MSG_0_ACTION, request[0]),
> +			GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV);
> +	/**
> +	 *  Let us take help of vf_get_ctxs_cfg() function to test responses
> +	 *  from Guc.
> +	 */

do we really need this comment?
> +	KUNIT_ASSERT_EQ(test,
> +			FIELD_GET(VF2GUC_QUERY_SINGLE_KLV_REQUEST_MSG_1_KEY, request[1]),
> +			GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY);
> +
> +	response_buf[0] = c->response_length;
> +	response_buf[1] = c->response_value;
> +
> +	return 0;
> +}
> +
> +static void test_ctxs(struct kunit *test)
> +{
> +	struct xe_gt *gt = test->priv;
> +	const struct config *c = test->param_value;
> +
> +	gt->sriov.vf.self_config.num_ctxs = c->orig;
> +	kunit_activate_static_stub(test, xe_guc_mmio_send_recv,
> +				   xe_guc_mmio_send_recv_stub_ctx);
> +	KUNIT_EXPECT_EQ(test, vf_get_ctxs_cfg(gt), c->expected_ret);
> +}
> +
> +static void test_dbs(struct kunit *test)
> +{
> +	struct xe_gt *gt = test->priv;
> +	const struct config *c = test->param_value;
> +
> +	gt->sriov.vf.self_config.num_dbs = c->orig;
> +	kunit_activate_static_stub(test, xe_guc_mmio_send_recv,
> +				   xe_guc_mmio_send_recv_stub_db);
> +	KUNIT_EXPECT_EQ(test, vf_get_dbs_cfg(gt), c->expected_ret);
> +}
> +
> +static void test_vram(struct kunit *test)
> +{
> +	struct xe_gt *gt = test->priv;
> +	const struct config_vram *c = test->param_value;
> +
> +	gt->tile->sriov.vf.self_config.lmem_size = c->orig;
> +	kunit_activate_static_stub(test, xe_guc_mmio_send_recv,
> +				   xe_guc_mmio_send_recv_stub_vram);
> +	KUNIT_EXPECT_EQ(test, vf_get_lmem_info(gt), c->expected_ret);
> +}
> +
> +static void test_ggtt(struct kunit *test)
> +{
> +	const struct config_ggtt *c = test->param_value;
> +	struct xe_gt *gt = test->priv;
> +	struct xe_device *xe = gt_to_xe(gt);
> +
> +	gt->tile->sriov.vf.self_config.ggtt_base = c->start_orig;
> +	gt->tile->sriov.vf.self_config.ggtt_size = c->size_orig;
> +
> +	xe->info.vram_flags = c->flags;

hmm, if flags are only used here, then we should name it c->vram_flags

but OTOH maybe it is overkill to test for 64K mis-alignment if we
already have test for 4K and the code under test is the same?

> +
> +	kunit_activate_static_stub(test, xe_guc_mmio_send_recv,
> +				   xe_guc_mmio_send_recv_stub_ggtt);
> +	KUNIT_EXPECT_EQ(test, vf_get_ggtt_info(gt), c->expected_ret);
> +}
> +
> +static void test_guc_mmio(struct kunit *test)
> +{
> +	const struct config_mmio *c = test->param_value;
> +	struct xe_gt *gt = test->priv;
> +
> +	gt->sriov.vf.self_config.num_ctxs = TEST_CTXS;
> +	kunit_activate_static_stub(test, xe_guc_mmio_send_recv,
> +				   xe_guc_mmio_send_recv_stub);
> +	KUNIT_EXPECT_EQ(test, vf_get_ctxs_cfg(gt), c->expected_ret);
> +}
> +
> +static int vf_gt_config_test_init(struct kunit *test)
> +{
> +	struct xe_pci_fake_data fake = {
> +		.sriov_mode = XE_SRIOV_MODE_VF,
> +		.platform = XE_BATTLEMAGE, /* any random DGFX platform with SR-IOV */
> +		.subplatform = XE_SUBPLATFORM_NONE,
> +		.graphics_verx100 = 2001,
> +	};
> +	struct xe_device *xe;
> +	struct xe_gt *gt;
> +
> +	test->priv = &fake;
> +	xe_kunit_helper_xe_device_test_init(test);
> +
> +	xe = test->priv;
> +	KUNIT_ASSERT_TRUE(test, IS_SRIOV_VF(xe));
> +	KUNIT_ASSERT_TRUE(test, IS_DGFX(xe));
> +
> +	gt = xe_root_mmio_gt(xe);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, gt);
> +	test->priv = gt;
> +
> +	return 0;
> +}
> +
> +static struct kunit_case vf_gt_config_test_cases[] = {
> +	KUNIT_CASE_PARAM(test_ctxs, ctx_testcase_gen_params),
> +	KUNIT_CASE_PARAM(test_dbs, db_testcase_gen_params),
> +	KUNIT_CASE_PARAM(test_vram, vram_testcase_gen_params),
> +	KUNIT_CASE_PARAM(test_ggtt, ggtt_testcase_gen_params),
> +	KUNIT_CASE_PARAM(test_guc_mmio, guc_mmio_resp_gen_params),
> +	{}
> +};
> +
> +static struct kunit_suite vf_gt_config_suite = {
> +	.name = "vf_gt_config",
> +	.test_cases = vf_gt_config_test_cases,
> +	.init = vf_gt_config_test_init,
> +};
> +
> +kunit_test_suite(vf_gt_config_suite);
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index be932ebe6a8d..d6b631cd4dfc 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -1684,3 +1684,7 @@ int xe_gt_sriov_vf_wait_valid_ggtt(struct xe_gt *gt)
>  
>  	return atomic_read(&gt->sriov.vf.migration.fixups_complete_count);
>  }
> +
> +#if IS_BUILTIN(CONFIG_DRM_XE_KUNIT_TEST)
> +#include "tests/xe_gt_sriov_vf_kunit.c"
> +#endif