[PATCH i-g-t 3/3] tests/xe_eudebug_online: Add set-breakpoint-sigint-debugger test

[Piatkowski, Dominik Karol]

Hi Christoph,

Test scenario came from the chat with Mika. The idea was to have complex enough test case (set-breakpoint) and just nuke the debugger with ctrl-c in random spot, hoping to hit interesting parts eventually.

I'll send a v2 soon with Mika in CC.

Thanks,
Dominik Karol

> -----Original Message-----
> From: Manszewski, Christoph <christoph.manszewski at intel.com>
> Sent: Monday, March 10, 2025 1:23 PM
> To: Piatkowski, Dominik Karol <dominik.karol.piatkowski at intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: Grzegorzek, Dominik <dominik.grzegorzek at intel.com>; Sokolowski, Jan
> <jan.sokolowski at intel.com>
> Subject: Re: [PATCH i-g-t 3/3] tests/xe_eudebug_online: Add set-breakpoint-
> sigint-debugger test
> 
> Hi Dominik,
> 
> On 24.02.2025 14:08, Dominik Karol Piątkowski wrote:
> > Add a test that sends SIGINT to the debugger thread with random timing
> > and checks if nothing breaks, exercising the scenario multiple times.
> 
> I have to admit I don't quite understand what this test is supposed to
> test. The worker loop you are interrupting polls for new events and
> reads them via a ioctl. Trying to understand what this test could test I
> think I found a corner case of our read_event ioctl. We can end up with
> a resource leak and missing event when the interrupt would happen during
> copy_to_user (after the event got pulled from the fifo, and before it
> got freed) but besides the possibility that this case would be triggered
> it would still go unnoticed by this test.
> 
> It feels to me like it is more a test for our IGT lib and session
> handling than actual Xe eudebug functionality. But I may be missing
> something here and perhaps a more verbose explanation of the intention
> of this test would prove me wrong.
> 
> Regards,
> Christoph
> 
> 
> >
> > Signed-off-by: Dominik Karol Piątkowski
> <dominik.karol.piatkowski at intel.com>
> > ---
> >   tests/intel/xe_eudebug_online.c | 75
> +++++++++++++++++++++++++++++++++
> >   1 file changed, 75 insertions(+)
> >
> > diff --git a/tests/intel/xe_eudebug_online.c
> b/tests/intel/xe_eudebug_online.c
> > index 472013fe1..8a97c8066 100644
> > --- a/tests/intel/xe_eudebug_online.c
> > +++ b/tests/intel/xe_eudebug_online.c
> > @@ -1506,6 +1506,77 @@ static void test_set_breakpoint_online(int fd,
> struct drm_xe_engine_class_instan
> >   	online_debug_data_destroy(data);
> >   }
> >
> > +/**
> > + * SUBTEST: set-breakpoint-sigint-debugger
> > + * Description:
> > + *	A variant of set-breakpoint that sends SIGINT to the debugger thread
> with random timing
> > + *	and checks if nothing breaks, exercising the scenario multiple times.
> > + */
> > +static void test_set_breakpoint_online_sigint_debugger(int fd,
> > +						       struct
> drm_xe_engine_class_instance *hwe,
> > +						       int flags)
> > +{
> > +	struct xe_eudebug_session *s;
> > +	struct online_debug_data *data;
> > +	struct timespec ts = { };
> > +	int loop_count = 0;
> > +	uint64_t sleep_time;
> > +	uint64_t set_breakpoint_time;
> > +	uint64_t max_sleep_time;
> > +
> > +	/*
> > +	 * Measure the average time required for basic set-breakpoint variant,
> > +	 * so sleep_time range is correct.
> > +	 */
> > +	igt_nsec_elapsed(&ts);
> > +	for (int i = 0; i < 10; i++)
> > +		test_set_breakpoint_online(fd, hwe, SHADER_NOP |
> TRIGGER_UFENCE_SET_BREAKPOINT);
> > +	set_breakpoint_time = igt_nsec_elapsed(&ts) / (NSEC_PER_MSEC /
> USEC_PER_MSEC) / 10;
> > +	igt_info("Average set-breakpoint execution time: %" PRIu64 " us\n",
> set_breakpoint_time);
> > +	max_sleep_time = set_breakpoint_time * 11 / 10;
> > +	igt_info("Maximum sleep_time: %" PRIu64 " us\n", max_sleep_time);
> > +
> > +	ts = (struct timespec) { };
> > +	igt_nsec_elapsed(&ts);
> > +
> > +	while (igt_seconds_elapsed(&ts) < 60) {
> > +		sleep_time = rand() % max_sleep_time;
> > +		igt_debug("Loop %d: SIGINT after %" PRIu64 " us\n",
> ++loop_count, sleep_time);
> > +
> > +		data = online_debug_data_create(hwe);
> > +		s = xe_eudebug_session_create(fd, run_online_client, flags,
> data);
> > +		xe_eudebug_debugger_add_trigger(s->debugger,
> DRM_XE_EUDEBUG_EVENT_OPEN,
> > +						open_trigger);
> > +		xe_eudebug_debugger_add_trigger(s->debugger,
> DRM_XE_EUDEBUG_EVENT_EXEC_QUEUE,
> > +						exec_queue_trigger);
> > +		xe_eudebug_debugger_add_trigger(s->debugger,
> DRM_XE_EUDEBUG_EVENT_VM,
> > +						vm_open_trigger);
> > +		xe_eudebug_debugger_add_trigger(s->debugger,
> DRM_XE_EUDEBUG_EVENT_METADATA,
> > +						create_metadata_trigger);
> > +		xe_eudebug_debugger_add_trigger(s->debugger,
> DRM_XE_EUDEBUG_EVENT_VM_BIND_UFENCE,
> > +						ufence_ack_set_bp_trigger);
> > +		xe_eudebug_debugger_add_trigger(s->debugger,
> DRM_XE_EUDEBUG_EVENT_EU_ATTENTION,
> > +
> 	eu_attention_resume_trigger);
> > +
> > +		igt_assert_eq(xe_eudebug_debugger_attach(s->debugger, s-
> >client), 0);
> > +		xe_eudebug_debugger_start_worker(s->debugger);
> > +		xe_eudebug_client_start(s->client);
> > +
> > +		usleep(sleep_time);
> > +		igt_assert_eq(pthread_kill(s->debugger->worker_thread,
> SIGINT), 0);
> > +		close(s->debugger->fd);
> > +		s->debugger->worker_state =
> DEBUGGER_WORKER_INACTIVE;
> > +
> > +		xe_eudebug_client_wait_done(s->client);
> > +
> > +		xe_eudebug_event_log_print(s->debugger->log, true);
> > +		xe_eudebug_event_log_print(s->client->log, true);
> > +
> > +		xe_eudebug_session_destroy(s);
> > +		online_debug_data_destroy(data);
> > +	}
> > +}
> > +
> >   /**
> >    * SUBTEST: pagefault-read
> >    * Description:
> > @@ -2426,6 +2497,10 @@ igt_main
> >   	test_gt_render_or_compute("set-breakpoint", fd, hwe)
> >   		test_set_breakpoint_online(fd, hwe, SHADER_NOP |
> TRIGGER_UFENCE_SET_BREAKPOINT);
> >
> > +	test_gt_render_or_compute("set-breakpoint-sigint-debugger", fd,
> hwe)
> > +		test_set_breakpoint_online_sigint_debugger(fd, hwe,
> > +							   SHADER_NOP |
> TRIGGER_UFENCE_SET_BREAKPOINT);
> > +
> >   	test_gt_render_or_compute("breakpoint-not-in-debug-mode", fd,
> hwe)
> >   		test_basic_online(fd, hwe, SHADER_BREAKPOINT |
> DISABLE_DEBUG_MODE);
> >