diff mbox series

drm/i915/perf: Consider OA buffer boundary when zeroing out reports

Message ID 20230616015035.616403-1-umesh.nerlige.ramappa@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/perf: Consider OA buffer boundary when zeroing out reports | expand

Commit Message

Umesh Nerlige Ramappa June 16, 2023, 1:50 a.m. UTC
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(-)

Comments

Dixit, Ashutosh June 16, 2023, 6:39 a.m. UTC | #1
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
>
diff mbox series

Patch

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;
+			u32 part = (u32)((void *)oa_buf_end - (void *)report32);
+
 			/* 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);
+			}
 		}
 	}