[PATCH i-g-t 4/4] tests/intel/xe_oa: Add an option to capture ftrace

[Umesh Nerlige Ramappa]

On Mon, Aug 25, 2025 at 10:18:12AM -0700, Dixit, Ashutosh wrote:
>On Sat, 23 Aug 2025 12:01:08 -0700, Dixit, Ashutosh wrote:
>>
>> On Fri, 22 Aug 2025 17:34:06 -0700, Umesh Nerlige Ramappa wrote:
>> >
>> > Capture ftrace with the --trace option when needed. This is not intended
>> > to run as default, but more of a debug functionality when manually
>> > running individual tests.
>>
>> Maybe mention here or in the commit title that we are capturing register
>> read/writes in ftrace.
>>
>> >
>> > In addition configure the ftrace buffer using the --trace_buf_size_mb
>> >
>> > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> > ---
>> >  tests/intel/xe_oa.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
>> >  1 file changed, 57 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
>> > index 7381e7b9bd44..e5473a5ff70f 100644
>> > --- a/tests/intel/xe_oa.c
>> > +++ b/tests/intel/xe_oa.c
>> > @@ -307,6 +307,8 @@ static struct oa_format lnl_oa_formats[XE_OA_FORMAT_MAX] = {
>> >		.bc_report = 0 },
>> >  };
>> >
>> > +static bool oa_trace = false;
>> > +static uint32_t oa_trace_buf_mb = 20;
>> >  static int drm_fd = -1;
>> >  static int sysfs = -1;
>> >  static int pm_fd = -1;
>> > @@ -389,6 +391,31 @@ static u32 get_stream_status(int fd)
>> >	return status.oa_status;
>> >  }
>> >
>> > +static void enable_trace_log(void)
>> > +{
>> > +	char cmd[64] = {0};
>> > +
>> > +	if (!oa_trace || oa_trace_buf_mb > 20)
>
>Actually, why is this oa_trace_buf_mb condition there in enable/disable? If
>we don't want the trace buf to be > 20 MB, maybe return an error in
>opt_handler?

Ah, right. That's easier to do. will change that. I would still use a 
default buffer size if the user passed a bad value.

>
>Also, do we really need this oa_trace_buf_mb option? Maybe just select a
>size and hardcode it? Or we need the option because we don't know the
>hardcoded value?
>
>Since we are only tracing xe_reg_rw, maybe even the default size is ok? How
>many reg r/w's are there for an OA IGT? Maybe not that many? Since the reg
>r/w's done by batches won't be captured here anyway, just the direct mmio
>r/w's?
>
>When we generate lots of ftrace, I have previously found the default ftrace
>buffer size is not sufficient and printf's get dropped. Previously I used
>512 KB (the largest size which worked in those old kernels) and that seemed
>sufficient. But anyway even larger values are ok.
>
Didn't think much about it though. I have a default of 20MB in my script 
somewhere, so copied it over. Sometimes I am dumping OA report contents 
to ftrace and it fills up quickly, so I find it helpful if I can pass it 
in the command line.

Thanks,
Umesh

>Anyway, ok to leave as is too. Just some ideas.
>
>> > +		return;
>> > +
>> > +	snprintf(cmd, sizeof(cmd) - 1, "echo %d > /sys/kernel/debug/tracing/buffer_size_kb", oa_trace_buf_mb * 1000);
>
>Maybe * 1024 instead?
>
>> > +	system(cmd);
>> > +	system("echo 0 > /sys/kernel/debug/tracing/tracing_on");
>> > +	system("echo > /sys/kernel/debug/tracing/trace");
>> > +	system("echo 1 > /sys/kernel/debug/tracing/events/xe/enable");
>> > +	system("echo 1 > /sys/kernel/debug/tracing/events/xe/xe_reg_rw/enable");
>> > +	system("echo 1 > /sys/kernel/debug/tracing/tracing_on");
>> > +}
>> > +
>> > +static void disable_trace_log(void)
>> > +{
>> > +	if (!oa_trace || oa_trace_buf_mb > 20)
>> > +		return;
>> > +
>> > +	system("echo 0 > /sys/kernel/debug/tracing/tracing_on");
>> > +	system("cat /sys/kernel/debug/tracing/trace");
>> > +}
>> > +
>> >  static void
>> >  dump_report(const uint32_t *report, uint32_t size, const char *message) {
>> >	uint32_t i;
>> > @@ -4937,7 +4964,34 @@ static const char *xe_engine_class_name(uint32_t engine_class)
>> >	igt_require_f(hwe, "no render engine\n"); \
>> >	igt_dynamic_f("rcs-%d", hwe->engine_instance)
>> >
>> > -igt_main
>> > +static int opt_handler(int opt, int opt_index, void *data)
>> > +{
>> > +	switch (opt) {
>> > +	case 'b':
>> > +		oa_trace_buf_mb = strtoul(optarg, NULL, 0);
>> > +		igt_debug("Trace buffer %d Mb\n", oa_trace_buf_mb);
>> > +		break;
>> > +	case 't':
>> > +		oa_trace = true;
>> > +		igt_debug("Trace enabled\n");
>> > +		break;
>> > +	default:
>> > +		return IGT_OPT_HANDLER_ERROR;
>> > +	}
>> > +
>> > +	return IGT_OPT_HANDLER_SUCCESS;
>> > +}
>> > +
>> > +static const char *help_str =  "  --trace | -t\t\tEnable ftrace\n"
>> > +			       "  --trace_buf_size_mb | -b\t\tSet ftrace buffer size in Mb (default = 0, max = 20)\n";
>
>s/Mb/MB/ ?
>
>Thanks.
>--
>Ashutosh
>
>> > +
>> > +static struct option long_options[] = {
>> > +	{"trace", 0, 0, 't'},
>> > +	{"trace_buf_size_mb", 0, 0, 'b'},
>> > +	{ NULL, 0, 0, 0 }
>> > +};
>> > +
>> > +igt_main_args("b:t", long_options, help_str, opt_handler, NULL)
>> >  {
>> >	const struct sync_section {
>> >		const char *name;
>> > @@ -4981,6 +5035,7 @@ igt_main
>> >		 */
>> >		igt_assert_eq(drm_fd, -1);
>> >
>> > +		enable_trace_log();
>> >		drm_fd = drm_open_driver(DRIVER_XE);
>> >		xe_dev = xe_device_get(drm_fd);
>> >
>> > @@ -5210,5 +5265,6 @@ igt_main
>> >			intel_xe_perf_free(intel_xe_perf);
>> >
>> >		drm_close_driver(drm_fd);
>> > +		disable_trace_log();
>>
>> Does this get disabled even if the test crashes? Anyway since it is for
>> debug and not enabled by default, this is:
>>
>> Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
>>
>> >	}
>> >  }
>> > --
>> > 2.43.0
>> >