Re: [PATCH] tests/intel/xe_oa: Ignore other errors when looking for buffer overflow

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 21 May 2025 13:25:02 -0700, Umesh Nerlige Ramappa wrote:
>
> On Wed, May 21, 2025 at 01:12:28PM -0700, Dixit, Ashutosh wrote:
> > On Tue, 20 May 2025 12:52:50 -0700, Umesh Nerlige Ramappa wrote:
> >>
> >> Occassionally other error bits will result in EIO and prematurely end
> >> the buffer-fill test. For this test, ignore other errors.
> >>
> >> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4565
> >
> > From CI buglog it looks like, a REPORT_LOST (0x1) status is returned before
> > BUFFER_OVERFLOW (0x2) status?
>
> In this case yes, but I have seem other occurrences where counter overflow
> bit is set before buffer overflow.
>
> >
> > Patch generally looks ok, but a question below.
> >
> >
> >> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> >> ---
> >>  tests/intel/xe_oa.c | 17 +++++++++++------
> >>  1 file changed, 11 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> >> index 73841a359daf..2022c5cb3955 100644
> >> --- a/tests/intel/xe_oa.c
> >> +++ b/tests/intel/xe_oa.c
> >> @@ -2520,7 +2520,6 @@ test_buffer_fill(const struct drm_xe_engine_class_instance *hwe)
> >>	char *buf = malloc(1024);
> >>	bool overflow_seen;
> >>	u32 oa_status;
> >> -	int len;
> >>
> >>	igt_debug("oa_period %s\n", pretty_print_oa_period(oa_period));
> >>	stream_fd = __perf_open(drm_fd, &param, true /* prevent_pm */);
> >> @@ -2535,16 +2534,22 @@ test_buffer_fill(const struct drm_xe_engine_class_instance *hwe)
> >>
> >>	errno = 0;
> >>	/* Read 0 bytes repeatedly until you see an EIO */
> >> -	while ((len = read(stream_fd, buf, 0)) == -1 && (errno == EINTR || errno == ENOSPC)) {
> >> +	while (-1 == read(stream_fd, buf, 0)) {
> >> +		if (errno == EIO) {
> >> +			oa_status = get_stream_status(stream_fd);
> >> +			overflow_seen = oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW;
> >> +			if (overflow_seen)
> >> +				break;
> >> +		}
> >>		usleep(100);
> >>	}
> >> -	igt_assert_eq(len, -1);
> >> -	igt_assert_eq(errno, EIO);
> >> +	igt_assert(overflow_seen);
> >>
> >> -	/* Ensure buffer overflowed */
> >> +	/* Make sure the buffer overflow is cleared */
> >> +	read(stream_fd, buf, 0);
> >
> > How does the buffer overflow status get cleared? We are still doing 0 size reads?
>
> Hoping I understood what you are asking:
>
> It would have been cleared as part of the read itself since the KMD
> implementation is doing a read-modify-write. Right? After that a subsequent
> read should have that bit set to 0.
>
> The 0 size reads do not seem to affect the OA status update. So the KMD
> state should be pointing to the last values stored during read().

OK, yes, so KMD is clearing the status and because overrun is enabled, the
next time buffer overflow is set will be after the whole buffer fills up
again. So hopefully we will not hit that. Otherwise we'll see what to
do. So for now this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

>
> Regards,
> Umesh
> >
> >>	oa_status = get_stream_status(stream_fd);
> >>	overflow_seen = oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW;
> >> -	igt_assert(overflow_seen);
> >> +	igt_assert_eq(overflow_seen, 0);
> >>
> >>	__perf_close(stream_fd);
> >>  }
> >> --
> >> 2.43.0
> >>
> >
> > Thanks.
> > --
> > Ashutosh