[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <n2tca2dc2821004271306nfc0c9c76o143b796e8af6ced8@mail.gmail.com>
Date: Tue, 27 Apr 2010 22:06:21 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Christian Kujau <lists@...dbynature.de>,
Zhenyu Wang <zhenyuw@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>, David.Woodhouse@...el.com,
dwmw2@...radead.org, eric@...olt.net, ben@...adent.org.uk,
gregkh@...e.de, Dave Airlie <airlied@...ux.ie>,
"Rafael J. Wysocki" <rjw@...k.pl>,
Maciej Rutecki <maciej.rutecki@...il.com>
Subject: Re: [PATCH] intel-agp.c: Fix crash when accessing nonexistent GTT
entries in i915
On Tue, Apr 27, 2010 at 7:57 PM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> On Thu, 25 Mar 2010 17:55:56 +0100
> Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:
>
>> On Wed, Mar 24, 2010 at 7:14 PM, Andrew Morton
>> <akpm@...ux-foundation.org> wrote:
>> > On Tue, 23 Mar 2010 12:40:05 +0100
>> > Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:
>> >
>> >> On Tue, Mar 23, 2010 at 5:14 AM, Christian Kujau <lists@...dbynature.de> wrote:
>> >> > On Mon, 22 Mar 2010 at 20:57, Andrew Morton wrote:
>> >> >> On Sun, 21 Mar 2010 16:30:20 +0100 Miguel Ojeda <miguel.ojeda.sandonis@...il.com> wrote:
>> >> >> > I bisected in order to find the commit 5877960869333e42ebeb733e8d9d5630ff96d350.
>> >> >
>> >> > I believe this[0] is fc61901373987ad61851ed001fe971f3ee8d96a3 upstream:
>> >>
>> >> Indeed. Also in
>> >>
>> >> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.32.y.git;a=commit;h=fc61901373987ad61851ed001fe971f3ee8d96a3
>> >
>> > Does reverting that patch from the current code fix the crash?
>>
>> Yes. In addition, applying the patch I provided also fixes it in current code.
>>
>
> Well great. A whole pile of new stuff has turned up in linux-next's
> drivers/char/agp/intel-agp.c. As far as I can tell none of it
> address the regression which you've reported and your patch no longer
> applies at all so I have to drop the patch.
>
> Perhaps "agp/intel: put back check that we have a driver for the
> bridge" fixes it, but it isn't tagged for -stable backporting.
I can try linux-next and see if it works again.
In addition, I have to test the regression with some newer X version
yet, as Zhenyu told me.
>
> Rafael, Maciej: if you're not already tracking this as a 2.6.32->2.6.33
> regression then please do so.
>
> David, can you please help us to get this sorted out in both 2.6.33.x
> and in mainline?
>
>
>
> From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
>
> Commit fc61901373987ad61851ed001fe971f3ee8d96a3 ("agp/intel-agp: Clear
> entire GTT on startup") (included since 2.6.32.4) crashes (locks up) the
> 82915G/GV/910GL Controller when intel-agp.c tries to access nonexistent
> GTT entries at:
>
> - for (i = intel_private.gtt_entries; i < current_size->num_entries; i++) {
> + for (i = intel_private.gtt_entries; i < intel_private.gtt_total_size; i++) {
>
> Rationale: I915 (gma900) has 128 MB of video memory (maximum), as per
> intel.com (
> http://www.intel.com/support/graphics/intel915g/sb/CS-012579.htm ) and
> lscpi:
>
> 00:02.0 VGA compatible controller: Intel Corporation 82915G/GV/910GL Integrated Graphics Controller (rev 04) (prog-if 00 [VGA controller])
> Subsystem: Intel Corporation Device 4147
> Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
> Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> Latency: 0
> Interrupt: pin A routed to IRQ 11
> Region 0: Memory at ff480000 (32-bit, non-prefetchable) [size=512K]
> Region 1: I/O ports at ec00 [size=8]
> Region 2: Memory at d8000000 (32-bit, prefetchable) [size=128M]
> Region 3: Memory at ff440000 (32-bit, non-prefetchable) [size=256K]
> Capabilities: <access denied>
>
> AFAIK, that implies that its gtt_total_size (in pages) should be 32K (as
> num_entries showed before the commit) instead of 64K.
>
> Note: The IS_I915 macro includes 945; however, only GMA900 (I915) had 128
> MB as the maximum AFAIK. Therefore, I divided the IS_I915 macro. I do
> not know about the "E7221" (please check).
>
> How to reproduce: Access kernel.org in iceweasel (Debian Lenny) and the X
> server will crash. Sometimes, the kernel freezes.
>
> The fix should be applied to stable series, as well as 2.6.33 and
> 2.6.34-rc1.
>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
> Cc: David Woodhouse <David.Woodhouse@...el.com>
> Cc: Eric Anholt <eric@...olt.net>
> Cc: Zhenyu Wang <zhenyuw@...ux.intel.com>
> Cc: Dave Airlie <airlied@...ux.ie>
> Cc: <stable@...nel.org>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> ---
>
> drivers/char/agp/intel-agp.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff -puN drivers/char/agp/intel-agp.c~intel-agpc-fix-crash-when-accessing-nonexistent-gtt-entries-in-i915 drivers/char/agp/intel-agp.c
> --- a/drivers/char/agp/intel-agp.c~intel-agpc-fix-crash-when-accessing-nonexistent-gtt-entries-in-i915
> +++ a/drivers/char/agp/intel-agp.c
> @@ -74,11 +74,11 @@ EXPORT_SYMBOL(intel_agp_enabled);
> #define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_HB 0x0104
> #define PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_IG 0x0106
>
> -/* cover 915 and 945 variants */
> #define IS_I915 (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_E7221_HB || \
> agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82915G_HB || \
> - agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82915GM_HB || \
> - agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945G_HB || \
> + agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82915GM_HB)
> +
> +#define IS_I945 (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945G_HB || \
> agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945GM_HB || \
> agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82945GME_HB)
>
> @@ -824,14 +824,14 @@ static void intel_i830_init_gtt_entries(
> break;
> case I915_GMCH_GMS_STOLEN_48M:
> /* Check it's really I915G */
> - if (IS_I915 || IS_I965 || IS_G33 || IS_G4X)
> + if (IS_I915 || IS_I945 || IS_I965 || IS_G33 || IS_G4X)
> gtt_entries = MB(48) - KB(size);
> else
> gtt_entries = 0;
> break;
> case I915_GMCH_GMS_STOLEN_64M:
> /* Check it's really I915G */
> - if (IS_I915 || IS_I965 || IS_G33 || IS_G4X)
> + if (IS_I915 || IS_I945 || IS_I965 || IS_G33 || IS_G4X)
> gtt_entries = MB(64) - KB(size);
> else
> gtt_entries = 0;
> @@ -1400,6 +1400,8 @@ static int intel_i915_create_gatt_table(
>
> if (IS_G33)
> gtt_map_size = 1024 * 1024; /* 1M on G33 */
> + else if (IS_I915)
> + gtt_map_size = 128 * 1024; /* 128K on I915 */
> intel_private.gtt = ioremap(temp2, gtt_map_size);
> if (!intel_private.gtt)
> return -ENOMEM;
> diff -puN /dev/null /dev/null
> _
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists