Re: [PATCH i-g-t 2/2] tests/intel/xe_oa: Replace hardcoded OA buffer size assumptions

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

On Sun, 17 Nov 2024 22:32:23 -0800, Sai Teja Pottumuttu wrote:
>
> A few xe_oa tests assumed that the default OA buffer size is 16MB
> and had calculations around it. These would fail if the default
> buffer size is changed on the kernel side.
>

I don't think this is correct. buf_size below refers to the size of the
user's buffer, not the size of the kernel OA buffer.

You also seem to be assuming that '65536 * report_size == oa_buffer_size'
(65536 * 256 == 16 MB) but report size is not only 256, Xe2 default report
size is 576 bytes and there are other report sizes too.

Maybe ok to do what this patch is doing for test_buffer_fill and
test_enable_disable, but we need to be careful about test_non_zero_reason
since that test is very useful for testing OA buffer wrap-arounds (and some
corner cases there).

I'd say let's drop this patch for now and revisit later if needed.

> Replace such assumptions with the OA buffer size we get from
> DRM_XE_OBSERVATION_IOCTL_INFO ioctl
>
> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> ---
>  tests/intel/xe_oa.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 4e4d0fabc..3b65212ae 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -2389,7 +2389,7 @@ test_buffer_fill(const struct drm_xe_engine_class_instance *hwe)
>		.properties_ptr = to_user_pointer(properties),
>	};
>	size_t report_size = get_oa_format(fmt).size;
> -	int buf_size = 65536 * report_size;
> +	int buf_size = oa_buffer_size;
>	uint8_t *buf = malloc(buf_size);
>	int len;
>	size_t oa_buf_size = oa_buffer_size;
> @@ -2533,7 +2533,7 @@ test_non_zero_reason(const struct drm_xe_engine_class_instance *hwe)
>		.num_properties = ARRAY_SIZE(properties) / 2,
>		.properties_ptr = to_user_pointer(properties),
>	};
> -	uint32_t buf_size = 3 * 65536 * report_size;
> +	uint32_t buf_size = 3 * oa_buffer_size;
>	uint8_t *buf = malloc(buf_size);
>	uint32_t total_len = 0;
>	const uint32_t *last_report;
> @@ -2615,7 +2615,7 @@ test_enable_disable(const struct drm_xe_engine_class_instance *hwe)
>		.properties_ptr = to_user_pointer(properties),
>	};
>	size_t report_size = get_oa_format(fmt).size;
> -	int buf_size = 65536 * report_size;
> +	int buf_size = oa_buffer_size;
>	uint8_t *buf = malloc(buf_size);
>	size_t oa_buf_size = oa_buffer_size;
>	int n_full_oa_reports = oa_buf_size / report_size;
> --
> 2.34.1
>

Thanks.
--
Ashutosh