Re: [PATCH i-g-t v2] tests/xe_debugfs: Add debugfs entry read/write validation in root-dir

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 15 Jun 2026, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> Hi Jani,
> On 2026-06-15 at 15:58:24 +0300, Jani Nikula wrote:
>> On Mon, 15 Jun 2026, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
>> > Hi Jani,
>> > On 2026-06-15 at 14:10:22 +0300, Jani Nikula wrote:
>> >> On Mon, 15 Jun 2026, Sobin Thomas <sobin.thomas@intel.com> wrote:
>> >> > Extend the root-dir subtest to validate additional optional Xe debugfs
>> >> > entries. Optional entries are those that may not be present on all
>> >> > platforms or configurations — their absence is not treated as a failure.
>> >> >
>> >> > - dgfx_pkg_residencies: Validate counter reads (non-empty)
>> >> > - dgfx_pcie_link_residencies: Validate PCIE LINK string content
>> >> > - sriov_info: Validate entry is non-empty
>> >> > - workarounds: Validate entry is non-empty
>> >> > - atomic_svm_timeslice_ms: Validate integer read/write
>> >> > - poor_man_system_atomic_support: Validate boolean read/write
>> >> > - disable_late_binding: Validate boolean read/write
>> >> >
>> >> > v2: Fixed the optional file check and added fail count.
>> >> >     Replaced igt_debug with igt_info in few places  (kamil)
>> >> 
>> >> Why is the plumbing in this patch so xe focused? Looks like a lot of
>> >> this could be IGT generic and driver agnostic, with just the debugfs
>> >> hierarchy and filenames and requirements for each file described per
>> >> driver.
>> >> 
>> >> BR,
>> >> Jani.
>> >> 
>> >
>> > imho we should keep core_debugfs free of driver-specific checks,
>> > as it do not know which entries are safe to write to. Also, it
>> > is good to have a simple read-all core tests for sysfs/debugs and
>> > keep them simple.
>> >
>> > Thats why driver-specific checks should be done in
>> > driver-specific test.
>> 
>> I don't think you understood what I meant at all.
>
> Well, feel free to write quick and short rfc with your proposed
> changes in code. I could comment on that.

The point is, enum debugfs_validate_type, struct check_entry,
validate_string(), mode_to_str(), validate_bool_file(),
validate_int_file(), validate_debugfs_file(), etc, are all driver
agnostic types and functions *already*, placed in driver specific code.

If anyone else wants to do similar validation for other drivers, they
either need to reinvent the wheel or refactor this. Which is just wasted
effort. If folks reinvent the wheel in other drivers, in subtly
different ways, consolidating them later is exponential wasted effort.

Which debugfs files a driver should have, or what they should contain,
is driver specific. How to check those debugfs files is not.


BR,
Jani.


>
> Btw did you mean that 'validate entry is non-empty' could be
> dropped from this patch? Now as I look again, they are already
> checked by core_debugsfs (or at least should be checked).
>
> Regards,
> Kamil
>
>
>> 
>> BR,
>> Jani.
>> 
>> >
>> > Regards,
>> > Kamil
>> >
>> >> 
>> >> >
>> >> > Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
>> >> > ---
>> >> >  tests/intel/xe_debugfs.c | 198 ++++++++++++++++++++++++++++++++++++---
>> > [cut]
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel