Re: [PATCH v4 4/4] drm/xe/tests: Add KUnit tests for VF provisioning error handling
Michal Wajdeczko <[email protected]> Wed, 5 Aug 2026 14:14:01 +0200
| Newsgroups | org.freedesktop.lists.intel-xe |
|---|---|
| Message-ID | <[email protected]> |
On 7/28/2026 11:50 AM, 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]> > --- > 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 | 212 ++++++++++++++++++ > drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 4 + > 2 files changed, 216 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..5444cad1eead > --- /dev/null > +++ b/drivers/gpu/drm/xe/tests/xe_gt_sriov_vf_kunit.c > @@ -0,0 +1,212 @@ > +// 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 0xdeadbeef00aa00ccull hmm, currently our GGTT is up to 4GB (32b) why choosing the test (workable) GGTT start value that huge? > + > +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, -ENODATA, "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", }, what about unaligned (non-2M) sizes? > +}; > + > +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", }, what about unaligned (non-4K) base/size? > +}; > + > +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); > + > +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_vram *c_vram; > + const struct config_ggtt *c_ggtt; > + const struct config *c; maybe it would be safer/cleaner to have separate stubs for each config type? then each test will activate proper stub (instead of activating this combo in init) > + > + if ((request[0] & 0xffff) != GUC_ACTION_VF2GUC_QUERY_SINGLE_KLV) > + return -EINVAL; > + > + switch (request[1]) { > + case GUC_KLV_VF_CFG_NUM_CONTEXTS_KEY: > + c = test->param_value; > + response_buf[0] = sizeof(u32) / sizeof(u32); > + response_buf[1] = c->query; > + break; > + > + case GUC_KLV_VF_CFG_NUM_DOORBELLS_KEY: > + c = test->param_value; > + response_buf[0] = sizeof(u32) / sizeof(u32); > + response_buf[1] = c->query; > + break; > + > + case GUC_KLV_VF_CFG_LMEM_SIZE_KEY: > + c_vram = test->param_value; > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = c_vram->query & 0xffffffff; > + response_buf[2] = (c_vram->query >> 32) & 0xffffffff; > + break; > + > + case GUC_KLV_VF_CFG_GGTT_START_KEY: > + c_ggtt = test->param_value; > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = c_ggtt->start_query & 0xffffffff; > + response_buf[2] = (c_ggtt->start_query >> 32) & 0xffffffff; > + break; > + > + case GUC_KLV_VF_CFG_GGTT_SIZE_KEY: > + c_ggtt = test->param_value; > + response_buf[0] = sizeof(u64) / sizeof(u32); > + response_buf[1] = c_ggtt->size_query & 0xffffffff; > + response_buf[2] = (c_ggtt->size_query >> 32) & 0xffffffff; > + break; > + > + default: > + return -EINVAL; > + } > + > + 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_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_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_EXPECT_EQ(test, vf_get_lmem_info(gt), c->expected_ret); > +} > + > +static void test_ggtt_size(struct kunit *test) test_ggtt ? > +{ > + const struct config_ggtt *c = test->param_value; > + struct xe_gt *gt = test->priv; > + > + gt->tile->sriov.vf.self_config.ggtt_base = c->start_orig; > + gt->tile->sriov.vf.self_config.ggtt_size = c->size_orig; > + KUNIT_EXPECT_EQ(test, vf_get_ggtt_info(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; > + > + kunit_activate_static_stub(test, xe_guc_mmio_send_recv, xe_guc_mmio_send_recv_stub); > + 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), also, what about adding test cases that will also inject GuC MMIO communication errors? and/or return too short/too long KLV? > + {} > +}; > + > +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 69e531be15ed..dd06b23d7a9d 100644 > --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c > @@ -1661,3 +1661,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