Re: [PATCH v5 6/6] drm/xe/tests: Add KUnit tests for VF provisioning error handling
Michal Wajdeczko <[email protected]>
| Newsgroups | org.freedesktop.lists.intel-xe |
|---|---|
| Message-ID | <[email protected]> |
On 8/10/2026 12:14 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 <[email protected]> > Cc: Michal Wajdeczko <[email protected]> > --- > 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 | 322 ++++++++++++++++++ > drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 4 + > 2 files changed, 326 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..9756c069fbc6 > --- /dev/null > +++ b/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c > @@ -0,0 +1,322 @@ > +// 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 0xa0a0a000ull > + > +struct config { > + u32 orig; > + u32 query; > + int expected_ret; > + const char *name; > +} vf_ctx_testcase[] = { > + {TEST_CTXS, TEST_CTXS, 0, "same", }, > + {0, 0, -ENODATA, "none", }, > + {0, GUC_ID_MAX + 1, -EPROTO, "overflow", }, > + {TEST_CTXS, 0, -EREMCHG, "lost", }, > + {TEST_CTXS, TEST_CTXS / 2, -EREMCHG, "reduced", }, > + {TEST_CTXS, TEST_CTXS * 2, -EREMCHG, "increased", }, > +}, vf_db_testcase[] = { > + {TEST_DBS, TEST_DBS, 0, "same", }, > + {0, 0, 0, "none", }, > + {0, GUC_NUM_DOORBELLS + 1, -EPROTO, "overflow", }, > + {TEST_DBS, 0, -EREMCHG, "lost", }, > + {TEST_DBS, TEST_DBS / 2, -EREMCHG, "reduced", }, > + {TEST_DBS, GUC_NUM_DOORBELLS - 1, -EREMCHG, "increased", }, > +}; > + > +struct config_vram { > + u64 orig; > + u64 query; > + int expected_ret; > + const char *name; > +} vf_vram_testcase[] = { > + {TEST_VRAM, TEST_VRAM, 0, "same", }, > + {0, 0, -ENODATA, "none", }, > + {TEST_VRAM / 2, TEST_VRAM + SZ_1G, -EREMCHG, "overflow", }, > + {TEST_VRAM / 2, 0, -EREMCHG, "lost", }, > + {TEST_VRAM / 2, TEST_VRAM / 4, -EREMCHG, "reduced", }, > + {TEST_VRAM / 2, TEST_VRAM, -EREMCHG, "increased", }, > + {TEST_VRAM - SZ_1M, TEST_VRAM - SZ_1M, -EREMCHG, "unaligned", }, bad error code for unaligned > +}; > + > +struct config_ggtt { > + u64 start_orig; > + u64 start_query; > + u64 size_orig; > + u64 size_query; > + int expected_ret; > + const char *name; > +} vf_ggtt_testcase[] = { > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE, > + TEST_GGTT_SIZE, 0, "same", }, > + {0, 0, 0, > + 0, -ENODATA, "none", }, > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE, > + TEST_GGTT_SIZE + SZ_1G, -EREMCHG, "overflow", }, > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE, > + 0, -ENODATA, "lost", }, > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE, > + TEST_GGTT_SIZE - SZ_1M, -EREMCHG, "reduced", }, > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE, > + TEST_GGTT_SIZE + SZ_1M, -EREMCHG, "increased", }, > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE - SZ_2K, > + TEST_GGTT_SIZE - SZ_2K, -EREMCHG, "unaligned_size_4K", }, > + {TEST_GGTT_START, TEST_GGTT_START, TEST_GGTT_SIZE - SZ_8K, > + TEST_GGTT_SIZE - SZ_8K, -EREMCHG, "unaligned_size_64K",}, > + {TEST_GGTT_START - SZ_2K, TEST_GGTT_START - SZ_2K, TEST_GGTT_SIZE, > + TEST_GGTT_SIZE, -EREMCHG, "unaligned_base_4k", }, > + {TEST_GGTT_START - SZ_8K, TEST_GGTT_START - SZ_8K, TEST_GGTT_SIZE, > + TEST_GGTT_SIZE, -EREMCHG, "unaligned_base_64k", }, bad error codes for 'unaligned' cases > +}; > + > +struct config_mmio { > + int expected_ret; > + const char *name; > +} guc_mmio_resp_testcase[] = { > + {0, "same", }, > + {-ENODATA, "none", }, > + {-EOVERFLOW, "overflow", }, > + {-EREMCHG, "lost", }, > + {-EOVERFLOW, "invalid", }, ??? > +}; > + > +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); > + > +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 ((request[0] & 0xffff) != GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV) > + return -EINVAL; are you sure that original xe_guc_mmio_send_recv() returns -EINVAL for such case? it doesn't make any sense to write test code against the stub that behaves differently than original function maybe just add KUNIT_ASSERT_EQ(test, GUC_HXG_TYPE_REQUEST, FIELD_GET(GUC_HXG_MSG_0_TYPE, request[0]); KUNIT_ASSERT_EQ(test, GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV, FIELD_GET(GUC_HXG_REQUEST_MSG_0_ACTION, request[0]); > + > + if (request[1] != GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY) > + return -EINVAL; > + > + 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 ((request[0] & 0xffff) != GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV) > + return -EINVAL; > + > + if (request[1] != GUC_KLV_VF_CFG_NUM_DOORBELLS_KEY) > + return -EINVAL; > + > + 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 ((request[0] & 0xffff) != GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV) > + return -EINVAL; > + > + if (request[1] != GUC_KLV_VF_CFG_LMEM_SIZE_KEY) > + return -EINVAL; > + > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = c->query & 0xffffffff; > + response_buf[2] = (c->query >> 32) & 0xffffffff; use upper_32_bits / lower_32_bits > + > + 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; > + > + if ((request[0] & 0xffff) != GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV) > + return -EINVAL; > + > + if (request[1] != GUC_KLV_VF_CFG_GGTT_START_KEY && > + request[1] != GUC_KLV_VF_CFG_GGTT_SIZE_KEY) > + return -EINVAL; > + > + if (request[1] == GUC_KLV_VF_CFG_GGTT_START_KEY) { > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = c->start_query & 0xffffffff; > + response_buf[2] = (c->start_query >> 32) & 0xffffffff; > + } else { > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = c->size_query & 0xffffffff; > + response_buf[2] = (c->size_query >> 32) & 0xffffffff; ditto > + } > + > + 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 ((request[0] & 0xffff) != GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV) > + return -EINVAL; > + /** > + * Let us take help of vf_get_ctxs_cfg() function to test responses > + * from Guc. > + */ > + if (request[1] != GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY) > + return -EINVAL; > + > + if (!strcmp(c->name, "same")) { > + response_buf[0] = sizeof(u32) / sizeof(u32); > + response_buf[1] = TEST_CTXS; > + } else if (!strcmp(c->name, "none")) { > + response_buf[0] = 0; > + response_buf[1] = 0; > + } else if (!strcmp(c->name, "overflow")) { > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = TEST_CTXS; > + } else if (!strcmp(c->name, "lost")) { > + response_buf[0] = sizeof(u32) / sizeof(u32); > + response_buf[1] = 0; > + } else if (!strcmp(c->name, "invalid")) { > + response_buf[0] = 0xffff; > + response_buf[1] = 0xffff; > + } something is wrong here we shouldn't really generate new test code per test name all these values should be already part of the "param" struct and IIRC the goal was to add test cases to make sure that VF code can deal also with communication errors (not just malformed values in the response) maybe the simplest way is to add to the "param" new field: int ret; which would be 0 by default, and it would be used by all stubs: int xe_guc_mmio_send_recv_stub_vram(...) { if (param->ret) return param->ret; ... return 0; } and in the test case definition: { .ret = -EIO, .expected_ret = -EIO, ... } > + > + 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_size(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; > + > + if (!strcmp(c->name, "unaligned_size_64K") || > + !strcmp(c->name, "unaligned_base_64k")) using param name as condition is fragile, maybe just add .flag to the config? > + xe->info.vram_flags = XE_VRAM_FLAGS_NEED64K; > + > + 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_size, 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 6c1878e4b1b2..05a6ad3348be 100644 > --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c > @@ -1685,3 +1685,7 @@ int xe_gt_sriov_vf_wait_valid_ggtt(struct xe_gt *gt) > > return atomic_read(>->sriov.vf.migration.fixups_complete_count); > } > + > +#if IS_BUILTIN(CONFIG_DRM_XE_KUNIT_TEST) > +#include "tests/xe_gt_sriov_vf_kunit.c" > +#endif