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

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

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@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));
> +	}
> +
> +	/* 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@intel.com>