Re: [Intel-gfx] [PATCH] drm/i915/perf: Consider OA buffer boundary when zeroing out reports

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

On Thu, 15 Jun 2023 18:50:35 -0700, Umesh Nerlige Ramappa wrote:
>
> For reports that are not powers of 2, reports at the end of the OA
> buffer may get split across the buffer boundary. When zeroing out such
> reports, take the split into consideration.
>
> Fixes: 09a36015d9a0 ("drm/i915/perf: Clear out entire reports after reading if not power of 2 size")
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index b5491a382bfd..9a8e329c5b5e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -867,8 +867,17 @@ static int gen8_append_oa_reports(struct i915_perf_stream *stream,
>			oa_report_id_clear(stream, report32);
>			oa_timestamp_clear(stream, report32);
>		} else {
> +			u8 *oa_buf_end = stream->oa_buffer.vaddr +
> +					 stream->oa_buffer.vma->size;

Probably: "u8 *oa_buf_end = oa_buf_base + OA_BUFFER_SIZE;" ?

We haven't used 'stream->oa_buffer.vma->size' much in this file so I think
better to stick to OA_BUFFER_SIZE.

> +			u32 part = (u32)((void *)oa_buf_end - (void *)report32);

This should be sufficient, no need to cast as above I think:
			u32 part = oa_buf_end - report32;
or
			u32 part = oa_buf_end - report;

> +
>			/* Zero out the entire report */
> -			memset(report32, 0, report_size);
> +			if (report_size <= part) {
> +				memset(report32, 0, report_size);
> +			} else {
> +				memset(report32, 0, part);
> +				memset(oa_buf_base, 0, report_size - part);
> +			}

Thanks for finding this:

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

>		}
>	}
>
> --
> 2.36.1
>