dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] drm/radeon: Make r100_cp_ring_info() and radeon_ring_gfx() safe
@ 2013-09-20 13:36 Alex Ivanov
  2013-09-20 18:33 ` Alex Deucher
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Ivanov @ 2013-09-20 13:36 UTC (permalink / raw)
  To: Maling list - DRI developers

Prevent NULL pointer dereference in case when radeon_ring_fini() did it's job.

Reading of r100_cp_ring_info and radeon_ring_gfx debugfs entries will lead to a KP if ring buffer was deallocated, e.g. on failed ring test. 
Seen on PA-RISC machine having "radeon: ring test failed (scratch(0x8504)=0xCAFEDEAD)" issue.

Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
---
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 2417571..413cdd1 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2933,10 +2933,11 @@ static int r100_debugfs_cp_ring_info(struct seq_file *m, void *data)
 	seq_printf(m, "CP_RB_RPTR 0x%08x\n", rdp);
 	seq_printf(m, "%u free dwords in ring\n", ring->ring_free_dw);
 	seq_printf(m, "%u dwords in ring\n", count);
-	for (j = 0; j <= count; j++) {
-		i = (rdp + j) & ring->ptr_mask;
-		seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
-	}
+	if (ring->ready)
+		for (j = 0; j <= count; j++) {
+			i = (rdp + j) & ring->ptr_mask;
+			seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
+		}
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 46a25f0..a890756 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -839,10 +839,11 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
 	 * packet that is the root issue
 	 */
 	i = (ring->rptr + ring->ptr_mask + 1 - 32) & ring->ptr_mask;
-	for (j = 0; j <= (count + 32); j++) {
-		seq_printf(m, "r[%5d]=0x%08x\n", i, ring->ring[i]);
-		i = (i + 1) & ring->ptr_mask;
-	}
+	if (ring->ready)
+		for (j = 0; j <= (count + 32); j++) {
+			seq_printf(m, "r[%5d]=0x%08x\n", i, ring->ring[i]);
+			i = (i + 1) & ring->ptr_mask;
+		}
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] drm/radeon: Make r100_cp_ring_info() and radeon_ring_gfx() safe
  2013-09-20 13:36 [PATCH 1/1] drm/radeon: Make r100_cp_ring_info() and radeon_ring_gfx() safe Alex Ivanov
@ 2013-09-20 18:33 ` Alex Deucher
  2013-09-22  1:18   ` [PATCH 1/1] drm/radeon: Don't run tests when underlying memory was uninitialized Alex Ivanov
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2013-09-20 18:33 UTC (permalink / raw)
  To: Alex Ivanov; +Cc: Maling list - DRI developers

On Fri, Sep 20, 2013 at 9:36 AM, Alex Ivanov <gnidorah@p0n4ik.tk> wrote:
> Prevent NULL pointer dereference in case when radeon_ring_fini() did it's job.
>
> Reading of r100_cp_ring_info and radeon_ring_gfx debugfs entries will lead to a KP if ring buffer was deallocated, e.g. on failed ring test.
> Seen on PA-RISC machine having "radeon: ring test failed (scratch(0x8504)=0xCAFEDEAD)" issue.
>
> Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>

Applied.  thanks!

Alex

> ---
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index 2417571..413cdd1 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -2933,10 +2933,11 @@ static int r100_debugfs_cp_ring_info(struct seq_file *m, void *data)
>         seq_printf(m, "CP_RB_RPTR 0x%08x\n", rdp);
>         seq_printf(m, "%u free dwords in ring\n", ring->ring_free_dw);
>         seq_printf(m, "%u dwords in ring\n", count);
> -       for (j = 0; j <= count; j++) {
> -               i = (rdp + j) & ring->ptr_mask;
> -               seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
> -       }
> +       if (ring->ready)
> +               for (j = 0; j <= count; j++) {
> +                       i = (rdp + j) & ring->ptr_mask;
> +                       seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
> +               }
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
> index 46a25f0..a890756 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -839,10 +839,11 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
>          * packet that is the root issue
>          */
>         i = (ring->rptr + ring->ptr_mask + 1 - 32) & ring->ptr_mask;
> -       for (j = 0; j <= (count + 32); j++) {
> -               seq_printf(m, "r[%5d]=0x%08x\n", i, ring->ring[i]);
> -               i = (i + 1) & ring->ptr_mask;
> -       }
> +       if (ring->ready)
> +               for (j = 0; j <= (count + 32); j++) {
> +                       seq_printf(m, "r[%5d]=0x%08x\n", i, ring->ring[i]);
> +                       i = (i + 1) & ring->ptr_mask;
> +               }
>         return 0;
>  }
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] drm/radeon: Don't run tests when underlying memory was uninitialized
  2013-09-20 18:33 ` Alex Deucher
@ 2013-09-22  1:18   ` Alex Ivanov
  2013-09-23 11:59     ` Alex Ivanov
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Ivanov @ 2013-09-22  1:18 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers

20.09.2013, в 22:33, Alex Deucher <alexdeucher@gmail.com> написал(а):

> On Fri, Sep 20, 2013 at 9:36 AM, Alex Ivanov <gnidorah@p0n4ik.tk> wrote:
>> Prevent NULL pointer dereference in case when radeon_ring_fini() did it's job.
>> 
>> Reading of r100_cp_ring_info and radeon_ring_gfx debugfs entries will lead to a KP if ring buffer was deallocated, e.g. on failed ring test.
>> Seen on PA-RISC machine having "radeon: ring test failed (scratch(0x8504)=0xCAFEDEAD)" issue.
>> 
>> Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
> 
> Applied.  thanks!
> 
> Alex

One more. Thank you!

Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
---
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index e29faa7..e6d1897 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1319,6 +1319,11 @@ int radeon_device_init(struct radeon_device *rdev,
                if (r)
                        return r;
        }
+       /* If ring buffer or PCI GART got uninitialized, we should't run tests */
+       if (!rdev->accel_working) {
+               DRM_INFO("radeon: acceleration disabled, skipping tests and benchmark.\n");
+               return 0;
+       }
        if ((radeon_testing & 1)) {
                radeon_test_moves(rdev);
        }



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] drm/radeon: Don't run tests when underlying memory was uninitialized
  2013-09-22  1:18   ` [PATCH 1/1] drm/radeon: Don't run tests when underlying memory was uninitialized Alex Ivanov
@ 2013-09-23 11:59     ` Alex Ivanov
  2013-09-23 14:44       ` Alex Deucher
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Ivanov @ 2013-09-23 11:59 UTC (permalink / raw)
  To: Maling list - DRI developers

P.S.: don't run means don't allow to run, by either feeding radeon.test=X
or radeon.benchmark=1

22.09.2013, в 5:18, Alex Ivanov <gnidorah@p0n4ik.tk> написал(а):

> 20.09.2013, в 22:33, Alex Deucher <alexdeucher@gmail.com> написал(а):
> 
>> On Fri, Sep 20, 2013 at 9:36 AM, Alex Ivanov <gnidorah@p0n4ik.tk> wrote:
>>> Prevent NULL pointer dereference in case when radeon_ring_fini() did it's job.
>>> 
>>> Reading of r100_cp_ring_info and radeon_ring_gfx debugfs entries will lead to a KP if ring buffer was deallocated, e.g. on failed ring test.
>>> Seen on PA-RISC machine having "radeon: ring test failed (scratch(0x8504)=0xCAFEDEAD)" issue.
>>> 
>>> Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
>> 
>> Applied.  thanks!
>> 
>> Alex
> 
> One more. Thank you!
> 
> Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
> ---
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index e29faa7..e6d1897 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1319,6 +1319,11 @@ int radeon_device_init(struct radeon_device *rdev,
>                if (r)
>                        return r;
>        }
> +       /* If ring buffer or PCI GART got uninitialized, we should't run tests */
> +       if (!rdev->accel_working) {
> +               DRM_INFO("radeon: acceleration disabled, skipping tests and benchmark.\n");
> +               return 0;
> +       }
>        if ((radeon_testing & 1)) {
>                radeon_test_moves(rdev);
>        }
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] drm/radeon: Don't run tests when underlying memory was uninitialized
  2013-09-23 11:59     ` Alex Ivanov
@ 2013-09-23 14:44       ` Alex Deucher
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Deucher @ 2013-09-23 14:44 UTC (permalink / raw)
  To: Alex Ivanov; +Cc: Maling list - DRI developers

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

On Mon, Sep 23, 2013 at 7:59 AM, Alex Ivanov <gnidorah@p0n4ik.tk> wrote:
> P.S.: don't run means don't allow to run, by either feeding radeon.test=X
> or radeon.benchmark=1

Good catch.  I applied a slightly different variant of the same idea.

Thanks,

Alex

>
> 22.09.2013, в 5:18, Alex Ivanov <gnidorah@p0n4ik.tk> написал(а):
>
>> 20.09.2013, в 22:33, Alex Deucher <alexdeucher@gmail.com> написал(а):
>>
>>> On Fri, Sep 20, 2013 at 9:36 AM, Alex Ivanov <gnidorah@p0n4ik.tk> wrote:
>>>> Prevent NULL pointer dereference in case when radeon_ring_fini() did it's job.
>>>>
>>>> Reading of r100_cp_ring_info and radeon_ring_gfx debugfs entries will lead to a KP if ring buffer was deallocated, e.g. on failed ring test.
>>>> Seen on PA-RISC machine having "radeon: ring test failed (scratch(0x8504)=0xCAFEDEAD)" issue.
>>>>
>>>> Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
>>>
>>> Applied.  thanks!
>>>
>>> Alex
>>
>> One more. Thank you!
>>
>> Signed-off-by: Alex Ivanov <gnidorah@p0n4ik.tk>
>> ---
>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
>> index e29faa7..e6d1897 100644
>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>> @@ -1319,6 +1319,11 @@ int radeon_device_init(struct radeon_device *rdev,
>>                if (r)
>>                        return r;
>>        }
>> +       /* If ring buffer or PCI GART got uninitialized, we should't run tests */
>> +       if (!rdev->accel_working) {
>> +               DRM_INFO("radeon: acceleration disabled, skipping tests and benchmark.\n");
>> +               return 0;
>> +       }
>>        if ((radeon_testing & 1)) {
>>                radeon_test_moves(rdev);
>>        }
>>
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #2: 0001-drm-radeon-disable-tests-benchmarks-if-accel-is-disa.patch --]
[-- Type: application/octet-stream, Size: 1556 bytes --]

From 97116713760ec8767034aecd1ea4b6f4cffc5f7e Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Mon, 23 Sep 2013 10:38:26 -0400
Subject: [PATCH] drm/radeon: disable tests/benchmarks if accel is disabled

The tests are only usable if the acceleration engines have
been successfully initialized.

Based on an initial patch from: Alex Ivanov <gnidorah@p0n4ik.tk>

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_device.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index e29faa7..841d0e0 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1320,13 +1320,22 @@ int radeon_device_init(struct radeon_device *rdev,
 			return r;
 	}
 	if ((radeon_testing & 1)) {
-		radeon_test_moves(rdev);
+		if (rdev->accel_working)
+			radeon_test_moves(rdev);
+		else
+			DRM_INFO("radeon: acceleration disabled, skipping move tests\n");
 	}
 	if ((radeon_testing & 2)) {
-		radeon_test_syncing(rdev);
+		if (rdev->accel_working)
+			radeon_test_syncing(rdev);
+		else
+			DRM_INFO("radeon: acceleration disabled, skipping sync tests\n");
 	}
 	if (radeon_benchmarking) {
-		radeon_benchmark(rdev, radeon_benchmarking);
+		if (rdev->accel_working)
+			radeon_benchmark(rdev, radeon_benchmarking);
+		else
+			DRM_INFO("radeon: acceleration disabled, skipping benchmarks\n");
 	}
 	return 0;
 }
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-09-23 14:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-20 13:36 [PATCH 1/1] drm/radeon: Make r100_cp_ring_info() and radeon_ring_gfx() safe Alex Ivanov
2013-09-20 18:33 ` Alex Deucher
2013-09-22  1:18   ` [PATCH 1/1] drm/radeon: Don't run tests when underlying memory was uninitialized Alex Ivanov
2013-09-23 11:59     ` Alex Ivanov
2013-09-23 14:44       ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).