[PATCH i-g-t v2 3/4] tests/intel/xe_oa: Add a test for tail address wrap

[Umesh Nerlige Ramappa]

On Wed, Aug 27, 2025 at 11:38:15AM -0700, Dixit, Ashutosh wrote:
>On Tue, 26 Aug 2025 15:43:00 -0700, Umesh Nerlige Ramappa wrote:
>>
>> Add a test to verify that the HW wraps to start of the OA buffer when a
>> report does not fit in the remaining space in the buffer.
>>
>> v2:
>> - Make test more robust
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> ---
>>  tests/intel/xe_oa.c | 97 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 97 insertions(+)
>>
>> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
>> index d10ef00d6abc..24442e4b83c6 100644
>> --- a/tests/intel/xe_oa.c
>> +++ b/tests/intel/xe_oa.c
>> @@ -4537,6 +4537,95 @@ static void closed_fd_and_unmapped_access(const struct drm_xe_engine_class_insta
>>	try_invalid_access(vaddr);
>>  }
>>
>> +static void read_reports(int fd, uint8_t *buf, size_t size)
>
>> +{
>> +	int len, total = 0;
>> +
>> +	while (total < size) {
>> +		len = read(fd, &buf[total], size - total);
>> +		if (len == -1 && (errno == EIO || errno == EINTR))
>> +			continue;
>> +
>> +		igt_assert(len);
>> +		total += len;
>> +	}
>> +}
>> +
>> +/**
>> + * SUBTEST: tail-address-wrap
>> + * Description: Test tail address wrap on odd format sizes. Ensure that the
>> + * format size is not a power of 2. This means that the last report will not be
>> + * broken down across the OA buffer end. Instead it will be written to the
>> + * beginning of the OA buffer. We will check the end of the buffer to ensure it
>> + * has zeroes in it.
>> + */
>> +static void
>> +test_tail_address_wrap(const struct drm_xe_engine_class_instance *hwe, size_t oa_buffer_size)
>> +{
>> +	struct intel_xe_perf_metric_set *test_set = metric_set(hwe);
>> +	uint64_t exponent = max_oa_exponent_for_period_lte(20000);
>> +	uint64_t buffer_size = oa_buffer_size ?: buffer_fill_size;
>
>We are providing oa_buffer_size in the caller, so maybe can just use
>oa_buffer_size? But otherwise ok to leave as is too.
>
>> +	uint64_t fmt = test_set->perf_oa_format;
>> +	uint64_t properties[] = {
>> +		DRM_XE_OA_PROPERTY_OA_UNIT_ID, 0,
>> +		DRM_XE_OA_PROPERTY_SAMPLE_OA, true,
>> +		DRM_XE_OA_PROPERTY_OA_METRIC_SET, test_set->perf_oa_metrics_set,
>> +		DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(fmt),
>> +		DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, exponent,
>> +		DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE, hwe->engine_instance,
>> +		DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE, buffer_size,
>> +		DRM_XE_OA_PROPERTY_OA_DISABLED, true,
>> +	};
>> +	struct intel_xe_oa_open_prop param = {
>> +		.num_properties = ARRAY_SIZE(properties) / 2,
>> +		.properties_ptr = to_user_pointer(properties),
>> +	};
>> +	uint32_t fmt_size = get_oa_format(fmt).size;
>> +	uint32_t fit_reports = buffer_size / fmt_size;
>> +	uint32_t extra_reports = 10;
>> +	uint32_t total_reports = fit_reports + extra_reports;
>> +	uint32_t total_size = total_reports * fmt_size;
>> +	uint8_t *buf = malloc(total_size);
>> +
>> +	uint32_t wrap_offset = (fit_reports * fmt_size) >> 2;
>> +	uint32_t end_offset = buffer_size >> 2;
>> +	uint32_t *zero_area, *buffer_end, *buffer_start;
>
>You can just use u32, instead of the longer uint32_t, for these...
>
>> +	int i;
>> +
>> +	/* Ensure report does not fit */
>
>This comment needs to be more specific, or just drop it.
>
>> +	igt_require(wrap_offset < end_offset);
>> +
>> +	stream_fd = __perf_open(drm_fd, &param, false);
>> +
>> +	/* Read fit_reports + extra_reports */
>> +	do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
>> +	read_reports(stream_fd, buf, total_size);
>> +	do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_DISABLE, 0);
>> +
>> +	/* Quick check for valid reports */
>> +	for (i = 0; i < total_reports; i++) {
>> +		uint32_t *report = (uint32_t *)&buf[i * fmt_size];
>> +
>> +		igt_assert(report_reason(report) && oa_timestamp(report, fmt));

Maybe we can hold off on this patch.

It looks like when the issue occurs, we will not be able to read reports 
greater than fit_reports because the kmd will not be able to find valid 
reports in the OA buffer after the wrap (since the TAIL reg is out of 
sync).

Will think of some other way to test this.

Thanks,
Umesh

>> +	}
>> +
>> +	/* mmap the buffer and check that the end of the buffer has zeroes */
>> +	buffer_start = mmap(0, buffer_size, PROT_READ, MAP_PRIVATE, stream_fd, 0);
>> +	igt_assert(buffer_start);
>> +
>> +	zero_area = buffer_start + wrap_offset;
>> +	buffer_end = buffer_start + end_offset;
>> +
>> +	/* Ensure HW did not write to this area */
>> +	while (zero_area < buffer_end)
>> +		igt_assert_eq(*zero_area++, 0);
>> +
>> +	munmap(buffer_start, buffer_size);
>> +
>> +	__perf_close(stream_fd);
>> +	free(buf);
>> +}
>> +
>>  /**
>>   * SUBTEST: map-oa-buffer
>>   * Description: Verify mapping of oa buffer
>> @@ -5095,6 +5184,14 @@ igt_main
>>				closed_fd_and_unmapped_access(hwe);
>>	}
>>
>> +	igt_subtest_with_dynamic("tail-address-wrap") {
>> +		long k = random() % num_buf_sizes;
>> +
>> +		igt_require(oau->capabilities & DRM_XE_OA_CAPS_OA_BUFFER_SIZE);
>> +		__for_one_hwe_in_oag_w_arg(hwe, buf_sizes[k].name)
>> +			test_tail_address_wrap(hwe, buf_sizes[k].size);
>> +	}
>> +
>>	igt_subtest_group {
>>		igt_fixture {
>>			perf_init_whitelist();
>
>Otherwise, this is also:
>
>Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>