From: Dave Jones <davej@redhat.com>
Subject: [rhel5] AGP corruption fixes.
Date: Wed, 22 Nov 2006 20:50:26 -0500
Bugzilla: 218107
Message-Id: <20061123015026.GA29538@redhat.com>
Changelog: AGP corruption fixes.


The i965 AGP support in RHEL5 has a really nasty data corruption problem
that is solved by the two commits below.  It only shows up on systems
with >4GB of ram, or systems with 4GB + a memory hole that supports
hoisting the remainder above the 4GB barrier.

The long and short of it is that the AGP code can't deal with addresses
this high, and strange things happen when it 'accidentally' occurs.
(Linus found this completely by accident by noticing that a loop bzero'ing
 malloc'd memory started scribbling over his screen).

The fix below does partially cripple some GART implementations
(namely AMD64), which *could* support having a GATT above the 4GB
mark -- now it's always forced to below 4GB. It's unlikely that this
will ever be noticed as a problem, but will likely get a follow-on
improvement upstream post 2.6.19.

If we ever backport 965 support to RHEL4, we'll need the same change there.

Horrible, horrible bug.

		Dave


commit 7d915a38985d2826acbdc9dc9cca8a93e23e5278
tree 6954d28e1e835a9043c1f201c0791c390e528e7f
parent b42172fc7b569a0ef2b0fa38d71382969074c0e2
author Linus Torvalds <torvalds@woody.osdl.org> 1164217074 -0800
committer Linus Torvalds <torvalds@woody.osdl.org> 1164217074 -0800

[AGP] Fix intel 965 AGP memory mapping function

This introduces a i965-specific "mask_memory()" function that knows
about the extended physical addresses that the i965 supports.  This
allows us to correctly map in physical memory in the >4GB range into the
GTT.

Also simplify/clean-up the i965 case for the aperture sizing by just
returning the fixed 512kB size from "fetch_size()".  We don't really
care that not all of the aperture may be visible - the only thing that
cares about the aperture size is the Intel "stolen memory" calculation,
which depends on the fixed size.

Cc: Keith Packard <keithp@keithp.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Dave Jones <davej@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

 drivers/char/agp/intel-agp.c |   31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d1ede7d..aceece7 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -387,11 +387,7 @@ static void intel_i830_init_gtt_entries(
 	/* We obtain the size of the GTT, which is also stored (for some
 	 * reason) at the top of stolen memory. Then we add 4KB to that
 	 * for the video BIOS popup, which is also stored in there. */
-
-	if (IS_I965)
-		size = 512 + 4;
-	else
-		size = agp_bridge->driver->fetch_size() + 4;
+	size = agp_bridge->driver->fetch_size() + 4;
 
 	if (agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82830_HB ||
 	    agp_bridge->dev->device == PCI_DEVICE_ID_INTEL_82845G_HB) {
@@ -805,6 +801,26 @@ static int intel_i915_create_gatt_table(
 
 	return 0;
 }
+
+/*
+ * The i965 supports 36-bit physical addresses, but to keep
+ * the format of the GTT the same, the bits that don't fit
+ * in a 32-bit word are shifted down to bits 4..7.
+ *
+ * Gcc is smart enough to notice that "(addr >> 28) & 0xf0"
+ * is always zero on 32-bit architectures, so no need to make
+ * this conditional.
+ */
+static unsigned long intel_i965_mask_memory(struct agp_bridge_data *bridge,
+	unsigned long addr, int type)
+{
+	/* Shift high bits down */
+	addr |= (addr >> 28) & 0xf0;
+
+	/* Type checking must be done elsewhere */
+	return addr | bridge->driver->masks[type].mask;
+}
+
 static int intel_i965_fetch_size(void)
 {
        struct aper_size_info_fixed *values;
@@ -832,7 +848,8 @@ static int intel_i965_fetch_size(void)
 
        agp_bridge->previous_size = agp_bridge->current_size = (void *)(values + offset);
 
-       return values[offset].size;
+	/* The i965 GTT is always sized as if it had a 512kB aperture size */
+	return 512;
 }
 
 /* The intel i965 automatically initializes the agp aperture during POST.
@@ -1584,7 +1601,7 @@ static struct agp_bridge_driver intel_i9
        .fetch_size             = intel_i965_fetch_size,
        .cleanup                = intel_i915_cleanup,
        .tlb_flush              = intel_i810_tlbflush,
-       .mask_memory            = intel_i810_mask_memory,
+       .mask_memory            = intel_i965_mask_memory,
        .masks                  = intel_i810_masks,
        .agp_enable             = intel_i810_agp_enable,
        .cache_flush            = global_cache_flush,






commit 66c669baa7d70b8d135da67f36c8dba12cea71b8
tree 8839924101a47a08d9da70f9bfcd8abb4023e466
parent 7d915a38985d2826acbdc9dc9cca8a93e23e5278
author Linus Torvalds <torvalds@woody.osdl.org> 1164236129 -0800
committer Linus Torvalds <torvalds@woody.osdl.org> 1164236129 -0800

[AGP] Allocate AGP pages with GFP_DMA32 by default

Not all graphic page remappers support physical addresses over the 4GB
mark for remapping, so while some do (the AMD64 GART always did, and I
just fixed the i965 to do so properly), we're safest off just forcing
GFP_DMA32 allocations to make sure graphics pages get allocated in the
low 32-bit address space by default.

AGP sub-drivers that really care, and can do better, could just choose
to implement their own allocator (or we could add another "64-bit safe"
default allocator for their use), but quite frankly, you're not likely
to care in practice.

So for now, this trivial change means that we won't be allocating pages
that we can't map correctly by mistake on x86-64.

[ On traditional 32-bit x86, this could never happen, because GFP_KERNEL
  would never allocate any highmem memory anyway ]

Acked-by: Andi Kleen <ak@suse.de>
Acked-by: Dave Jones <davej@redhat.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

 drivers/char/agp/generic.c   |    2 +-
 drivers/char/agp/intel-agp.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index c392001..5ff457b 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -1054,7 +1054,7 @@ void *agp_generic_alloc_page(struct agp_
 {
 	struct page * page;
 
-	page = alloc_page(GFP_KERNEL);
+	page = alloc_page(GFP_KERNEL | GFP_DMA32);
 	if (page == NULL)
 		return NULL;
 
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index aceece7..555b3a8 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -169,7 +169,7 @@ static void *i8xx_alloc_pages(void)
 {
 	struct page * page;
 
-	page = alloc_pages(GFP_KERNEL, 2);
+	page = alloc_pages(GFP_KERNEL | GFP_DMA32, 2);
 	if (page == NULL)
 		return NULL;
 



-- 
http://www.codemonkey.org.uk