tags 572168 + upstream patch
thanks
Jonathan Nieder wrote:
> On Fri, Mar 05, 2010 at 05:00:27PM +0100, Tormod Volden wrote:
>> The old version just grepped the lspci output for "Memory" and "K" to
>> find the control region. The new one looks for a region of size
>> 64K, which fails for your class of cards (yours is 16K). The region
>> index will then be uninitialized and if it happens to be 0, your frame
>> buffer will be accessed instead...
>
> Thanks for the analysis. Patches coming in a moment.
Here are some patches against upstream master that seem to fix this.
I am attaching them rather than sending them as separate messages
since the Debian BTS deals better with that.
Thoughts?
Jonathan Nieder (3):
radeontool: completely skip early cards with --skip
radeontool: error out for missing control or fb region
radeontool: handle r128 again
radeontool.c | 21 ++++++++++++++-------
1 files changed, 14 insertions(+), 7 deletions(-)
>From c73c35d8d17035e4a61b60c66713613108a08723 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnie...@gmail.com>
Date: Fri, 5 Mar 2010 10:38:43 -0600
Subject: [PATCH 1/3] radeontool: completely skip early cards with --skip
radeontool looks for the control and fb regions for early cards
with --skip even though that they are not going to be used. If
radeontool fails to detect one of those regions for the chosen
card, the result can be a setup with one region from one card
and the other from another.
This also improves error handling: if only one card is present
but --skip=1 was supplied, then without this patch radeontool
will not actually error out.
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
radeontool.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/radeontool.c b/radeontool.c
index 907927f..a67e010 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -888,6 +888,9 @@ static void map_radeon_cntl_mem(void)
device->device_id, device->device_class);
}
+ if (skip-- != 0) {
+ continue;
+ }
for (i = 0; i < 6; i++) {
if (device->regions[i].size == 64 * 1024)
ctrl_region = i;
@@ -895,9 +898,7 @@ static void map_radeon_cntl_mem(void)
fb_region = i;
}
avivo_device = device;
- if(skip-- == 0) {
- break;
- }
+ break;
}
}
--
1.7.0
>From c353c73d99c08995ee88f89e3a32f36dcec9447b Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnie...@gmail.com>
Date: Fri, 5 Mar 2010 10:56:18 -0600
Subject: [PATCH 2/3] radeontool: error out for missing control or fb region
Do not blithely use an uninitialized value when the expected
memory areas cannot be found.
This is triggered by trying to use radeontool on an r128.
Noticed-by: Tormod Volden <debian.tor...@gmail.com>
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
radeontool.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/radeontool.c b/radeontool.c
index a67e010..100b255 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -33,7 +33,7 @@ int skip = 0;
/* Not the address but what it points to is volatile. */
struct pci_device *avivo_device = NULL;
unsigned char * volatile radeon_cntl_mem;
-unsigned int ctrl_region, fb_region;
+unsigned int ctrl_region = -1, fb_region = -1;
unsigned char * volatile fb_mem;
unsigned char * volatile ctrl_mem;
static void radeon_rom_legacy_mmio_table(unsigned char *bios, int offset);
@@ -860,7 +860,8 @@ void radeon_cmd_stretch(char *param)
}
-/* Here we fork() and exec() the lspci command to look for the Radeon hardware
address. */
+/* Here we used to fork() and exec() the lspci command
+ * to look for the Radeon hardware address. */
static void map_radeon_cntl_mem(void)
{
struct pci_slot_match match;
@@ -897,6 +898,10 @@ static void map_radeon_cntl_mem(void)
else if (device->regions[i].size >= 128 * 1024 * 1024)
fb_region = i;
}
+ if (ctrl_region == (unsigned int) -1)
+ fatal("cannot find ctrl region\n");
+ if (fb_region == (unsigned int) -1)
+ fatal("cannot find fb region\n");
avivo_device = device;
break;
}
--
1.7.0
>From 11ab9caae843754ab92643693637b174e7de7825 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnie...@gmail.com>
Date: Fri, 5 Mar 2010 11:00:07 -0600
Subject: [PATCH 3/3] radeontool: handle r128 again
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
An r128 has a tiny control region and a small framebuffer, but
radeontool can handle it anyway.
In particular, this gets 'radeontool light off' (and 'on'), which is
often used to work around bugs in Dell suspend support, working again
for the r128. It hasn’t been working since the change to use
libpciaccess.
Analysis-by: Tormod Volden <debian.tor...@gmail.com>
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
radeontool.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/radeontool.c b/radeontool.c
index 100b255..919b92f 100644
--- a/radeontool.c
+++ b/radeontool.c
@@ -893,9 +893,10 @@ static void map_radeon_cntl_mem(void)
continue;
}
for (i = 0; i < 6; i++) {
- if (device->regions[i].size == 64 * 1024)
+ if (device->regions[i].size == 64 * 1024 ||
+ device->regions[i].size == 16 * 1024)
ctrl_region = i;
- else if (device->regions[i].size >= 128 * 1024 * 1024)
+ else if (device->regions[i].size >= 64 * 1024 * 1024)
fb_region = i;
}
if (ctrl_region == (unsigned int) -1)
--
1.7.0
