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
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(-)
diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
index 88c1ebe..156cd5d 100644
--- a/arch/x86/include/asm/gart.h
+++ b/arch/x86/include/asm/gart.h
@@ -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);
}
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 5c66a2f..3d2661c 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -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;
--
1.7.1
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(-)
diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
index 43085bf..88c1ebe 100644
--- a/arch/x86/include/asm/gart.h
+++ b/arch/x86/include/asm/gart.h
@@ -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)
--
1.7.1
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(-)
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 86d1ad4..5c66a2f 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -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);
--
1.7.1
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(-)
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 82ada01..b117efd 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -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;
--
1.7.1
On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> 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
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> 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
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Mon, Apr 18, 2011 at 10:46:24AM -0400, H. Peter Anvin wrote:
> On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> > 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
--
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
On 04/18/2011 07:46 AM, H. Peter Anvin wrote:
> On 04/18/2011 06:45 AM, Joerg Roedel wrote:
>> 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
On Mon, Apr 18, 2011 at 10:45:19AM -0400, H. Peter Anvin wrote:
> On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> > 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.
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
--
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
On Mon, Apr 18, 2011 at 10:54:17AM -0400, H. Peter Anvin wrote:
> On 04/18/2011 07:46 AM, H. Peter Anvin wrote:
> > On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> >> 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
--
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
* Roedel, Joerg <[email protected]> wrote:
> On Mon, Apr 18, 2011 at 10:54:17AM -0400, H. Peter Anvin wrote:
> > On 04/18/2011 07:46 AM, H. Peter Anvin wrote:
> > > On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> > >> 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.
Does the file want to be renamed to pci-amd_gart_64.c or so?
Thanks,
Ingo
On 04/18/2011 07:56 AM, Roedel, Joerg wrote:
> On Mon, Apr 18, 2011 at 10:45:19AM -0400, H. Peter Anvin wrote:
>> On 04/18/2011 06:45 AM, Joerg Roedel wrote:
>>> 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.
>
> 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
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On 04/18/2011 07:52 AM, Roedel, Joerg wrote:
>>
>> 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
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
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]
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/include/asm/gart.h | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
index 43085bf..88c1ebe 100644
--- a/arch/x86/include/asm/gart.h
+++ b/arch/x86/include/asm/gart.h
@@ -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)
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]
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(-)
diff --git a/arch/x86/include/asm/gart.h b/arch/x86/include/asm/gart.h
index 88c1ebe..156cd5d 100644
--- a/arch/x86/include/asm/gart.h
+++ b/arch/x86/include/asm/gart.h
@@ -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);
}
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 86d1ad4..73fb469 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -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;
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]
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/pci-gart_64.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 82ada01..b117efd 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -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;
On Mon, Apr 18, 2011 at 08:51:50AM -0700, H. Peter Anvin wrote:
> On 04/18/2011 07:56 AM, Roedel, Joerg wrote:
> > On Mon, Apr 18, 2011 at 10:45:19AM -0400, H. Peter Anvin wrote:
> >> On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> >>> 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.
> >
> > 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.
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
On 04/18/2011 10:36 AM, Joerg Roedel wrote:
>>
>> 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
On Mon, Apr 18, 2011 at 10:39:05AM -0700, H. Peter Anvin wrote:
> 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
On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> 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.
> 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
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]
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/aperture_64.c | 34 +++++++++++++++++++---------------
1 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 73fb469..3d2661c 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -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);
On Mon, Apr 18, 2011 at 10:39:05AM -0700, H. Peter Anvin wrote:
> 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
On 04/18/2011 12:06 PM, Joerg Roedel wrote:
> On Mon, Apr 18, 2011 at 10:39:05AM -0700, H. Peter Anvin wrote:
>> 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.
>
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
On Mon, Apr 18, 2011 at 11:50:33AM -0400, Ingo Molnar wrote:
>
> * Roedel, Joerg <[email protected]> wrote:
>
> > On Mon, Apr 18, 2011 at 10:54:17AM -0400, H. Peter Anvin wrote:
> > > On 04/18/2011 07:46 AM, H. Peter Anvin wrote:
> > > > On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> > > >> 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.
>
> 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
--
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
* Roedel, Joerg <[email protected]> wrote:
> On Mon, Apr 18, 2011 at 11:50:33AM -0400, Ingo Molnar wrote:
> >
> > * Roedel, Joerg <[email protected]> wrote:
> >
> > > On Mon, Apr 18, 2011 at 10:54:17AM -0400, H. Peter Anvin wrote:
> > > > On 04/18/2011 07:46 AM, H. Peter Anvin wrote:
> > > > > On 04/18/2011 06:45 AM, Joerg Roedel wrote:
> > > > >> 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.
> >
> > 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
On Tue, Apr 19, 2011 at 03:25:20AM -0400, Ingo Molnar wrote:
> * Roedel, Joerg <[email protected]> wrote:
> > Hmm, that seems long. How about just amd64_gart.c?
>
> Sure.
Okay, here it is.
>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%)
diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
index 092e596..6512fde 100644
--- a/Documentation/x86/x86_64/boot-options.txt
+++ b/Documentation/x86/x86_64/boot-options.txt
@@ -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
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 7338ef2..2c49bb2 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/amd64_gart.c
similarity index 100%
rename from arch/x86/kernel/pci-gart_64.c
rename to arch/x86/kernel/amd64_gart.c
--
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
On 04/19/2011 12:25 AM, Ingo Molnar wrote:
>>>>
>>>> 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
On Tue, Apr 19, 2011 at 09:43:54AM -0400, H. Peter Anvin wrote:
> On 04/19/2011 12:25 AM, Ingo Molnar wrote:
> >>>>
> >>>> 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
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
--
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
On 04/19/2011 07:29 AM, Roedel, Joerg wrote:
>>
>> 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
On Wed, Apr 20, 2011 at 01:38:28PM -0400, H. Peter Anvin wrote:
> On 04/19/2011 07:29 AM, Roedel, Joerg wrote:
> > 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.
>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%)
diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
index 092e596..c54b4f5 100644
--- a/Documentation/x86/x86_64/boot-options.txt
+++ b/Documentation/x86/x86_64/boot-options.txt
@@ -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
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 7338ef2..97ebf82e 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/amd_gart_64.c
similarity index 100%
rename from arch/x86/kernel/pci-gart_64.c
rename to arch/x86/kernel/amd_gart_64.c
--
1.7.4.1