Radeon: race in dp aux code on atom scratch

Commit Message

Alex Deucher Nov. 10, 2014, 4:35 p.m. UTC

On Mon, Nov 10, 2014 at 2:47 AM, Dave Airlie <[e-mail]> wrote:
> Hey,
>
> I think I spotted a problem in the atombios auxch code, we lock the chan
> mutex, access atom scratch and then call atom execute.
>
> Now atom execute is protected by a mutex, but if multiple aux channels are
> accessed we will collide on the scratch area,
>
> Any ideas on best fix?

Just adding another mutex for the scratch buffer seems like the
easiest solution to me.  How about the attached patch?

Alex

>
> Dave.

Patch

From 7979bc7d4ee00d05960bceed35d929de28683cef Mon Sep 17 00:00:00 2001
From: Alex Deucher <[e-mail]>
Date: Mon, 10 Nov 2014 11:32:42 -0500
Subject: [PATCH] drm/radeon: protect atom scratch buffer with a mutex

There is just one scratch buffer, but it can be used
by multiple atom clients.

Signed-off-by: Alex Deucher <[e-mail]>
Cc: [e-mail]
---
 drivers/gpu/drm/radeon/atom.c          | 4 ++++
 drivers/gpu/drm/radeon/atom.h          | 2 ++
 drivers/gpu/drm/radeon/atombios_dp.c   | 4 ++++
 drivers/gpu/drm/radeon/atombios_i2c.c  | 4 ++++
 drivers/gpu/drm/radeon/radeon_device.c | 1 +
 5 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 15da7ef..604e1a2 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -1221,6 +1221,8 @@  int atom_execute_table(struct atom_context *ctx, int index, uint32_t * params)
 {
 	int r;
 
+	if (ctx->driver_uses_scratch == false)
+		mutex_lock(&ctx->scratch_mutex);
 	mutex_lock(&ctx->mutex);
 	/* reset data block */
 	ctx->data_block = 0;
@@ -1234,6 +1236,8 @@  int atom_execute_table(struct atom_context *ctx, int index, uint32_t * params)
 	ctx->divmul[0] = 0;
 	ctx->divmul[1] = 0;
 	r = atom_execute_table_locked(ctx, index, params);
+	if (ctx->driver_uses_scratch == false)
+		mutex_unlock(&ctx->scratch_mutex);
 	mutex_unlock(&ctx->mutex);
 	return r;
 }
diff --git a/drivers/gpu/drm/radeon/atom.h b/drivers/gpu/drm/radeon/atom.h
index feba6b8..b1322bf 100644
--- a/drivers/gpu/drm/radeon/atom.h
+++ b/drivers/gpu/drm/radeon/atom.h
@@ -137,6 +137,8 @@  struct atom_context {
 	uint8_t shift;
 	int cs_equal, cs_above;
 	int io_mode;
+	struct mutex scratch_mutex;
+	bool driver_uses_scratch;
 	uint32_t *scratch;
 	int scratch_size_bytes;
 };
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
index 95d5d4a..b338f19 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -100,6 +100,8 @@  static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
 	memset(&args, 0, sizeof(args));
 
 	mutex_lock(&chan->mutex);
+	mutex_lock(&rdev->mode_info.atom_context->scratch_mutex);
+	rdev->mode_info.atom_context->driver_uses_scratch = true;
 
 	base = (unsigned char *)(rdev->mode_info.atom_context->scratch + 1);
 
@@ -147,6 +149,8 @@  static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
 
 	r = recv_bytes;
 done:
+	rdev->mode_info.atom_context->driver_uses_scratch = false;
+	mutex_unlock(&rdev->mode_info.atom_context->scratch_mutex);
 	mutex_unlock(&chan->mutex);
 
 	return r;
diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c b/drivers/gpu/drm/radeon/atombios_i2c.c
index 9c570fb..0dfd489 100644
--- a/drivers/gpu/drm/radeon/atombios_i2c.c
+++ b/drivers/gpu/drm/radeon/atombios_i2c.c
@@ -48,6 +48,8 @@  static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
 	memset(&args, 0, sizeof(args));
 
 	mutex_lock(&chan->mutex);
+	mutex_lock(&rdev->mode_info.atom_context->scratch_mutex);
+	rdev->mode_info.atom_context->driver_uses_scratch = true;
 
 	base = (unsigned char *)rdev->mode_info.atom_context->scratch;
 
@@ -95,6 +97,8 @@  static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
 		radeon_atom_copy_swap(buf, base, num, false);
 
 done:
+	rdev->mode_info.atom_context->driver_uses_scratch = false;
+	mutex_unlock(&rdev->mode_info.atom_context->scratch_mutex);
 	mutex_unlock(&chan->mutex);
 
 	return r;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index ea26769..258e1c9 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -954,6 +954,7 @@  int radeon_atombios_init(struct radeon_device *rdev)
 	mutex_init(&rdev->mode_info.atom_context->mutex);
 	radeon_atom_initialize_bios_scratch_regs(rdev->ddev);
 	atom_allocate_fb_scratch(rdev->mode_info.atom_context);
+	mutex_init(&rdev->mode_info.atom_context->scratch_mutex);
 	return 0;
 }
 
-- 
1.8.3.1