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

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

On 7/10/2026 2:19 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, LMEM, or GGTT assignment.
> 
> Add KUnit test cases that use the guc_action_query_single_klv() static
> stub to inject bad KLV responses and verify that VF can return a negative
> error code rather than silently accepting the invalid configuration.
> 
> The following error scenarios are covered per resource:
> 
>   Contexts : zero, less-by-4K, invalid , one, more-by-4K
>   Doorbells: zero, less-by-32, invalid , one, more-by-32
>   LMEM     : zero, less-by-32M, invalid , one-GB, more-by-1G
>   GGTT     : zero size, size-less-by-4K, size-invalid
> 
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/tests/xe_gt_sriov_vf.c | 387 ++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.c       |   4 +
>  2 files changed, 391 insertions(+)
>  create mode 100644 drivers/gpu/drm/xe/tests/xe_gt_sriov_vf.c
> 
> diff --git a/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf.c
> new file mode 100644
> index 000000000000..529513255dc3
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf.c

add _kunit suffix, per [1]

[1] https://elixir.bootlin.com/linux/v7.2-rc2/source/Documentation/dev-tools/kunit/style.rst#L192

> @@ -0,0 +1,387 @@
> +// SPDX-License-Identifier: GPL-2.0 AND MIT
> +/*
> + * Copyright © 2026-2027 Intel Corporation

ohh, you have a time machine ;)

> + */
> +
> +#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_vram_types.h"
> +#include "xe_sriov_pf_helpers.h"
> +
> +#define TEST_MAX_VFS    63
> +#define TEST_VRAM       0x7a800000ull   /* random size that works on 32-bit */
> +#define TEST_GGTT_SIZE  SZ_2G
> +
> +static int guc_action_query_single_klv_stub(struct xe_guc *guc, u32 key, u32 *value, u32 value_len);
> +static int vf_gt_config_test_init(struct kunit *test);

reorg your code to avoid forward decl for static functions

> +
> +enum vf_config_types {
> +	CTX_CONFIG,
> +	DB_CONFIG,
> +	VRAM_CONFIG,
> +	GGTT_CONFIG,
> +};
> +
> +struct vf_config_type {
> +	enum vf_config_types type;
> +};
> +
> +enum ctx_config_types {
> +	CTX_CONFIG_DEFAULT,
> +	CTX_CONFIG_ZERO_CTXS,
> +	CTX_CONFIG_LESS_BY_4K_CTXS,
> +	CTX_CONFIG_INVALID_CTXS,
> +	CTX_CONFIG_ONE_CTX,
> +	CTX_CONFIG_MORE_BY_4K,
> +	CTX_CONFIG_MAX_CTXS,
> +
> +};
> +
> +struct vf_ctx_config_case {
> +	struct vf_config_type config_type;
> +	enum ctx_config_types ctx_type;
> +	const char *test_name;
> +};
> +
> +static const struct vf_ctx_config_case vf_ctx_config_cases[] = {
> +	{.config_type = { .type = CTX_CONFIG },
> +		.ctx_type = CTX_CONFIG_ZERO_CTXS,
> +		.test_name = "contexts_zero_1vf"
> +	},
> +	{.config_type = { .type = CTX_CONFIG },
> +		.ctx_type = CTX_CONFIG_LESS_BY_4K_CTXS,
> +		.test_name = "contexts_less_by_4K_1vf"
> +	},
> +	{.config_type = { .type = CTX_CONFIG },
> +		.ctx_type = CTX_CONFIG_INVALID_CTXS,
> +		.test_name = "contexts_invalid_1vf"
> +	},
> +	{.config_type = { .type = CTX_CONFIG },
> +		.ctx_type = CTX_CONFIG_ONE_CTX,
> +		.test_name = "context_one_1vf"
> +	},
> +	{.config_type = { .type = CTX_CONFIG },
> +		.ctx_type = CTX_CONFIG_MORE_BY_4K,
> +		.test_name = "contexts_more_by_4K_1vf"
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM_DESC(vf_ctx_config, vf_ctx_config_cases, test_name);
> +
> +enum db_config_types {
> +	DB_CONFIG_DEFAULT = CTX_CONFIG_MAX_CTXS,
> +	DB_CONFIG_ZERO_DOORBELLS,
> +	DB_CONFIG_LESS_BY_32_DOORBELLS,
> +	DB_CONFIG_INVALID_DOORBELLS,
> +	DB_CONFIG_ONE_DOORBELL,
> +	DB_CONFIG_MORE_BY_32_DOORBELLS,
> +};
> +
> +struct vf_db_config_case {
> +	struct vf_config_type config_type;
> +	enum db_config_types db_type;
> +	const char *test_name;
> +};
> +
> +static const struct vf_db_config_case vf_db_config_cases[] = {
> +	{.config_type = { .type = DB_CONFIG },
> +		.db_type = DB_CONFIG_ZERO_DOORBELLS,
> +		.test_name = "doorbells_zero_1vf"
> +	},
> +	{.config_type = { .type = DB_CONFIG },
> +		.db_type = DB_CONFIG_LESS_BY_32_DOORBELLS,
> +		.test_name = "doorbells_less_by_32_1vf"
> +	},
> +	{.config_type = { .type = DB_CONFIG },
> +		.db_type = DB_CONFIG_INVALID_DOORBELLS,
> +		.test_name = "doorbells_invalid_1vf"
> +	},
> +	{.config_type = { .type = DB_CONFIG },
> +		.db_type = DB_CONFIG_ONE_DOORBELL,
> +		.test_name = "doorbells_one_1vf"
> +	},
> +	{.config_type = { .type = DB_CONFIG },
> +		.db_type = DB_CONFIG_MORE_BY_32_DOORBELLS,
> +		.test_name = "doorbells_more_by_32_1vf"
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM_DESC(vf_db_config, vf_db_config_cases, test_name);
> +
> +enum vram_config_types {
> +	VRAM_CONFIG_DEFAULT,
> +	VRAM_CONFIG_ZERO_VRAM,
> +	VRAM_CONFIG_LESS_BY_32M_VRAM,
> +	VRAM_CONFIG_INVALID_VRAM,
> +	VRAM_CONFIG_ONE_GB_VRAM,
> +	VRAM_CONFIG_MORE_BY_1G_VRAM,
> +};
> +
> +struct vf_vram_config_case {
> +	struct vf_config_type config_type;
> +	enum vram_config_types vram_type;
> +	const char *test_name;
> +};
> +
> +static const struct vf_vram_config_case vf_vram_config_cases[] = {
> +	{.config_type = { .type = VRAM_CONFIG },
> +		.vram_type = VRAM_CONFIG_ZERO_VRAM,
> +		.test_name = "vram_zero_1vf"
> +	},
> +	{.config_type = { .type = VRAM_CONFIG },
> +		.vram_type = VRAM_CONFIG_LESS_BY_32M_VRAM,
> +		.test_name = "vram_less_by_32M_1vf"
> +	},
> +	{.config_type = { .type = VRAM_CONFIG },
> +		.vram_type = VRAM_CONFIG_INVALID_VRAM,
> +		.test_name = "vram_invalid_1vf"
> +	},
> +	{.config_type = { .type = VRAM_CONFIG },
> +		.vram_type = VRAM_CONFIG_ONE_GB_VRAM,
> +		.test_name = "vram_one_gb_1vf"
> +	},
> +	{.config_type = { .type = VRAM_CONFIG },
> +		.vram_type = VRAM_CONFIG_MORE_BY_1G_VRAM,
> +		.test_name = "vram_more_by_1G_1vf"
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM_DESC(vf_vram_config, vf_vram_config_cases, test_name);
> +
> +enum ggtt_config_types {
> +	GGTT_CONFIG_DEFAULT,
> +	GGTT_CONFIG_SIZE_INVALID,
> +	GGTT_CONFIG_SIZE_ZERO,
> +	GGTT_CONFIG_SIZE_LESS_BY_4K,
> +};
> +
> +struct vf_ggtt_config_case {
> +	struct vf_config_type config_type;
> +	enum ggtt_config_types ggtt_type;
> +	const char *test_name;
> +};
> +
> +static const struct vf_ggtt_config_case ggtt_config_cases[] = {
> +	{.config_type = { .type = GGTT_CONFIG },
> +		.ggtt_type = GGTT_CONFIG_SIZE_ZERO,
> +		.test_name = "ggtt_size_zero_1vf"
> +	},
> +	{.config_type = { .type = GGTT_CONFIG },
> +		.ggtt_type = GGTT_CONFIG_SIZE_LESS_BY_4K,
> +		.test_name = "ggtt_size_less_by_4K_1vf"
> +	},
> +	{.config_type = { .type = GGTT_CONFIG },
> +		.ggtt_type = GGTT_CONFIG_SIZE_INVALID,
> +		.test_name = "ggtt_size_invalid_1vf"
> +	},
> +};
> +
> +KUNIT_ARRAY_PARAM_DESC(vf_ggtt_config, ggtt_config_cases, test_name);
> +

can we have something more like:

struct config {
	u32 orig;	// value to preset in vf.selfconfig
	u32 query;	// value to return from query 
	int expected_ret;
	const char *name;
} testcase [] = {
	{ .orig = 0, .query = 0, .ret = -ENODATA, .name = "none", },
	{ .orig = 0, .query = 65536, .ret = -EINVAL, .name = "overflow", },
	{ .orig = 1024, .query = 0, .ret = -EREMCHG, .name = "lost", },
	{ .orig = 1024, .query = 512, .ret = -EREMCHG, .name = "reduced", },
	{ .orig = 1024, .query = 2048, .ret = -EREMCHG, .name = "increased", },
	...
};

to keep the test values in place?

> +static void vf_gt_config_1vf(struct kunit *test)
> +{
> +	int config_type = ((struct vf_config_type *)test->param_value)->type;
> +	struct xe_gt *gt = test->priv;
> +
> +	kunit_activate_static_stub(test, guc_action_query_single_klv,
> +				   guc_action_query_single_klv_stub);
> +
> +	if (config_type == CTX_CONFIG) {

define separate test function for each config category
there should be no "if" here

> +		const struct vf_ctx_config_case *c = test->param_value;
> +		enum ctx_config_types ctx_type = c->ctx_type;
> +
> +		test->priv = &ctx_type;
> +
> +		KUNIT_EXPECT_LT(test, vf_get_submission_cfg(gt), 0);
> +	} else if (config_type == DB_CONFIG) {
> +		const struct vf_db_config_case *c = test->param_value;
> +		enum db_config_types db_type = c->db_type;
> +
> +		test->priv = &db_type;
> +
> +		KUNIT_EXPECT_LT(test, vf_get_submission_cfg(gt), 0);
> +	} else if (config_type == VRAM_CONFIG) {
> +		const struct vf_vram_config_case *c = test->param_value;
> +		enum vram_config_types vram_type = c->vram_type;
> +
> +		test->priv = &vram_type;
> +
> +		KUNIT_EXPECT_LT(test, vf_get_lmem_info(gt), 0);
> +	} else if (config_type == GGTT_CONFIG) {
> +		const struct vf_ggtt_config_case *c = test->param_value;
> +		enum ggtt_config_types ggtt_type = c->ggtt_type;
> +
> +		test->priv = &ggtt_type;
> +
> +		KUNIT_EXPECT_LT(test, vf_get_ggtt_info(gt), 0);
> +	}
> +
> +	kunit_deactivate_static_stub(test, guc_action_query_single_klv);

likely not needed

> +}
> +
> +static int guc_action_query_single_klv_stub(struct xe_guc *guc, u32 key, u32 *value, u32 value_len)
> +{
> +	struct xe_gt *gt = guc_to_gt(guc);
> +	struct xe_device *xe = gt_to_xe(gt);
> +	int *type = kunit_get_current_test()->priv;
> +
> +	switch (key) {
> +	case GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY:
> +		switch (*type) {
> +		case CTX_CONFIG_ZERO_CTXS:
> +			*value = 0;
> +			break;
> +		case CTX_CONFIG_LESS_BY_4K_CTXS:
> +			*value = gt->sriov.vf.self_config.num_ctxs - SZ_4K;
> +			break;
> +		case CTX_CONFIG_INVALID_CTXS:
> +			*value = GUC_ID_MAX + SZ_1K; /* greater than max context value */
> +			break;
> +		case CTX_CONFIG_ONE_CTX:
> +			*value = 1;
> +			break;
> +		case CTX_CONFIG_MORE_BY_4K:
> +			*value = gt->sriov.vf.self_config.num_ctxs + SZ_4K;
> +			break;
> +		default:
> +			*value = gt->sriov.vf.self_config.num_ctxs;
> +			break;
> +		}
> +		break;
> +	case GUC_KLV_VF_CFG_NUM_DOORBELLS_KEY:
> +		switch (*type) {
> +		case DB_CONFIG_ZERO_DOORBELLS:
> +			*value = 0;
> +			break;
> +		case DB_CONFIG_LESS_BY_32_DOORBELLS:
> +			*value = gt->sriov.vf.self_config.num_dbs - SZ_32;
> +			break;
> +		case DB_CONFIG_INVALID_DOORBELLS:
> +			*value = GUC_NUM_DOORBELLS + 10; /* greater than max doorbells */
> +			break;
> +		case DB_CONFIG_ONE_DOORBELL:
> +			*value = 1;
> +			break;
> +		case DB_CONFIG_MORE_BY_32_DOORBELLS:
> +			*value = gt->sriov.vf.self_config.num_dbs + SZ_32;
> +			break;
> +		default:
> +			*value = gt->sriov.vf.self_config.num_dbs;
> +			break;
> +		}
> +		break;
> +	case GUC_KLV_VF_CFG_LMEM_SIZE_KEY:
> +		switch (*type) {
> +		case VRAM_CONFIG_ZERO_VRAM:
> +			*value = 0;
> +			break;
> +		case VRAM_CONFIG_LESS_BY_32M_VRAM:
> +			*value = xe->tiles[0].sriov.vf.self_config.lmem_size - SZ_32M;
> +			break;
> +		case VRAM_CONFIG_INVALID_VRAM:
> +			*value = xe->tiles[0].sriov.vf.self_config.lmem_size * 2;
> +			break;
> +		case VRAM_CONFIG_MORE_BY_1G_VRAM:
> +			*value = xe->tiles[0].sriov.vf.self_config.lmem_size + SZ_1G;
> +			break;
> +		case VRAM_CONFIG_ONE_GB_VRAM:
> +			*value = SZ_1G;
> +			break;
> +		default:
> +			*value = xe->tiles[0].sriov.vf.self_config.lmem_size;
> +			break;
> +		}
> +		break;
> +	case GUC_KLV_VF_CFG_GGTT_SIZE_KEY:
> +		switch (*type) {
> +		case GGTT_CONFIG_SIZE_INVALID:
> +			*value = SZ_4K;
> +			break;
> +		case GGTT_CONFIG_SIZE_ZERO:
> +			*value = 0;
> +			break;
> +		case GGTT_CONFIG_SIZE_LESS_BY_4K:
> +			*value = xe->tiles[0].sriov.vf.self_config.ggtt_size - SZ_4K;
> +			break;
> +		default:
> +			*value = xe->tiles[0].sriov.vf.self_config.ggtt_size;
> +			break;
> +		}
> +		break;
> +	}
> +	return 0;
> +}
> +
> +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_vram_region *vram;
> +	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));
> +
> +	gt = xe_root_mmio_gt(xe);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, gt);
> +	test->priv = gt;
> +
> +	/* pretend it has some VRAM */
> +	KUNIT_ASSERT_TRUE(test, IS_DGFX(xe));
> +	vram = kunit_kzalloc(test, sizeof(*vram), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vram);
> +	vram->usable_size = TEST_VRAM;
> +	xe->mem.vram = vram;
> +	xe->tiles[0].mem.vram = vram;

do we need this?

> +
> +	/* pretend we have a valid LMTT */
> +	KUNIT_ASSERT_TRUE(test, xe_device_has_lmtt(xe));
> +	KUNIT_ASSERT_GE(test, GRAPHICS_VERx100(xe), 1260);
> +	xe->tiles[0].sriov.pf.lmtt.ops = &lmtt_ml_ops;
> +
> +	/* pretend it can support up to 63 VFs */
> +	xe->sriov.pf.device_total_vfs = TEST_MAX_VFS;
> +	xe->sriov.pf.driver_max_vfs = TEST_MAX_VFS;
> +	KUNIT_ASSERT_EQ(test, xe_sriov_pf_get_totalvfs(xe), 63);

your fake device is VF, so don't touch PF-only fields

> +
> +	KUNIT_ASSERT_EQ(test, xe_sriov_init(xe), 0);
> +
> +	/* more sanity checks */
> +	KUNIT_EXPECT_EQ(test, GUC_ID_MAX + 1, SZ_64K);
> +	KUNIT_EXPECT_EQ(test, GUC_NUM_DOORBELLS, SZ_256);
> +
> +	gt->sriov.vf.self_config.num_ctxs = SZ_32K;
> +	gt->sriov.vf.self_config.num_dbs = 128;
> +	xe->tiles[0].sriov.vf.self_config.lmem_size = TEST_VRAM;
> +	xe->tiles[0].sriov.vf.self_config.ggtt_size = TEST_GGTT_SIZE;

why? the test goal should be to verify that VF correctly detects and
rejects any invalid config received from GuC/PF (as this would be
the part of the bootstrap)

the only case where we would need these preset, is the restore,
where we just expect that all config, but GGTT start, is the same
as previously received - but I guess this is a separate testcase
or to be done within the test scenario itself

> +
> +	return 0;
> +}
> +
> +static struct kunit_case vf_gt_config_test_cases[] = {
> +	KUNIT_CASE_PARAM(vf_gt_config_1vf, vf_ctx_config_gen_params),
> +	KUNIT_CASE_PARAM(vf_gt_config_1vf, vf_db_config_gen_params),
> +	KUNIT_CASE_PARAM(vf_gt_config_1vf, vf_vram_config_gen_params),
> +	KUNIT_CASE_PARAM(vf_gt_config_1vf, vf_ggtt_config_gen_params),

there should be separate test names
and use better names, as now it would be listed as:

	vf_gt_config.vf_gt_config_1vf
	vf_gt_config.vf_gt_config_1vf
	vf_gt_config.vf_gt_config_1vf
	vf_gt_config.vf_gt_config_1vf


> +	{}
> +};
> +
> +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 802c982ea39b..7b4b88cb81d7 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -1579,3 +1579,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.c"
> +#endif