[PATCH i-g-t v2 1/2] tests/intel/xe_pmu: Add engine activity test for all functions
Riana Tauro
riana.tauro at intel.com
Thu Mar 20 13:51:51 UTC 2025
Hi Umesh
On 3/20/2025 4:41 AM, Umesh Nerlige Ramappa wrote:
> I missed out some comments, adding it here:
>
> On Wed, Mar 19, 2025 at 03:22:00PM -0700, Umesh Nerlige Ramappa wrote:
>> On Wed, Mar 12, 2025 at 11:54:30AM +0530, Riana Tauro wrote:
>>> Add a test that runs workload on all functions simultaneously and
>>> validates that all engines are equally and fully loaded for the
>>> entire execution quantum of the function
>>>
>>> v2: add a different function for function config
>>> add function details to log
>>> move pmu_fd to struct
>>> enable and provision VF's (Umesh)
>>>
>>> Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>>> ---
>>> tests/intel/xe_pmu.c | 130 +++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 120 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
>>> index 66edf24ad..a95ee1777 100644
>>> --- a/tests/intel/xe_pmu.c
>>> +++ b/tests/intel/xe_pmu.c
>>> @@ -14,18 +14,22 @@
>>>
>>> #include "igt.h"
>>> #include "igt_perf.h"
>>> +#include "igt_sriov_device.h"
>>> #include "igt_sysfs.h"
>>>
>>> #include "xe/xe_gt.h"
>>> #include "xe/xe_ioctl.h"
>>> #include "xe/xe_spin.h"
>>> +#include "xe/xe_sriov_provisioning.h"
>>>
>>> #define SLEEP_DURATION 2 /* in seconds */
>>> +#define TOTAL_EXEC_QUANTUM 128 /* in ms */
>>> /* flag masks */
>>> #define TEST_LOAD BIT(0)
>>> #define TEST_TRAILING_IDLE BIT(1)
>>>
>>> const double tolerance = 0.1;
>>> +char xe_device[NAME_MAX];
>>
>> These globals should be static.
>>
>> Optionally, for easier reviews, it's better to split the patch in two
>> - (1) make xe_device global (2) Add the engine activity test.
>>
>> With just the static change, this is
>>
>> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>>
>> Thanks,
>> Umesh
>>
>>>
>>> #define test_each_engine(test, fd, hwe) \
>>> igt_subtest_with_dynamic(test) \
>>> @@ -92,7 +96,7 @@ static unsigned long read_idle_residency(int fd,
>>> int gt)
>>> return residency;
>>> }
>>>
>>> -static uint64_t add_format_config(const char *xe_device, const char
>>> *format, uint64_t val)
>>> +static uint64_t add_format_config(const char *format, uint64_t val)
>>> {
>>> int ret;
>>> uint32_t shift;
>>> @@ -105,26 +109,30 @@ static uint64_t add_format_config(const char
>>> *xe_device, const char *format, uin
>>> return config;
>>> }
>>>
>>> -static uint64_t get_event_config(int xe, unsigned int gt, struct
>>> drm_xe_engine_class_instance *eci,
>>> +static uint64_t get_event_config(unsigned int gt, struct
>>> drm_xe_engine_class_instance *eci,
>>> const char *event)
>>> {
>>> - char xe_device[100];
>>> uint64_t pmu_config = 0;
>>> int ret;
>>>
>>> - xe_perf_device(xe, xe_device, sizeof(xe_device));
>>> ret = perf_event_config(xe_device, event, &pmu_config);
>>> igt_assert(ret >= 0);
>>> - pmu_config |= add_format_config(xe_device, "gt", gt);
>>> + pmu_config |= add_format_config("gt", gt);
>>>
>>> if (eci) {
>>> - pmu_config |= add_format_config(xe_device, "engine_class",
>>> eci->engine_class);
>>> - pmu_config |= add_format_config(xe_device,
>>> "engine_instance", eci->engine_instance);
>>> + pmu_config |= add_format_config("engine_class", eci-
>>> >engine_class);
>>> + pmu_config |= add_format_config("engine_instance", eci-
>>> >engine_instance);
>>> }
>>>
>>> return pmu_config;
>>> }
>>>
>>> +static uint64_t get_event_config_fn(unsigned int gt, int function,
>>> + struct drm_xe_engine_class_instance *eci, const
>>> char *event)
>>> +{
>>> + return get_event_config(gt, eci, event) |
>>> add_format_config("function", function);
>>> +}
>>> +
>>> /**
>>> * SUBTEST: engine-activity-idle
>>> * Description: Test to validate engine activity shows no load when idle
>>> @@ -144,10 +152,10 @@ static void engine_activity(int fd, struct
>>> drm_xe_engine_class_instance *eci, un
>>> uint32_t vm;
>>> int pmu_fd[2];
>>>
>>> - config = get_event_config(fd, eci->gt_id, eci, "engine-active-
>>> ticks");
>>> + config = get_event_config(eci->gt_id, eci, "engine-active-ticks");
>>> pmu_fd[0] = open_group(fd, config, -1);
>>>
>>> - config = get_event_config(fd, eci->gt_id, eci, "engine-total-
>>> ticks");
>>> + config = get_event_config(eci->gt_id, eci, "engine-total-ticks");
>>> pmu_fd[1] = open_group(fd, config, pmu_fd[0]);
>>>
>>> vm = xe_vm_create(fd, 0, 0);
>>> @@ -188,6 +196,73 @@ static void engine_activity(int fd, struct
>>> drm_xe_engine_class_instance *eci, un
>>> igt_assert(!engine_active_ticks);
>>> }
>>>
>>> +/**
>>> + * SUBTEST: all-fn-engine-activity-load
>>> + * Description: Test to validate engine activity by running load on
>>> all functions simultaneously
>>> + */
>>> +static void engine_activity_all_fn(int fd, struct
>>> drm_xe_engine_class_instance *eci, int num_fns)
>>> +{
>>> + uint64_t config, engine_active_ticks, engine_total_ticks;
>>> + uint64_t after[2 * num_fns], before[2 * num_fns];
>>> + struct pmu_function {
>>> + struct xe_cork *cork;
>>> + uint32_t vm;
>>> + uint64_t pmu_fd[2];
>>> + int fd;
>>> + } fn[num_fns];
>>> + int i;
>>> +
>>> + fn[0].pmu_fd[0] = -1;
>>> + for (i = 0; i < num_fns; i++) {
>>> + config = get_event_config_fn(eci->gt_id, i, eci, "engine-
>>> active-ticks");
>>> + fn[i].pmu_fd[0] = open_group(fd, config, fn[0].pmu_fd[0]);
>>> +
>>> + config = get_event_config_fn(eci->gt_id, i, eci, "engine-
>>> total-ticks");
>>> + fn[i].pmu_fd[1] = open_group(fd, config, fn[0].pmu_fd[0]);
>>> +
>>> + if (i > 0)
>>> + fn[i].fd = igt_sriov_open_vf_drm_device(fd, i);
>>> + else
>>> + fn[i].fd = fd;
>>> +
>>> + igt_assert_fd(fn[i].fd);
>>> +
>>> + fn[i].vm = xe_vm_create(fn[i].fd, 0, 0);
>>> + fn[i].cork = xe_cork_create_opts(fn[i].fd, eci, fn[i].vm, 1,
>>> 1);
>>> + xe_cork_sync_start(fn[i].fd, fn[i].cork);
>>> + }
>>> +
>>> + pmu_read_multi(fn[0].pmu_fd[0], 2 * num_fns, before);
>>> + usleep(SLEEP_DURATION * USEC_PER_SEC);
>>> + pmu_read_multi(fn[0].pmu_fd[0], 2 * num_fns, after);
>>> +
>>> + for (i = 0; i < num_fns; i++) {
>>> + int idx = i * 2;
>>> +
>>> + xe_cork_sync_end(fn[i].fd, fn[i].cork);
>>> + engine_active_ticks = after[idx] - before[idx];
>>> + engine_total_ticks = after[idx + 1] - before[idx + 1];
>>> +
>>> + igt_debug("[%d] Engine active ticks: after %ld, before %ld
>>> delta %ld\n", i,
>>> + after[idx], before[idx], engine_active_ticks);
>>> + igt_debug("[%d] Engine total ticks: after %ld, before %ld
>>> delta %ld\n", i,
>>> + after[idx + 1], before[idx + 1], engine_total_ticks);
>>> +
>>> + if (fn[i].cork)
>>> + xe_cork_destroy(fn[i].fd, fn[i].cork);
>>> +
>>> + xe_vm_destroy(fn[i].fd, fn[i].vm);
>>> +
>>> + close(fn[i].pmu_fd[0]);
>>> + close(fn[i].pmu_fd[1]);
>>> +
>>> + if (i > 0)
>>> + close(fn[i].fd);
>>> +
>>> + assert_within_epsilon(engine_active_ticks,
>>> engine_total_ticks, tolerance);
>>> + }
>>> +}
>>> +
>>> /**
>>> * SUBTEST: gt-c6-idle
>>> * Description: Basic residency test to validate idle residency
>>> @@ -202,7 +277,7 @@ static void test_gt_c6_idle(int xe, unsigned int gt)
>>> uint64_t val;
>>>
>>> /* Get the PMU config for the gt-c6 event */
>>> - pmu_config = get_event_config(xe, gt, NULL, "gt-c6-residency");
>>> + pmu_config = get_event_config(gt, NULL, "gt-c6-residency");
>>>
>>> pmu_fd = open_pmu(xe, pmu_config);
>>>
>>> @@ -226,13 +301,34 @@ static void test_gt_c6_idle(int xe, unsigned
>>> int gt)
>>> close(pmu_fd);
>>> }
>>>
>>> +static unsigned int enable_and_provision_vfs(int fd)
>>> +{
>>> + unsigned int gt, num_vfs;
>>> +
>>> + igt_sriov_enable_vfs(fd, 2);
>>> + num_vfs = igt_sriov_get_enabled_vfs(fd);
>>> + igt_require(num_vfs);
>
> igt_assert(num_vfs == 2)
>
> I am thinking maybe you could use globals instead of the define and
> initialize them here.
>
> pf_quanta_msec = 64;
> vf_quanta_msec = 32;
> total_quanta = pf_quanta_msec + (num_vfs * vf_quanta_msec);
>
Okay will fix this>>> +
>>> + /* Set 32ms for VF execution quantum and 64ms for PF execution
>>> quantum */
>>> + xe_for_each_gt(fd, gt) {
>>> + xe_sriov_set_sched_if_idle(fd, gt, 0);
>>> + for (int fn = 0; fn <= num_vfs; fn++)
>>> + xe_sriov_set_exec_quantum_ms(fd, fn, gt, fn ?
>>> TOTAL_EXEC_QUANTUM / 4 :
>>> + TOTAL_EXEC_QUANTUM / 2);
>>> + }
>
> maybe also cover the case for sched_if_idle = 1?For both tests?>
> Thanks,
> Umesh
>
>>> +
>>> + return num_vfs;
>>> +}
>>> +
>>> igt_main
>>> {
>>> int fd, gt;
>>> + unsigned int num_fns;
>>> struct drm_xe_engine_class_instance *eci;
>>>
>>> igt_fixture {
>>> fd = drm_open_driver(DRIVER_XE);
>>> + xe_perf_device(fd, xe_device, sizeof(xe_device));
>>> }
>>>
>>> igt_describe("Validate PMU gt-c6 residency counters when idle");
>>> @@ -254,6 +350,20 @@ igt_main
>>> test_each_engine("engine-activity-load", fd, eci)
>>> engine_activity(fd, eci, TEST_LOAD);
>>>
>>> + igt_subtest_group {
>>> + igt_fixture {
>>> + igt_require(igt_sriov_is_pf(fd));
>>> + num_fns = enable_and_provision_vfs(fd) + 1;
>
> How does this work if VFs are already provisioned before starting the test?It fails to write to num_vfs. The sriov tests in igt skip in that case
igt_require(igt_sriov_get_enabled_vfs(pf_fd) == 0);
Thanks for catching this. Will do the same for these tests.>
>
>>> + }
>>> +
>>> + igt_describe("Validate engine activity on all functions");
>>> + test_each_engine("all-fn-engine-activity-load", fd, eci)
>>> + engine_activity_all_fn(fd, eci, num_fns);
>>> +
>>> + igt_fixture
>>> + igt_sriov_disable_vfs(fd);
>>> + }
>>> +
>>> igt_fixture {
>>> close(fd);
>>> }
>>> --
>>> 2.47.1
>>>
More information about the igt-dev
mailing list