[PATCH i-g-t v1] tools/xe_guc_logger: Add guc logger for Xe
Dong, Zhanjun
zhanjun.dong at intel.com
Tue Apr 8 14:46:31 UTC 2025
Thanks for review.
Please see my inline comments below.
Regards,
Zhanjun Dong
On 2025-04-08 5:10 a.m., Peter Senna Tschudin wrote:
> Dear Zhanjun,
>
> On 3/12/2025 10:30 PM, Zhanjun Dong wrote:
>> Add guc logger for Xe, support save guc log in LFD format.
>
> I tested this patch and found the behavior of the tool a bit
> confusing. It behaves more like a test than a standalone tool.
> For example:
>
> $ sudo ./build/tools/xe_guc_logger -i /sys/kernel/debug/dri/0000:03:00.0/gt0/uc/guc_log
> IGT-Version: 2.0-g4b22256d0 (x86_64) (Linux: 6.14.0-xe x86_64)
> Using IGT_SRANDOM=1744102201 for randomisation
> SUCCESS (0.371s)
>
> $ sudo ./build/tools/xe_guc_logger --help
> Usage: xe_guc_logger [OPTIONS]
> --list-subtests
> --show-testlist
> --run-subtest <pattern>
> --dynamic-subtest <pattern>
> --debug[=log-domain]
> --interactive-debug[=domain]
> --skip-crc-compare
> --trace-on-oops
> --hook [<events>:]<cmd>
> --help-hook
> --help-description
> --describe
> --device filters
> --version
> --help|-h
> -i --inputfile=name name of the guc log file, including the path
> -o --outputfile=name name of the output file, including the location, where logs will be stored
> -v --verbosity=level verbosity level of output
>
> This makes it appear as if the logger is a test case rather
> than a tool. I suggest removing this test-like behavior. You
> can look at other examples under the tools/ directory, such
> as lsgpu and igt_facts, which are implemented as tools.
Right, the tool should not has the testcase help message, to be updated.
>
> Additionally, the tool is hard to use without reading the
> source code. It would help to:
> - Automatically locate the guc_log files in /sys/kernel/debug/
> - Provide a clearer error message when the input file is
> not specified
>
> Right now, omitting the -i option results in a test-like
> assertion failure with no guidance:
>
> $ sudo ./build/tools/xe_guc_logger
> IGT-Version: 2.0-g4b22256d0 (x86_64) (Linux: 6.14.0-xe x86_64)
> Using IGT_SRANDOM=1744102681 for randomisation
> (xe_guc_logger:2445) CRITICAL: Test assertion failure function load_guc_log, file ../tools/xe_guc_logger.c:267:
> (xe_guc_logger:2445) CRITICAL: Failed assertion: fd
> (xe_guc_logger:2445) CRITICAL: Last errno: 2, No such file or directory
> (xe_guc_logger:2445) CRITICAL: couldn't open the file: guc_log
> Stack trace:
> #0 ../lib/igt_core.c:2065 __igt_fail_assert()
> #1 ../tools/xe_guc_logger.c:166 main()
> #2 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main()
> #3 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> #4 [_start+0x25]
> Test xe_guc_logger failed.
> **** DEBUG ****
> (xe_guc_logger:2445) igt_core-INFO: IGT-Version: 2.0-g4b22256d0 (x86_64) (Linux: 6.14.0-xe x86_64)
> (xe_guc_logger:2445) igt_core-INFO: Using IGT_SRANDOM=1744102681 for randomisation
> (xe_guc_logger:2445) CRITICAL: Test assertion failure function load_guc_log, file ../tools/xe_guc_logger.c:267:
> (xe_guc_logger:2445) CRITICAL: Failed assertion: fd
> (xe_guc_logger:2445) CRITICAL: Last errno: 2, No such file or directory
> (xe_guc_logger:2445) CRITICAL: couldn't open the file: guc_log
> (xe_guc_logger:2445) igt_core-INFO: Stack trace:
> (xe_guc_logger:2445) igt_core-INFO: #0 ../lib/igt_core.c:2065 __igt_fail_assert()
> (xe_guc_logger:2445) igt_core-INFO: #1 ../tools/xe_guc_logger.c:166 main()
> (xe_guc_logger:2445) igt_core-INFO: #2 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main()
> (xe_guc_logger:2445) igt_core-INFO: #3 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> (xe_guc_logger:2445) igt_core-INFO: #4 [_start+0x25]
> **** END ****
> FAIL (0.046s)
>
> Instead, please consider printing a helpful usage message,
> such as:
>
> $ sudo ./build/tools/xe_guc_logger
> ERROR: Please use -i to specify the guc_log file
> (e.g. -i /sys/kernel/debug/dri/0000:03:00.0/gt0/uc/guc_log)
'-i' is designed to let user specifiy data source, it would be usedful
for mulptile gpu system. Thanks for take time to test it, I will
consider better handling on this. >
> Lastly, it would be helpful to clarify the purpose of the
> output file. What does the tool provide that a simple cat
> on the guc_log file wouldn’t? A short explanation in the
> source code and the --help output would help users and
> developers understand this tool.
Good point, will add that later.
Next rev will have dependency with KMD/guc changes, this tool will not
moving forward before those changes finished.>
> Thank you,
>
> Peter>
>> Reference:
>> https://coredocs.intel.com/InterfaceDocs/sphinx/core/kmd_log_file_format.html?highlight=lfd
>>
>> Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
>> ---
>> Cc: John Harrison <John.C.Harrison at Intel.com>
>> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
>>
>> tools/lfd.h | 590 ++++++++++++++++++++++++++++++++++++++++
>> tools/lfd_default.h | 39 +++
>> tools/meson.build | 1 +
>> tools/xe_guc_logger.c | 615 ++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 1245 insertions(+)
>> create mode 100644 tools/lfd.h
>> create mode 100644 tools/lfd_default.h
>> create mode 100644 tools/xe_guc_logger.c
... Omitted ...
More information about the igt-dev
mailing list