[RFC][PATCH 0/4] enabling graphics memory dma remapping

[RFC][PATCH 0/4] enabling graphics memory dma remapping

[Zhenyu Wang]

Intel IOMMU (a.k.a VT-d) is under rapid deployment on desktop
and mobile platforms. As platform provides multiple dma remap
engines for devices like those lives on south bridge (net, sound,
etc.), and we also have one engine specific to graphics device.

If this engine is functioning, the access to graphics memory
routine is to first look up in GART table, which return virtual
dma address that will further be route to graphics DMAR engine,
which then looking up DMAR table to get real physical address.

Current intel iommu function in kernel is providing a graphics
workaround kernel config (CONFIG_DMAR_GFX_WA) to make a 1:1 mapping
initialization for graphics device, so what we fill in GART table
got from agpgart page alloc is identical to virtual dma address in
DMAR table. No change needs to be made for graphics driver.

Following patches try to add dma remapping function to agpgart
module, so we won't depend on intel iommu graphics workaround. 
As DMAR is only available on x86_64 now, so these patches can only
work for x86_64 system.

Here're three patches below:

- intel_iommu-explicit-export-current-graphics-dmar-status.patch
  
  This exports current status of graphics dma remap engine, which
  depends on current platform iommu support, kernel config or runtime
  config. This info will be used in agp module for detect whether
  we should do remapping or not.

- AGP-Add-generic-support-for-graphics-dma-remapping.patch

  This adds new hooks into agp bridge driver to do map/unmap when
  bind/unbind memory into GART table. The aim is to provide generic
  interfaces to let driver implement hardware specific operations.

- AGP-intel_agp-add-support-for-graphics-dma-remapping.patch

  This implements graphics dma remapping support in intel_agp module,
  current only desktop chipset G33 series can use it.

Thanks.

[RFC][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

[PATCH] [intel_iommu] explicit export current graphics dmar status

To make it possbile to tell other modules about curent
graphics dmar engine status, that could decide if graphics
driver should remap physical address to dma address.

Also this one trys to make dmar_disabled really present
current status of DMAR, which would be used for completely
express graphics dmar engine is active or not.

Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
---
 drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
 include/linux/dmar.h      |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e079a52..81a0abc 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,7 +53,7 @@
 static void domain_remove_dev_info(struct dmar_domain *domain);
 
 static int dmar_disabled;
-static int __initdata dmar_map_gfx = 1;
+static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
@@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
 }
 __setup("intel_iommu=", intel_iommu_setup);
 
+int intel_iommu_gfx_remapping(void)
+{
+#ifndef CONFIG_DMAR_GFX_WA
+	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
+		return 1;
+#endif
+	return 0;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
+
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static struct kmem_cache *iommu_iova_cache;
@@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
 
 void __init detect_intel_iommu(void)
 {
-	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
+	if (dmar_disabled)
+		return;
+	if (swiotlb || no_iommu || iommu_detected) {
+		dmar_disabled = 1;
 		return;
+	}
 	if (early_dmar_detect()) {
 		iommu_detected = 1;
 	}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index ffb6439..8ae435e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
 extern int dmar_table_init(void);
 extern int early_dmar_detect(void);
 
+extern int intel_iommu_gfx_remapping(void);
+
 extern struct list_head dmar_drhd_units;
 extern struct list_head dmar_rmrr_units;
 
@@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
 {
 	return -ENODEV;
 }
+static inline int intel_iommu_gfx_remapping(void)
+{
+	return 0;
+}
 
 #endif /* !CONFIG_DMAR */
 #endif /* __DMAR_H__ */
-- 
1.5.3.5

[RFC][PATCH 2/4][AGP] Add generic support for graphics dma remapping

[Zhenyu Wang]

[PATCH] [AGP] Add generic support for graphics dma remapping

New driver hooks for support graphics memory dma remapping
are introduced in this patch. It makes generic code can
tell if current device needs dma remapping, then call driver
provided interfaces for mapping and unmapping. Change has
also been made to handle scratch_page in remapping case.

Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
---
 drivers/char/agp/agp.h      |   14 ++++++++++++++
 drivers/char/agp/backend.c  |   21 ++++++++++++++++++++-
 drivers/char/agp/generic.c  |   14 ++++++++++++++
 include/linux/agp_backend.h |    7 +++++++
 4 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index b83824c..7806da1 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -118,6 +118,18 @@ struct agp_bridge_driver {
 	void *(*agp_alloc_page)(struct agp_bridge_data *);
 	void (*agp_destroy_page)(void *, int flags);
         int (*agp_type_to_mask_type) (struct agp_bridge_data *, int);
+
+	/* Tell current graphics dma remapping engine status, there might
+	 * be driver or platform specific way to detect. */
+	int (*agp_require_remapping)(void);
+
+	/* This can support single page remapping via void *virt for like
+	 * scratch_page, or normal bunch of mem page range via struct
+	 * agp_memory *mem.
+	 */
+	int (*agp_map_page)(struct agp_memory *mem, void *virt, dma_addr_t *ret, int is_single);
+
+	void (*agp_unmap_page)(struct agp_memory *mem, dma_addr_t ret, int is_single);
 };
 
 struct agp_bridge_data {
@@ -132,6 +144,8 @@ struct agp_bridge_data {
 	u32 *gatt_table_real;
 	unsigned long scratch_page;
 	unsigned long scratch_page_real;
+	dma_addr_t scratch_page_dma;
+	u8 scratch_page_is_mapped;
 	unsigned long gart_bus_addr;
 	unsigned long gatt_bus_addr;
 	u32 mode;
diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index 832ded2..cc6d648 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -151,7 +151,20 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
 
 		bridge->scratch_page_real = virt_to_gart(addr);
 		bridge->scratch_page =
-		    bridge->driver->mask_memory(bridge, bridge->scratch_page_real, 0);
+			bridge->driver->mask_memory(bridge, bridge->scratch_page_real, 0);
+
+		if (bridge->driver->agp_require_remapping &&
+				bridge->driver->agp_require_remapping()) {
+			if (bridge->driver->agp_map_page(NULL, addr, &bridge->scratch_page_dma, 1)) {
+				printk(KERN_ERR PFX "unable to map scratch page\n");
+				rc = -ENOMEM;
+				goto err_out;
+			}
+			bridge->scratch_page_is_mapped = TRUE;
+			bridge->scratch_page =
+				bridge->driver->mask_memory(bridge,
+						bridge->scratch_page_dma, 0);
+		}
 	}
 
 	size_value = bridge->driver->fetch_size();
@@ -189,6 +202,9 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
 
 err_out:
 	if (bridge->driver->needs_scratch_page) {
+		if (bridge->scratch_page_is_mapped)
+			bridge->driver->agp_unmap_page(NULL, bridge->scratch_page_dma, 1);
+
 		bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 						 AGP_PAGE_DESTROY_UNMAP);
 		flush_agp_mappings();
@@ -217,6 +233,9 @@ static void agp_backend_cleanup(struct agp_bridge_data *bridge)
 
 	if (bridge->driver->agp_destroy_page &&
 	    bridge->driver->needs_scratch_page) {
+		if (bridge->scratch_page_is_mapped)
+			bridge->driver->agp_unmap_page(NULL, bridge->scratch_page_dma, 1);
+
 		bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 						 AGP_PAGE_DESTROY_UNMAP);
 		flush_agp_mappings();
diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 64b2f6d..8966c94 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -415,6 +415,14 @@ int agp_bind_memory(struct agp_memory *curr, off_t pg_start)
 		curr->bridge->driver->cache_flush();
 		curr->is_flushed = TRUE;
 	}
+
+	if (curr->bridge->driver->agp_require_remapping &&
+			curr->bridge->driver->agp_require_remapping()) {
+		ret_val = curr->bridge->driver->agp_map_page(curr, NULL, NULL, 0);
+		if (ret_val)
+			return ret_val;
+	}
+
 	ret_val = curr->bridge->driver->insert_memory(curr, pg_start, curr->type);
 
 	if (ret_val != 0)
@@ -454,6 +462,12 @@ int agp_unbind_memory(struct agp_memory *curr)
 
 	curr->is_bound = FALSE;
 	curr->pg_start = 0;
+
+	if (curr->is_mapped) {
+		if (curr->bridge->driver->agp_unmap_page)
+			curr->bridge->driver->agp_unmap_page(curr, 0, 0);
+		curr->is_mapped = FALSE;
+	}
 	return 0;
 }
 EXPORT_SYMBOL(agp_unbind_memory);
diff --git a/include/linux/agp_backend.h b/include/linux/agp_backend.h
index abc521c..5b7c431 100644
--- a/include/linux/agp_backend.h
+++ b/include/linux/agp_backend.h
@@ -87,7 +87,14 @@ struct agp_memory {
 	u32 physical;
 	u8 is_bound;
 	u8 is_flushed;
+	u8 is_mapped;
         u8 vmalloc_flag;
+	/* Following are used in graphics dma remapping case,
+	 * for map/unmap via sg interfaces.
+	 */
+	struct scatterlist *sg_list;
+	int num_sg;
+	u8 sg_vmalloc_flag;
 };
 
 #define AGP_NORMAL_MEMORY 0
-- 
1.5.3.5

[RFC][PATCH 3/4][AGP] intel_agp: add support for graphics dma remapping on G33

[Zhenyu Wang]

[PATCH] [AGP] intel_agp: add support for graphics dma remapping on G33

When graphics dma remapping engine is active, we must fill
gart table with dma address from dmar engine, as now graphics
device access to graphics memory must go through dma remapping
table to get real physical address.

Add support on G33 chipset, which has graphics device specific
dmar engine avaiable.

Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
---
 drivers/char/agp/Kconfig     |   12 +++-
 drivers/char/agp/intel-agp.c |  134 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig
index ccb1fa8..e60c2b0 100644
--- a/drivers/char/agp/Kconfig
+++ b/drivers/char/agp/Kconfig
@@ -74,8 +74,16 @@ config AGP_INTEL
 	  on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850, 860, 875,
 	  E7205 and E7505 chipsets and full support for the 810, 815, 830M,
 	  845G, 852GM, 855GM, 865G and I915 integrated graphics chipsets.
-
-
+	
+config AGP_INTEL_DMAR
+	bool
+	depends on AGP_INTEL && DMAR && !DMAR_GFX_WA
+	default y
+	help
+	  This option will enable graphics address remapping with Intel
+	  DMAR engine aka VT-d if you don't select graphics workaround on
+	  Intel DMAR. The graphics address filled to gart table will be
+	  changed by remapping within graphics DMAR engine for domain.
 
 config AGP_NVIDIA
 	tristate "NVIDIA nForce/nForce2 chipset support"
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index d879619..7424667 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -9,6 +9,9 @@
 #include <linux/pagemap.h>
 #include <linux/agp_backend.h>
 #include "agp.h"
+#ifdef CONFIG_AGP_INTEL_DMAR
+#include <linux/dmar.h>
+#endif
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB      0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG      0x2972
@@ -115,6 +118,97 @@ static struct _intel_private {
 	int gtt_entries;			/* i830+ */
 } intel_private;
 
+static int intel_agp_require_remapping(void)
+{
+#ifdef CONFIG_AGP_INTEL_DMAR
+	return intel_iommu_gfx_remapping();
+#else
+	return 0;
+#endif
+}
+
+#ifdef CONFIG_AGP_INTEL_DMAR
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t *ret, int is_single)
+{
+	int i;
+	struct scatterlist *sg;
+
+	if (is_single) {
+		*ret = pci_map_single(intel_private.pcidev, virt, PAGE_SIZE,
+				PCI_DMA_BIDIRECTIONAL);
+		if (pci_dma_mapping_error(*ret))
+			return -EINVAL;
+		return 0;
+	}
+
+	DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
+
+	if ((mem->page_count * sizeof(*mem->sg_list)) < 2*PAGE_SIZE)
+	    mem->sg_list = kcalloc(mem->page_count, sizeof(*mem->sg_list), GFP_KERNEL);
+
+	if (mem->sg_list == NULL) {
+	    mem->sg_list = vmalloc(mem->page_count * sizeof(*mem->sg_list));
+	    mem->sg_vmalloc_flag = 1;
+	}
+
+	if (!mem->sg_list) {
+	    mem->sg_vmalloc_flag = 0;
+	    return -ENOMEM;
+	}
+	sg_init_table(mem->sg_list, mem->page_count);
+
+	sg = mem->sg_list;
+	for (i = 0 ; i < mem->page_count; i++, sg = sg_next(sg))
+		sg_set_buf(sg, gart_to_virt(mem->memory[i]), PAGE_SIZE);
+
+	mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
+					mem->page_count, PCI_DMA_BIDIRECTIONAL);
+	if (mem->num_sg == 0) {
+		if (mem->sg_vmalloc_flag)
+			vfree(mem->sg_list);
+		else
+			kfree(mem->sg_list);
+		mem->sg_list = NULL;
+		mem->sg_vmalloc_flag = 0;
+		return -ENOMEM;
+	}
+	mem->is_mapped = TRUE;
+	return 0;
+}
+
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int is_single)
+{
+	if (is_single) {
+		pci_unmap_single(intel_private.pcidev, addr, PAGE_SIZE,
+				PCI_DMA_BIDIRECTIONAL);
+		return;
+	}
+	DBG("try unmapping %lu pages\n", (unsigned long)mem->page_count);
+
+	if (!mem->is_mapped)
+		return;
+
+	if (mem->num_sg)
+		pci_unmap_sg(intel_private.pcidev, mem->sg_list, mem->page_count,
+				PCI_DMA_BIDIRECTIONAL);
+	if (mem->sg_vmalloc_flag)
+		vfree(mem->sg_list);
+	else
+		kfree(mem->sg_list);
+	mem->sg_vmalloc_flag = 0;
+	mem->sg_list = NULL;
+	mem->num_sg = 0;
+}
+#else
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t *ret, int is_single)
+{
+    return 0;
+}
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int is_single)
+{
+}
+#endif
+
 static int intel_i810_fetch_size(void)
 {
 	u32 smram_miscc;
@@ -847,9 +941,40 @@ static int intel_i915_insert_entries(struct agp_memory *mem,off_t pg_start,
 	if (!mem->is_flushed)
 		global_cache_flush();
 
-	for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
-		writel(agp_bridge->driver->mask_memory(agp_bridge,
-			mem->memory[i], mask_type), intel_private.gtt+j);
+	/* map mem if current graphics dmar engine is active
+	 * and workaround is not applied. */
+	if (mem->is_mapped) {
+		struct scatterlist *sg;
+
+		j = pg_start;
+		if (mem->num_sg == mem->page_count) {
+			for_each_sg(mem->sg_list, sg, mem->page_count, i) {
+				writel(agp_bridge->driver->mask_memory(agp_bridge,
+							sg_dma_address(sg),
+							mask_type),
+						intel_private.gtt+j);
+				j++;
+			}
+		} else {
+			/* sg may merge pages, but we have to seperate
+			 * per-page addr for GTT */
+			unsigned int len, m;
+			for_each_sg(mem->sg_list, sg, mem->num_sg, i) {
+				len = sg_dma_len(sg) / PAGE_SIZE;
+				for (m = 0; m < len; m++) {
+					writel(agp_bridge->driver->mask_memory(agp_bridge,
+								sg_dma_address(sg) + m * PAGE_SIZE, mask_type),
+							intel_private.gtt+j);
+					j++;
+				}
+			}
+		}
+	} else {
+		for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
+			writel(agp_bridge->driver->mask_memory(agp_bridge,
+						mem->memory[i], mask_type),
+					intel_private.gtt+j);
+		}
 	}
 
 	readl(intel_private.gtt+j-1);
@@ -1796,6 +1921,9 @@ static const struct agp_bridge_driver intel_g33_driver = {
 	.agp_alloc_page         = agp_generic_alloc_page,
 	.agp_destroy_page       = agp_generic_destroy_page,
 	.agp_type_to_mask_type  = intel_i830_type_to_mask_type,
+	.agp_require_remapping  = intel_agp_require_remapping,
+	.agp_map_page		= intel_agp_map_pages,
+	.agp_unmap_page		= intel_agp_unmap_pages,
 };
 
 static int find_gmch(u16 device)
-- 
1.5.3.5

[agp-mm][PATCH 0/4] enabling graphics memory dma remapping

[Zhenyu Wang]

On 2007.12.18 13:08:09 +0000, Zhenyu Wang wrote:
> 
> Here're three patches below:
> 
> - intel_iommu-explicit-export-current-graphics-dmar-status.patch
>   
>   This exports current status of graphics dma remap engine, which
>   depends on current platform iommu support, kernel config or runtime
>   config. This info will be used in agp module for detect whether
>   we should do remapping or not.
> 
> - AGP-Add-generic-support-for-graphics-dma-remapping.patch
> 
>   This adds new hooks into agp bridge driver to do map/unmap when
>   bind/unbind memory into GART table. The aim is to provide generic
>   interfaces to let driver implement hardware specific operations.
> 
> - AGP-intel_agp-add-support-for-graphics-dma-remapping.patch
> 
>   This implements graphics dma remapping support in intel_agp module,
>   current only desktop chipset G33 series can use it.

Above patch series are against master branch, following threads try to
rebase them to Airlie's agp-mm tree for -mm inclusion.

Thanks.

[agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

[agp-mm] [intel_iommu] explicit export current graphics dmar status

To make it possbile to tell other modules about curent
graphics dmar engine status, that could decide if graphics
driver should remap physical address to dma address.

Also this one trys to make dmar_disabled really present
current status of DMAR, which would be used for completely
express graphics dmar engine is active or not.

Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
---
 drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
 include/linux/dmar.h      |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e079a52..81a0abc 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,7 +53,7 @@
 static void domain_remove_dev_info(struct dmar_domain *domain);
 
 static int dmar_disabled;
-static int __initdata dmar_map_gfx = 1;
+static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
@@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
 }
 __setup("intel_iommu=", intel_iommu_setup);
 
+int intel_iommu_gfx_remapping(void)
+{
+#ifndef CONFIG_DMAR_GFX_WA
+	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
+		return 1;
+#endif
+	return 0;
+}
+EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
+
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 static struct kmem_cache *iommu_iova_cache;
@@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
 
 void __init detect_intel_iommu(void)
 {
-	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
+	if (dmar_disabled)
+		return;
+	if (swiotlb || no_iommu || iommu_detected) {
+		dmar_disabled = 1;
 		return;
+	}
 	if (early_dmar_detect()) {
 		iommu_detected = 1;
 	}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index ffb6439..8ae435e 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
 extern int dmar_table_init(void);
 extern int early_dmar_detect(void);
 
+extern int intel_iommu_gfx_remapping(void);
+
 extern struct list_head dmar_drhd_units;
 extern struct list_head dmar_rmrr_units;
 
@@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
 {
 	return -ENODEV;
 }
+static inline int intel_iommu_gfx_remapping(void)
+{
+	return 0;
+}
 
 #endif /* !CONFIG_DMAR */
 #endif /* __DMAR_H__ */
-- 
1.5.3.4

[agp-mm][PATCH 2/4][AGP] Add generic support for graphics dma remapping

[Zhenyu Wang]

[agp-mm] [AGP] Add generic support for graphics dma remapping

New driver hooks for support graphics memory dma remapping
are introduced in this patch. It makes generic code can
tell if current device needs dma remapping, then call driver
provided interfaces for mapping and unmapping. Change has
also been made to handle scratch_page in remapping case.

Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
---
 drivers/char/agp/agp.h      |   13 +++++++++++++
 drivers/char/agp/backend.c  |   21 ++++++++++++++++++++-
 drivers/char/agp/generic.c  |   14 ++++++++++++++
 include/linux/agp_backend.h |    7 +++++++
 4 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
index 9ec9374..2c78a28 100644
--- a/drivers/char/agp/agp.h
+++ b/drivers/char/agp/agp.h
@@ -119,6 +119,17 @@ struct agp_bridge_driver {
 	void (*agp_destroy_page)(void *, int flags);
 	int (*agp_type_to_mask_type) (struct agp_bridge_data *, int);
 	void (*chipset_flush)(struct agp_bridge_data *);
+	/* Tell current graphics dma remapping engine status, there might
+	 * be driver or platform specific way to detect. */
+	int (*agp_require_remapping)(void);
+
+	/* This can support single page remapping via void *virt for like
+	 * scratch_page, or normal bunch of mem page range via struct
+	 * agp_memory *mem.
+	 */
+	int (*agp_map_page)(struct agp_memory *mem, void *virt, dma_addr_t *ret, int is_single);
+
+	void (*agp_unmap_page)(struct agp_memory *mem, dma_addr_t ret, int is_single);
 };
 
 struct agp_bridge_data {
@@ -133,6 +144,8 @@ struct agp_bridge_data {
 	u32 *gatt_table_real;
 	unsigned long scratch_page;
 	unsigned long scratch_page_real;
+	dma_addr_t scratch_page_dma;
+	u8 scratch_page_is_mapped;
 	unsigned long gart_bus_addr;
 	unsigned long gatt_bus_addr;
 	u32 mode;
diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index f9c180c..7898051 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -151,7 +151,20 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
 
 		bridge->scratch_page_real = virt_to_gart(addr);
 		bridge->scratch_page =
-		    bridge->driver->mask_memory(bridge, bridge->scratch_page_real, 0);
+			bridge->driver->mask_memory(bridge, bridge->scratch_page_real, 0);
+
+		if (bridge->driver->agp_require_remapping &&
+				bridge->driver->agp_require_remapping()) {
+			if (bridge->driver->agp_map_page(NULL, addr, &bridge->scratch_page_dma, 1)) {
+				printk(KERN_ERR PFX "unable to map scratch page\n");
+				rc = -ENOMEM;
+				goto err_out;
+			}
+			bridge->scratch_page_is_mapped = TRUE;
+			bridge->scratch_page =
+				bridge->driver->mask_memory(bridge,
+						bridge->scratch_page_dma, 0);
+		}
 	}
 
 	size_value = bridge->driver->fetch_size();
@@ -189,6 +202,9 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
 
 err_out:
 	if (bridge->driver->needs_scratch_page) {
+		if (bridge->scratch_page_is_mapped)
+			bridge->driver->agp_unmap_page(NULL, bridge->scratch_page_dma, 1);
+
 		bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 						 AGP_PAGE_DESTROY_UNMAP);
 		flush_agp_mappings();
@@ -217,6 +233,9 @@ static void agp_backend_cleanup(struct agp_bridge_data *bridge)
 
 	if (bridge->driver->agp_destroy_page &&
 	    bridge->driver->needs_scratch_page) {
+		if (bridge->scratch_page_is_mapped)
+			bridge->driver->agp_unmap_page(NULL, bridge->scratch_page_dma, 1);
+
 		bridge->driver->agp_destroy_page(gart_to_virt(bridge->scratch_page_real),
 						 AGP_PAGE_DESTROY_UNMAP);
 		flush_agp_mappings();
diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c
index 8c67b4f..d2a502c 100644
--- a/drivers/char/agp/generic.c
+++ b/drivers/char/agp/generic.c
@@ -422,6 +422,14 @@ int agp_bind_memory(struct agp_memory *curr, off_t pg_start)
 		curr->bridge->driver->cache_flush();
 		curr->is_flushed = TRUE;
 	}
+
+	if (curr->bridge->driver->agp_require_remapping &&
+			curr->bridge->driver->agp_require_remapping()) {
+		ret_val = curr->bridge->driver->agp_map_page(curr, NULL, NULL, 0);
+		if (ret_val)
+			return ret_val;
+	}
+
 	ret_val = curr->bridge->driver->insert_memory(curr, pg_start, curr->type);
 
 	if (ret_val != 0)
@@ -461,6 +469,12 @@ int agp_unbind_memory(struct agp_memory *curr)
 
 	curr->is_bound = FALSE;
 	curr->pg_start = 0;
+
+	if (curr->is_mapped) {
+		if (curr->bridge->driver->agp_unmap_page)
+			curr->bridge->driver->agp_unmap_page(curr, 0, 0);
+		curr->is_mapped = FALSE;
+	}
 	return 0;
 }
 EXPORT_SYMBOL(agp_unbind_memory);
diff --git a/include/linux/agp_backend.h b/include/linux/agp_backend.h
index 03e3454..64c7476 100644
--- a/include/linux/agp_backend.h
+++ b/include/linux/agp_backend.h
@@ -87,7 +87,14 @@ struct agp_memory {
 	u32 physical;
 	u8 is_bound;
 	u8 is_flushed;
+	u8 is_mapped;
         u8 vmalloc_flag;
+	/* Following are used in graphics dma remapping case,
+	 * for map/unmap via sg interfaces.
+	 */
+	struct scatterlist *sg_list;
+	int num_sg;
+	u8 sg_vmalloc_flag;
 };
 
 #define AGP_NORMAL_MEMORY 0
-- 
1.5.3.4

[agp-mm][PATCH 3/4][AGP] intel_agp: add support for graphics dma remapping on G33

[Zhenyu Wang]

[agp-mm] [AGP] intel_agp: add support for graphics dma remapping on G33

When graphics dma remapping engine is active, we must fill
gart table with dma address from dmar engine, as now graphics
device access to graphics memory must go through dma remapping
table to get real physical address.

Add support on G33 chipset, which has graphics device specific
dmar engine avaiable.

Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
---
 drivers/char/agp/Kconfig     |   12 +++-
 drivers/char/agp/intel-agp.c |  134 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 141 insertions(+), 5 deletions(-)

diff --git a/drivers/char/agp/Kconfig b/drivers/char/agp/Kconfig
index ccb1fa8..e60c2b0 100644
--- a/drivers/char/agp/Kconfig
+++ b/drivers/char/agp/Kconfig
@@ -74,8 +74,16 @@ config AGP_INTEL
 	  on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850, 860, 875,
 	  E7205 and E7505 chipsets and full support for the 810, 815, 830M,
 	  845G, 852GM, 855GM, 865G and I915 integrated graphics chipsets.
-
-
+	
+config AGP_INTEL_DMAR
+	bool
+	depends on AGP_INTEL && DMAR && !DMAR_GFX_WA
+	default y
+	help
+	  This option will enable graphics address remapping with Intel
+	  DMAR engine aka VT-d if you don't select graphics workaround on
+	  Intel DMAR. The graphics address filled to gart table will be
+	  changed by remapping within graphics DMAR engine for domain.
 
 config AGP_NVIDIA
 	tristate "NVIDIA nForce/nForce2 chipset support"
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index f443682..31a939d 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -9,6 +9,9 @@
 #include <linux/pagemap.h>
 #include <linux/agp_backend.h>
 #include "agp.h"
+#ifdef CONFIG_AGP_INTEL_DMAR
+#include <linux/dmar.h>
+#endif
 
 #define PCI_DEVICE_ID_INTEL_82946GZ_HB      0x2970
 #define PCI_DEVICE_ID_INTEL_82946GZ_IG      0x2972
@@ -123,6 +126,97 @@ static struct _intel_private {
 	struct resource ifp_resource;
 } intel_private;
 
+static int intel_agp_require_remapping(void)
+{
+#ifdef CONFIG_AGP_INTEL_DMAR
+	return intel_iommu_gfx_remapping();
+#else
+	return 0;
+#endif
+}
+
+#ifdef CONFIG_AGP_INTEL_DMAR
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t *ret, int is_single)
+{
+	int i;
+	struct scatterlist *sg;
+
+	if (is_single) {
+		*ret = pci_map_single(intel_private.pcidev, virt, PAGE_SIZE,
+				PCI_DMA_BIDIRECTIONAL);
+		if (pci_dma_mapping_error(*ret))
+			return -EINVAL;
+		return 0;
+	}
+
+	DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
+
+	if ((mem->page_count * sizeof(*mem->sg_list)) < 2*PAGE_SIZE)
+	    mem->sg_list = kcalloc(mem->page_count, sizeof(*mem->sg_list), GFP_KERNEL);
+
+	if (mem->sg_list == NULL) {
+	    mem->sg_list = vmalloc(mem->page_count * sizeof(*mem->sg_list));
+	    mem->sg_vmalloc_flag = 1;
+	}
+
+	if (!mem->sg_list) {
+	    mem->sg_vmalloc_flag = 0;
+	    return -ENOMEM;
+	}
+	sg_init_table(mem->sg_list, mem->page_count);
+
+	sg = mem->sg_list;
+	for (i = 0 ; i < mem->page_count; i++, sg = sg_next(sg))
+		sg_set_buf(sg, gart_to_virt(mem->memory[i]), PAGE_SIZE);
+
+	mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
+					mem->page_count, PCI_DMA_BIDIRECTIONAL);
+	if (mem->num_sg == 0) {
+		if (mem->sg_vmalloc_flag)
+			vfree(mem->sg_list);
+		else
+			kfree(mem->sg_list);
+		mem->sg_list = NULL;
+		mem->sg_vmalloc_flag = 0;
+		return -ENOMEM;
+	}
+	mem->is_mapped = TRUE;
+	return 0;
+}
+
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int is_single)
+{
+	if (is_single) {
+		pci_unmap_single(intel_private.pcidev, addr, PAGE_SIZE,
+				PCI_DMA_BIDIRECTIONAL);
+		return;
+	}
+	DBG("try unmapping %lu pages\n", (unsigned long)mem->page_count);
+
+	if (!mem->is_mapped)
+		return;
+
+	if (mem->num_sg)
+		pci_unmap_sg(intel_private.pcidev, mem->sg_list, mem->page_count,
+				PCI_DMA_BIDIRECTIONAL);
+	if (mem->sg_vmalloc_flag)
+		vfree(mem->sg_list);
+	else
+		kfree(mem->sg_list);
+	mem->sg_vmalloc_flag = 0;
+	mem->sg_list = NULL;
+	mem->num_sg = 0;
+}
+#else
+static int intel_agp_map_pages(struct agp_memory *mem, void *virt, dma_addr_t *ret, int is_single)
+{
+    return 0;
+}
+static void intel_agp_unmap_pages(struct agp_memory *mem, dma_addr_t addr, int is_single)
+{
+}
+#endif
+
 static int intel_i810_fetch_size(void)
 {
 	u32 smram_miscc;
@@ -991,9 +1085,40 @@ static int intel_i915_insert_entries(struct agp_memory *mem,off_t pg_start,
 	if (!mem->is_flushed)
 		global_cache_flush();
 
-	for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
-		writel(agp_bridge->driver->mask_memory(agp_bridge,
-			mem->memory[i], mask_type), intel_private.gtt+j);
+	/* map mem if current graphics dmar engine is active
+	 * and workaround is not applied. */
+	if (mem->is_mapped) {
+		struct scatterlist *sg;
+
+		j = pg_start;
+		if (mem->num_sg == mem->page_count) {
+			for_each_sg(mem->sg_list, sg, mem->page_count, i) {
+				writel(agp_bridge->driver->mask_memory(agp_bridge,
+							sg_dma_address(sg),
+							mask_type),
+						intel_private.gtt+j);
+				j++;
+			}
+		} else {
+			/* sg may merge pages, but we have to seperate
+			 * per-page addr for GTT */
+			unsigned int len, m;
+			for_each_sg(mem->sg_list, sg, mem->num_sg, i) {
+				len = sg_dma_len(sg) / PAGE_SIZE;
+				for (m = 0; m < len; m++) {
+					writel(agp_bridge->driver->mask_memory(agp_bridge,
+								sg_dma_address(sg) + m * PAGE_SIZE, mask_type),
+							intel_private.gtt+j);
+					j++;
+				}
+			}
+		}
+	} else {
+		for (i = 0, j = pg_start; i < mem->page_count; i++, j++) {
+			writel(agp_bridge->driver->mask_memory(agp_bridge,
+						mem->memory[i], mask_type),
+					intel_private.gtt+j);
+		}
 	}
 
 	readl(intel_private.gtt+j-1);
@@ -1947,6 +2072,9 @@ static const struct agp_bridge_driver intel_g33_driver = {
 	.agp_destroy_page       = agp_generic_destroy_page,
 	.agp_type_to_mask_type  = intel_i830_type_to_mask_type,
 	.chipset_flush		= intel_i915_chipset_flush,
+	.agp_require_remapping  = intel_agp_require_remapping,
+	.agp_map_page		= intel_agp_map_pages,
+	.agp_unmap_page		= intel_agp_unmap_pages,
 };
 
 static int find_gmch(u16 device)
-- 
1.5.3.4

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

On 2007.12.19 13:26:08 +0000, Zhenyu Wang wrote:
> 
> [agp-mm] [intel_iommu] explicit export current graphics dmar status
> 
> To make it possbile to tell other modules about curent
> graphics dmar engine status, that could decide if graphics
> driver should remap physical address to dma address.
> 
> Also this one trys to make dmar_disabled really present
> current status of DMAR, which would be used for completely
> express graphics dmar engine is active or not.
> 
> Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
> ---
>  drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
>  include/linux/dmar.h      |    6 ++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index e079a52..81a0abc 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -53,7 +53,7 @@
>  static void domain_remove_dev_info(struct dmar_domain *domain);
>  
>  static int dmar_disabled;
> -static int __initdata dmar_map_gfx = 1;
> +static int dmar_map_gfx = 1;
>  static int dmar_forcedac;
>  
>  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
>  }
>  __setup("intel_iommu=", intel_iommu_setup);
>  
> +int intel_iommu_gfx_remapping(void)
> +{
> +#ifndef CONFIG_DMAR_GFX_WA
> +	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> +		return 1;
> +#endif
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> +
>  static struct kmem_cache *iommu_domain_cache;
>  static struct kmem_cache *iommu_devinfo_cache;
>  static struct kmem_cache *iommu_iova_cache;
> @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
>  
>  void __init detect_intel_iommu(void)
>  {
> -	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> +	if (dmar_disabled)
> +		return;
> +	if (swiotlb || no_iommu || iommu_detected) {
> +		dmar_disabled = 1;
>  		return;
> +	}
>  	if (early_dmar_detect()) {
>  		iommu_detected = 1;
>  	}
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index ffb6439..8ae435e 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
>  extern int dmar_table_init(void);
>  extern int early_dmar_detect(void);
>  
> +extern int intel_iommu_gfx_remapping(void);
> +
>  extern struct list_head dmar_drhd_units;
>  extern struct list_head dmar_rmrr_units;
>  
> @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
>  {
>  	return -ENODEV;
>  }
> +static inline int intel_iommu_gfx_remapping(void)
> +{
> +	return 0;
> +}
>  
>  #endif /* !CONFIG_DMAR */
>  #endif /* __DMAR_H__ */
> -- 
> 1.5.3.4
> 

Any comments to this one? Is it ok to push this iommu patch with
agp dma remapping patches to next -mm?

Thanks.

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[mark gross]

Sorry for the late reply.
comments below...


On Fri, Jan 04, 2008 at 09:53:38AM +0800, Zhenyu Wang wrote:
> On 2007.12.19 13:26:08 +0000, Zhenyu Wang wrote:
> > 
> > [agp-mm] [intel_iommu] explicit export current graphics dmar status
> > 
> > To make it possbile to tell other modules about curent
> > graphics dmar engine status, that could decide if graphics
> > driver should remap physical address to dma address.
> > 
> > Also this one trys to make dmar_disabled really present
> > current status of DMAR, which would be used for completely
> > express graphics dmar engine is active or not.

do you think this exporting will be needed?
If so why and when would someone use it?

> > 
> > Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
> > ---
> >  drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
> >  include/linux/dmar.h      |    6 ++++++
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > index e079a52..81a0abc 100644
> > --- a/drivers/pci/intel-iommu.c
> > +++ b/drivers/pci/intel-iommu.c
> > @@ -53,7 +53,7 @@
> >  static void domain_remove_dev_info(struct dmar_domain *domain);
> >  
> >  static int dmar_disabled;
> > -static int __initdata dmar_map_gfx = 1;
> > +static int dmar_map_gfx = 1;
> >  static int dmar_forcedac;
> >  
> >  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> > @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
> >  }
> >  __setup("intel_iommu=", intel_iommu_setup);
> >  
> > +int intel_iommu_gfx_remapping(void)
> > +{
> > +#ifndef CONFIG_DMAR_GFX_WA
> > +	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> > +		return 1;
> > +#endif
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> > +
> >  static struct kmem_cache *iommu_domain_cache;
> >  static struct kmem_cache *iommu_devinfo_cache;
> >  static struct kmem_cache *iommu_iova_cache;
> > @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
> >  
> >  void __init detect_intel_iommu(void)
> >  {
> > -	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> > +	if (dmar_disabled)
> > +		return;
> > +	if (swiotlb || no_iommu || iommu_detected) {
> > +		dmar_disabled = 1;
> >  		return;

Why the bloat?  This block of 7 lines looks like it does the same as the
2 you replaced.

> > +	}
> >  	if (early_dmar_detect()) {
> >  		iommu_detected = 1;
> >  	}
> > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > index ffb6439..8ae435e 100644
> > --- a/include/linux/dmar.h
> > +++ b/include/linux/dmar.h
> > @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
> >  extern int dmar_table_init(void);
> >  extern int early_dmar_detect(void);
> >  
> > +extern int intel_iommu_gfx_remapping(void);
> > +
> >  extern struct list_head dmar_drhd_units;
> >  extern struct list_head dmar_rmrr_units;
> >  
> > @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
> >  {
> >  	return -ENODEV;
> >  }
> > +static inline int intel_iommu_gfx_remapping(void)
> > +{
> > +	return 0;
> > +}

Um this function is silly lets not post it.

> >  
> >  #endif /* !CONFIG_DMAR */
> >  #endif /* __DMAR_H__ */
> > -- 
> > 1.5.3.4
> > 
> 
> Any comments to this one? Is it ok to push this iommu patch with
> agp dma remapping patches to next -mm?
> 

Its not ready for posting.

--mgross

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

Mark, sorry for missing this for long time...

On 2008.01.08 12:44:20 -0800, mark gross wrote:
> > > 
> > > [agp-mm] [intel_iommu] explicit export current graphics dmar status
> > > 
> > > To make it possbile to tell other modules about curent
> > > graphics dmar engine status, that could decide if graphics
> > > driver should remap physical address to dma address.
> > > 
> > > Also this one trys to make dmar_disabled really present
> > > current status of DMAR, which would be used for completely
> > > express graphics dmar engine is active or not.
> 
> do you think this exporting will be needed?
> If so why and when would someone use it?

This is used for our graphics driver module to know if we have to
do dma remapping in iommu case, both in kernel config and kernel
boot time param config. 

The simplest case is that DMAR_GFX_WA is on, which no dma mapping
will act in intel_agp.

If no DMAR_GFX_WA, we still have boot param to turn off whole intel
iommu (intel_iommu=off), just turn off graphics remap engine
(intel_iommu=igfx_off). So this exported interface is used to know
that in runtime.

> 
> > > 
> > > Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
> > > ---
> > >  drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
> > >  include/linux/dmar.h      |    6 ++++++
> > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > > index e079a52..81a0abc 100644
> > > --- a/drivers/pci/intel-iommu.c
> > > +++ b/drivers/pci/intel-iommu.c
> > > @@ -53,7 +53,7 @@
> > >  static void domain_remove_dev_info(struct dmar_domain *domain);
> > >  
> > >  static int dmar_disabled;
> > > -static int __initdata dmar_map_gfx = 1;
> > > +static int dmar_map_gfx = 1;
> > >  static int dmar_forcedac;
> > >  
> > >  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> > > @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
> > >  }
> > >  __setup("intel_iommu=", intel_iommu_setup);
> > >  
> > > +int intel_iommu_gfx_remapping(void)
> > > +{
> > > +#ifndef CONFIG_DMAR_GFX_WA
> > > +	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> > > +		return 1;
> > > +#endif
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> > > +
> > >  static struct kmem_cache *iommu_domain_cache;
> > >  static struct kmem_cache *iommu_devinfo_cache;
> > >  static struct kmem_cache *iommu_iova_cache;
> > > @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
> > >  
> > >  void __init detect_intel_iommu(void)
> > >  {
> > > -	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> > > +	if (dmar_disabled)
> > > +		return;
> > > +	if (swiotlb || no_iommu || iommu_detected) {
> > > +		dmar_disabled = 1;
> > >  		return;
> 
> Why the bloat?  This block of 7 lines looks like it does the same as the
> 2 you replaced.

No, dmar_disabled is not set in origin, which can't tell if it's turned
off with (intel_iommu=off) later.

> 
> > > +	}
> > >  	if (early_dmar_detect()) {
> > >  		iommu_detected = 1;
> > >  	}
> > > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > > index ffb6439..8ae435e 100644
> > > --- a/include/linux/dmar.h
> > > +++ b/include/linux/dmar.h
> > > @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
> > >  extern int dmar_table_init(void);
> > >  extern int early_dmar_detect(void);
> > >  
> > > +extern int intel_iommu_gfx_remapping(void);
> > > +
> > >  extern struct list_head dmar_drhd_units;
> > >  extern struct list_head dmar_rmrr_units;
> > >  
> > > @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
> > >  {
> > >  	return -ENODEV;
> > >  }
> > > +static inline int intel_iommu_gfx_remapping(void)
> > > +{
> > > +	return 0;
> > > +}
> 
> Um this function is silly lets not post it.
> 
> > >  
> > >  #endif /* !CONFIG_DMAR */
> > >  #endif /* __DMAR_H__ */
> > > -- 
> > > 1.5.3.4
> > > 
> > 
> > Any comments to this one? Is it ok to push this iommu patch with
> > agp dma remapping patches to next -mm?
> > 
> 
> Its not ready for posting.
> 
> --mgross

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[mark gross]

On Fri, Jan 25, 2008 at 11:02:55AM +0800, Zhenyu Wang wrote:
> 
> Mark, sorry for missing this for long time...
> 
> On 2008.01.08 12:44:20 -0800, mark gross wrote:
> > > > 
> > > > [agp-mm] [intel_iommu] explicit export current graphics dmar status
> > > > 
> > > > To make it possbile to tell other modules about curent
> > > > graphics dmar engine status, that could decide if graphics
> > > > driver should remap physical address to dma address.
> > > > 
> > > > Also this one trys to make dmar_disabled really present
> > > > current status of DMAR, which would be used for completely
> > > > express graphics dmar engine is active or not.
> > 
> > do you think this exporting will be needed?
> > If so why and when would someone use it?
> 
> This is used for our graphics driver module to know if we have to
> do dma remapping in iommu case, both in kernel config and kernel
> boot time param config. 

ok. How soon do you need this export?

> 
> The simplest case is that DMAR_GFX_WA is on, which no dma mapping
> will act in intel_agp.
> 
> If no DMAR_GFX_WA, we still have boot param to turn off whole intel
> iommu (intel_iommu=off), just turn off graphics remap engine
> (intel_iommu=igfx_off). So this exported interface is used to know
> that in runtime.
> 
> > 
> > > > 
> > > > Signed-off-by: Zhenyu Wang <zhenyu.z.wang@intel.com>
> > > > ---
> > > >  drivers/pci/intel-iommu.c |   18 ++++++++++++++++--
> > > >  include/linux/dmar.h      |    6 ++++++
> > > >  2 files changed, 22 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> > > > index e079a52..81a0abc 100644
> > > > --- a/drivers/pci/intel-iommu.c
> > > > +++ b/drivers/pci/intel-iommu.c
> > > > @@ -53,7 +53,7 @@
> > > >  static void domain_remove_dev_info(struct dmar_domain *domain);
> > > >  
> > > >  static int dmar_disabled;
> > > > -static int __initdata dmar_map_gfx = 1;
> > > > +static int dmar_map_gfx = 1;
> > > >  static int dmar_forcedac;
> > > >  
> > > >  #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
> > > > @@ -86,6 +86,16 @@ static int __init intel_iommu_setup(char *str)
> > > >  }
> > > >  __setup("intel_iommu=", intel_iommu_setup);
> > > >  
> > > > +int intel_iommu_gfx_remapping(void)
> > > > +{
> > > > +#ifndef CONFIG_DMAR_GFX_WA
> > > > +	if (!dmar_disabled && iommu_detected && dmar_map_gfx)
> > > > +		return 1;
> > > > +#endif
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(intel_iommu_gfx_remapping);
> > > > +
> > > >  static struct kmem_cache *iommu_domain_cache;
> > > >  static struct kmem_cache *iommu_devinfo_cache;
> > > >  static struct kmem_cache *iommu_iova_cache;
> > > > @@ -2189,8 +2199,12 @@ static void __init iommu_exit_mempool(void)
> > > >  
> > > >  void __init detect_intel_iommu(void)
> > > >  {
> > > > -	if (swiotlb || no_iommu || iommu_detected || dmar_disabled)
> > > > +	if (dmar_disabled)
> > > > +		return;
> > > > +	if (swiotlb || no_iommu || iommu_detected) {
> > > > +		dmar_disabled = 1;
> > > >  		return;
> > 
> > Why the bloat?  This block of 7 lines looks like it does the same as the
> > 2 you replaced.
> 
> No, dmar_disabled is not set in origin, which can't tell if it's turned
> off with (intel_iommu=off) later.
> 

oops, I missed that how about something like

if (swiotlb || no_iommu || iommu_detected || dmar_disabled) {
	dmar_disabled = 1;
	return;
}

I guess your way is ok too.

> > 
> > > > +	}
> > > >  	if (early_dmar_detect()) {
> > > >  		iommu_detected = 1;
> > > >  	}
> > > > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > > > index ffb6439..8ae435e 100644
> > > > --- a/include/linux/dmar.h
> > > > +++ b/include/linux/dmar.h
> > > > @@ -47,6 +47,8 @@ extern int intel_iommu_init(void);
> > > >  extern int dmar_table_init(void);
> > > >  extern int early_dmar_detect(void);
> > > >  
> > > > +extern int intel_iommu_gfx_remapping(void);
> > > > +
> > > >  extern struct list_head dmar_drhd_units;
> > > >  extern struct list_head dmar_rmrr_units;
> > > >  
> > > > @@ -81,6 +83,10 @@ static inline int intel_iommu_init(void)
> > > >  {
> > > >  	return -ENODEV;
> > > >  }
> > > > +static inline int intel_iommu_gfx_remapping(void)
> > > > +{
> > > > +	return 0;
> > > > +}
> > 
> > Um this function is silly lets not post it.
> > 
> > > >  
> > > >  #endif /* !CONFIG_DMAR */
> > > >  #endif /* __DMAR_H__ */
> > > > -- 
> > > > 1.5.3.4
> > > > 
> > > 
> > > Any comments to this one? Is it ok to push this iommu patch with
> > > agp dma remapping patches to next -mm?
> > > 
> > 
> > Its not ready for posting.

send me your current patch and eta for the graphics module patches so I
can better coordinate with you.

--mgross

Re: [agp-mm][PATCH 1/4][intel_iommu] explicit export current graphics dmar status

[Zhenyu Wang]

On 2008.01.25 10:08:20 -0800, mark gross wrote:
> > This is used for our graphics driver module to know if we have to
> > do dma remapping in iommu case, both in kernel config and kernel
> > boot time param config. 
> 
> ok. How soon do you need this export?
> 

As graphics part of these patches depend on this one, it should go
first, or along with agpgart patches, that should first be in Airlie's
agp-mm queue.

> send me your current patch and eta for the graphics module patches so I
> can better coordinate with you.

You can find my current patches against agp-mm tree (so equally against
current -mm kernel) here, http://people.freedesktop.org/~zhen/, fixed
with some warnings from checkpatch.