Commit 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") changed the LBM allocation logic from first allocating the LBM memory for the plane to running mode_set, adding a gap in the LBM, and then running the dlist allocation filling that gap.
The gap was introduced by incrementing the dlist array index, but was never checking whether or not we were over the array length, leading eventually to memory corruptions if we ever crossed this limit.
vc4_dlist_write had that logic though, and was reallocating a larger dlist array when reaching the end of the buffer. Let's share the logic between both functions.
Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Eric Anholt eric@anholt.net Fixes: 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_plane.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index b5586c92bfe5..3d33fe3dacea 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -227,7 +227,7 @@ static void vc4_plane_reset(struct drm_plane *plane) __drm_atomic_helper_plane_reset(plane, &vc4_state->base); }
-static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) +static void vc4_dlist_counter_increment(struct vc4_plane_state *vc4_state) { if (vc4_state->dlist_count == vc4_state->dlist_size) { u32 new_size = max(4u, vc4_state->dlist_count * 2); @@ -242,7 +242,15 @@ static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) vc4_state->dlist_size = new_size; }
- vc4_state->dlist[vc4_state->dlist_count++] = val; + vc4_state->dlist_count++; +} + +static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) +{ + unsigned int idx = vc4_state->dlist_count; + + vc4_dlist_counter_increment(vc4_state); + vc4_state->dlist[idx] = val; }
/* Returns the scl0/scl1 field based on whether the dimensions need to @@ -1057,8 +1065,10 @@ static int vc4_plane_mode_set(struct drm_plane *plane, * be set when calling vc4_plane_allocate_lbm(). */ if (vc4_state->y_scaling[0] != VC4_SCALING_NONE || - vc4_state->y_scaling[1] != VC4_SCALING_NONE) - vc4_state->lbm_offset = vc4_state->dlist_count++; + vc4_state->y_scaling[1] != VC4_SCALING_NONE) { + vc4_state->lbm_offset = vc4_state->dlist_count; + vc4_dlist_counter_increment(vc4_state); + }
if (num_planes > 1) { /* Emit Cb/Cr as channel 0 and Y as channel
Am 29.01.21 um 17:06 schrieb Maxime Ripard:
Commit 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") changed the LBM allocation logic from first allocating the LBM memory for the plane to running mode_set, adding a gap in the LBM, and then running the dlist allocation filling that gap.
The gap was introduced by incrementing the dlist array index, but was never checking whether or not we were over the array length, leading eventually to memory corruptions if we ever crossed this limit.
vc4_dlist_write had that logic though, and was reallocating a larger dlist array when reaching the end of the buffer. Let's share the logic between both functions.
Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Eric Anholt eric@anholt.net Fixes: 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") Signed-off-by: Maxime Ripard maxime@cerno.tech
Acked-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/vc4/vc4_plane.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index b5586c92bfe5..3d33fe3dacea 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -227,7 +227,7 @@ static void vc4_plane_reset(struct drm_plane *plane) __drm_atomic_helper_plane_reset(plane, &vc4_state->base); }
-static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) +static void vc4_dlist_counter_increment(struct vc4_plane_state *vc4_state) { if (vc4_state->dlist_count == vc4_state->dlist_size) { u32 new_size = max(4u, vc4_state->dlist_count * 2); @@ -242,7 +242,15 @@ static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) vc4_state->dlist_size = new_size; }
- vc4_state->dlist[vc4_state->dlist_count++] = val;
- vc4_state->dlist_count++;
+}
+static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) +{
unsigned int idx = vc4_state->dlist_count;
vc4_dlist_counter_increment(vc4_state);
vc4_state->dlist[idx] = val; }
/* Returns the scl0/scl1 field based on whether the dimensions need to
@@ -1057,8 +1065,10 @@ static int vc4_plane_mode_set(struct drm_plane *plane, * be set when calling vc4_plane_allocate_lbm(). */ if (vc4_state->y_scaling[0] != VC4_SCALING_NONE ||
vc4_state->y_scaling[1] != VC4_SCALING_NONE)vc4_state->lbm_offset = vc4_state->dlist_count++;
vc4_state->y_scaling[1] != VC4_SCALING_NONE) {vc4_state->lbm_offset = vc4_state->dlist_count;vc4_dlist_counter_increment(vc4_state);}if (num_planes > 1) { /* Emit Cb/Cr as channel 0 and Y as channel
Hi Maxime
On Fri, 29 Jan 2021 at 16:07, Maxime Ripard maxime@cerno.tech wrote:
Commit 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") changed the LBM allocation logic from first allocating the LBM memory for the plane to running mode_set, adding a gap in the LBM, and then running the dlist allocation filling that gap.
The gap was introduced by incrementing the dlist array index, but was never checking whether or not we were over the array length, leading eventually to memory corruptions if we ever crossed this limit.
vc4_dlist_write had that logic though, and was reallocating a larger dlist array when reaching the end of the buffer. Let's share the logic between both functions.
Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Eric Anholt eric@anholt.net Fixes: 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") Signed-off-by: Maxime Ripard maxime@cerno.tech
Looks good.
Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
drivers/gpu/drm/vc4/vc4_plane.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index b5586c92bfe5..3d33fe3dacea 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -227,7 +227,7 @@ static void vc4_plane_reset(struct drm_plane *plane) __drm_atomic_helper_plane_reset(plane, &vc4_state->base); }
-static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) +static void vc4_dlist_counter_increment(struct vc4_plane_state *vc4_state) { if (vc4_state->dlist_count == vc4_state->dlist_size) { u32 new_size = max(4u, vc4_state->dlist_count * 2); @@ -242,7 +242,15 @@ static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) vc4_state->dlist_size = new_size; }
vc4_state->dlist[vc4_state->dlist_count++] = val;
vc4_state->dlist_count++;+}
+static void vc4_dlist_write(struct vc4_plane_state *vc4_state, u32 val) +{
unsigned int idx = vc4_state->dlist_count;vc4_dlist_counter_increment(vc4_state);vc4_state->dlist[idx] = val;}
/* Returns the scl0/scl1 field based on whether the dimensions need to @@ -1057,8 +1065,10 @@ static int vc4_plane_mode_set(struct drm_plane *plane, * be set when calling vc4_plane_allocate_lbm(). */ if (vc4_state->y_scaling[0] != VC4_SCALING_NONE ||
vc4_state->y_scaling[1] != VC4_SCALING_NONE)vc4_state->lbm_offset = vc4_state->dlist_count++;
vc4_state->y_scaling[1] != VC4_SCALING_NONE) {vc4_state->lbm_offset = vc4_state->dlist_count;vc4_dlist_counter_increment(vc4_state);} if (num_planes > 1) { /* Emit Cb/Cr as channel 0 and Y as channel-- 2.29.2
On Fri, Jan 29, 2021 at 05:06:47PM +0100, Maxime Ripard wrote:
Commit 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") changed the LBM allocation logic from first allocating the LBM memory for the plane to running mode_set, adding a gap in the LBM, and then running the dlist allocation filling that gap.
The gap was introduced by incrementing the dlist array index, but was never checking whether or not we were over the array length, leading eventually to memory corruptions if we ever crossed this limit.
vc4_dlist_write had that logic though, and was reallocating a larger dlist array when reaching the end of the buffer. Let's share the logic between both functions.
Cc: Boris Brezillon boris.brezillon@collabora.com Cc: Eric Anholt eric@anholt.net Fixes: 0a038c1c29a7 ("drm/vc4: Move LBM creation out of vc4_plane_mode_set()") Signed-off-by: Maxime Ripard maxime@cerno.tech
Applied, Maxime
dri-devel@lists.freedesktop.org
-
Dave Stevenson -
Maxime Ripard -
Thomas Zimmermann