Re: [PATCH i-g-t v3] tests/intel/xe_ras: Add test for GPU health indicator

["Tauro, Riana" <riana.tauro@intel.com>]

On 08-09-2026 10:51, Purkait, Soham wrote:
>
> On 04-09-2026 11:54, Tauro, Riana wrote:
>>
>> On 02-09-2026 08:28, Soham Purkait wrote:
>>> Add a new Xe RAS test exercising the gpu_health sysfs attribute
>>> exposed by the Xe driver on platforms that provide the system
>>> controller. The attribute reports and allows updating the GPU
>>> health state.
>>>
>>> The gpu-health subtest validates each valid state by writing it
>>> and reading the value back, and checks that invalid writes are
>>> rejected with -EINVAL, guarding against regressions in the
>>> implementation.
>>>
>>> v1:
>>>   - Platform-conditional skip w/o fd leak. (Anirban)
>>>   - Restore via exit handler. (Anirban)
>>>
>>> v2:
>>>   - Remove dead-code and extra comments. (Anirban)
>>>   - Initialize exit handler under igt_subtest. (Anirban)
>>>
>>> v3:
>>>   - Move igt_info to the normal cleanup path when
>>>     restoring gpu_health. (Anirban)
>>>
>>> Signed-off-by: Soham Purkait <soham.purkait@intel.com>
>>> Reviewed-by: Sk Anirban <sk.anirban@intel.com>
>>> ---
>>>   tests/intel/xe_ras.c | 131 
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   tests/meson.build    |   1 +
>>>   2 files changed, 132 insertions(+)
>>>   create mode 100644 tests/intel/xe_ras.c
>>>
>>> diff --git a/tests/intel/xe_ras.c b/tests/intel/xe_ras.c
>>> new file mode 100644
>>> index 000000000..f3b507ce0
>>> --- /dev/null
>>> +++ b/tests/intel/xe_ras.c
>>> @@ -0,0 +1,131 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2026 Intel Corporation
>>> + */
>>> +
>>> +#include <errno.h>
>>> +#include <string.h>
>>> +#include <unistd.h>
>>> +
>>> +#include "igt.h"
>>> +#include "igt_sysfs.h"
>>> +
>>> +/**
>>> + * TEST: Test Xe RAS (Reliability, Availability, Serviceability) 
>>> functionality
>>> + * Category: Core
>>> + * Mega feature: RAS
>>> + * Sub-category: RAS tests
>>> + * Functionality: ras
>>> + * Test category: Functional tests
>>> + *
>>> + * SUBTEST: gpu-health
>>> + * Description: Verify the gpu_health sysfs attribute accepts each 
>>> valid
>>> + *        state (ok/warning/critical) and rejects invalid writes
>>> + *        with EINVAL.
>>> + */
>>> +
>>> +IGT_TEST_DESCRIPTION("Tests for Xe RAS");
>>> +
>>> +#define GPU_HEALTH_ATTR    "device/gpu_health"
>>> +
>>> +static const char * const gpu_health_states[] = {
>>> +    "ok",
>>> +    "warning",
>>> +    "critical",
>>> +};
>>> +
>>> +static struct {
>>> +    int sys_fd;
>>> +    char *orig_health;
>>> +} gpu_health_ctx = { .sys_fd = -1 };
>>> +
>>> +static void restore_gpu_health(int sig)
>>> +{
>>> +    if (gpu_health_ctx.sys_fd < 0 || !gpu_health_ctx.orig_health)
>>> +        return;
>>> +
>>> +    igt_sysfs_set(gpu_health_ctx.sys_fd, GPU_HEALTH_ATTR, 
>>> gpu_health_ctx.orig_health);
>>> +    close(gpu_health_ctx.sys_fd);
>>> +    gpu_health_ctx.sys_fd = -1;
>>> +    if (sig)
>>> +        return;
>>> +    igt_info("Restored initial gpu_health to '%s'\n", 
>>> gpu_health_ctx.orig_health);
>>> +    free(gpu_health_ctx.orig_health);
>>> +    gpu_health_ctx.orig_health = NULL;
>>> +}
>>> +
>>> +static bool valid_gpu_health(const char *s)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(gpu_health_states); i++)
>>> +        if (!strcmp(s, gpu_health_states[i]))
>>> +            return true;
>>> +
>>> +    return false;
>>> +}
>>> +
>>> +static void test_gpu_health(int xe)
>>> +{
>>> +    char *health = NULL;
>>> +    int ret;
>>> +    int i;
>>> +
>>> +    gpu_health_ctx.sys_fd = igt_sysfs_open(xe);
>>> +    igt_assert(gpu_health_ctx.sys_fd >= 0);
>>> +    if (!igt_sysfs_has_attr(gpu_health_ctx.sys_fd, GPU_HEALTH_ATTR)) {
>>> +        close(gpu_health_ctx.sys_fd);
>>> +        gpu_health_ctx.sys_fd = -1;
>>> +        igt_skip("gpu_health sysfs attribute not exposed by 
>>> driver\n");
>>
>> Won't exit handler be called here?
> No its the device sysfs for which  gpu-health exit handler does not run.

I meant assert here.

>> Wouldn't it be better to skip on gpu health sysfs open.
>
> Apart from this test technically no one is supposed to open gpu health 
> sysfs. If so, should this fail ? skip may not be a right option.

How is skip related to other tests using it? use igt_skip_on if gpu 
health is not available. This will prevent failures on non-supported 
platforms.

>
>>
>>> +    }
>>> +
>>> +    gpu_health_ctx.orig_health = 
>>> igt_sysfs_get(gpu_health_ctx.sys_fd, GPU_HEALTH_ATTR);
>>> +    igt_assert_f(gpu_health_ctx.orig_health, "Failed to read %s\n", 
>>> GPU_HEALTH_ATTR);
>>> +    igt_info("Initial gpu_health: %s\n", gpu_health_ctx.orig_health);
>>> + igt_assert_f(valid_gpu_health(gpu_health_ctx.orig_health),
>>> +             "Unexpected initial gpu_health value: '%s'\n",
>>> +             gpu_health_ctx.orig_health);
>>> +
>>> +    ret = igt_sysfs_write(gpu_health_ctx.sys_fd, GPU_HEALTH_ATTR, 
>>> "bogus", strlen("bogus"));
>>> +    igt_assert_f(ret == -EINVAL,
>>> +             "Write of invalid value to %s returned %d, expected 
>>> -EINVAL\n",
>>> +             GPU_HEALTH_ATTR, ret);
>>> +
>>> +    for (i = 0; i < ARRAY_SIZE(gpu_health_states); i++) {
>>> +        const char *state = gpu_health_states[i];
>>> +
>>> +        igt_info("Setting gpu_health to '%s'\n", state);
>>> +
>>> +        igt_assert_f(igt_sysfs_set(gpu_health_ctx.sys_fd, 
>>> GPU_HEALTH_ATTR, state),
>>> +                 "Failed to write '%s' to %s\n",
>>> +                 state, GPU_HEALTH_ATTR);
>>> +
>>> +        health = igt_sysfs_get(gpu_health_ctx.sys_fd, 
>>> GPU_HEALTH_ATTR);
>>> +        igt_assert(health);
>>> +        igt_assert_f(!strcmp(health, state),
>>> +                 "gpu_health readback mismatch: wrote '%s', read 
>>> '%s'\n",
>>> +                 state, health);
>>> +        igt_info("Verified gpu_health state readback for '%s' 
>>> successfully\n",
>>> +             state);
>>> +        free(health);
>>> +    }
>>> +
>>> +    restore_gpu_health(0);
>>> +}
>>> +
>>> +int igt_main()
>>> +{
>>> +    int xe;
>>> +
>>> +    igt_fixture()
>>> +        xe = drm_open_driver(DRIVER_XE);
>>> +
>>> +    igt_subtest("gpu-health") {
>>> +        igt_install_exit_handler(restore_gpu_health);
>>> +        test_gpu_health(xe);
>>
>> Wouldn't it be better to have dedicated subtests for each value 
>> instead of
>> adding all testcases in a single function.
>
> What about dynamic subtest for all these 4 cases under gpu-health 
> subtest?

Sure. It should be per value.

Thanks
Riana

>
> Thanks,
> Soham
>>
>> Would be easier to find during analysis as well
>>
>> Thanks
>> Riana
>>
>>> +    }
>>> +
>>> +    igt_fixture()
>>> +        drm_close_driver(xe);
>>> +}
>>> diff --git a/tests/meson.build b/tests/meson.build
>>> index 1ac89bab7..3c2d4fdda 100644
>>> --- a/tests/meson.build
>>> +++ b/tests/meson.build
>>> @@ -333,6 +333,7 @@ intel_xe_progs = [
>>>       'xe_prime_self_import',
>>>       'xe_pxp',
>>>       'xe_query',
>>> +    'xe_ras',
>>>       'xe_render_copy',
>>>       'xe_vm',
>>>       'xe_userptr_pressure',