[PATCH v2] drm/log: Batch vmap/vunmap and flush for record drawing

[PATCH v2] drm/log: Batch vmap/vunmap and flush for record drawing

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

drm_log_draw_new_line() calls drm_log_clear_line() and drm_log_draw_line(),
which independently call drm_client_buffer_vmap_local(),
drm_client_buffer_vunmap_local(), and drm_client_buffer_flush(). For each
call to drm_log_draw_new_line(), this results in 2 vmap/vunmap pairs and
2 flush calls, even though vmap_local maps the entire framebuffer
each time.

Refactor drm_log_clear_line() and drm_log_draw_line() to accept a
pre-mapped iosys_map by value, removing the per-line vmap/vunmap/flush
calls. Move the single vmap/vunmap pair and flush up to
drm_log_draw_new_line(), which now maps once before clearing and
drawing, then issues a single flush after unmapping.

Also remove the now-unused drm_rect from both functions.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
v1->v2:
   Remove unused drm_rect from drm_log_clear_line() and
   drm_log_draw_line().

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index ac10c978f917..52563e2a1c45 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -94,37 +94,28 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
 	}
 }
 
-static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line)
+static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line,
+			       struct iosys_map map)
 {
 	struct drm_framebuffer *fb = scanout->buffer->fb;
 	unsigned long height = scanout->scaled_font_h;
-	struct iosys_map map;
-	struct drm_rect r = DRM_RECT_INIT(0, line * height, fb->width, height);
 
-	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
-		return;
-	iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb->pitches[0]);
-	drm_client_buffer_vunmap_local(scanout->buffer);
-	drm_client_buffer_flush(scanout->buffer, &r);
+	iosys_map_memset(&map, line * height * fb->pitches[0], 0, height * fb->pitches[0]);
 }
 
 static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
-			      unsigned int len, unsigned int prefix_len)
+			      unsigned int len, unsigned int prefix_len,
+			      struct iosys_map map)
 {
 	struct drm_framebuffer *fb = scanout->buffer->fb;
-	struct iosys_map map;
 	const struct font_desc *font = scanout->font;
 	size_t font_pitch = DIV_ROUND_UP(font->width, 8);
 	const u8 *src;
 	u32 px_width = fb->format->cpp[0];
-	struct drm_rect r = DRM_RECT_INIT(0, scanout->line * scanout->scaled_font_h,
-					  fb->width, (scanout->line + 1) * scanout->scaled_font_h);
 	u32 i;
 
-	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
-		return;
+	iosys_map_incr(&map, scanout->line * scanout->scaled_font_h * fb->pitches[0]);
 
-	iosys_map_incr(&map, r.y1 * fb->pitches[0]);
 	for (i = 0; i < len && i < scanout->columns; i++) {
 		u32 color = (i < prefix_len) ? scanout->prefix_color : scanout->front_color;
 		src = font_data_glyph_buf(font->data, font->width, font->height,
@@ -139,21 +130,40 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
 	scanout->line++;
 	if (scanout->line >= scanout->rows)
 		scanout->line = 0;
-	drm_client_buffer_vunmap_local(scanout->buffer);
-	drm_client_buffer_flush(scanout->buffer, &r);
 }
 
 static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
-				  const char *s, unsigned int len, unsigned int prefix_len)
+				  const char *s, unsigned int len,
+				  unsigned int prefix_len)
 {
+	struct iosys_map map;
+	struct drm_framebuffer *fb = scanout->buffer->fb;
+	u32 height = scanout->scaled_font_h;
+	u32 line = scanout->line;
+	u32 y2;
+	struct drm_rect dirty;
+
+	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
+		return;
+
 	if (scanout->line == 0) {
-		drm_log_clear_line(scanout, 0);
-		drm_log_clear_line(scanout, 1);
-		drm_log_clear_line(scanout, 2);
-	} else if (scanout->line + 2 < scanout->rows)
-		drm_log_clear_line(scanout, scanout->line + 2);
+		drm_log_clear_line(scanout, 0, map);
+		drm_log_clear_line(scanout, 1, map);
+		drm_log_clear_line(scanout, 2, map);
+		y2 = min(3, scanout->rows) * height;
+	} else if (scanout->line + 2 < scanout->rows) {
+		drm_log_clear_line(scanout, scanout->line + 2, map);
+		y2 = (line + 3) * height;
+	} else {
+		y2 = (line + 1) * height;
+	}
+
+	drm_log_draw_line(scanout, s, len, prefix_len, map);
+
+	drm_client_buffer_vunmap_local(scanout->buffer);
 
-	drm_log_draw_line(scanout, s, len, prefix_len);
+	dirty = DRM_RECT_INIT(0, line * height, fb->width, y2 - line * height);
+	drm_client_buffer_flush(scanout->buffer, &dirty);
 }
 
 /*
--
2.25.1

No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH v2] drm/log: Batch vmap/vunmap and flush for record drawing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional clearing of lines 0, 1, and 2 can write past the end of the framebuffer if `scanout->rows` is less than 3.
--

--- Patch [1]: [PATCH v2] drm/log: Batch vmap/vunmap and flush for record drawing ---
commit 59469456017da9a89a891c48422d122f14da3c7a
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/log: Batch vmap/vunmap and flush for record drawing

This commit refactors drm_log_clear_line and drm_log_draw_line to accept a
pre-mapped iosys_map by value, removing the per-line vmap/vunmap/flush
calls. The single vmap/vunmap pair and flush are moved to
drm_log_draw_new_line.

> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index e3e02c84a4cfe..97eddae3973f3 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c

[ ... ]

> @@ -134,21 +125,40 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
>  	scanout->line++;
>  	if (scanout->line >= scanout->rows)
>  		scanout->line = 0;
> -	drm_client_buffer_vunmap_local(scanout->buffer);
> -	drm_client_buffer_flush(scanout->buffer, &r);
>  }
>  
>  static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
> -				  const char *s, unsigned int len, unsigned int prefix_len)
> +				  const char *s, unsigned int len,
> +				  unsigned int prefix_len)
>  {
> +	struct iosys_map map;
> +	struct drm_framebuffer *fb = scanout->buffer->fb;
> +	u32 height = scanout->scaled_font_h;
> +	u32 line = scanout->line;
> +	u32 y2;
> +	struct drm_rect dirty;
> +
> +	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
> +		return;
> +
>  	if (scanout->line == 0) {
> -		drm_log_clear_line(scanout, 0);
> -		drm_log_clear_line(scanout, 1);
> -		drm_log_clear_line(scanout, 2);
> +		drm_log_clear_line(scanout, 0, map);
> +		drm_log_clear_line(scanout, 1, map);
> +		drm_log_clear_line(scanout, 2, map);

[Severity: High]
This is a pre-existing issue, but can unconditionally clearing lines 0, 1,
and 2 write past the end of the framebuffer if scanout->rows is less than 3?

If the display has a very small vertical resolution, or if the scale module
parameter is set to a large value, scanout->rows can be less than 3.

When scanout->line == 0, drm_log_draw_new_line unconditionally clears line 2.
This calls drm_log_clear_line, which does:

static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line,
                               struct iosys_map map)
{
        ...
        iosys_map_memset(&map, line * height * fb->pitches[0], 0, height * fb->pitches[0]);
}

If line * height exceeds the actual framebuffer height, does this result in
an out-of-bounds memory write that could corrupt adjacent kernel memory?

> +		y2 = min(3, scanout->rows) * height;
> +	} else if (scanout->line + 2 < scanout->rows) {
> -		drm_log_clear_line(scanout, scanout->line + 2);
> +		drm_log_clear_line(scanout, scanout->line + 2, map);
> +		y2 = (line + 3) * height;
> +	} else {
> +		y2 = (line + 1) * height;
> +	}
> +
> -	drm_log_draw_line(scanout, s, len, prefix_len);
> +	drm_log_draw_line(scanout, s, len, prefix_len, map);
> +
> +	drm_client_buffer_vunmap_local(scanout->buffer);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730105950.670892-1-oushixiong1025@163.com?part=1