Re: [PATCH 1/3] drm/xe/oa: Use xe_map layer

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

On Thu, 09 Apr 2026 16:17:52 -0700, Ashutosh Dixit wrote:
>

Hi Umesh,

> +/* Because there is no iosys_map_copy_to_user() or copy_to_user_from_iomem() */
> +static int xe_oa_copy_to_user(struct xe_oa_stream *stream, void __user *dst, u32 head, u32 len)
> +{
> +	u32 __user *dptr = dst;
> +
> +	xe_gt_assert(stream->gt, len % 4 == 0);
> +
> +	for (int i = 0; i < len / 4; i++) {
> +		struct iosys_map *map = &stream->oa_buffer.bo->vmap;
> +		u32 val = xe_map_rd(stream->oa->xe, map, head, u32);
> +		int ret = put_user(val, dptr++);
> +
> +		if (ret)
> +			return ret;
> +		head += 4;
> +	}
> +
> +	return 0;
> +}

Unfortunately, with this copy_to_user() implementation we are seeing buffer
overflow, implying there is significant performance impact, even when OA
buffer is in system memory:

  Starting subtest: non-zero-reason
  Starting dynamic subtest: oag-0
  (xe_oa:6798) CRITICAL: Test assertion failure function test_non_zero_reason, file ../tests/intel/xe_oa.c:2693:
  (xe_oa:6798) CRITICAL: Failed assertion: !(oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW)

That is, we are unable to read data at the rate at which HW is writing
data, causing buffer overrun.

Therefore in v3 I have basically reverted to the copy_to_user()
implementation of v1, except that I have taken the copy_to_user() out of
xe_oa.c and moved it to xe_map.h, to make that implementation "official"
(and OA code to strictly use the xe_map layer). This seems to be the best
approach to me, it avoids unnecessary copies and works correctly on all of
our systems of interest.

Also, the previous code, without this patch, is of course doing this exact
same thing, except that the use of the xe_map layer make these assumptions
explicit.

If you still don't think we should be doing this, I can try to get a second
opinion about this patch. Thanks.