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

["Purkait, Soham" <soham.purkait@intel.com>]

On 30-09-2026 10:21, Tauro, Riana wrote:
>
> On 15-09-2026 19:02, 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:
>>   - Add platform-conditional skip and exit handler. (Anirban)
>>
>> v2:
>>   - Add dynamic subtests for each case. (Riana)
>>
>> Signed-off-by: Soham Purkait <soham.purkait@intel.com>
>> Reviewed-by: Sk Anirban <sk.anirban@intel.com>
>> ---
>>   tests/intel/xe_ras.c | 146 +++++++++++++++++++++++++++++++++++++++++++
>>   tests/meson.build    |   1 +
>>   2 files changed, 147 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..91233e72f
>> --- /dev/null
>> +++ b/tests/intel/xe_ras.c
>> @@ -0,0 +1,146 @@
>> +// 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.
> Provide a brief description. Don't we need description for each subtest.
>
> ++Karthik for Category check
>
>> + */
>> +
>> +IGT_TEST_DESCRIPTION("Tests for Xe RAS");
>> +
>> +#define GPU_HEALTH_ATTR    "device/gpu_health"
>> +
>> +static const char * const gpu_health_states[] = {
>> +    "ok",
>> +    "warning",
>> +    "critical",
>> +    "bogus",
> invalid?
>> +};
>> +
>> +#define GPU_HEALTH_VALID_STATE_COUNT (ARRAY_SIZE(gpu_health_states) 
>> - 1)
>> +
>> +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)
>> +        return;
>> +
>> +    if (gpu_health_ctx.orig_health)
>> +        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;
> Why?

Is free() or igt_info() safe to call when fatal signal is being invoked?
Seems like these are not async-signal-safe.

Thanks,
Soham
>> +    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;
>> +
>> +    if (!s)
>> +        return false;
>> +
>> +    for (i = 0; i < GPU_HEALTH_VALID_STATE_COUNT; i++)
>> +        if (!strcmp(s, gpu_health_states[i]))
>> +            return true;
>> +
>> +    return false;
>> +}
>> +
>> +static void setup_gpu_health(int xe)
>> +{
>> +    gpu_health_ctx.sys_fd = igt_sysfs_open(xe);
>> +    igt_assert(gpu_health_ctx.sys_fd >= 0);
>> +
>> +    igt_skip_on_f(!igt_sysfs_has_attr(gpu_health_ctx.sys_fd, 
>> GPU_HEALTH_ATTR),
>> +              "gpu_health sysfs attribute not exposed by driver\n");
>> +
>> +    gpu_health_ctx.orig_health = 
>> igt_sysfs_get(gpu_health_ctx.sys_fd, GPU_HEALTH_ATTR);
>> +    igt_assert_f(valid_gpu_health(gpu_health_ctx.orig_health),
>> +             "Unexpected initial gpu_health value: '%s'\n",
>> +             gpu_health_ctx.orig_health);
>> +    igt_info("Initial gpu_health: %s\n", gpu_health_ctx.orig_health);
>> +}
>> +
>> +static void test_gpu_health_state(const char *state)
>> +{
>> +    char *health;
>> +    int ret;
>> +
>> +    if (!valid_gpu_health(state)) {
>> +        ret = igt_sysfs_write(gpu_health_ctx.sys_fd, GPU_HEALTH_ATTR,
>> +                      state, strlen(state));
>> +        igt_assert_f(ret == -EINVAL,
>> +                 "Write of invalid value to %s returned %d, expected 
>> -EINVAL\n",
>> +                 GPU_HEALTH_ATTR, ret);
>> +        return;
>> +    }
>> +
>> +    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);
>> +}
>> +
>> +int igt_main()
>> +{
>> +    int xe;
>> +    int state;
>> +
>> +    igt_fixture() {
>> +        xe = drm_open_driver(DRIVER_XE);
>> +        igt_install_exit_handler(restore_gpu_health);
> This should be inside igt_subtest
>> +    }
>> +
>> +    igt_subtest_with_dynamic("gpu-health") {
>> +        setup_gpu_health(xe);
>> +
>> +        for (state = 0; state < ARRAY_SIZE(gpu_health_states); 
>> state++) {
>> +            igt_dynamic_f("%s",
>> + valid_gpu_health(gpu_health_states[state]) ?
>> +                      gpu_health_states[state] : "invalid-input") {
>
> Why do we need valid check here?. This is anyway done in test_gpu_health
>
> Thanks
> Riana
>
>> + test_gpu_health_state(gpu_health_states[state]);
>> +            }
>> +        }
>> +
>> +        restore_gpu_health(0);
>> +    }
>> +
>> +    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',