x86: some GART cleanups

PATCH In review Joerg Roedel 5 participants 15 messages Oct 23, 2007 – Oct 24, 2007

Subsystems (4)

x86 USB PCI Agpgart driver

lore

Share & tools

lore

Joerg Roedel

author

2007-10-23 17:42 UTC #0 lore raw

cover x86: some GART cleanups

This series of patches contains cleanups for the GART IOMMU implementation in Linux. It mostly renames symbols to make clear that they belong only to the GART and not to the implementations for other IOMMUs. It also makes some unnecessary exported functions static in the GART implementation.

Joerg Roedel

author

2007-10-23 17:43 UTC #1 lore raw

PATCH 1/4 x86 gart: rename iommu.h to gart.h

This patch renames the include file asm-x86/iommu.h to asm-x86/gart.h to make clear to which IOMMU implementation it belongs. The patch also adds "GART" to the Kconfig line.

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

---
 arch/x86/kernel/aperture_64.c       |    2 +-
 arch/x86/kernel/early-quirks.c      |    4 ++--
 arch/x86/kernel/pci-calgary_64.c    |    2 +-
 arch/x86/kernel/pci-dma_64.c        |    2 +-
 arch/x86/kernel/pci-gart_64.c       |    2 +-
 arch/x86/kernel/pci-nommu_64.c      |    2 +-
 arch/x86/kernel/pci-swiotlb_64.c    |    2 +-
 arch/x86/kernel/reboot_64.c         |    2 +-
 arch/x86_64/Kconfig                 |    2 +-
 drivers/pci/intel-iommu.c           |    2 +-
 include/asm-x86/{iommu.h => gart.h} |    4 ++--
 11 files changed, 13 insertions(+), 13 deletions(-)
 rename include/asm-x86/{iommu.h => gart.h} (92%)

arch/x86/kernel/aperture_64.c +1 -1

index 8f681ca..70c854f 100644
@@ -20,7 +20,7 @@
 #include <linux/ioport.h>
 #include <asm/e820.h>
 #include <asm/io.h>
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include <asm/pci-direct.h>
 #include <asm/dma.h>
 #include <asm/k8.h>

arch/x86/kernel/early-quirks.c +2 -2

index dc34acb..d6e6e83 100644
@@ -17,8 +17,8 @@
 #include <asm/io_apic.h>
 #include <asm/apic.h>
 
-#ifdef CONFIG_IOMMU
-#include <asm/iommu.h>
+#ifdef CONFIG_GART_IOMMU
+#include <asm/gart.h>
 #endif
 
 static void __init via_bugs(void)

arch/x86/kernel/pci-calgary_64.c +1 -1

index 1a20fe3..6bf1f71 100644
@@ -36,7 +36,7 @@
 #include <linux/pci.h>
 #include <linux/delay.h>
 #include <linux/scatterlist.h>
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include <asm/calgary.h>
 #include <asm/tce.h>
 #include <asm/pci-direct.h>

arch/x86/kernel/pci-dma_64.c +1 -1

index 393e272..730339f 100644
@@ -9,7 +9,7 @@
 #include <linux/module.h>
 #include <linux/dmar.h>
 #include <asm/io.h>
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include <asm/calgary.h>
 
 int iommu_merge __read_mostly = 1;

arch/x86/kernel/pci-gart_64.c +1 -1

index c56e9ee..0e72abf 100644
@@ -30,7 +30,7 @@
 #include <asm/mtrr.h>
 #include <asm/pgtable.h>
 #include <asm/proto.h>
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include <asm/cacheflush.h>
 #include <asm/swiotlb.h>
 #include <asm/dma.h>

arch/x86/kernel/pci-nommu_64.c +1 -1

index faf70bd..ab08e18 100644
@@ -7,7 +7,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/scatterlist.h>
 
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include <asm/processor.h>
 #include <asm/dma.h>

arch/x86/kernel/pci-swiotlb_64.c +1 -1

index b2f405e..102866d 100644
@@ -5,7 +5,7 @@
 #include <linux/module.h>
 #include <linux/dma-mapping.h>
 
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include <asm/swiotlb.h>
 #include <asm/dma.h>

arch/x86/kernel/reboot_64.c +1 -1

index 776eb06..71b13c5 100644
@@ -17,7 +17,7 @@
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
 #include <asm/apic.h>
-#include <asm/iommu.h>
+#include <asm/gart.h>
 
 /*
  * Power off function, if any

arch/x86_64/Kconfig +1 -1

index c2d2499..198adad 100644
@@ -480,7 +480,7 @@ config HPET_EMULATE_RTC
 # Mark as embedded because too many people got it wrong.
 # The code disables itself when not needed.
 config IOMMU
-	bool "IOMMU support" if EMBEDDED
+	bool "GART IOMMU support" if EMBEDDED
 	default y
 	select SWIOTLB
 	select AGP

drivers/pci/intel-iommu.c +1 -1

index b3d7031..10712b9 100644
@@ -34,7 +34,7 @@
 #include "intel-iommu.h"
 #include <asm/proto.h> /* force_iommu in this header in x86-64*/
 #include <asm/cacheflush.h>
-#include <asm/iommu.h>
+#include <asm/gart.h>
 #include "pci.h"
 
 #define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)

include/asm-x86/gart.h renamed from include/asm-x86/iommu.h +2 -2

similarity index 92%
rename from include/asm-x86/iommu.h
rename to include/asm-x86/gart.h
index 5af471f..07862fd 100644
@@ -1,5 +1,5 @@
-#ifndef _ASM_X8664_IOMMU_H
-#define _ASM_X8664_IOMMU_H 1
+#ifndef _ASM_X8664_GART_H
+#define _ASM_X8664_GART_H 1
 
 extern void pci_iommu_shutdown(void);
 extern void no_iommu_init(void);

Signature · 1 line

1.5.2.5

Muli Ben-Yehuda

Acked-by on v1 1/4

2007-10-23 19:06 UTC #2 lore raw

Re: [PATCH 1/4] x86 gart: rename iommu.h to gart.h

Quoted 5 lines · Joerg Roedel

This patch renames the include file asm-x86/iommu.h to asm-x86/gart.h to make clear to which IOMMU implementation it belongs. The patch also adds "GART" to the Kconfig line.
Signed-off-by Joerg Roedel <[email protected]>

Acked-by Muli Ben-Yehuda <[email protected]>

Cheers,
Muli

Signature · 4 lines

SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007

Joerg Roedel

author

2007-10-23 17:41 UTC #3 lore raw

PATCH 2/4 x86 gart: rename CONFIG_IOMMU to CONFIG_GART_IOMMU

In reply to Joerg Roedel

This patch renames the IOMMU config option to GART_IOMMU because in fact it means the GART and not general support for an IOMMU on x86.

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

---
 arch/x86/kernel/Makefile_64    |    2 +-
 arch/x86/kernel/early-quirks.c |    2 +-
 arch/x86/kernel/pci-dma_64.c   |    6 +++---
 arch/x86_64/Kconfig            |    4 ++--
 arch/x86_64/Kconfig.debug      |    2 +-
 arch/x86_64/defconfig          |    2 +-
 drivers/char/agp/Kconfig       |    4 ++--
 drivers/char/agp/amd64-agp.c   |    2 +-
 drivers/usb/core/message.c     |    2 +-
 include/asm-x86/gart.h         |    2 +-
 include/asm-x86/pci_64.h       |    2 +-
 11 files changed, 15 insertions(+), 15 deletions(-)

arch/x86/kernel/Makefile_64 +1 -1

index dec06e7..02cfd08 100644
@@ -29,7 +29,7 @@ obj-$(CONFIG_PM)		+= suspend_64.o
 obj-$(CONFIG_HIBERNATION)	+= suspend_asm_64.o
 obj-$(CONFIG_CPU_FREQ)		+= cpu/cpufreq/
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
-obj-$(CONFIG_IOMMU)		+= pci-gart_64.o aperture_64.o
+obj-$(CONFIG_GART_IOMMU)	+= pci-gart_64.o aperture_64.o
 obj-$(CONFIG_CALGARY_IOMMU)	+= pci-calgary_64.o tce_64.o
 obj-$(CONFIG_SWIOTLB)		+= pci-swiotlb_64.o
 obj-$(CONFIG_KPROBES)		+= kprobes_64.o

arch/x86/kernel/early-quirks.c +1 -1

index d6e6e83..5330745 100644
@@ -23,7 +23,7 @@
 
 static void __init via_bugs(void)
 {
-#ifdef CONFIG_IOMMU
+#ifdef CONFIG_GART_IOMMU
 	if ((end_pfn > MAX_DMA32_PFN ||  force_iommu) &&
 	    !iommu_aperture_allowed) {
 		printk(KERN_INFO

arch/x86/kernel/pci-dma_64.c +3 -3

index 730339f..66b3dc5 100644
@@ -275,7 +275,7 @@ __init int iommu_setup(char *p)
 			swiotlb = 1;
 #endif
 
-#ifdef CONFIG_IOMMU
+#ifdef CONFIG_GART_IOMMU
 		gart_parse_options(p);
 #endif

@@ -298,7 +298,7 @@ void __init pci_iommu_alloc(void)
 	 * The order of these functions is important for
 	 * fall-back/fail-over reasons
 	 */
-#ifdef CONFIG_IOMMU
+#ifdef CONFIG_GART_IOMMU
 	iommu_hole_init();
 #endif

@@ -321,7 +321,7 @@ static int __init pci_iommu_init(void)
 
 	intel_iommu_init();
 
-#ifdef CONFIG_IOMMU
+#ifdef CONFIG_GART_IOMMU
 	gart_iommu_init();
 #endif

arch/x86_64/Kconfig +2 -2

index 198adad..b986b27 100644
@@ -479,7 +479,7 @@ config HPET_EMULATE_RTC
 
 # Mark as embedded because too many people got it wrong.
 # The code disables itself when not needed.
-config IOMMU
+config GART_IOMMU
 	bool "GART IOMMU support" if EMBEDDED
 	default y
 	select SWIOTLB

@@ -687,7 +687,7 @@ source kernel/Kconfig.hz
 
 config K8_NB
 	def_bool y
-	depends on AGP_AMD64 || IOMMU || (PCI && NUMA)
+	depends on AGP_AMD64 || GART_IOMMU || (PCI && NUMA)
 
 endmenu

arch/x86_64/Kconfig.debug +1 -1

index 775d211..6035468 100644
@@ -17,7 +17,7 @@ config DEBUG_RODATA
 	 If in doubt, say "N".
 
 config IOMMU_DEBUG
-       depends on IOMMU && DEBUG_KERNEL
+       depends on GART_IOMMU && DEBUG_KERNEL
        bool "Enable IOMMU debugging"
        help
          Force the IOMMU to on even when you have less than 4GB of

arch/x86_64/defconfig +1 -1

index b091c5e..38a83f9 100644
@@ -170,7 +170,7 @@ CONFIG_HOTPLUG_CPU=y
 CONFIG_ARCH_ENABLE_MEMORY_HOTPLUG=y
 CONFIG_HPET_TIMER=y
 CONFIG_HPET_EMULATE_RTC=y
-CONFIG_IOMMU=y
+CONFIG_GART_IOMMU=y
 # CONFIG_CALGARY_IOMMU is not set
 CONFIG_SWIOTLB=y
 CONFIG_X86_MCE=y

drivers/char/agp/Kconfig +2 -2

index f22c253..ccb1fa8 100644
@@ -56,9 +56,9 @@ config AGP_AMD
 	  X on AMD Irongate, 761, and 762 chipsets.
 
 config AGP_AMD64
-	tristate "AMD Opteron/Athlon64 on-CPU GART support" if !IOMMU
+	tristate "AMD Opteron/Athlon64 on-CPU GART support" if !GART_IOMMU
 	depends on AGP && X86
-	default y if IOMMU
+	default y if GART_IOMMU
 	help
 	  This option gives you AGP support for the GLX component of
 	  X using the on-CPU northbridge of the AMD Athlon64/Opteron CPUs.

drivers/char/agp/amd64-agp.c +1 -1

index d95662e..d8200ac 100644
@@ -787,7 +787,7 @@ static void __exit agp_amd64_cleanup(void)
 
 /* On AMD64 the PCI driver needs to initialize this driver early
    for the IOMMU, so it has to be called via a backdoor. */
-#ifndef CONFIG_IOMMU
+#ifndef CONFIG_GART_IOMMU
 module_init(agp_amd64_init);
 module_exit(agp_amd64_cleanup);
 #endif

drivers/usb/core/message.c +1 -1

index 90d64a8..9157cfb 100644
@@ -434,7 +434,7 @@ int usb_sg_init (
 		if (dma) {
 			io->urbs [i]->transfer_dma = sg_dma_address (sg + i);
 			len = sg_dma_len (sg + i);
-#if defined(CONFIG_HIGHMEM) || defined(CONFIG_IOMMU)
+#if defined(CONFIG_HIGHMEM) || defined(CONFIG_GART_IOMMU)
 			io->urbs[i]->transfer_buffer = NULL;
 #else
 			io->urbs[i]->transfer_buffer = sg_virt(&sg[i]);

include/asm-x86/gart.h +1 -1

index 07862fd..cb7e989 100644
@@ -5,7 +5,7 @@ extern void pci_iommu_shutdown(void);
 extern void no_iommu_init(void);
 extern int force_iommu, no_iommu;
 extern int iommu_detected;
-#ifdef CONFIG_IOMMU
+#ifdef CONFIG_GART_IOMMU
 extern void gart_iommu_init(void);
 extern void gart_iommu_shutdown(void);
 extern void __init gart_parse_options(char *);

include/asm-x86/pci_64.h +1 -1

index 9baa46d..ef54226 100644
@@ -37,7 +37,7 @@ extern int iommu_setup(char *opt);
  */
 #define PCI_DMA_BUS_IS_PHYS (dma_ops->is_phys)
 
-#if defined(CONFIG_IOMMU) || defined(CONFIG_CALGARY_IOMMU)
+#if defined(CONFIG_GART_IOMMU) || defined(CONFIG_CALGARY_IOMMU)
 
 #define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)	\
 	dma_addr_t ADDR_NAME;

Signature · 1 line

1.5.2.5

Muli Ben-Yehuda

Acked-by on v1 2/4

2007-10-24 01:24 UTC #4 lore raw

Re: [PATCH 2/4] x86 gart: rename CONFIG_IOMMU to CONFIG_GART_IOMMU

Quoted 4 lines · Joerg Roedel

This patch renames the IOMMU config option to GART_IOMMU because in fact it means the GART and not general support for an IOMMU on x86.
Signed-off-by Joerg Roedel <[email protected]>

Acked-by Muli Ben-Yehuda <[email protected]>

Cheers,
Muli

Signature · 4 lines

SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007

Joerg Roedel

author

2007-10-23 17:42 UTC #5 lore raw

PATCH 3/4 x86 gart: make some variables and functions static

In reply to Joerg Roedel

This patch makes some functions and variables static in pci-gart_64.c which are not used somewhere else.

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

---
 arch/x86/kernel/pci-gart_64.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

arch/x86/kernel/pci-gart_64.c +4 -4

index 0e72abf..7097ef8 100644
@@ -36,11 +36,11 @@
 #include <asm/dma.h>
 #include <asm/k8.h>
 
-unsigned long iommu_bus_base;	/* GART remapping area (physical) */
+static unsigned long iommu_bus_base;	/* GART remapping area (physical) */
 static unsigned long iommu_size; 	/* size of remapping area bytes */
 static unsigned long iommu_pages;	/* .. and in pages */
 
-u32 *iommu_gatt_base; 		/* Remapping table */
+static u32 *iommu_gatt_base; 		/* Remapping table */
 
 /* If this is disabled the IOMMU will use an optimized flushing strategy
    of only flushing when an mapping is reused. With it true the GART is flushed

@@ -135,8 +135,8 @@ static void flush_gart(void)
 /* Debugging aid for drivers that don't free their IOMMU tables */
 static void **iommu_leak_tab; 
 static int leak_trace;
-int iommu_leak_pages = 20; 
-void dump_leak(void)
+static int iommu_leak_pages = 20;
+static void dump_leak(void)
 {
 	int i;
 	static int dump;

Signature · 1 line

1.5.2.5

Muli Ben-Yehuda

Acked-by on v1 3/4

2007-10-24 01:24 UTC #6 lore raw

Re: [PATCH 3/4] x86 gart: make some variables and functions static

Quoted 4 lines · Joerg Roedel

This patch makes some functions and variables static in pci-gart_64.c which are not used somewhere else.
Signed-off-by Joerg Roedel <[email protected]>

Acked-by Muli Ben-Yehuda <[email protected]>

Cheers,
Muli

Signature · 4 lines

SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007

Joerg Roedel

author

2007-10-23 17:42 UTC #7 lore raw

PATCH 4/4 x86 gart: rename symbols only used for the GART implementation

In reply to Joerg Roedel

This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.

It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.

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

---
 arch/x86/kernel/aperture_64.c  |   12 ++++++------
 arch/x86/kernel/early-quirks.c |    4 ++--
 arch/x86/kernel/pci-dma_64.c   |    2 +-
 arch/x86/kernel/pci-gart_64.c  |    8 ++++----
 include/asm-x86/gart.h         |   12 ++++++------
 5 files changed, 19 insertions(+), 19 deletions(-)

arch/x86/kernel/aperture_64.c +6 -6

index 70c854f..3423d00 100644
@@ -25,9 +25,9 @@
 #include <asm/dma.h>
 #include <asm/k8.h>
 
-int iommu_aperture;
-int iommu_aperture_disabled __initdata = 0;
-int iommu_aperture_allowed __initdata = 0;
+int gart_aperture;
+int gart_aperture_disabled __initdata = 0;
+int gart_aperture_allowed __initdata = 0;
 
 int fallback_aper_order __initdata = 1; /* 64MB */
 int fallback_aper_force __initdata = 0;

@@ -204,14 +204,14 @@ static __u32 __init search_agp_bridge(u32 *order, int *valid_agp)
 	return 0;
 }
 
-void __init iommu_hole_init(void)
+void __init gart_hole_init(void)
 { 
 	int fix, num; 
 	u32 aper_size, aper_alloc = 0, aper_order = 0, last_aper_order = 0;
 	u64 aper_base, last_aper_base = 0;
 	int valid_agp = 0;
 
-	if (iommu_aperture_disabled || !fix_aperture || !early_pci_allowed())
+	if (gart_aperture_disabled || !fix_aperture || !early_pci_allowed())
 		return;
 
 	printk(KERN_INFO  "Checking aperture...\n");

@@ -222,7 +222,7 @@ void __init iommu_hole_init(void)
 			continue;
 
 		iommu_detected = 1;
-		iommu_aperture = 1; 
+		gart_aperture = 1;
 
 		aper_order = (read_pci_config(0, num, 3, 0x90) >> 1) & 7; 
 		aper_size = (32 * 1024 * 1024) << aper_order;

arch/x86/kernel/early-quirks.c +2 -2

index 5330745..b0527af 100644
@@ -25,11 +25,11 @@ static void __init via_bugs(void)
 {
 #ifdef CONFIG_GART_IOMMU
 	if ((end_pfn > MAX_DMA32_PFN ||  force_iommu) &&
-	    !iommu_aperture_allowed) {
+	    !gart_aperture_allowed) {
 		printk(KERN_INFO
 		       "Looks like a VIA chipset. Disabling IOMMU."
 		       " Override with iommu=allowed\n");
-		iommu_aperture_disabled = 1;
+		gart_aperture_disabled = 1;
 	}
 #endif
 }

arch/x86/kernel/pci-dma_64.c +1 -1

index 66b3dc5..20389f9 100644
@@ -299,7 +299,7 @@ void __init pci_iommu_alloc(void)
 	 * fall-back/fail-over reasons
 	 */
 #ifdef CONFIG_GART_IOMMU
-	iommu_hole_init();
+	gart_hole_init();
 #endif
 
 #ifdef CONFIG_CALGARY_IOMMU

arch/x86/kernel/pci-gart_64.c +4 -4

index 7097ef8..c54c87d 100644
@@ -628,12 +628,12 @@ void __init gart_iommu_init(void)
 		return;
 
 	/* Did we detect a different HW IOMMU? */
-	if (iommu_detected && !iommu_aperture)
+	if (iommu_detected && !gart_aperture)
 		return;
 
 	if (no_iommu ||
 	    (!force_iommu && end_pfn <= MAX_DMA32_PFN) ||
-	    !iommu_aperture ||
+	    !gart_aperture ||
 	    (no_agp && init_k8_gatt(&info) < 0)) {
 		if (end_pfn > MAX_DMA32_PFN) {
 			printk(KERN_ERR "WARNING more than 4GB of memory "

@@ -734,9 +734,9 @@ void __init gart_parse_options(char *p)
 		fix_aperture = 0;
 	/* duplicated from pci-dma.c */
 	if (!strncmp(p,"force",5))
-		iommu_aperture_allowed = 1;
+		gart_aperture_allowed = 1;
 	if (!strncmp(p,"allowed",7))
-		iommu_aperture_allowed = 1;
+		gart_aperture_allowed = 1;
 	if (!strncmp(p, "memaper", 7)) {
 		fallback_aper_force = 1;
 		p += 7;

include/asm-x86/gart.h +6 -6

index cb7e989..70c6889 100644
@@ -9,16 +9,16 @@ extern int iommu_detected;
 extern void gart_iommu_init(void);
 extern void gart_iommu_shutdown(void);
 extern void __init gart_parse_options(char *);
-extern void iommu_hole_init(void);
+extern void gart_hole_init(void);
 extern int fallback_aper_order;
 extern int fallback_aper_force;
-extern int iommu_aperture;
-extern int iommu_aperture_allowed;
-extern int iommu_aperture_disabled;
+extern int gart_aperture;
+extern int gart_aperture_allowed;
+extern int gart_aperture_disabled;
 extern int fix_aperture;
 #else
-#define iommu_aperture 0
-#define iommu_aperture_allowed 0
+#define gart_aperture 0
+#define gart_aperture_allowed 0
 
 static inline void gart_iommu_shutdown(void)
 {

Signature · 1 line

1.5.2.5

Andi Kleen

2007-10-23 17:44 UTC #8 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

Quoted 5 lines · On Tuesday 23 October 2007 19:41:33 Joerg Roedel wrote:

This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.
It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.

That makes it still potentially conflicting with the AGP GART code.

-Andi

Joerg Roedel

author

2007-10-23 17:48 UTC #9 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

Quoted 8 lines · Andi Kleen

Quoted 5 lines · On Tuesday 23 October 2007 19:41:33 Joerg Roedel wrote:
This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.
It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.
That makes it still potentially conflicting with the AGP GART code.

Maybe yes. But the AGP GART driver conflicts with the GART IOMMU config option. So I don't see a problem here.

Joerg

Signature · 6 lines

           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Dave Jones

2007-10-23 17:59 UTC #10 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

Quoted 5 lines · Joerg Roedel

Quoted 4 lines · Andi Kleen
Quoted 3 lines · On Tuesday 23 October 2007 19:41:33 Joerg Roedel wrote:
This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.
That makes it still potentially conflicting with the AGP GART code.
Maybe yes. But the AGP GART driver conflicts with the GART IOMMU config option. So I don't see a problem here.

It shouldn't. It's perfectly feasible to use both IOMMU and GART for AGP at the same time.

Dave

Signature · 1 line

http://www.codemonkey.org.uk

H. Peter Anvin

reviewer

2007-10-23 18:05 UTC #11 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

Quoted 9 lines · Dave Jones

Quoted 8 lines · Joerg Roedel
Quoted 7 lines · Andi Kleen
Quoted 5 lines · On Tuesday 23 October 2007 19:41:33 Joerg Roedel wrote:
This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.
It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
That makes it still potentially conflicting with the AGP GART code.
Maybe yes. But the AGP GART driver conflicts with the GART IOMMU config option. So I don't see a problem here.
It shouldn't. It's perfectly feasible to use both IOMMU and GART for AGP at the same time.

And even if did, it's a big difference between being not able to compile in two drivers in the same kernel and not being able to use them at the same time.

Now, it looks to me the AGP GART code mostly calls itself agpgart.

-hpa

Andi Kleen

2007-10-23 18:11 UTC #12 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

Quoted 15 lines · On Tuesday 23 October 2007 20:04:36 H. Peter Anvin wrote:

Quoted 14 lines · Dave Jones
Quoted 12 lines · Joerg Roedel
Quoted 8 lines · Andi Kleen
Quoted 5 lines · On Tuesday 23 October 2007 19:41:33 Joerg Roedel wrote:
This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.
It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.
That makes it still potentially conflicting with the AGP GART code.
Maybe yes. But the AGP GART driver conflicts with the GART IOMMU config option. So I don't see a problem here.

Quoted 7 lines

Quoted 3 lines
It shouldn't. It's perfectly feasible to use both IOMMU and GART for AGP at the same time.
And even if did, it's a big difference between being not able to compile in two drivers in the same kernel and not being able to use them at the same time.

You can already compile them in all. The functions are not really conflicting -- otherwise allyesconfig would not work.

I think his goal was to get an prefix that describes the module uniquely. gart_* clearly does not fulfill that criteria.

So basically he's replacing an ambigious-with-other-IOMMU-implementations prefix with an ambigious-with-AGP prefix. Seems like a rather pointless change.

-And

Joerg Roedel

author

2007-10-23 19:06 UTC #13 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

Quoted 27 lines · Andi Kleen

Quoted 15 lines · On Tuesday 23 October 2007 20:04:36 H. Peter Anvin wrote:
Quoted 14 lines · Dave Jones
Quoted 12 lines · Joerg Roedel
Quoted 8 lines · Andi Kleen
Quoted 5 lines · On Tuesday 23 October 2007 19:41:33 Joerg Roedel wrote:
This patch renames the 4 symbols iommu_hole_init(), iommu_aperture, iommu_aperture_allowed, iommu_aperture_disabled.
It replaces the iommu_ with gart_ in the symbol name. All these symbols are only used for the GART implementation of IOMMUs.
That makes it still potentially conflicting with the AGP GART code.
Maybe yes. But the AGP GART driver conflicts with the GART IOMMU config option. So I don't see a problem here.
Quoted 7 lines
Quoted 3 lines
It shouldn't. It's perfectly feasible to use both IOMMU and GART for AGP at the same time.
And even if did, it's a big difference between being not able to compile in two drivers in the same kernel and not being able to use them at the same time.
You can already compile them in all. The functions are not really conflicting -- otherwise allyesconfig would not work.

Quoted 6 lines

I think his goal was to get an prefix that describes the module uniquely. gart_* clearly does not fulfill that criteria.
So basically he's replacing an ambigious-with-other-IOMMU-implementations prefix with an ambigious-with-AGP prefix. Seems like a rather pointless change.

Ok, sorry, right. I misunderstood the Kconfig entry for the AGP GART driver. I will change the prefix to gart_iommu and resubmit.

Joerg

Signature · 6 lines

           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Muli Ben-Yehuda

2007-10-24 01:41 UTC #14 lore raw

Re: [PATCH 4/4] x86 gart: rename symbols only used for the GART implementation

In reply to Andi Kleen

Quoted 6 lines · Andi Kleen

I think his goal was to get an prefix that describes the module uniquely. gart_* clearly does not fulfill that criteria.
So basically he's replacing an ambigious-with-other-IOMMU-implementations prefix with an ambigious-with-AGP prefix. Seems like a rather pointless change.

Not at all - the present situation where GART specific code masquerades as generic IOMMU code is confusing. I've heard from more than one person who was confused by "how come I can enable an IOMMU while CONFIG_IOMMU is off?" and "how come some iommu_xxx options (which were really gart options...) don't actually affect the (Calgary) IOMMU"?

If the variable names clash the correct solution is to s/gart/agpgart/ in the AGP code and then continue renaming gart specific bits 'gart' rather than 'iommu'.

Cheers,
Muli

Signature · 4 lines

SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007