* [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case
@ 2008-10-14 22:58 Brandon Philips
2008-10-20 21:21 ` Andrew Morton
0 siblings, 1 reply; 7+ messages in thread
From: Brandon Philips @ 2008-10-14 22:58 UTC (permalink / raw)
To: Dave Airlie, Zhenyu Wang, Jesse Barnes; +Cc: linux-kernel
This is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was
reverted because it broke older X.org driver. This patch only fixes
the 1MB stolen case since it causes an oops.
Xorg will not work without the accompanying patch[1] but avoiding an
oops and making it possible to work with patched xorg driver is
reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
> static void intel_i830_init_gtt_entries(void)
...
> } else if (IS_G33) {
> /* G33's GTT size defined in gmch_ctrl */
> switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
> case G33_PGETBL_SIZE_1M:
> size = 1024;
> break;
...
> size += 4;
size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH
control we get to here:
> } else {
> switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> case I855_GMCH_GMS_STOLEN_1M:
> gtt_entries = MB(1) - KB(size);
> break;
MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024
MB(1) - KB(1028) = -4096
> gtt_entries /= KB(4);
> intel_private.gtt_entries = gtt_entries;
We end up with -1 in gtt_entries.
This leads to intel_i915_configure reading/writing to areas outside of
mapped memory and the oops.
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
Signed-off-by: Brandon Philips <bphilips@suse.de>
---
drivers/char/agp/intel-agp.c | 8 ++++++++
1 file changed, 8 insertions(+)
Index: linux-2.6/drivers/char/agp/intel-agp.c
===================================================================
--- linux-2.6.orig/drivers/char/agp/intel-agp.c
+++ linux-2.6/drivers/char/agp/intel-agp.c
@@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries(
} else {
switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
+ if (IS_G33) {
+ size = 0;
+ printk(KERN_WARNING PFX
+ "Warning: G33 chipset with 1MB"
+ " allocated. Older X.org Intel drivers"
+ " will not work.\n");
+ WARN_ON(1);
+ }
gtt_entries = MB(1) - KB(size);
break;
case I855_GMCH_GMS_STOLEN_4M:
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case
2008-10-14 22:58 [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case Brandon Philips
@ 2008-10-20 21:21 ` Andrew Morton
2008-10-20 21:44 ` Dave Airlie
2008-10-29 5:23 ` [PATCH] " Brandon Philips
0 siblings, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2008-10-20 21:21 UTC (permalink / raw)
To: Brandon Philips; +Cc: airlied, zhenyu.z.wang, jbarnes, linux-kernel
On Tue, 14 Oct 2008 15:58:17 -0700
Brandon Philips <bphilips@suse.de> wrote:
> This is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was
> reverted because it broke older X.org driver. This patch only fixes
> the 1MB stolen case since it causes an oops.
>
> Xorg will not work without the accompanying patch[1] but avoiding an
> oops and making it possible to work with patched xorg driver is
> reasonable.
>
> [1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
>
> Explanation of the oops:
>
> > static void intel_i830_init_gtt_entries(void)
> ...
> > } else if (IS_G33) {
> > /* G33's GTT size defined in gmch_ctrl */
> > switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
> > case G33_PGETBL_SIZE_1M:
> > size = 1024;
> > break;
> ...
> > size += 4;
>
> size = 1028
>
> Then since we have the BIOS setting 1MB for the device in the GMCH
> control we get to here:
>
> > } else {
> > switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> > case I855_GMCH_GMS_STOLEN_1M:
> > gtt_entries = MB(1) - KB(size);
> > break;
>
> MB(1) = 1 * 1024 * 1024
> KB(1028) = 1028 * 1024
>
> MB(1) - KB(1028) = -4096
>
> > gtt_entries /= KB(4);
> > intel_private.gtt_entries = gtt_entries;
>
> We end up with -1 in gtt_entries.
>
> This leads to intel_i915_configure reading/writing to areas outside of
> mapped memory and the oops.
>
> Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
>
> Signed-off-by: Brandon Philips <bphilips@suse.de>
>
> ---
> drivers/char/agp/intel-agp.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: linux-2.6/drivers/char/agp/intel-agp.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/agp/intel-agp.c
> +++ linux-2.6/drivers/char/agp/intel-agp.c
> @@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries(
> } else {
> switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> case I855_GMCH_GMS_STOLEN_1M:
> + if (IS_G33) {
> + size = 0;
> + printk(KERN_WARNING PFX
> + "Warning: G33 chipset with 1MB"
> + " allocated. Older X.org Intel drivers"
> + " will not work.\n");
> + WARN_ON(1);
> + }
> gtt_entries = MB(1) - KB(size);
> break;
> case I855_GMCH_GMS_STOLEN_4M:
Is the bug which this patch addresses present in the 2.6.27 kernel?
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case
2008-10-20 21:21 ` Andrew Morton
@ 2008-10-20 21:44 ` Dave Airlie
2008-10-20 21:52 ` Arjan van de Ven
2008-10-29 5:23 ` [PATCH] " Brandon Philips
1 sibling, 1 reply; 7+ messages in thread
From: Dave Airlie @ 2008-10-20 21:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: Brandon Philips, zhenyu.z.wang, jbarnes, linux-kernel
On Mon, 2008-10-20 at 14:21 -0700, Andrew Morton wrote:
> On Tue, 14 Oct 2008 15:58:17 -0700
> Brandon Philips <bphilips@suse.de> wrote:
>
> > This is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was
> > reverted because it broke older X.org driver. This patch only fixes
> > the 1MB stolen case since it causes an oops.
> >
> > Xorg will not work without the accompanying patch[1] but avoiding an
> > oops and making it possible to work with patched xorg driver is
> > reasonable.
> >
> > [1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
> >
> > Explanation of the oops:
> >
> > > static void intel_i830_init_gtt_entries(void)
> > ...
> > > } else if (IS_G33) {
> > > /* G33's GTT size defined in gmch_ctrl */
> > > switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
> > > case G33_PGETBL_SIZE_1M:
> > > size = 1024;
> > > break;
> > ...
> > > size += 4;
> >
> > size = 1028
> >
> > Then since we have the BIOS setting 1MB for the device in the GMCH
> > control we get to here:
> >
> > > } else {
> > > switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> > > case I855_GMCH_GMS_STOLEN_1M:
> > > gtt_entries = MB(1) - KB(size);
> > > break;
> >
> > MB(1) = 1 * 1024 * 1024
> > KB(1028) = 1028 * 1024
> >
> > MB(1) - KB(1028) = -4096
> >
> > > gtt_entries /= KB(4);
> > > intel_private.gtt_entries = gtt_entries;
> >
> > We end up with -1 in gtt_entries.
> >
> > This leads to intel_i915_configure reading/writing to areas outside of
> > mapped memory and the oops.
> >
> > Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
> >
> > Signed-off-by: Brandon Philips <bphilips@suse.de>
> >
> > ---
> > drivers/char/agp/intel-agp.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > Index: linux-2.6/drivers/char/agp/intel-agp.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/char/agp/intel-agp.c
> > +++ linux-2.6/drivers/char/agp/intel-agp.c
> > @@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries(
> > } else {
> > switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> > case I855_GMCH_GMS_STOLEN_1M:
> > + if (IS_G33) {
> > + size = 0;
> > + printk(KERN_WARNING PFX
> > + "Warning: G33 chipset with 1MB"
> > + " allocated. Older X.org Intel drivers"
> > + " will not work.\n");
> > + WARN_ON(1);
> > + }
> > gtt_entries = MB(1) - KB(size);
> > break;
> > case I855_GMCH_GMS_STOLEN_4M:
>
> Is the bug which this patch addresses present in the 2.6.27 kernel?
>
I've been a bit wary about this patch, but on re-review I suppose it
should be fine. We'll just get WARN_ONs in places we don't really want
them, and I'm sure Arjan will come complaining about them from
kerneloops.
However as I said at KS there is no nice answer to this, we are
hopelessly screwed.
Dave.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case
2008-10-20 21:44 ` Dave Airlie
@ 2008-10-20 21:52 ` Arjan van de Ven
2008-10-29 4:41 ` Brandon Philips
2008-10-29 5:22 ` [PATCH v2] " Brandon Philips
0 siblings, 2 replies; 7+ messages in thread
From: Arjan van de Ven @ 2008-10-20 21:52 UTC (permalink / raw)
To: Dave Airlie
Cc: Andrew Morton, Brandon Philips, zhenyu.z.wang, jbarnes,
linux-kernel
On Tue, 21 Oct 2008 07:44:46 +1000
Dave Airlie <airlied@redhat.com> wrote:
> > + printk(KERN_WARNING PFX
> > > + "Warning: G33 chipset
> > > with 1MB"
> > > + " allocated. Older X.org
> > > Intel drivers"
> > > + " will not work.\n");
> > > + WARN_ON(1);
> > > + }
> > > gtt_entries = MB(1) - KB(size);
> > > break;
> > > case I855_GMCH_GMS_STOLEN_4M:
> >
> > Is the bug which this patch addresses present in the 2.6.27 kernel?
> >
>
> I've been a bit wary about this patch, but on re-review I suppose it
> should be fine. We'll just get WARN_ONs in places we don't really want
> them, and I'm sure Arjan will come complaining about them from
> kerneloops.
>
the patch is not so nice for this;
the printk+WARN_ON() really should be using WARN() instead; that also
allows me to filter these guys out easily if needed.
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case
2008-10-20 21:52 ` Arjan van de Ven
@ 2008-10-29 4:41 ` Brandon Philips
2008-10-29 5:22 ` [PATCH v2] " Brandon Philips
1 sibling, 0 replies; 7+ messages in thread
From: Brandon Philips @ 2008-10-29 4:41 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Dave Airlie, Andrew Morton, zhenyu.z.wang, jbarnes, linux-kernel
On 14:52 Mon 20 Oct 2008, Arjan van de Ven wrote:
> On Tue, 21 Oct 2008 07:44:46 +1000
> Dave Airlie <airlied@redhat.com> wrote:
> > > + printk(KERN_WARNING PFX
> > > > + "Warning: G33 chipset
> > > > with 1MB"
> > > > + " allocated. Older X.org
> > > > Intel drivers"
> > > > + " will not work.\n");
> > > > + WARN_ON(1);
> > > > + }
> > > > gtt_entries = MB(1) - KB(size);
> > > > break;
> > > > case I855_GMCH_GMS_STOLEN_4M:
> > >
> > > Is the bug which this patch addresses present in the 2.6.27 kernel?
> > >
> >
> > I've been a bit wary about this patch, but on re-review I suppose it
> > should be fine. We'll just get WARN_ONs in places we don't really want
> > them, and I'm sure Arjan will come complaining about them from
> > kerneloops.
I figured it was better to be noisy than to have people complaining
about X.org being broken.
Should it be something other than a WARN?
> the patch is not so nice for this;
> the printk+WARN_ON() really should be using WARN() instead; that also
> allows me to filter these guys out easily if needed.
Ok. I will send a new patch in a moment using WARN().
Thanks,
Brandon
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] intel-agp: Avoid oops for G33 on 1MB stolen case
2008-10-20 21:52 ` Arjan van de Ven
2008-10-29 4:41 ` Brandon Philips
@ 2008-10-29 5:22 ` Brandon Philips
1 sibling, 0 replies; 7+ messages in thread
From: Brandon Philips @ 2008-10-29 5:22 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Dave Airlie, Andrew Morton, zhenyu.z.wang, jbarnes, linux-kernel
This is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was
reverted because it broke older X.org driver. This patch only fixes
the 1MB stolen case since it causes an oops.
Xorg will not work without the accompanying patch[1] but avoiding an
oops and making it possible to work with patched xorg driver is
reasonable.
[1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
Explanation of the oops:
> static void intel_i830_init_gtt_entries(void)
...
> } else if (IS_G33) {
> /* G33's GTT size defined in gmch_ctrl */
> switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
> case G33_PGETBL_SIZE_1M:
> size = 1024;
> break;
...
> size += 4;
size = 1028
Then since we have the BIOS setting 1MB for the device in the GMCH
control we get to here:
> } else {
> switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> case I855_GMCH_GMS_STOLEN_1M:
> gtt_entries = MB(1) - KB(size);
> break;
MB(1) = 1 * 1024 * 1024
KB(1028) = 1028 * 1024
MB(1) - KB(1028) = -4096
> gtt_entries /= KB(4);
> intel_private.gtt_entries = gtt_entries;
We end up with -1 in gtt_entries.
This leads to intel_i915_configure reading/writing to areas outside of
mapped memory and the oops.
Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
Signed-off-by: Brandon Philips <bphilips@suse.de>
---
drivers/char/agp/intel-agp.c | 7 +++++++
1 file changed, 7 insertions(+)
Index: linux-2.6/drivers/char/agp/intel-agp.c
===================================================================
--- linux-2.6.orig/drivers/char/agp/intel-agp.c
+++ linux-2.6/drivers/char/agp/intel-agp.c
@@ -561,6 +561,13 @@ static void intel_i830_init_gtt_entries(
} else {
switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
case I855_GMCH_GMS_STOLEN_1M:
+ if (IS_G33) {
+ size = 0;
+ WARN(1, KERN_WARNING
+ "Warning: G33 chip with 1MB allocated."
+ " Older X.org Intel drivers will not"
+ " work.\n");
+ }
gtt_entries = MB(1) - KB(size);
break;
case I855_GMCH_GMS_STOLEN_4M:
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case
2008-10-20 21:21 ` Andrew Morton
2008-10-20 21:44 ` Dave Airlie
@ 2008-10-29 5:23 ` Brandon Philips
1 sibling, 0 replies; 7+ messages in thread
From: Brandon Philips @ 2008-10-29 5:23 UTC (permalink / raw)
To: Andrew Morton; +Cc: airlied, zhenyu.z.wang, jbarnes, linux-kernel
On 14:21 Mon 20 Oct 2008, Andrew Morton wrote:
> On Tue, 14 Oct 2008 15:58:17 -0700
> Brandon Philips <bphilips@suse.de> wrote:
>
> > This is similar to f443675affe3f16dd428e46f0f7fd3f4d703eeab which was
> > reverted because it broke older X.org driver. This patch only fixes
> > the 1MB stolen case since it causes an oops.
> >
> > Xorg will not work without the accompanying patch[1] but avoiding an
> > oops and making it possible to work with patched xorg driver is
> > reasonable.
> >
> > [1] http://ifup.org/~philips/review/xf86-video-intel-G33-1mb.patch
> >
> > Explanation of the oops:
> >
> > > static void intel_i830_init_gtt_entries(void)
> > ...
> > > } else if (IS_G33) {
> > > /* G33's GTT size defined in gmch_ctrl */
> > > switch (gmch_ctrl & G33_PGETBL_SIZE_MASK) {
> > > case G33_PGETBL_SIZE_1M:
> > > size = 1024;
> > > break;
> > ...
> > > size += 4;
> >
> > size = 1028
> >
> > Then since we have the BIOS setting 1MB for the device in the GMCH
> > control we get to here:
> >
> > > } else {
> > > switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> > > case I855_GMCH_GMS_STOLEN_1M:
> > > gtt_entries = MB(1) - KB(size);
> > > break;
> >
> > MB(1) = 1 * 1024 * 1024
> > KB(1028) = 1028 * 1024
> >
> > MB(1) - KB(1028) = -4096
> >
> > > gtt_entries /= KB(4);
> > > intel_private.gtt_entries = gtt_entries;
> >
> > We end up with -1 in gtt_entries.
> >
> > This leads to intel_i915_configure reading/writing to areas outside of
> > mapped memory and the oops.
> >
> > Bugzilla: https://bugzilla.novell.com/show_bug.cgi?id=391261
> >
> > Signed-off-by: Brandon Philips <bphilips@suse.de>
> >
> > ---
> > drivers/char/agp/intel-agp.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > Index: linux-2.6/drivers/char/agp/intel-agp.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/char/agp/intel-agp.c
> > +++ linux-2.6/drivers/char/agp/intel-agp.c
> > @@ -559,6 +559,14 @@ static void intel_i830_init_gtt_entries(
> > } else {
> > switch (gmch_ctrl & I855_GMCH_GMS_MASK) {
> > case I855_GMCH_GMS_STOLEN_1M:
> > + if (IS_G33) {
> > + size = 0;
> > + printk(KERN_WARNING PFX
> > + "Warning: G33 chipset with 1MB"
> > + " allocated. Older X.org Intel drivers"
> > + " will not work.\n");
> > + WARN_ON(1);
> > + }
> > gtt_entries = MB(1) - KB(size);
> > break;
> > case I855_GMCH_GMS_STOLEN_4M:
>
> Is the bug which this patch addresses present in the 2.6.27 kernel?
Yes, ever since 3a4986955c0d9806e96a9d738ef7c40cb0cdaba3
Thanks,
Brandon
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-10-29 5:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-14 22:58 [PATCH] intel-agp: Avoid oops for G33 on 1MB stolen case Brandon Philips
2008-10-20 21:21 ` Andrew Morton
2008-10-20 21:44 ` Dave Airlie
2008-10-20 21:52 ` Arjan van de Ven
2008-10-29 4:41 ` Brandon Philips
2008-10-29 5:22 ` [PATCH v2] " Brandon Philips
2008-10-29 5:23 ` [PATCH] " Brandon Philips
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox