Skip to contentSkip to conversation

gart cleanups and fixes

PATCH In review Joerg Roedel 6 participants 31 messages Apr 18, 2011 – May 10, 2011

Subsystems (2)
Documentation x86
lore
Share & tools
lore

Joerg Roedel

author
#0 lore raw

cover gart cleanups and fixes

Hi,

here is a patch-set to clean things up and fix certain issues with the GART driver in x86-linux. The first patch changes the alignment requirements for the aperture to be aper_size instead of hard-coded 512MB as discussed in another thread.
Patch two is preperation for patch three which makes sure that the bit to disable cache probes on gart table walks is always set, even when the BIOS sets up the aperture correctly and Linux does not remap then. The last patch adds a check for a GART hardware constraint. The GART can only map addresses in the first 1TB of physical memory. This is now checked with the last patch.

Regards,

Joerg

Diffstat:

 arch/x86/include/asm/gart.h   |   24 ++++++++++++------------
 arch/x86/kernel/aperture_64.c |   36 ++++++++++++++++++++----------------
 arch/x86/kernel/pci-gart_64.c |    9 ++++++++-
 3 files changed, 40 insertions(+), 29 deletions(-)

Shortlog:

Joerg Roedel (4):
x86, gart: Don't enforce GART aperture lower-bound by alignment x86, gart: Convert spaces to tabs in enable_gart_translation x86, gart: Set DISTLBWALKPRB bit always
x86, gart: Make sure GART does not map physmem above 1TB

author
#1 lore raw

PATCH 1/4 x86, gart: Don't enforce GART aperture lower-bound by alignment

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB. This is enforced by using 512MB as the lower-bound address in the allocation range.

Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
---
 arch/x86/kernel/aperture_64.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)
arch/x86/kernel/aperture_64.c +19 -15
index 86d1ad4..5c66a2f 100644
@@ -30,6 +30,22 @@
 #include <asm/amd_nb.h>
 #include <asm/x86_init.h>
 
+/*
+ * Using 512M as goal, in case kexec will load kernel_big
+ * that will do the on-position decompress, and could overlap with
+ * with the gart aperture that is used.
+ * Sequence:
+ * kernel_small
+ * ==> kexec (with kdump trigger path or gart still enabled)
+ * ==> kernel_small (gart area become e820_reserved)
+ * ==> kexec (with kdump trigger path or gart still enabled)
+ * ==> kerne_big (uncompressed size will be big than 64M or 128M)
+ * So don't use 512M below as gart iommu, leave the space for kernel
+ * code for safe.
+ */
+#define GART_MIN_ADDR	(512ULL << 20)
+#define GART_MAX_ADDR	(1ULL   << 32)
+
 int gart_iommu_aperture;
 int gart_iommu_aperture_disabled __initdata;
 int gart_iommu_aperture_allowed __initdata;

@@ -70,21 +86,9 @@ static u32 __init allocate_aperture(void)
 	 * memory. Unfortunately we cannot move it up because that would
 	 * make the IOMMU useless.
 	 */
-	/*
-	 * using 512M as goal, in case kexec will load kernel_big
-	 * that will do the on position decompress, and  could overlap with
-	 * that position with gart that is used.
-	 * sequende:
-	 * kernel_small
-	 * ==> kexec (with kdump trigger path or previous doesn't shutdown gart)
-	 * ==> kernel_small(gart area become e820_reserved)
-	 * ==> kexec (with kdump trigger path or previous doesn't shutdown gart)
-	 * ==> kerne_big (uncompressed size will be big than 64M or 128M)
-	 * so don't use 512M below as gart iommu, leave the space for kernel
-	 * code for safe
-	 */
-	addr = memblock_find_in_range(0, 1ULL<<32, aper_size, 512ULL<<20);
-	if (addr == MEMBLOCK_ERROR || addr + aper_size > 0xffffffff) {
+	addr = memblock_find_in_range(GART_MIN_ADDR, GART_MAX_ADDR,
+				      aper_size, aper_size);
+	if (addr == MEMBLOCK_ERROR || addr + aper_size > GART_MAX_ADDR) {
 		printk(KERN_ERR
 			"Cannot allocate aperture memory hole (%lx,%uK)\n",
 				addr, aper_size>>10);
Signature · 1 line
1.7.1
reviewer
#2 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 8 lines · Joerg Roedel

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB. This is enforced by using 512MB as the lower-bound address in the allocation range.

Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>

Better implementation of the existing bounds, yes, but I think the algorithm is still wrong. Specifically, 512 MiB seems to have been the maximum address of the kernel at some point, but that is historic at this point, at least on 64 bits.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#3 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 7 lines · H. Peter Anvin
Quoted 6 lines · Joerg Roedel

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB. This is enforced by using 512MB as the lower-bound address in the allocation range.

Quoted 7 lines
Quoted 2 lines
Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>

Better implementation of the existing bounds, yes, but I think the algorithm is still wrong. Specifically, 512 MiB seems to have been the maximum address of the kernel at some point, but that is historic at this point, at least on 64 bits.

I am fine with a smaller lower-bound, but I am not sure what a better choice is. The comment about kexec seems to be valid. It shouldn't matter for kdump because in this case the memory is allocated independently and the kdump kernel will only use this part, but for other kexec uses it is a bit harder. Probably any number we choose as a lower bound is an arbitrary choice at some point. But I am open for suggestions/corrections to this.

Regards,

Joerg

Signature · 5 lines
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
reviewer
#4 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 16 lines · Joerg
Quoted 7 lines · H. Peter Anvin
Quoted 6 lines · Joerg Roedel

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB. This is enforced by using 512MB as the lower-bound address in the allocation range.

Quoted 7 lines
Quoted 2 lines
Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>

Better implementation of the existing bounds, yes, but I think the algorithm is still wrong. Specifically, 512 MiB seems to have been the maximum address of the kernel at some point, but that is historic at this point, at least on 64 bits.

Quoted 8 lines

I am fine with a smaller lower-bound, but I am not sure what a better choice is. The comment about kexec seems to be valid. It shouldn't matter for kdump because in this case the memory is allocated independently and the kdump kernel will only use this part, but for other kexec uses it is a bit harder. Probably any number we choose as a lower bound is an arbitrary choice at some point. But I am open for suggestions/corrections to this.

The right thing to do for in-place kexec it to turn it off, not rely on any specific magic addresses. We have had this problem with a number of drivers in the context of kexec.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#5 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 26 lines · H. Peter Anvin
Quoted 16 lines · Joerg
Quoted 7 lines · H. Peter Anvin
Quoted 6 lines · Joerg Roedel

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB. This is enforced by using 512MB as the lower-bound address in the allocation range.

Quoted 7 lines
Quoted 2 lines
Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>

Better implementation of the existing bounds, yes, but I think the algorithm is still wrong. Specifically, 512 MiB seems to have been the maximum address of the kernel at some point, but that is historic at this point, at least on 64 bits.

Quoted 8 lines

I am fine with a smaller lower-bound, but I am not sure what a better choice is. The comment about kexec seems to be valid. It shouldn't matter for kdump because in this case the memory is allocated independently and the kdump kernel will only use this part, but for other kexec uses it is a bit harder. Probably any number we choose as a lower bound is an arbitrary choice at some point. But I am open for suggestions/corrections to this.

Quoted 3 lines

The right thing to do for in-place kexec it to turn it off, not rely on any specific magic addresses. We have had this problem with a number of drivers in the context of kexec.

Ok, so changing the start-address to zero seems the best thing to do. This way the GART will working again on machines with less than 512MB of RAM (and a BIOS that does not initialize the GART as we want it).

Regards,

Joerg

reviewer
#6 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 9 lines · Joerg Roedel
Quoted 4 lines

The right thing to do for in-place kexec it to turn it off, not rely on any specific magic addresses. We have had this problem with a number of drivers in the context of kexec.

Ok, so changing the start-address to zero seems the best thing to do. This way the GART will working again on machines with less than 512MB of RAM (and a BIOS that does not initialize the GART as we want it).

I'm going to apply your patch as-is, based on the notion that this is a change of mechanism, and a change of policy is a separate thing. However, we should address the shutdown of the GART first, and then we can move the start address to zero (again, two separate patches.)

-hpa

#7 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 4 lines · H. Peter Anvin

I'm going to apply your patch as-is, based on the notion that this is a change of mechanism, and a change of policy is a separate thing. However, we should address the shutdown of the GART first, and then we can move the start address to zero (again, two separate patches.)

Right, that sounds like a better way to go. I'll implement that soon, thanks.

Joerg

#8 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

In reply to H. Peter Anvin
Quoted 4 lines · H. Peter Anvin

I'm going to apply your patch as-is, based on the notion that this is a change of mechanism, and a change of policy is a separate thing. However, we should address the shutdown of the GART first, and then we can move the start address to zero (again, two separate patches.)

Hmm, thinking again about this, setting the lower-bound to 0 may break kdump setups. The kdump kernel is then able to allocate an aperture too and that may use half of the memory from the kdump-system so that the dump-process triggers OOM.
As it is now, the GART is automatically disabled when the BIOS doesn't setup it correctly and the machine has not more than 512MB of memory. Either we keep it as is or, with the changes suggested above, we handle it like soft-iotlb and don't use gart for dma-api at all when max_pfn is smaller than 4G.

Joerg

reviewer
#9 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

Quoted 6 lines · Joerg Roedel
Quoted 4 lines · H. Peter Anvin

I'm going to apply your patch as-is, based on the notion that this is a change of mechanism, and a change of policy is a separate thing. However, we should address the shutdown of the GART first, and then we can move the start address to zero (again, two separate patches.)

Quoted 10 lines

Hmm, thinking again about this, setting the lower-bound to 0 may break kdump setups. The kdump kernel is then able to allocate an aperture too and that may use half of the memory from the kdump-system so that the dump-process triggers OOM.
As it is now, the GART is automatically disabled when the BIOS doesn't setup it correctly and the machine has not more than 512MB of memory. Either we keep it as is or, with the changes suggested above, we handle it like soft-iotlb and don't use gart for dma-api at all when max_pfn is smaller than 4G.

OK, kdump is no longer limited to 512 MiB and so that number is spurious in the context of kdump. If that is broken it is already broken. Thus the 512 MiB number already is looking spurious.

-hpa

Acked-by on v1 1/4
#10 lore raw

Re: [PATCH 1/4] x86, gart: Don't enforce GART aperture lower-bound by alignment

In reply to Joerg Roedel
Quoted 4 lines · Joerg Roedel

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB.

Now, memblock is used instead of bootmem, so we don't need big alignment anymore.

Quoted 5 lines

This is enforced by using 512MB
as the lower-bound address in the allocation range.

Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
Acked-by Yinghai Lu <[email protected]>

Thanks

Yinghai

author
#11 lore raw

[tip:x86/gart] x86, gart: Don't enforce GART aperture lower-bound by alignment

In reply to Joerg Roedel
Commit-ID:  c387aa3a1a910ce00b86f3a85082d24f144db256
Gitweb:     http://git.kernel.org/tip/c387aa3a1a910ce00b86f3a85082d24f144db256
Author:     Joerg Roedel <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:45:43 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 10:31:38 -0700

x86, gart: Don't enforce GART aperture lower-bound by alignment

This patch changes the allocation of the GART aperture to enforce only natural alignment instead of aligning it on 512MB. This big alignment was used to force the GART aperture to be over 512MB. This is enforced by using 512MB as the lower-bound address in the allocation range.

[ hpa: The actual number 512 MiB needs to be revisited, too. ]

Cc Yinghai Lu <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by H. Peter Anvin <[email protected]>
---
 arch/x86/kernel/aperture_64.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)
arch/x86/kernel/aperture_64.c +19 -15
index 73fb469..3d2661c 100644
@@ -30,6 +30,22 @@
 #include <asm/amd_nb.h>
 #include <asm/x86_init.h>
 
+/*
+ * Using 512M as goal, in case kexec will load kernel_big
+ * that will do the on-position decompress, and could overlap with
+ * with the gart aperture that is used.
+ * Sequence:
+ * kernel_small
+ * ==> kexec (with kdump trigger path or gart still enabled)
+ * ==> kernel_small (gart area become e820_reserved)
+ * ==> kexec (with kdump trigger path or gart still enabled)
+ * ==> kerne_big (uncompressed size will be big than 64M or 128M)
+ * So don't use 512M below as gart iommu, leave the space for kernel
+ * code for safe.
+ */
+#define GART_MIN_ADDR	(512ULL << 20)
+#define GART_MAX_ADDR	(1ULL   << 32)
+
 int gart_iommu_aperture;
 int gart_iommu_aperture_disabled __initdata;
 int gart_iommu_aperture_allowed __initdata;

@@ -70,21 +86,9 @@ static u32 __init allocate_aperture(void)
 	 * memory. Unfortunately we cannot move it up because that would
 	 * make the IOMMU useless.
 	 */
-	/*
-	 * using 512M as goal, in case kexec will load kernel_big
-	 * that will do the on position decompress, and  could overlap with
-	 * that position with gart that is used.
-	 * sequende:
-	 * kernel_small
-	 * ==> kexec (with kdump trigger path or previous doesn't shutdown gart)
-	 * ==> kernel_small(gart area become e820_reserved)
-	 * ==> kexec (with kdump trigger path or previous doesn't shutdown gart)
-	 * ==> kerne_big (uncompressed size will be big than 64M or 128M)
-	 * so don't use 512M below as gart iommu, leave the space for kernel
-	 * code for safe
-	 */
-	addr = memblock_find_in_range(0, 1ULL<<32, aper_size, 512ULL<<20);
-	if (addr == MEMBLOCK_ERROR || addr + aper_size > 0xffffffff) {
+	addr = memblock_find_in_range(GART_MIN_ADDR, GART_MAX_ADDR,
+				      aper_size, aper_size);
+	if (addr == MEMBLOCK_ERROR || addr + aper_size > GART_MAX_ADDR) {
 		printk(KERN_ERR
 			"Cannot allocate aperture memory hole (%lx,%uK)\n",
 				addr, aper_size>>10);
author
#12 lore raw

PATCH 2/4 x86, gart: Convert spaces to tabs in enable_gart_translation

In reply to Joerg Roedel

Probably by copy&paste this function was indented by spaces. Convert this to tabs.

Signed-off-by Joerg Roedel <[email protected]>
---
 arch/x86/include/asm/gart.h |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)
arch/x86/include/asm/gart.h +11 -11
index 43085bf..88c1ebe 100644
@@ -75,17 +75,17 @@ static inline void enable_gart_translation(struct pci_dev *dev, u64 addr)
 {
 	u32 tmp, ctl;
 
-        /* address of the mappings table */
-        addr >>= 12;
-        tmp = (u32) addr<<4;
-        tmp &= ~0xf;
-        pci_write_config_dword(dev, AMD64_GARTTABLEBASE, tmp);
-
-        /* Enable GART translation for this hammer. */
-        pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &ctl);
-        ctl |= GARTEN;
-        ctl &= ~(DISGARTCPU | DISGARTIO);
-        pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
+	/* address of the mappings table */
+	addr >>= 12;
+	tmp = (u32) addr<<4;
+	tmp &= ~0xf;
+	pci_write_config_dword(dev, AMD64_GARTTABLEBASE, tmp);
+
+	/* Enable GART translation for this hammer. */
+	pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &ctl);
+	ctl |= GARTEN;
+	ctl &= ~(DISGARTCPU | DISGARTIO);
+	pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
 }
 
 static inline int aperture_valid(u64 aper_base, u32 aper_size, u32 min_size)
Signature · 1 line
1.7.1
author
#13 lore raw

[tip:x86/urgent] x86, gart: Convert spaces to tabs in enable_gart_translation

Commit-ID:  af289bfe15fc92ecfbf6d8312713815b33e452c0
Gitweb:     http://git.kernel.org/tip/af289bfe15fc92ecfbf6d8312713815b33e452c0
Author:     Joerg Roedel <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:45:44 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 09:26:48 -0700

x86, gart: Convert spaces to tabs in enable_gart_translation

Probably by copy&paste this function was indented by spaces. Convert this to tabs.

Signed-off-by Joerg Roedel <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by H. Peter Anvin <[email protected]>
---
 arch/x86/include/asm/gart.h |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)
arch/x86/include/asm/gart.h +11 -11
index 43085bf..88c1ebe 100644
@@ -75,17 +75,17 @@ static inline void enable_gart_translation(struct pci_dev *dev, u64 addr)
 {
 	u32 tmp, ctl;
 
-        /* address of the mappings table */
-        addr >>= 12;
-        tmp = (u32) addr<<4;
-        tmp &= ~0xf;
-        pci_write_config_dword(dev, AMD64_GARTTABLEBASE, tmp);
-
-        /* Enable GART translation for this hammer. */
-        pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &ctl);
-        ctl |= GARTEN;
-        ctl &= ~(DISGARTCPU | DISGARTIO);
-        pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
+	/* address of the mappings table */
+	addr >>= 12;
+	tmp = (u32) addr<<4;
+	tmp &= ~0xf;
+	pci_write_config_dword(dev, AMD64_GARTTABLEBASE, tmp);
+
+	/* Enable GART translation for this hammer. */
+	pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &ctl);
+	ctl |= GARTEN;
+	ctl &= ~(DISGARTCPU | DISGARTIO);
+	pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
 }
 
 static inline int aperture_valid(u64 aper_base, u32 aper_size, u32 min_size)
author
#14 lore raw

PATCH 3/4 x86, gart: Set DISTLBWALKPRB bit always

In reply to Joerg Roedel

The DISTLBWALKPRB bit must be set for the GART because the gatt table is mapped UC. But the current code does not set the bit at boot when the BIOS setup the aperture correctly. Fix that by setting this bit when enabling the GART instead of the other places.

Cc [email protected]
Cc Borislav Petkov <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
---
 arch/x86/include/asm/gart.h   |    4 ++--
 arch/x86/kernel/aperture_64.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/include/asm/gart.h +2 -2
index 88c1ebe..156cd5d 100644
@@ -66,7 +66,7 @@ static inline void gart_set_size_and_enable(struct pci_dev *dev, u32 order)
 	 * Don't enable translation but enable GART IO and CPU accesses.
 	 * Also, set DISTLBWALKPRB since GART tables memory is UC.
 	 */
-	ctl = DISTLBWALKPRB | order << 1;
+	ctl = order << 1;
 
 	pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
 }

@@ -83,7 +83,7 @@ static inline void enable_gart_translation(struct pci_dev *dev, u64 addr)
 
 	/* Enable GART translation for this hammer. */
 	pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &ctl);
-	ctl |= GARTEN;
+	ctl |= GARTEN | DISTLBWALKPRB;
 	ctl &= ~(DISGARTCPU | DISGARTIO);
 	pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
 }
arch/x86/kernel/aperture_64.c +1 -1
index 5c66a2f..3d2661c 100644
@@ -503,7 +503,7 @@ out:
 		 * Don't enable translation yet but enable GART IO and CPU
 		 * accesses and set DISTLBWALKPRB since GART table memory is UC.
 		 */
-		u32 ctl = DISTLBWALKPRB | aper_order << 1;
+		u32 ctl = aper_order << 1;
 
 		bus = amd_nb_bus_dev_ranges[i].bus;
 		dev_base = amd_nb_bus_dev_ranges[i].dev_base;
Signature · 1 line
1.7.1
author
#15 lore raw

[tip:x86/urgent] x86, gart: Set DISTLBWALKPRB bit always

Commit-ID:  c34151a742d84ae65db2088ea30495063f697fbe
Gitweb:     http://git.kernel.org/tip/c34151a742d84ae65db2088ea30495063f697fbe
Author:     Joerg Roedel <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:45:45 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 09:26:48 -0700

x86, gart: Set DISTLBWALKPRB bit always

The DISTLBWALKPRB bit must be set for the GART because the gatt table is mapped UC. But the current code does not set the bit at boot when the BIOS setup the aperture correctly. Fix that by setting this bit when enabling the GART instead of the other places.

Cc <[email protected]>
Cc Borislav Petkov <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by H. Peter Anvin <[email protected]>
---
 arch/x86/include/asm/gart.h   |    4 ++--
 arch/x86/kernel/aperture_64.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/include/asm/gart.h +2 -2
index 88c1ebe..156cd5d 100644
@@ -66,7 +66,7 @@ static inline void gart_set_size_and_enable(struct pci_dev *dev, u32 order)
 	 * Don't enable translation but enable GART IO and CPU accesses.
 	 * Also, set DISTLBWALKPRB since GART tables memory is UC.
 	 */
-	ctl = DISTLBWALKPRB | order << 1;
+	ctl = order << 1;
 
 	pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
 }

@@ -83,7 +83,7 @@ static inline void enable_gart_translation(struct pci_dev *dev, u64 addr)
 
 	/* Enable GART translation for this hammer. */
 	pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &ctl);
-	ctl |= GARTEN;
+	ctl |= GARTEN | DISTLBWALKPRB;
 	ctl &= ~(DISGARTCPU | DISGARTIO);
 	pci_write_config_dword(dev, AMD64_GARTAPERTURECTL, ctl);
 }
arch/x86/kernel/aperture_64.c +1 -1
index 86d1ad4..73fb469 100644
@@ -499,7 +499,7 @@ out:
 		 * Don't enable translation yet but enable GART IO and CPU
 		 * accesses and set DISTLBWALKPRB since GART table memory is UC.
 		 */
-		u32 ctl = DISTLBWALKPRB | aper_order << 1;
+		u32 ctl = aper_order << 1;
 
 		bus = amd_nb_bus_dev_ranges[i].bus;
 		dev_base = amd_nb_bus_dev_ranges[i].dev_base;
author
#16 lore raw

PATCH 4/4 x86, gart: Make sure GART does not map physmem above 1TB

In reply to Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Cc [email protected]
Signed-off-by Joerg Roedel <[email protected]>
---
 arch/x86/kernel/pci-gart_64.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
arch/x86/kernel/pci-gart_64.c +8 -1
index 82ada01..b117efd 100644
@@ -81,6 +81,9 @@ static u32 gart_unmapped_entry;
 #define AGPEXTERN
 #endif
 
+/* GART can only remap to physical addresses < 1TB */
+#define GART_MAX_PHYS_ADDR	(1ULL << 40)
+
 /* backdoor interface to AGP driver */
 AGPEXTERN int agp_memory_reserved;
 AGPEXTERN __u32 *agp_gatt_table;

@@ -212,9 +215,13 @@ static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
 				size_t size, int dir, unsigned long align_mask)
 {
 	unsigned long npages = iommu_num_pages(phys_mem, size, PAGE_SIZE);
-	unsigned long iommu_page = alloc_iommu(dev, npages, align_mask);
+	unsigned long iommu_page;
 	int i;
 
+	if (unlikely(phys_mem + size > GART_MAX_PHYS_ADDR))
+		return bad_dma_addr;
+
+	iommu_page = alloc_iommu(dev, npages, align_mask);
 	if (iommu_page == -1) {
 		if (!nonforced_iommu(dev, phys_mem, size))
 			return phys_mem;
Signature · 1 line
1.7.1
reviewer
#17 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Where does this limit come from?

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#18 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 8 lines · H. Peter Anvin
Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Where does this limit come from?

From the hardware. A GART PTE is only 32 bits in size so that it cannot

be expanded. The physical address bits 32-39 are mapped 11:4 in the PTE. This can not be architecturally expanded to more than 40 physical bits.

Regards,

Joerg

Signature · 5 lines
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
reviewer
#19 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 7 lines · Joerg
Quoted 2 lines

Where does this limit come from?

From the hardware. A GART PTE is only 32 bits in size so that it cannot be expanded. The physical address bits 32-39 are mapped 11:4 in the PTE. This can not be architecturally expanded to more than 40 physical bits.

AFAIK the GART format is implementation-specific (unless some standard crept up when I wasn't looking -- this happens from time to time); this file seems, despite its generic name, to be AMD K8 specific.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#20 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

In reply to H. Peter Anvin
Quoted 6 lines · H. Peter Anvin
Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Where does this limit come from?

Nevermind... despite the generic name of the file it looks to be specific to the K8 GART.

-hpa

author
#21 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 10 lines · H. Peter Anvin
Quoted 6 lines · H. Peter Anvin
Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Where does this limit come from?

Nevermind... despite the generic name of the file it looks to be specific to the K8 GART.

Yes, this file mostly contains the dma-ops implementation that uses the K8 gart plus some k8-gart specific initialization functions.

Joerg

Signature · 5 lines
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
#22 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 7 lines · Roedel, Joerg
Quoted 6 lines · H. Peter Anvin
Quoted 5 lines · H. Peter Anvin
Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Where does this limit come from?

Nevermind... despite the generic name of the file it looks to be specific to the K8 GART.

Yes, this file mostly contains the dma-ops implementation that uses the K8 gart plus some k8-gart specific initialization functions.

Does the file want to be renamed to pci-amd_gart_64.c or so?

Thanks,

Ingo

author
#23 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 12 lines · Ingo Molnar
Quoted 9 lines · Roedel, Joerg
Quoted 8 lines · H. Peter Anvin
Quoted 6 lines · H. Peter Anvin
Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Where does this limit come from?

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Nevermind... despite the generic name of the file it looks to be specific to the K8 GART.

Yes, this file mostly contains the dma-ops implementation that uses the K8 gart plus some k8-gart specific initialization functions.

Does the file want to be renamed to pci-amd_gart_64.c or so?

Hmm, that seems long. How about just amd64_gart.c?

Joerg

Signature · 5 lines
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
#24 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 16 lines · Roedel, Joerg
Quoted 15 lines · Ingo Molnar
Quoted 12 lines · Roedel, Joerg
Quoted 10 lines · H. Peter Anvin
Quoted 6 lines · H. Peter Anvin
Quoted 3 lines · Joerg Roedel

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Where does this limit come from?

Nevermind... despite the generic name of the file it looks to be specific to the K8 GART.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Yes, this file mostly contains the dma-ops implementation that uses the K8 gart plus some k8-gart specific initialization functions.

Does the file want to be renamed to pci-amd_gart_64.c or so?

Hmm, that seems long. How about just amd64_gart.c?

Sure.

Thanks,

Ingo

author
#25 lore raw

PATCH 1/1 x86, gart: Rename pci-gart_64.c to amd64_gart.c

Quoted 5 lines · Ingo Molnar
Quoted 1 line · Roedel, Joerg

Hmm, that seems long. How about just amd64_gart.c?

Sure.

Okay, here it is.

Quoted 1 line

From 35842cba238969f102c6b24c95325e5c09fa3fed Mon Sep 17 00:00:00 2001

From: Joerg Roedel <[email protected]>
Date: Tue, 19 Apr 2011 11:04:32 +0200
Subject: [PATCH] x86, gart: Rename pci-gart_64.c to amd64_gart.c

This file contains only code relevant for the AMD NB GART present in most 64 bit AMD processors. Rename the file so that its name represent that it is specific to AMD hardware.

Signed-off-by Joerg Roedel <[email protected]>
---
 Documentation/x86/x86_64/boot-options.txt       |    2 +-
 arch/x86/kernel/Makefile                        |    2 +-
 arch/x86/kernel/{pci-gart_64.c => amd64_gart.c} |    0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/kernel/{pci-gart_64.c => amd64_gart.c} (100%)
Documentation/x86/x86_64/boot-options.txt +1 -1
index 092e596..6512fde 100644
@@ -206,7 +206,7 @@ IOMMU (input/output memory management unit)
       (e.g. because you have < 3 GB memory).
       Kernel boot message: "PCI-DMA: Disabling IOMMU"
 
-   2. <arch/x86_64/kernel/pci-gart.c>: AMD GART based hardware IOMMU.
+   2. <arch/x86_64/kernel/amd64_gart.c>: AMD GART based hardware IOMMU.
       Kernel boot message: "PCI-DMA: using GART IOMMU"
 
    3. <arch/x86_64/kernel/pci-swiotlb.c> : Software IOMMU implementation. Used
arch/x86/kernel/Makefile +1 -1
index 7338ef2..2c49bb2 100644
@@ -117,7 +117,7 @@ obj-$(CONFIG_OF)			+= devicetree.o
 ifeq ($(CONFIG_X86_64),y)
 	obj-$(CONFIG_AUDIT)		+= audit_64.o
 
-	obj-$(CONFIG_GART_IOMMU)	+= pci-gart_64.o aperture_64.o
+	obj-$(CONFIG_GART_IOMMU)	+= amd64_gart.o aperture_64.o
 	obj-$(CONFIG_CALGARY_IOMMU)	+= pci-calgary_64.o tce_64.o
 	obj-$(CONFIG_AMD_IOMMU)		+= amd_iommu_init.o amd_iommu.o
arch/x86/kernel/amd64_gart.c renamed from arch/x86/kernel/pci-gart_64.c +0 -0
similarity index 100%
rename from arch/x86/kernel/pci-gart_64.c
rename to arch/x86/kernel/amd64_gart.c
Signature · 8 lines
1.7.1

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
#26 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

In reply to Ingo Molnar
Quoted 8 lines · Ingo Molnar
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

Yes, this file mostly contains the dma-ops implementation that uses the K8 gart plus some k8-gart specific initialization functions.

Does the file want to be renamed to pci-amd_gart_64.c or so?

Hmm, that seems long. How about just amd64_gart.c?

Arguably we call this the k8_nb in other places, so my suggestion would be k8_gart.c

-hpa

author
#27 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 5 lines · H. Peter Anvin
Quoted 4 lines · Ingo Molnar
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines

Yes, this file mostly contains the dma-ops implementation that uses the K8 gart plus some k8-gart specific initialization functions.

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

Does the file want to be renamed to pci-amd_gart_64.c or so?

Hmm, that seems long. How about just amd64_gart.c?

Arguably we call this the k8_nb in other places, so my suggestion would be k8_gart.c

The k8_nb was renamed to amd_nb recently. The NB is implemented in all 64 bit AMD processors and there are more today then K8. The k8_nb was historically chosen and misleading today.

Joerg

Signature · 5 lines
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
#28 lore raw

Re: [PATCH 4/4] x86, gart: Make sure GART does not map physmem above 1TB

Quoted 8 lines · Joerg
Quoted 3 lines

Arguably we call this the k8_nb in other places, so my suggestion would be k8_gart.c

The k8_nb was renamed to amd_nb recently. The NB is implemented in all 64 bit AMD processors and there are more today then K8. The k8_nb was historically chosen and misleading today.

OK, fair enough. However, unless it is actually used in 32-bit mode calling it amd_gart_64.c is probably better than amd64_gart.c since an "amd64" processor can also be run in 32-bit mode.

-hpa

author
#29 lore raw

PATCH 1/1 x86, gart: Rename pci-gart_64.c to amd_gart_64.c 1TB

Quoted 9 lines · H. Peter Anvin
Quoted 3 lines · Joerg

The k8_nb was renamed to amd_nb recently. The NB is implemented in all 64 bit AMD processors and there are more today then K8. The k8_nb was historically chosen and misleading today.

OK, fair enough. However, unless it is actually used in 32-bit mode calling it amd_gart_64.c is probably better than amd64_gart.c since an "amd64" processor can also be run in 32-bit mode.

Fine with me :) Here is the patch.

Quoted 1 line

From fffcda1183e93df84ad73ba7eb7782a5c354e2b3 Mon Sep 17 00:00:00 2001

From: Joerg Roedel <[email protected]>
Date: Tue, 10 May 2011 17:22:06 +0200
Subject: [PATCH] x86, gart: Rename pci-gart_64.c to amd_gart_64.c

This file only contains code relevant for the northbridge gart in AMD processors. This patch renames the file to represent this fact in the filename.

Signed-off-by Joerg Roedel <[email protected]>
---
 Documentation/x86/x86_64/boot-options.txt        |    2 +-
 arch/x86/kernel/Makefile                         |    2 +-
 arch/x86/kernel/{pci-gart_64.c => amd_gart_64.c} |    0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/kernel/{pci-gart_64.c => amd_gart_64.c} (100%)
Documentation/x86/x86_64/boot-options.txt +1 -1
index 092e596..c54b4f5 100644
@@ -206,7 +206,7 @@ IOMMU (input/output memory management unit)
       (e.g. because you have < 3 GB memory).
       Kernel boot message: "PCI-DMA: Disabling IOMMU"
 
-   2. <arch/x86_64/kernel/pci-gart.c>: AMD GART based hardware IOMMU.
+   2. <arch/x86/kernel/amd_gart_64.c>: AMD GART based hardware IOMMU.
       Kernel boot message: "PCI-DMA: using GART IOMMU"
 
    3. <arch/x86_64/kernel/pci-swiotlb.c> : Software IOMMU implementation. Used
arch/x86/kernel/Makefile +1 -1
index 7338ef2..97ebf82e 100644
@@ -117,7 +117,7 @@ obj-$(CONFIG_OF)			+= devicetree.o
 ifeq ($(CONFIG_X86_64),y)
 	obj-$(CONFIG_AUDIT)		+= audit_64.o
 
-	obj-$(CONFIG_GART_IOMMU)	+= pci-gart_64.o aperture_64.o
+	obj-$(CONFIG_GART_IOMMU)	+= amd_gart_64.o aperture_64.o
 	obj-$(CONFIG_CALGARY_IOMMU)	+= pci-calgary_64.o tce_64.o
 	obj-$(CONFIG_AMD_IOMMU)		+= amd_iommu_init.o amd_iommu.o
arch/x86/kernel/amd_gart_64.c renamed from arch/x86/kernel/pci-gart_64.c +0 -0
similarity index 100%
rename from arch/x86/kernel/pci-gart_64.c
rename to arch/x86/kernel/amd_gart_64.c
Signature · 1 line
1.7.4.1
author
#30 lore raw

[tip:x86/urgent] x86, gart: Make sure GART does not map physmem above 1TB

In reply to Joerg Roedel
Commit-ID:  665d3e2af83c8fbd149534db8f57d82fa6fa6753
Gitweb:     http://git.kernel.org/tip/665d3e2af83c8fbd149534db8f57d82fa6fa6753
Author:     Joerg Roedel <[email protected]>
AuthorDate: Mon, 18 Apr 2011 15:45:46 +0200
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 18 Apr 2011 09:26:49 -0700

x86, gart: Make sure GART does not map physmem above 1TB

The GART can only map physical memory below 1TB. Make sure the gart driver in the kernel does not try to map memory above 1TB.

Cc <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by H. Peter Anvin <[email protected]>
---
 arch/x86/kernel/pci-gart_64.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
arch/x86/kernel/pci-gart_64.c +8 -1
index 82ada01..b117efd 100644
@@ -81,6 +81,9 @@ static u32 gart_unmapped_entry;
 #define AGPEXTERN
 #endif
 
+/* GART can only remap to physical addresses < 1TB */
+#define GART_MAX_PHYS_ADDR	(1ULL << 40)
+
 /* backdoor interface to AGP driver */
 AGPEXTERN int agp_memory_reserved;
 AGPEXTERN __u32 *agp_gatt_table;

@@ -212,9 +215,13 @@ static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
 				size_t size, int dir, unsigned long align_mask)
 {
 	unsigned long npages = iommu_num_pages(phys_mem, size, PAGE_SIZE);
-	unsigned long iommu_page = alloc_iommu(dev, npages, align_mask);
+	unsigned long iommu_page;
 	int i;
 
+	if (unlikely(phys_mem + size > GART_MAX_PHYS_ADDR))
+		return bad_dma_addr;
+
+	iommu_page = alloc_iommu(dev, npages, align_mask);
 	if (iommu_page == -1) {
 		if (!nonforced_iommu(dev, phys_mem, size))
 			return phys_mem;