[PATCH 0/23] AMD IOMMU 2.6.28 updates for review

[PATCH 0/23] AMD IOMMU 2.6.28 updates for review

[Joerg Roedel]

Hi all,

this is the patch series with all 2.6.28 updates I currently have for
the AMD IOMMU driver. The highlights in this series are the
implementation of lazy IOTLB flushing which increases performance and
code for AMD IOMMU event handling. Any events the hardware will send are
now visible. There are also a lot of minor fixes and cleanups which came
up during code review and LKML discussions from various persons. The
patches are relative to current tip/master branch.
Please review.

Here is the shortlog and the diffstat:

FUJITA Tomonori (1):
      AMD IOMMU: avoid unnecessary low zone allocation in alloc_coherent

Joerg Roedel (22):
      AMD IOMMU: check for invalid device pointers
      AMD IOMMU: move TLB flushing to the map/unmap helper functions
      AMD IOMMU: implement lazy IO/TLB flushing
      AMD IOMMU: add branch hints to completion wait checks
      AMD IOMMU: align alloc_coherent addresses properly
      AMD IOMMU: add event buffer allocation
      AMD IOMMU: save pci segment from ACPI tables
      AMD IOMMU: save pci_dev instead of devid
      AMD IOMMU: add MSI interrupt support
      AMD IOMMU: add event handling code
      AMD IOMMU: enable event logging
      AMD IOMMU: allow IO page faults from devices
      AMD IOMMU: add dma_supported callback
      AMD IOMMU: don't assing preallocated protection domains to devices
      AMD IOMMU: some set_device_domain cleanups
      AMD IOMMU: replace memset with __GFP_ZERO in alloc_coherent
      AMD IOMMU: simplify dma_mask_to_pages
      AMD IOMMU: free domain bitmap with its allocation order
      AMD IOMMU: remove unnecessary cast to u64 in the init code
      AMD IOMMU: calculate IVHD size with a function
      AMD IOMMU: use cmd_buf_size when freeing the command buffer
      add AMD IOMMU tree to MAINTAINERS file

 Documentation/kernel-parameters.txt |    5 +
 MAINTAINERS                         |    1 +
 arch/x86/Kconfig                    |    1 +
 arch/x86/kernel/amd_iommu.c         |  301 +++++++++++++++++++++++++++++------
 arch/x86/kernel/amd_iommu_init.c    |  194 ++++++++++++++++++++--
 include/asm-x86/amd_iommu.h         |    3 +
 include/asm-x86/amd_iommu_types.h   |   64 +++++++-
 7 files changed, 497 insertions(+), 72 deletions(-)

Thanks,

Joerg

[PATCH 01/23] AMD IOMMU: check for invalid device pointers

[Joerg Roedel]

Currently AMD IOMMU code triggers a BUG_ON if NULL is passed as the
device. This is inconsistent with other IOMMU implementations. This
patch fixes it.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   43 +++++++++++++++++++++++++++++++++++--------
 1 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 01c68c3..695e0fc 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -646,6 +646,18 @@ static void set_device_domain(struct amd_iommu *iommu,
  *****************************************************************************/
 
 /*
+ * This function checks if the driver got a valid device from the caller to
+ * avoid dereferencing invalid pointers.
+ */
+static bool check_device(struct device *dev)
+{
+	if (!dev || !dev->dma_mask)
+		return false;
+
+	return true;
+}
+
+/*
  * In the dma_ops path we only have the struct device. This function
  * finds the corresponding IOMMU, the protection domain and the
  * requestor id for a given device.
@@ -661,18 +673,19 @@ static int get_device_resources(struct device *dev,
 	struct pci_dev *pcidev;
 	u16 _bdf;
 
-	BUG_ON(!dev || dev->bus != &pci_bus_type || !dev->dma_mask);
+	*iommu = NULL;
+	*domain = NULL;
+	*bdf = 0xffff;
+
+	if (dev->bus != &pci_bus_type)
+		return 0;
 
 	pcidev = to_pci_dev(dev);
 	_bdf = calc_devid(pcidev->bus->number, pcidev->devfn);
 
 	/* device not translated by any IOMMU in the system? */
-	if (_bdf > amd_iommu_last_bdf) {
-		*iommu = NULL;
-		*domain = NULL;
-		*bdf = 0xffff;
+	if (_bdf > amd_iommu_last_bdf)
 		return 0;
-	}
 
 	*bdf = amd_iommu_alias_table[_bdf];
 
@@ -826,6 +839,9 @@ static dma_addr_t map_single(struct device *dev, phys_addr_t paddr,
 	u16 devid;
 	dma_addr_t addr;
 
+	if (!check_device(dev))
+		return bad_dma_address;
+
 	get_device_resources(dev, &iommu, &domain, &devid);
 
 	if (iommu == NULL || domain == NULL)
@@ -860,7 +876,8 @@ static void unmap_single(struct device *dev, dma_addr_t dma_addr,
 	struct protection_domain *domain;
 	u16 devid;
 
-	if (!get_device_resources(dev, &iommu, &domain, &devid))
+	if (!check_device(dev) ||
+	    !get_device_resources(dev, &iommu, &domain, &devid))
 		/* device not handled by any AMD IOMMU */
 		return;
 
@@ -910,6 +927,9 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 	phys_addr_t paddr;
 	int mapped_elems = 0;
 
+	if (!check_device(dev))
+		return 0;
+
 	get_device_resources(dev, &iommu, &domain, &devid);
 
 	if (!iommu || !domain)
@@ -967,7 +987,8 @@ static void unmap_sg(struct device *dev, struct scatterlist *sglist,
 	u16 devid;
 	int i;
 
-	if (!get_device_resources(dev, &iommu, &domain, &devid))
+	if (!check_device(dev) ||
+	    !get_device_resources(dev, &iommu, &domain, &devid))
 		return;
 
 	spin_lock_irqsave(&domain->lock, flags);
@@ -999,6 +1020,9 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	u16 devid;
 	phys_addr_t paddr;
 
+	if (!check_device(dev))
+		return NULL;
+
 	virt_addr = (void *)__get_free_pages(flag, get_order(size));
 	if (!virt_addr)
 		return 0;
@@ -1047,6 +1071,9 @@ static void free_coherent(struct device *dev, size_t size,
 	struct protection_domain *domain;
 	u16 devid;
 
+	if (!check_device(dev))
+		return;
+
 	get_device_resources(dev, &iommu, &domain, &devid);
 
 	if (!iommu || !domain)
-- 
1.5.6.4

[PATCH 02/23] AMD IOMMU: move TLB flushing to the map/unmap helper functions

[Joerg Roedel]

This patch moves the invocation of the flushing functions to the
map/unmap helper functions because its common code in all dma_ops
relevant mapping/unmapping functions.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   19 +++++--------------
 1 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 695e0fc..691e023 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -795,6 +795,9 @@ static dma_addr_t __map_single(struct device *dev,
 	}
 	address += offset;
 
+	if (unlikely(iommu_has_npcache(iommu)))
+		iommu_flush_pages(iommu, dma_dom->domain.id, address, size);
+
 out:
 	return address;
 }
@@ -825,6 +828,8 @@ static void __unmap_single(struct amd_iommu *iommu,
 	}
 
 	dma_ops_free_addresses(dma_dom, dma_addr, pages);
+
+	iommu_flush_pages(iommu, dma_dom->domain.id, dma_addr, size);
 }
 
 /*
@@ -853,9 +858,6 @@ static dma_addr_t map_single(struct device *dev, phys_addr_t paddr,
 	if (addr == bad_dma_address)
 		goto out;
 
-	if (iommu_has_npcache(iommu))
-		iommu_flush_pages(iommu, domain->id, addr, size);
-
 	if (iommu->need_sync)
 		iommu_completion_wait(iommu);
 
@@ -885,8 +887,6 @@ static void unmap_single(struct device *dev, dma_addr_t dma_addr,
 
 	__unmap_single(iommu, domain->priv, dma_addr, size, dir);
 
-	iommu_flush_pages(iommu, domain->id, dma_addr, size);
-
 	if (iommu->need_sync)
 		iommu_completion_wait(iommu);
 
@@ -948,9 +948,6 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 			mapped_elems++;
 		} else
 			goto unmap;
-		if (iommu_has_npcache(iommu))
-			iommu_flush_pages(iommu, domain->id, s->dma_address,
-					  s->dma_length);
 	}
 
 	if (iommu->need_sync)
@@ -996,8 +993,6 @@ static void unmap_sg(struct device *dev, struct scatterlist *sglist,
 	for_each_sg(sglist, s, nelems, i) {
 		__unmap_single(iommu, domain->priv, s->dma_address,
 			       s->dma_length, dir);
-		iommu_flush_pages(iommu, domain->id, s->dma_address,
-				  s->dma_length);
 		s->dma_address = s->dma_length = 0;
 	}
 
@@ -1048,9 +1043,6 @@ static void *alloc_coherent(struct device *dev, size_t size,
 		goto out;
 	}
 
-	if (iommu_has_npcache(iommu))
-		iommu_flush_pages(iommu, domain->id, *dma_addr, size);
-
 	if (iommu->need_sync)
 		iommu_completion_wait(iommu);
 
@@ -1082,7 +1074,6 @@ static void free_coherent(struct device *dev, size_t size,
 	spin_lock_irqsave(&domain->lock, flags);
 
 	__unmap_single(iommu, domain->priv, dma_addr, size, DMA_BIDIRECTIONAL);
-	iommu_flush_pages(iommu, domain->id, dma_addr, size);
 
 	if (iommu->need_sync)
 		iommu_completion_wait(iommu);
-- 
1.5.6.4

[PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

The IO/TLB flushing on every unmaping operation is the most expensive
part there and not strictly necessary. It is sufficient to do the flush
before any entries are reused. This is patch implements lazy IO/TLB
flushing which does exactly this.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 Documentation/kernel-parameters.txt |    5 +++++
 arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
 arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
 include/asm-x86/amd_iommu_types.h   |    9 +++++++++
 4 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c2e00ee..5f0aefe 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
 			isolate - enable device isolation (each device, as far
 			          as possible, will get its own protection
 			          domain)
+			unmap_flush - enable flushing of IO/TLB entries when
+			              they are unmapped. Otherwise they are
+				      flushed before they will be reused, which
+				      is a lot of faster
+
 	amd_iommu_size= [HW,X86-64]
 			Define the size of the aperture for the AMD IOMMU
 			driver. Possible values are:
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 691e023..0569098 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -203,6 +203,14 @@ static int iommu_flush_pages(struct amd_iommu *iommu, u16 domid,
 	return 0;
 }
 
+/* Flush the whole IO/TLB for a given protection domain */
+static void iommu_flush_tlb(struct amd_iommu *iommu, u16 domid)
+{
+	u64 address = CMD_INV_IOMMU_ALL_PAGES_ADDRESS;
+
+	iommu_queue_inv_iommu_pages(iommu, address, domid, 0, 1);
+}
+
 /****************************************************************************
  *
  * The functions below are used the create the page table mappings for
@@ -386,14 +394,18 @@ static unsigned long dma_ops_alloc_addresses(struct device *dev,
 			PAGE_SIZE) >> PAGE_SHIFT;
 	limit = limit < size ? limit : size;
 
-	if (dom->next_bit >= limit)
+	if (dom->next_bit >= limit) {
 		dom->next_bit = 0;
+		dom->need_flush = true;
+	}
 
 	address = iommu_area_alloc(dom->bitmap, limit, dom->next_bit, pages,
 			0 , boundary_size, 0);
-	if (address == -1)
+	if (address == -1) {
 		address = iommu_area_alloc(dom->bitmap, limit, 0, pages,
 				0, boundary_size, 0);
+		dom->need_flush = true;
+	}
 
 	if (likely(address != -1)) {
 		dom->next_bit = address + pages;
@@ -553,6 +565,8 @@ static struct dma_ops_domain *dma_ops_domain_alloc(struct amd_iommu *iommu,
 	dma_dom->bitmap[0] = 1;
 	dma_dom->next_bit = 0;
 
+	dma_dom->need_flush = false;
+
 	/* Intialize the exclusion range if necessary */
 	if (iommu->exclusion_start &&
 	    iommu->exclusion_start < dma_dom->aperture_size) {
@@ -795,7 +809,10 @@ static dma_addr_t __map_single(struct device *dev,
 	}
 	address += offset;
 
-	if (unlikely(iommu_has_npcache(iommu)))
+	if (unlikely(dma_dom->need_flush && !amd_iommu_unmap_flush)) {
+		iommu_flush_tlb(iommu, dma_dom->domain.id);
+		dma_dom->need_flush = false;
+	} else if (unlikely(iommu_has_npcache(iommu)))
 		iommu_flush_pages(iommu, dma_dom->domain.id, address, size);
 
 out:
@@ -829,7 +846,8 @@ static void __unmap_single(struct amd_iommu *iommu,
 
 	dma_ops_free_addresses(dma_dom, dma_addr, pages);
 
-	iommu_flush_pages(iommu, dma_dom->domain.id, dma_addr, size);
+	if (amd_iommu_unmap_flush)
+		iommu_flush_pages(iommu, dma_dom->domain.id, dma_addr, size);
 }
 
 /*
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index a69cc0f..da631ab 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -121,6 +121,7 @@ LIST_HEAD(amd_iommu_unity_map);		/* a list of required unity mappings
 					   we find in ACPI */
 unsigned amd_iommu_aperture_order = 26; /* size of aperture in power of 2 */
 int amd_iommu_isolate;			/* if 1, device isolation is enabled */
+bool amd_iommu_unmap_flush;		/* if true, flush on every unmap */
 
 LIST_HEAD(amd_iommu_list);		/* list of all AMD IOMMUs in the
 					   system */
@@ -995,6 +996,11 @@ int __init amd_iommu_init(void)
 	else
 		printk("disabled\n");
 
+	if (amd_iommu_unmap_flush)
+		printk(KERN_INFO "AMD IOMMU: IO/TLB flush on unmap enabled\n");
+	else
+		printk(KERN_INFO "AMD IOMMU: Lazy IO/TLB flushing enabled\n");
+
 out:
 	return ret;
 
@@ -1057,8 +1063,10 @@ void __init amd_iommu_detect(void)
 static int __init parse_amd_iommu_options(char *str)
 {
 	for (; *str; ++str) {
-		if (strcmp(str, "isolate") == 0)
+		if (strncmp(str, "isolate", 7) == 0)
 			amd_iommu_isolate = 1;
+		if (strncmp(str, "unmap_flush", 11) == 0)
+			amd_iommu_unmap_flush = true;
 	}
 
 	return 1;
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 1ffa4e5..82a26fb 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -196,6 +196,9 @@ struct dma_ops_domain {
 	 * just calculate its address in constant time.
 	 */
 	u64 **pte_pages;
+
+	/* This will be set to true when TLB needs to be flushed */
+	bool need_flush;
 };
 
 /*
@@ -322,6 +325,12 @@ extern unsigned long *amd_iommu_pd_alloc_bitmap;
 /* will be 1 if device isolation is enabled */
 extern int amd_iommu_isolate;
 
+/*
+ * If true, the addresses will be flushed on unmap time, not when
+ * they are reused
+ */
+extern bool amd_iommu_unmap_flush;
+
 /* takes a PCI device id and prints it out in a readable form */
 static inline void print_devid(u16 devid, int nl)
 {
-- 
1.5.6.4

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[FUJITA Tomonori]

On Wed, 17 Sep 2008 18:52:37 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> The IO/TLB flushing on every unmaping operation is the most expensive
> part there and not strictly necessary. It is sufficient to do the flush
> before any entries are reused. This is patch implements lazy IO/TLB
> flushing which does exactly this.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  Documentation/kernel-parameters.txt |    5 +++++
>  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
>  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
>  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
>  4 files changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index c2e00ee..5f0aefe 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
>  			isolate - enable device isolation (each device, as far
>  			          as possible, will get its own protection
>  			          domain)
> +			unmap_flush - enable flushing of IO/TLB entries when
> +			              they are unmapped. Otherwise they are
> +				      flushed before they will be reused, which
> +				      is a lot of faster
> +

Would it be nice to have consistency of IOMMU parameters?

VT-d also has the kernel-boot option for this lazy flushing trick
though VT-d 'strict' option is more vague than 'unmap_flush'

It would be also nice to have consistency of IOMMU behavior.

VT-d enables the lazy flushing trick by default and has the boot
option to disable it.

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> On Wed, 17 Sep 2008 18:52:37 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > The IO/TLB flushing on every unmaping operation is the most expensive
> > part there and not strictly necessary. It is sufficient to do the flush
> > before any entries are reused. This is patch implements lazy IO/TLB
> > flushing which does exactly this.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  Documentation/kernel-parameters.txt |    5 +++++
> >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> >  4 files changed, 45 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > index c2e00ee..5f0aefe 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> >  			isolate - enable device isolation (each device, as far
> >  			          as possible, will get its own protection
> >  			          domain)
> > +			unmap_flush - enable flushing of IO/TLB entries when
> > +			              they are unmapped. Otherwise they are
> > +				      flushed before they will be reused, which
> > +				      is a lot of faster
> > +
> 
> Would it be nice to have consistency of IOMMU parameters?

True. We should merge common parameters across IOMMUs into the
iommu= parameter some time in the future, I think. It would also be the
place for the IOMMU size parameter.

> VT-d also has the kernel-boot option for this lazy flushing trick
> though VT-d 'strict' option is more vague than 'unmap_flush'
> 
> It would be also nice to have consistency of IOMMU behavior.
> 
> VT-d enables the lazy flushing trick by default and has the boot
> option to disable it.

This is exactly what AMD IOMMU with this patch does too. The
amd_iommu=unmap_flush parameter disables lazy IO/TLB flushing.


Joerg

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[FUJITA Tomonori]

On Wed, 17 Sep 2008 21:28:27 +0200
Joerg Roedel <joro@8bytes.org> wrote:

> On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 18:52:37 +0200
> > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > 
> > > The IO/TLB flushing on every unmaping operation is the most expensive
> > > part there and not strictly necessary. It is sufficient to do the flush
> > > before any entries are reused. This is patch implements lazy IO/TLB
> > > flushing which does exactly this.
> > > 
> > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > ---
> > >  Documentation/kernel-parameters.txt |    5 +++++
> > >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > index c2e00ee..5f0aefe 100644
> > > --- a/Documentation/kernel-parameters.txt
> > > +++ b/Documentation/kernel-parameters.txt
> > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> > >  			isolate - enable device isolation (each device, as far
> > >  			          as possible, will get its own protection
> > >  			          domain)
> > > +			unmap_flush - enable flushing of IO/TLB entries when
> > > +			              they are unmapped. Otherwise they are
> > > +				      flushed before they will be reused, which
> > > +				      is a lot of faster
> > > +
> > 
> > Would it be nice to have consistency of IOMMU parameters?
> 
> True. We should merge common parameters across IOMMUs into the
> iommu= parameter some time in the future, I think. It would also be the
> place for the IOMMU size parameter.

Hmm, now is better than the future? I think that now you can add
something like 'disable_batching_flush' as a common parameter and
change AMD IOMMU to use it.


> > VT-d also has the kernel-boot option for this lazy flushing trick
> > though VT-d 'strict' option is more vague than 'unmap_flush'
> > 
> > It would be also nice to have consistency of IOMMU behavior.
> > 
> > VT-d enables the lazy flushing trick by default and has the boot
> > option to disable it.
> 
> This is exactly what AMD IOMMU with this patch does too. The
> amd_iommu=unmap_flush parameter disables lazy IO/TLB flushing.

Oops, I misread the description.

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> On Wed, 17 Sep 2008 21:28:27 +0200
> Joerg Roedel <joro@8bytes.org> wrote:
> 
> > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > 
> > > > The IO/TLB flushing on every unmaping operation is the most expensive
> > > > part there and not strictly necessary. It is sufficient to do the flush
> > > > before any entries are reused. This is patch implements lazy IO/TLB
> > > > flushing which does exactly this.
> > > > 
> > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > ---
> > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > index c2e00ee..5f0aefe 100644
> > > > --- a/Documentation/kernel-parameters.txt
> > > > +++ b/Documentation/kernel-parameters.txt
> > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> > > >  			isolate - enable device isolation (each device, as far
> > > >  			          as possible, will get its own protection
> > > >  			          domain)
> > > > +			unmap_flush - enable flushing of IO/TLB entries when
> > > > +			              they are unmapped. Otherwise they are
> > > > +				      flushed before they will be reused, which
> > > > +				      is a lot of faster
> > > > +
> > > 
> > > Would it be nice to have consistency of IOMMU parameters?
> > 
> > True. We should merge common parameters across IOMMUs into the
> > iommu= parameter some time in the future, I think. It would also be the
> > place for the IOMMU size parameter.
> 
> Hmm, now is better than the future? I think that now you can add
> something like 'disable_batching_flush' as a common parameter and
> change AMD IOMMU to use it.

Ok, I introduce a parameter called '[no]fullflush' like the GART already uses.
We can remove this parameter from gart_parse_options() later then.

Joerg

-- 
           |           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

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> On Wed, 17 Sep 2008 21:28:27 +0200
> Joerg Roedel <joro@8bytes.org> wrote:
> 
> > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > 
> > > > The IO/TLB flushing on every unmaping operation is the most expensive
> > > > part there and not strictly necessary. It is sufficient to do the flush
> > > > before any entries are reused. This is patch implements lazy IO/TLB
> > > > flushing which does exactly this.
> > > > 
> > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > ---
> > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > index c2e00ee..5f0aefe 100644
> > > > --- a/Documentation/kernel-parameters.txt
> > > > +++ b/Documentation/kernel-parameters.txt
> > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> > > >  			isolate - enable device isolation (each device, as far
> > > >  			          as possible, will get its own protection
> > > >  			          domain)
> > > > +			unmap_flush - enable flushing of IO/TLB entries when
> > > > +			              they are unmapped. Otherwise they are
> > > > +				      flushed before they will be reused, which
> > > > +				      is a lot of faster
> > > > +
> > > 
> > > Would it be nice to have consistency of IOMMU parameters?
> > 
> > True. We should merge common parameters across IOMMUs into the
> > iommu= parameter some time in the future, I think. It would also be the
> > place for the IOMMU size parameter.
> 
> Hmm, now is better than the future? I think that now you can add
> something like 'disable_batching_flush' as a common parameter and
> change AMD IOMMU to use it.

Ok, I queued the following patch in the AMD IOMMU updates and changed
this patch to use iommu_fullflush instead. Is this patch ok? It changes
the behavior of GART to use lazy flushing by default. But I don't think
that this is a problem.

commit 9769771290fddcfc0362c5d30242151d4eb1cc46
Author: Joerg Roedel <joerg.roedel@amd.com>
Date:   Thu Sep 18 15:23:43 2008 +0200

    x86: move GART TLB flushing options to generic code
    
    The GART currently implements the iommu=[no]fullflush command line
    parameters which influence its IO/TLB flushing strategy. This patch
    makes these parameters generic so that they can be used by the AMD IOMMU
    too.
    
    Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c2e00ee..569527e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is defined in the file
 		nomerge
 		forcesac
 		soft
+		fullflush
+			Flush IO/TLB at every deallocation
+		nofullflush
+			Flush IO/TLB only when addresses are reused (default)
 
 
 	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
index 72ffb53..42c887b 100644
--- a/Documentation/x86/x86_64/boot-options.txt
+++ b/Documentation/x86/x86_64/boot-options.txt
@@ -229,8 +229,6 @@ IOMMU (input/output memory management unit)
   iommu options only relevant to the AMD GART hardware IOMMU:
     <size>             Set the size of the remapping area in bytes.
     allowed            Overwrite iommu off workarounds for specific chipsets.
-    fullflush          Flush IOMMU on each allocation (default).
-    nofullflush        Don't use IOMMU fullflush.
     leak               Turn on simple iommu leak tracing (only when
                        CONFIG_IOMMU_LEAK is on). Default number of leak pages
                        is 20.
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 23882c4..6dae123 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -16,6 +16,15 @@ EXPORT_SYMBOL(dma_ops);
 
 static int iommu_sac_force __read_mostly;
 
+/*
+ * 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 for every mapping. Problem is that doing the lazy flush seems
+ * to trigger bugs with some popular PCI cards, in particular 3ware (but
+ * has been also also seen with Qlogic at least).
+ */
+int iommu_fullflush;
+
 #ifdef CONFIG_IOMMU_DEBUG
 int panic_on_overflow __read_mostly = 1;
 int force_iommu __read_mostly = 1;
@@ -171,6 +180,10 @@ static __init int iommu_setup(char *p)
 		}
 		if (!strncmp(p, "nomerge", 7))
 			iommu_merge = 0;
+		if (!strncmp(p, "fullflush", 8))
+			iommu_fullflush = 1;
+		if (!strncmp(p, "nofullflush", 11))
+			iommu_fullflush = 0;
 		if (!strncmp(p, "forcesac", 8))
 			iommu_sac_force = 1;
 		if (!strncmp(p, "allowdac", 8))
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 9739d56..508ef47 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -45,15 +45,6 @@ static unsigned long iommu_pages;	/* .. and in pages */
 
 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 for every mapping. Problem is that doing the lazy flush seems
- * to trigger bugs with some popular PCI cards, in particular 3ware (but
- * has been also also seen with Qlogic at least).
- */
-int iommu_fullflush = 1;
-
 /* Allocation bitmap for the remapping area: */
 static DEFINE_SPINLOCK(iommu_bitmap_lock);
 /* Guarded by iommu_bitmap_lock: */
@@ -901,10 +892,6 @@ void __init gart_parse_options(char *p)
 #endif
 	if (isdigit(*p) && get_option(&p, &arg))
 		iommu_size = arg;
-	if (!strncmp(p, "fullflush", 8))
-		iommu_fullflush = 1;
-	if (!strncmp(p, "nofullflush", 11))
-		iommu_fullflush = 0;
 	if (!strncmp(p, "noagp", 5))
 		no_agp = 1;
 	if (!strncmp(p, "noaperture", 10))
diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
index 546ad31..bcebc37 100644
--- a/include/asm-x86/iommu.h
+++ b/include/asm-x86/iommu.h
@@ -7,6 +7,7 @@ extern struct dma_mapping_ops nommu_dma_ops;
 extern int force_iommu, no_iommu;
 extern int iommu_detected;
 extern int dmar_disabled;
+extern int iommu_fullflush;
 
 extern unsigned long iommu_num_pages(unsigned long addr, unsigned long len);
 

-- 
           |           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

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[FUJITA Tomonori]

On Thu, 18 Sep 2008 16:03:50 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 21:28:27 +0200
> > Joerg Roedel <joro@8bytes.org> wrote:
> > 
> > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > 
> > > > > The IO/TLB flushing on every unmaping operation is the most expensive
> > > > > part there and not strictly necessary. It is sufficient to do the flush
> > > > > before any entries are reused. This is patch implements lazy IO/TLB
> > > > > flushing which does exactly this.
> > > > > 
> > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > ---
> > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > > index c2e00ee..5f0aefe 100644
> > > > > --- a/Documentation/kernel-parameters.txt
> > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> > > > >  			isolate - enable device isolation (each device, as far
> > > > >  			          as possible, will get its own protection
> > > > >  			          domain)
> > > > > +			unmap_flush - enable flushing of IO/TLB entries when
> > > > > +			              they are unmapped. Otherwise they are
> > > > > +				      flushed before they will be reused, which
> > > > > +				      is a lot of faster
> > > > > +
> > > > 
> > > > Would it be nice to have consistency of IOMMU parameters?
> > > 
> > > True. We should merge common parameters across IOMMUs into the
> > > iommu= parameter some time in the future, I think. It would also be the
> > > place for the IOMMU size parameter.
> > 
> > Hmm, now is better than the future? I think that now you can add
> > something like 'disable_batching_flush' as a common parameter and
> > change AMD IOMMU to use it.
> 
> Ok, I queued the following patch in the AMD IOMMU updates and changed
> this patch to use iommu_fullflush instead. Is this patch ok? It changes
> the behavior of GART to use lazy flushing by default. But I don't think
> that this is a problem.
> 
> commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> Author: Joerg Roedel <joerg.roedel@amd.com>
> Date:   Thu Sep 18 15:23:43 2008 +0200
> 
>     x86: move GART TLB flushing options to generic code
>     
>     The GART currently implements the iommu=[no]fullflush command line
>     parameters which influence its IO/TLB flushing strategy. This patch
>     makes these parameters generic so that they can be used by the AMD IOMMU
>     too.
>     
>     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index c2e00ee..569527e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is defined in the file
>  		nomerge
>  		forcesac
>  		soft
> +		fullflush
> +			Flush IO/TLB at every deallocation
> +		nofullflush
> +			Flush IO/TLB only when addresses are reused (default)

I'm not sure about making 'nofullflush' a generic option. Enabling
nofullflush option doesn't change anything. So what's the point of the
option?

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

On Fri, Sep 19, 2008 at 08:10:32AM +0900, FUJITA Tomonori wrote:
> On Thu, 18 Sep 2008 16:03:50 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > > On Wed, 17 Sep 2008 21:28:27 +0200
> > > Joerg Roedel <joro@8bytes.org> wrote:
> > > 
> > > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > > 
> > > > > > The IO/TLB flushing on every unmaping operation is the most expensive
> > > > > > part there and not strictly necessary. It is sufficient to do the flush
> > > > > > before any entries are reused. This is patch implements lazy IO/TLB
> > > > > > flushing which does exactly this.
> > > > > > 
> > > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > > ---
> > > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > > >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> > > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > > > index c2e00ee..5f0aefe 100644
> > > > > > --- a/Documentation/kernel-parameters.txt
> > > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> > > > > >  			isolate - enable device isolation (each device, as far
> > > > > >  			          as possible, will get its own protection
> > > > > >  			          domain)
> > > > > > +			unmap_flush - enable flushing of IO/TLB entries when
> > > > > > +			              they are unmapped. Otherwise they are
> > > > > > +				      flushed before they will be reused, which
> > > > > > +				      is a lot of faster
> > > > > > +
> > > > > 
> > > > > Would it be nice to have consistency of IOMMU parameters?
> > > > 
> > > > True. We should merge common parameters across IOMMUs into the
> > > > iommu= parameter some time in the future, I think. It would also be the
> > > > place for the IOMMU size parameter.
> > > 
> > > Hmm, now is better than the future? I think that now you can add
> > > something like 'disable_batching_flush' as a common parameter and
> > > change AMD IOMMU to use it.
> > 
> > Ok, I queued the following patch in the AMD IOMMU updates and changed
> > this patch to use iommu_fullflush instead. Is this patch ok? It changes
> > the behavior of GART to use lazy flushing by default. But I don't think
> > that this is a problem.
> > 
> > commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> > Author: Joerg Roedel <joerg.roedel@amd.com>
> > Date:   Thu Sep 18 15:23:43 2008 +0200
> > 
> >     x86: move GART TLB flushing options to generic code
> >     
> >     The GART currently implements the iommu=[no]fullflush command line
> >     parameters which influence its IO/TLB flushing strategy. This patch
> >     makes these parameters generic so that they can be used by the AMD IOMMU
> >     too.
> >     
> >     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > 
> > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > index c2e00ee..569527e 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is defined in the file
> >  		nomerge
> >  		forcesac
> >  		soft
> > +		fullflush
> > +			Flush IO/TLB at every deallocation
> > +		nofullflush
> > +			Flush IO/TLB only when addresses are reused (default)
> 
> I'm not sure about making 'nofullflush' a generic option. Enabling
> nofullflush option doesn't change anything. So what's the point of the
> option?

Backwards compatability with the GART code. These two options are
basically just moved from the GART code to pci-dma.c. But otherwise its
pointless, I can remove it if everybody else agrees.

Joerg

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[FUJITA Tomonori]

On Fri, 19 Sep 2008 08:29:46 +0200
Joerg Roedel <joro@8bytes.org> wrote:

> > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > index c2e00ee..569527e 100644
> > > --- a/Documentation/kernel-parameters.txt
> > > +++ b/Documentation/kernel-parameters.txt
> > > @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is defined in the file
> > >  		nomerge
> > >  		forcesac
> > >  		soft
> > > +		fullflush
> > > +			Flush IO/TLB at every deallocation
> > > +		nofullflush
> > > +			Flush IO/TLB only when addresses are reused (default)
> > 
> > I'm not sure about making 'nofullflush' a generic option. Enabling
> > nofullflush option doesn't change anything. So what's the point of the
> > option?
> 
> Backwards compatability with the GART code. These two options are
> basically just moved from the GART code to pci-dma.c. But otherwise its
> pointless, I can remove it if everybody else agrees.

The compatibility with the GART code doesn't mean that we need to have
this pointless option as a common option. You can make 'fullflush' a
common option and the meaningless 'nofullflush' can live in GART.

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

Hi All,

FUJITA mentioned that I forgot to discuss this patch with you guys, the
implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
would like to hear your opinion on that patch. He is right with that.
The patch is already in tip/iommu but can easily be reverted if there
are fundamental objections.
The patch basically moves the iommu=fullflush and iommu=nofullflush
option from the GART code to pci-dma.c. So we can use these parameters
in other IOMMU implementations too. Since Intel VT-d is implementing
also lazy IO/TLB flushing it would benefit from this generic parameter
too. I am not so sure about Calgary, but we have other parameters for
iommu= which doesn't affect all hardware IOMMUs.

Joerg

On Thu, Sep 18, 2008 at 04:03:50PM +0200, Joerg Roedel wrote:
> On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 21:28:27 +0200
> > Joerg Roedel <joro@8bytes.org> wrote:
> > 
> > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > 
> > > > > The IO/TLB flushing on every unmaping operation is the most expensive
> > > > > part there and not strictly necessary. It is sufficient to do the flush
> > > > > before any entries are reused. This is patch implements lazy IO/TLB
> > > > > flushing which does exactly this.
> > > > > 
> > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > ---
> > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > >  arch/x86/kernel/amd_iommu.c         |   26 ++++++++++++++++++++++----
> > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > > index c2e00ee..5f0aefe 100644
> > > > > --- a/Documentation/kernel-parameters.txt
> > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters. It is defined in the file
> > > > >  			isolate - enable device isolation (each device, as far
> > > > >  			          as possible, will get its own protection
> > > > >  			          domain)
> > > > > +			unmap_flush - enable flushing of IO/TLB entries when
> > > > > +			              they are unmapped. Otherwise they are
> > > > > +				      flushed before they will be reused, which
> > > > > +				      is a lot of faster
> > > > > +
> > > > 
> > > > Would it be nice to have consistency of IOMMU parameters?
> > > 
> > > True. We should merge common parameters across IOMMUs into the
> > > iommu= parameter some time in the future, I think. It would also be the
> > > place for the IOMMU size parameter.
> > 
> > Hmm, now is better than the future? I think that now you can add
> > something like 'disable_batching_flush' as a common parameter and
> > change AMD IOMMU to use it.
> 
> Ok, I queued the following patch in the AMD IOMMU updates and changed
> this patch to use iommu_fullflush instead. Is this patch ok? It changes
> the behavior of GART to use lazy flushing by default. But I don't think
> that this is a problem.
> 
> commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> Author: Joerg Roedel <joerg.roedel@amd.com>
> Date:   Thu Sep 18 15:23:43 2008 +0200
> 
>     x86: move GART TLB flushing options to generic code
>     
>     The GART currently implements the iommu=[no]fullflush command line
>     parameters which influence its IO/TLB flushing strategy. This patch
>     makes these parameters generic so that they can be used by the AMD IOMMU
>     too.
>     
>     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index c2e00ee..569527e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is defined in the file
>  		nomerge
>  		forcesac
>  		soft
> +		fullflush
> +			Flush IO/TLB at every deallocation
> +		nofullflush
> +			Flush IO/TLB only when addresses are reused (default)
>  
>  
>  	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
> diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
> index 72ffb53..42c887b 100644
> --- a/Documentation/x86/x86_64/boot-options.txt
> +++ b/Documentation/x86/x86_64/boot-options.txt
> @@ -229,8 +229,6 @@ IOMMU (input/output memory management unit)
>    iommu options only relevant to the AMD GART hardware IOMMU:
>      <size>             Set the size of the remapping area in bytes.
>      allowed            Overwrite iommu off workarounds for specific chipsets.
> -    fullflush          Flush IOMMU on each allocation (default).
> -    nofullflush        Don't use IOMMU fullflush.
>      leak               Turn on simple iommu leak tracing (only when
>                         CONFIG_IOMMU_LEAK is on). Default number of leak pages
>                         is 20.
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 23882c4..6dae123 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -16,6 +16,15 @@ EXPORT_SYMBOL(dma_ops);
>  
>  static int iommu_sac_force __read_mostly;
>  
> +/*
> + * 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 for every mapping. Problem is that doing the lazy flush seems
> + * to trigger bugs with some popular PCI cards, in particular 3ware (but
> + * has been also also seen with Qlogic at least).
> + */
> +int iommu_fullflush;
> +
>  #ifdef CONFIG_IOMMU_DEBUG
>  int panic_on_overflow __read_mostly = 1;
>  int force_iommu __read_mostly = 1;
> @@ -171,6 +180,10 @@ static __init int iommu_setup(char *p)
>  		}
>  		if (!strncmp(p, "nomerge", 7))
>  			iommu_merge = 0;
> +		if (!strncmp(p, "fullflush", 8))
> +			iommu_fullflush = 1;
> +		if (!strncmp(p, "nofullflush", 11))
> +			iommu_fullflush = 0;
>  		if (!strncmp(p, "forcesac", 8))
>  			iommu_sac_force = 1;
>  		if (!strncmp(p, "allowdac", 8))
> diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
> index 9739d56..508ef47 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -45,15 +45,6 @@ static unsigned long iommu_pages;	/* .. and in pages */
>  
>  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 for every mapping. Problem is that doing the lazy flush seems
> - * to trigger bugs with some popular PCI cards, in particular 3ware (but
> - * has been also also seen with Qlogic at least).
> - */
> -int iommu_fullflush = 1;
> -
>  /* Allocation bitmap for the remapping area: */
>  static DEFINE_SPINLOCK(iommu_bitmap_lock);
>  /* Guarded by iommu_bitmap_lock: */
> @@ -901,10 +892,6 @@ void __init gart_parse_options(char *p)
>  #endif
>  	if (isdigit(*p) && get_option(&p, &arg))
>  		iommu_size = arg;
> -	if (!strncmp(p, "fullflush", 8))
> -		iommu_fullflush = 1;
> -	if (!strncmp(p, "nofullflush", 11))
> -		iommu_fullflush = 0;
>  	if (!strncmp(p, "noagp", 5))
>  		no_agp = 1;
>  	if (!strncmp(p, "noaperture", 10))
> diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
> index 546ad31..bcebc37 100644
> --- a/include/asm-x86/iommu.h
> +++ b/include/asm-x86/iommu.h
> @@ -7,6 +7,7 @@ extern struct dma_mapping_ops nommu_dma_ops;
>  extern int force_iommu, no_iommu;
>  extern int iommu_detected;
>  extern int dmar_disabled;
> +extern int iommu_fullflush;
>  
>  extern unsigned long iommu_num_pages(unsigned long addr, unsigned long len);
>  
> 
> -- 
>            |           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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[FUJITA Tomonori]

On Fri, 19 Sep 2008 19:43:39 +0200
Joerg Roedel <joro@8bytes.org> wrote:

> Hi All,
> 
> FUJITA mentioned that I forgot to discuss this patch with you guys, the
> implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
> would like to hear your opinion on that patch. He is right with that.
> The patch is already in tip/iommu but can easily be reverted if there
> are fundamental objections.
> The patch basically moves the iommu=fullflush and iommu=nofullflush
> option from the GART code to pci-dma.c. So we can use these parameters
> in other IOMMU implementations too. Since Intel VT-d is implementing
> also lazy IO/TLB flushing it would benefit from this generic parameter
> too. I am not so sure about Calgary, but we have other parameters for
> iommu= which doesn't affect all hardware IOMMUs.

nofullflush is pointless. It doesn't change any kernel behavior. Yes,
GART has it and we can't remove it because we exported it to
users. But please don't add such pointless option to the generic
options just for GART compatibility.

fullflush might be useful as a generic option to disable lazy IOTLB
flushing. But I'm not sure that Calgary uses it. VT-d already has
'strict' option for it and we can't change it. If VT-d wants to
support both 'strict' and 'fullflush' for disabling lazy IOTLB
flushing, it would make sense. But if VT-d doesn't want, it is useful
only for GART and AMD IOMMU. If so, I don't think that it is very
useful but I'm not against adding it.

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

On Sat, Sep 20, 2008 at 03:40:34AM +0900, FUJITA Tomonori wrote:
> On Fri, 19 Sep 2008 19:43:39 +0200
> Joerg Roedel <joro@8bytes.org> wrote:
> 
> > Hi All,
> > 
> > FUJITA mentioned that I forgot to discuss this patch with you guys, the
> > implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
> > would like to hear your opinion on that patch. He is right with that.
> > The patch is already in tip/iommu but can easily be reverted if there
> > are fundamental objections.
> > The patch basically moves the iommu=fullflush and iommu=nofullflush
> > option from the GART code to pci-dma.c. So we can use these parameters
> > in other IOMMU implementations too. Since Intel VT-d is implementing
> > also lazy IO/TLB flushing it would benefit from this generic parameter
> > too. I am not so sure about Calgary, but we have other parameters for
> > iommu= which doesn't affect all hardware IOMMUs.
> 
> nofullflush is pointless. It doesn't change any kernel behavior. Yes,
> GART has it and we can't remove it because we exported it to
> users. But please don't add such pointless option to the generic
> options just for GART compatibility.

I think maybe we can remove it completly. It doesn't change any behavior
for GART too and as an unknown option it will be silently ignored by the
command line parser. So removing this option completly will not break
the user interface, imho.
 
> fullflush might be useful as a generic option to disable lazy IOTLB
> flushing. But I'm not sure that Calgary uses it. VT-d already has
> 'strict' option for it and we can't change it. If VT-d wants to
> support both 'strict' and 'fullflush' for disabling lazy IOTLB
> flushing, it would make sense. But if VT-d doesn't want, it is useful
> only for GART and AMD IOMMU. If so, I don't think that it is very
> useful but I'm not against adding it.

I think it will be usefull for VT-d too. As Ingo stated some time ago,
it makes sense to have a common set of options independent of the
specific type of hardware IOMMU in the system. And all hardware IOMMUs
for x86 implement lazy IO/TLB flushing. Only Calgary does not allow to
disable it. So iommu=fullflush is a good candidate for a common
parameter.

Joerg

RE: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Keshavamurthy, Anil S]

-----Original Message-----
From: Joerg Roedel [mailto:joro@8bytes.org] 
Sent: Friday, September 19, 2008 10:44 AM
To: Raj, Ashok; Li, Shaohua; Keshavamurthy, Anil S; muli@il.ibm.com
Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
linux-kernel@vger.kernel.org; Ingo Molnar
Subject: Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

Hi All,

FUJITA mentioned that I forgot to discuss this patch with you guys, the
implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
would like to hear your opinion on that patch. He is right with that.
The patch is already in tip/iommu but can easily be reverted if there
are fundamental objections.
The patch basically moves the iommu=fullflush and iommu=nofullflush
option from the GART code to pci-dma.c. So we can use these parameters
in other IOMMU implementations too. Since Intel VT-d is implementing
also lazy IO/TLB flushing it would benefit from this generic parameter
too. I am not so sure about Calgary, but we have other parameters for
iommu= which doesn't affect all hardware IOMMUs.

Joerg

On Thu, Sep 18, 2008 at 04:03:50PM +0200, Joerg Roedel wrote:
> On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 21:28:27 +0200
> > Joerg Roedel <joro@8bytes.org> wrote:
> > 
> > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > 
> > > > > The IO/TLB flushing on every unmaping operation is the most
expensive
> > > > > part there and not strictly necessary. It is sufficient to do
the flush
> > > > > before any entries are reused. This is patch implements lazy
IO/TLB
> > > > > flushing which does exactly this.
> > > > > 
> > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > ---
> > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > >  arch/x86/kernel/amd_iommu.c         |   26
++++++++++++++++++++++----
> > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
> > > > > index c2e00ee..5f0aefe 100644
> > > > > --- a/Documentation/kernel-parameters.txt
> > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters.
It is defined in the file
> > > > >  			isolate - enable device isolation (each
device, as far
> > > > >  			          as possible, will get its own
protection
> > > > >  			          domain)
> > > > > +			unmap_flush - enable flushing of IO/TLB
entries when
> > > > > +			              they are unmapped.
Otherwise they are
> > > > > +				      flushed before they will
be reused, which
> > > > > +				      is a lot of faster
> > > > > +
> > > > 
> > > > Would it be nice to have consistency of IOMMU parameters?
> > > 
> > > True. We should merge common parameters across IOMMUs into the
> > > iommu= parameter some time in the future, I think. It would also
be the
> > > place for the IOMMU size parameter.
> > 
> > Hmm, now is better than the future? I think that now you can add
> > something like 'disable_batching_flush' as a common parameter and
> > change AMD IOMMU to use it.
> 
> Ok, I queued the following patch in the AMD IOMMU updates and changed
> this patch to use iommu_fullflush instead. Is this patch ok? It
changes
> the behavior of GART to use lazy flushing by default. But I don't
think
> that this is a problem.
> 
> commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> Author: Joerg Roedel <joerg.roedel@amd.com>
> Date:   Thu Sep 18 15:23:43 2008 +0200
> 
>     x86: move GART TLB flushing options to generic code
>     
>     The GART currently implements the iommu=[no]fullflush command line
>     parameters which influence its IO/TLB flushing strategy. This
patch
>     makes these parameters generic so that they can be used by the AMD
IOMMU
>     too.
>     
>     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
> index c2e00ee..569527e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is
defined in the file
>  		nomerge
>  		forcesac
>  		soft
> +		fullflush
> +			Flush IO/TLB at every deallocation
> +		nofullflush
> +			Flush IO/TLB only when addresses are reused
(default)
>  
>  
>  	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
> diff --git a/Documentation/x86/x86_64/boot-options.txt
b/Documentation/x86/x86_64/boot-options.txt
> index 72ffb53..42c887b 100644
> --- a/Documentation/x86/x86_64/boot-options.txt
> +++ b/Documentation/x86/x86_64/boot-options.txt
> @@ -229,8 +229,6 @@ IOMMU (input/output memory management unit)
>    iommu options only relevant to the AMD GART hardware IOMMU:
>      <size>             Set the size of the remapping area in bytes.
>      allowed            Overwrite iommu off workarounds for specific
chipsets.
> -    fullflush          Flush IOMMU on each allocation (default).
> -    nofullflush        Don't use IOMMU fullflush.
>      leak               Turn on simple iommu leak tracing (only when
>                         CONFIG_IOMMU_LEAK is on). Default number of
leak pages
>                         is 20.
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 23882c4..6dae123 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -16,6 +16,15 @@ EXPORT_SYMBOL(dma_ops);
>  
>  static int iommu_sac_force __read_mostly;
>  
> +/*
> + * 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 for every mapping. Problem is that doing the lazy flush
seems
> + * to trigger bugs with some popular PCI cards, in particular 3ware
(but
> + * has been also also seen with Qlogic at least).
> + */
> +int iommu_fullflush;
> +
>  #ifdef CONFIG_IOMMU_DEBUG
>  int panic_on_overflow __read_mostly = 1;
>  int force_iommu __read_mostly = 1;
> @@ -171,6 +180,10 @@ static __init int iommu_setup(char *p)
>  		}
>  		if (!strncmp(p, "nomerge", 7))
>  			iommu_merge = 0;
> +		if (!strncmp(p, "fullflush", 8))
> +			iommu_fullflush = 1;
> +		if (!strncmp(p, "nofullflush", 11))
> +			iommu_fullflush = 0;
>  		if (!strncmp(p, "forcesac", 8))
>  			iommu_sac_force = 1;
>  		if (!strncmp(p, "allowdac", 8))
> diff --git a/arch/x86/kernel/pci-gart_64.c
b/arch/x86/kernel/pci-gart_64.c
> index 9739d56..508ef47 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -45,15 +45,6 @@ static unsigned long iommu_pages;	/* .. and in
pages */
>  
>  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 for every mapping. Problem is that doing the lazy flush
seems
> - * to trigger bugs with some popular PCI cards, in particular 3ware
(but
> - * has been also also seen with Qlogic at least).
> - */
> -int iommu_fullflush = 1;
> -
>  /* Allocation bitmap for the remapping area: */
>  static DEFINE_SPINLOCK(iommu_bitmap_lock);
>  /* Guarded by iommu_bitmap_lock: */
> @@ -901,10 +892,6 @@ void __init gart_parse_options(char *p)
>  #endif
>  	if (isdigit(*p) && get_option(&p, &arg))
>  		iommu_size = arg;
> -	if (!strncmp(p, "fullflush", 8))
> -		iommu_fullflush = 1;
> -	if (!strncmp(p, "nofullflush", 11))
> -		iommu_fullflush = 0;
>  	if (!strncmp(p, "noagp", 5))
>  		no_agp = 1;
>  	if (!strncmp(p, "noaperture", 10))
> diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
> index 546ad31..bcebc37 100644
> --- a/include/asm-x86/iommu.h
> +++ b/include/asm-x86/iommu.h
> @@ -7,6 +7,7 @@ extern struct dma_mapping_ops nommu_dma_ops;
>  extern int force_iommu, no_iommu;
>  extern int iommu_detected;
>  extern int dmar_disabled;
> +extern int iommu_fullflush;
>  
>  extern unsigned long iommu_num_pages(unsigned long addr, unsigned
long len);
>  
> 
> -- 
>            |           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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Keshavamurthy, Anil S]

-----Original Message-----
From: Keshavamurthy, Anil S 
Sent: Friday, September 19, 2008 11:47 AM
To: 'Joerg Roedel'; Raj, Ashok; Li, Shaohua; muli@il.ibm.com
Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
linux-kernel@vger.kernel.org; Ingo Molnar
Subject: RE: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing



-----Original Message-----
From: Joerg Roedel [mailto:joro@8bytes.org] 
Sent: Friday, September 19, 2008 10:44 AM
To: Raj, Ashok; Li, Shaohua; Keshavamurthy, Anil S; muli@il.ibm.com
Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
linux-kernel@vger.kernel.org; Ingo Molnar
Subject: Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

Hi All,

FUJITA mentioned that I forgot to discuss this patch with you guys, the
implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
would like to hear your opinion on that patch. He is right with that.
The patch is already in tip/iommu but can easily be reverted if there
are fundamental objections.
The patch basically moves the iommu=fullflush and iommu=nofullflush
option from the GART code to pci-dma.c. So we can use these parameters
in other IOMMU implementations too. Since Intel VT-d is implementing
also lazy IO/TLB flushing it would benefit from this generic parameter
too. I am not so sure about Calgary, but we have other parameters for
iommu= which doesn't affect all hardware IOMMUs.

Joerg

On Thu, Sep 18, 2008 at 04:03:50PM +0200, Joerg Roedel wrote:
> On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 21:28:27 +0200
> > Joerg Roedel <joro@8bytes.org> wrote:
> > 
> > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > 
> > > > > The IO/TLB flushing on every unmaping operation is the most
expensive
> > > > > part there and not strictly necessary. It is sufficient to do
the flush
> > > > > before any entries are reused. This is patch implements lazy
IO/TLB
> > > > > flushing which does exactly this.
> > > > > 
> > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > ---
> > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > >  arch/x86/kernel/amd_iommu.c         |   26
++++++++++++++++++++++----
> > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
> > > > > index c2e00ee..5f0aefe 100644
> > > > > --- a/Documentation/kernel-parameters.txt
> > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters.
It is defined in the file
> > > > >  			isolate - enable device isolation (each
device, as far
> > > > >  			          as possible, will get its own
protection
> > > > >  			          domain)
> > > > > +			unmap_flush - enable flushing of IO/TLB
entries when
> > > > > +			              they are unmapped.
Otherwise they are
> > > > > +				      flushed before they will
be reused, which
> > > > > +				      is a lot of faster
> > > > > +
> > > > 
> > > > Would it be nice to have consistency of IOMMU parameters?
> > > 
> > > True. We should merge common parameters across IOMMUs into the
> > > iommu= parameter some time in the future, I think. It would also
be the
> > > place for the IOMMU size parameter.
> > 
> > Hmm, now is better than the future? I think that now you can add
> > something like 'disable_batching_flush' as a common parameter and
> > change AMD IOMMU to use it.
> 
> Ok, I queued the following patch in the AMD IOMMU updates and changed
> this patch to use iommu_fullflush instead. Is this patch ok? It
changes
> the behavior of GART to use lazy flushing by default. But I don't
think
> that this is a problem.
> 
> commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> Author: Joerg Roedel <joerg.roedel@amd.com>
> Date:   Thu Sep 18 15:23:43 2008 +0200
> 
>     x86: move GART TLB flushing options to generic code
>     
>     The GART currently implements the iommu=[no]fullflush command line
>     parameters which influence its IO/TLB flushing strategy. This
patch
>     makes these parameters generic so that they can be used by the AMD
IOMMU
>     too.
>     
>     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
> index c2e00ee..569527e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is
defined in the file
>  		nomerge
>  		forcesac
>  		soft
> +		fullflush
> +			Flush IO/TLB at every deallocation
> +		nofullflush
> +			Flush IO/TLB only when addresses are reused
(default)
>  
>  
>  	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
> diff --git a/Documentation/x86/x86_64/boot-options.txt
b/Documentation/x86/x86_64/boot-options.txt
> index 72ffb53..42c887b 100644
> --- a/Documentation/x86/x86_64/boot-options.txt
> +++ b/Documentation/x86/x86_64/boot-options.txt
> @@ -229,8 +229,6 @@ IOMMU (input/output memory management unit)
>    iommu options only relevant to the AMD GART hardware IOMMU:
>      <size>             Set the size of the remapping area in bytes.
>      allowed            Overwrite iommu off workarounds for specific
chipsets.
> -    fullflush          Flush IOMMU on each allocation (default).
> -    nofullflush        Don't use IOMMU fullflush.
>      leak               Turn on simple iommu leak tracing (only when
>                         CONFIG_IOMMU_LEAK is on). Default number of
leak pages
>                         is 20.
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 23882c4..6dae123 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -16,6 +16,15 @@ EXPORT_SYMBOL(dma_ops);
>  
>  static int iommu_sac_force __read_mostly;
>  
> +/*
> + * 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 for every mapping. Problem is that doing the lazy flush
seems
> + * to trigger bugs with some popular PCI cards, in particular 3ware
(but
> + * has been also also seen with Qlogic at least).
> + */
> +int iommu_fullflush;
> +
>  #ifdef CONFIG_IOMMU_DEBUG
>  int panic_on_overflow __read_mostly = 1;
>  int force_iommu __read_mostly = 1;
> @@ -171,6 +180,10 @@ static __init int iommu_setup(char *p)
>  		}
>  		if (!strncmp(p, "nomerge", 7))
>  			iommu_merge = 0;
> +		if (!strncmp(p, "fullflush", 8))
> +			iommu_fullflush = 1;
> +		if (!strncmp(p, "nofullflush", 11))
> +			iommu_fullflush = 0;
>  		if (!strncmp(p, "forcesac", 8))
>  			iommu_sac_force = 1;
>  		if (!strncmp(p, "allowdac", 8))
> diff --git a/arch/x86/kernel/pci-gart_64.c
b/arch/x86/kernel/pci-gart_64.c
> index 9739d56..508ef47 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -45,15 +45,6 @@ static unsigned long iommu_pages;	/* .. and in
pages */
>  
>  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 for every mapping. Problem is that doing the lazy flush
seems
> - * to trigger bugs with some popular PCI cards, in particular 3ware
(but
> - * has been also also seen with Qlogic at least).
> - */
> -int iommu_fullflush = 1;
> -
>  /* Allocation bitmap for the remapping area: */
>  static DEFINE_SPINLOCK(iommu_bitmap_lock);
>  /* Guarded by iommu_bitmap_lock: */
> @@ -901,10 +892,6 @@ void __init gart_parse_options(char *p)
>  #endif
>  	if (isdigit(*p) && get_option(&p, &arg))
>  		iommu_size = arg;
> -	if (!strncmp(p, "fullflush", 8))
> -		iommu_fullflush = 1;
> -	if (!strncmp(p, "nofullflush", 11))
> -		iommu_fullflush = 0;
>  	if (!strncmp(p, "noagp", 5))
>  		no_agp = 1;
>  	if (!strncmp(p, "noaperture", 10))
> diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
> index 546ad31..bcebc37 100644
> --- a/include/asm-x86/iommu.h
> +++ b/include/asm-x86/iommu.h
> @@ -7,6 +7,7 @@ extern struct dma_mapping_ops nommu_dma_ops;
>  extern int force_iommu, no_iommu;
>  extern int iommu_detected;
>  extern int dmar_disabled;
> +extern int iommu_fullflush;
>  
>  extern unsigned long iommu_num_pages(unsigned long addr, unsigned
long len);
>  
> 
> -- 
>            |           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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Keshavamurthy, Anil S]

Adding David Wookhouse who is now the official maintainer for intel VT-d
driver.
-----Original Message-----
From: Keshavamurthy, Anil S 
Sent: Friday, September 19, 2008 11:47 AM
To: 'Joerg Roedel'; Raj, Ashok; Li, Shaohua; muli@il.ibm.com
Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
linux-kernel@vger.kernel.org; Ingo Molnar
Subject: RE: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing



-----Original Message-----
From: Joerg Roedel [mailto:joro@8bytes.org] 
Sent: Friday, September 19, 2008 10:44 AM
To: Raj, Ashok; Li, Shaohua; Keshavamurthy, Anil S; muli@il.ibm.com
Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
linux-kernel@vger.kernel.org; Ingo Molnar
Subject: Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

Hi All,

FUJITA mentioned that I forgot to discuss this patch with you guys, the
implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
would like to hear your opinion on that patch. He is right with that.
The patch is already in tip/iommu but can easily be reverted if there
are fundamental objections.
The patch basically moves the iommu=fullflush and iommu=nofullflush
option from the GART code to pci-dma.c. So we can use these parameters
in other IOMMU implementations too. Since Intel VT-d is implementing
also lazy IO/TLB flushing it would benefit from this generic parameter
too. I am not so sure about Calgary, but we have other parameters for
iommu= which doesn't affect all hardware IOMMUs.

Joerg

On Thu, Sep 18, 2008 at 04:03:50PM +0200, Joerg Roedel wrote:
> On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 21:28:27 +0200
> > Joerg Roedel <joro@8bytes.org> wrote:
> > 
> > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > 
> > > > > The IO/TLB flushing on every unmaping operation is the most
expensive
> > > > > part there and not strictly necessary. It is sufficient to do
the flush
> > > > > before any entries are reused. This is patch implements lazy
IO/TLB
> > > > > flushing which does exactly this.
> > > > > 
> > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > ---
> > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > >  arch/x86/kernel/amd_iommu.c         |   26
++++++++++++++++++++++----
> > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
> > > > > index c2e00ee..5f0aefe 100644
> > > > > --- a/Documentation/kernel-parameters.txt
> > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters.
It is defined in the file
> > > > >  			isolate - enable device isolation (each
device, as far
> > > > >  			          as possible, will get its own
protection
> > > > >  			          domain)
> > > > > +			unmap_flush - enable flushing of IO/TLB
entries when
> > > > > +			              they are unmapped.
Otherwise they are
> > > > > +				      flushed before they will
be reused, which
> > > > > +				      is a lot of faster
> > > > > +
> > > > 
> > > > Would it be nice to have consistency of IOMMU parameters?
> > > 
> > > True. We should merge common parameters across IOMMUs into the
> > > iommu= parameter some time in the future, I think. It would also
be the
> > > place for the IOMMU size parameter.
> > 
> > Hmm, now is better than the future? I think that now you can add
> > something like 'disable_batching_flush' as a common parameter and
> > change AMD IOMMU to use it.
> 
> Ok, I queued the following patch in the AMD IOMMU updates and changed
> this patch to use iommu_fullflush instead. Is this patch ok? It
changes
> the behavior of GART to use lazy flushing by default. But I don't
think
> that this is a problem.
> 
> commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> Author: Joerg Roedel <joerg.roedel@amd.com>
> Date:   Thu Sep 18 15:23:43 2008 +0200
> 
>     x86: move GART TLB flushing options to generic code
>     
>     The GART currently implements the iommu=[no]fullflush command line
>     parameters which influence its IO/TLB flushing strategy. This
patch
>     makes these parameters generic so that they can be used by the AMD
IOMMU
>     too.
>     
>     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
> index c2e00ee..569527e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is
defined in the file
>  		nomerge
>  		forcesac
>  		soft
> +		fullflush
> +			Flush IO/TLB at every deallocation
> +		nofullflush
> +			Flush IO/TLB only when addresses are reused
(default)
>  
>  
>  	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
> diff --git a/Documentation/x86/x86_64/boot-options.txt
b/Documentation/x86/x86_64/boot-options.txt
> index 72ffb53..42c887b 100644
> --- a/Documentation/x86/x86_64/boot-options.txt
> +++ b/Documentation/x86/x86_64/boot-options.txt
> @@ -229,8 +229,6 @@ IOMMU (input/output memory management unit)
>    iommu options only relevant to the AMD GART hardware IOMMU:
>      <size>             Set the size of the remapping area in bytes.
>      allowed            Overwrite iommu off workarounds for specific
chipsets.
> -    fullflush          Flush IOMMU on each allocation (default).
> -    nofullflush        Don't use IOMMU fullflush.
>      leak               Turn on simple iommu leak tracing (only when
>                         CONFIG_IOMMU_LEAK is on). Default number of
leak pages
>                         is 20.
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 23882c4..6dae123 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -16,6 +16,15 @@ EXPORT_SYMBOL(dma_ops);
>  
>  static int iommu_sac_force __read_mostly;
>  
> +/*
> + * 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 for every mapping. Problem is that doing the lazy flush
seems
> + * to trigger bugs with some popular PCI cards, in particular 3ware
(but
> + * has been also also seen with Qlogic at least).
> + */
> +int iommu_fullflush;
> +
>  #ifdef CONFIG_IOMMU_DEBUG
>  int panic_on_overflow __read_mostly = 1;
>  int force_iommu __read_mostly = 1;
> @@ -171,6 +180,10 @@ static __init int iommu_setup(char *p)
>  		}
>  		if (!strncmp(p, "nomerge", 7))
>  			iommu_merge = 0;
> +		if (!strncmp(p, "fullflush", 8))
> +			iommu_fullflush = 1;
> +		if (!strncmp(p, "nofullflush", 11))
> +			iommu_fullflush = 0;
>  		if (!strncmp(p, "forcesac", 8))
>  			iommu_sac_force = 1;
>  		if (!strncmp(p, "allowdac", 8))
> diff --git a/arch/x86/kernel/pci-gart_64.c
b/arch/x86/kernel/pci-gart_64.c
> index 9739d56..508ef47 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -45,15 +45,6 @@ static unsigned long iommu_pages;	/* .. and in
pages */
>  
>  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 for every mapping. Problem is that doing the lazy flush
seems
> - * to trigger bugs with some popular PCI cards, in particular 3ware
(but
> - * has been also also seen with Qlogic at least).
> - */
> -int iommu_fullflush = 1;
> -
>  /* Allocation bitmap for the remapping area: */
>  static DEFINE_SPINLOCK(iommu_bitmap_lock);
>  /* Guarded by iommu_bitmap_lock: */
> @@ -901,10 +892,6 @@ void __init gart_parse_options(char *p)
>  #endif
>  	if (isdigit(*p) && get_option(&p, &arg))
>  		iommu_size = arg;
> -	if (!strncmp(p, "fullflush", 8))
> -		iommu_fullflush = 1;
> -	if (!strncmp(p, "nofullflush", 11))
> -		iommu_fullflush = 0;
>  	if (!strncmp(p, "noagp", 5))
>  		no_agp = 1;
>  	if (!strncmp(p, "noaperture", 10))
> diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
> index 546ad31..bcebc37 100644
> --- a/include/asm-x86/iommu.h
> +++ b/include/asm-x86/iommu.h
> @@ -7,6 +7,7 @@ extern struct dma_mapping_ops nommu_dma_ops;
>  extern int force_iommu, no_iommu;
>  extern int iommu_detected;
>  extern int dmar_disabled;
> +extern int iommu_fullflush;
>  
>  extern unsigned long iommu_num_pages(unsigned long addr, unsigned
long len);
>  
> 
> -- 
>            |           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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Joerg Roedel]

Hey David,

can you add an entry to the MAINTAINERs file for Intel IOMMU? I found it
difficult to find the responsible person for this thread.

Joerg

On Fri, Sep 19, 2008 at 11:48:50AM -0700, Keshavamurthy, Anil S wrote:
> 
> Adding David Wookhouse who is now the official maintainer for intel VT-d
> driver.
> -----Original Message-----
> From: Keshavamurthy, Anil S 
> Sent: Friday, September 19, 2008 11:47 AM
> To: 'Joerg Roedel'; Raj, Ashok; Li, Shaohua; muli@il.ibm.com
> Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Ingo Molnar
> Subject: RE: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing
> 
> 
> 
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org] 
> Sent: Friday, September 19, 2008 10:44 AM
> To: Raj, Ashok; Li, Shaohua; Keshavamurthy, Anil S; muli@il.ibm.com
> Cc: Joerg Roedel; FUJITA Tomonori; iommu@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Ingo Molnar
> Subject: Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing
> 
> Hi All,
> 
> FUJITA mentioned that I forgot to discuss this patch with you guys, the
> implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
> would like to hear your opinion on that patch. He is right with that.
> The patch is already in tip/iommu but can easily be reverted if there
> are fundamental objections.
> The patch basically moves the iommu=fullflush and iommu=nofullflush
> option from the GART code to pci-dma.c. So we can use these parameters
> in other IOMMU implementations too. Since Intel VT-d is implementing
> also lazy IO/TLB flushing it would benefit from this generic parameter
> too. I am not so sure about Calgary, but we have other parameters for
> iommu= which doesn't affect all hardware IOMMUs.
> 
> Joerg
> 
> On Thu, Sep 18, 2008 at 04:03:50PM +0200, Joerg Roedel wrote:
> > On Thu, Sep 18, 2008 at 10:29:24AM +0900, FUJITA Tomonori wrote:
> > > On Wed, 17 Sep 2008 21:28:27 +0200
> > > Joerg Roedel <joro@8bytes.org> wrote:
> > > 
> > > > On Thu, Sep 18, 2008 at 04:20:18AM +0900, FUJITA Tomonori wrote:
> > > > > On Wed, 17 Sep 2008 18:52:37 +0200
> > > > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > > 
> > > > > > The IO/TLB flushing on every unmaping operation is the most
> expensive
> > > > > > part there and not strictly necessary. It is sufficient to do
> the flush
> > > > > > before any entries are reused. This is patch implements lazy
> IO/TLB
> > > > > > flushing which does exactly this.
> > > > > > 
> > > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > > > ---
> > > > > >  Documentation/kernel-parameters.txt |    5 +++++
> > > > > >  arch/x86/kernel/amd_iommu.c         |   26
> ++++++++++++++++++++++----
> > > > > >  arch/x86/kernel/amd_iommu_init.c    |   10 +++++++++-
> > > > > >  include/asm-x86/amd_iommu_types.h   |    9 +++++++++
> > > > > >  4 files changed, 45 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> > > > > > index c2e00ee..5f0aefe 100644
> > > > > > --- a/Documentation/kernel-parameters.txt
> > > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > > @@ -284,6 +284,11 @@ and is between 256 and 4096 characters.
> It is defined in the file
> > > > > >  			isolate - enable device isolation (each
> device, as far
> > > > > >  			          as possible, will get its own
> protection
> > > > > >  			          domain)
> > > > > > +			unmap_flush - enable flushing of IO/TLB
> entries when
> > > > > > +			              they are unmapped.
> Otherwise they are
> > > > > > +				      flushed before they will
> be reused, which
> > > > > > +				      is a lot of faster
> > > > > > +
> > > > > 
> > > > > Would it be nice to have consistency of IOMMU parameters?
> > > > 
> > > > True. We should merge common parameters across IOMMUs into the
> > > > iommu= parameter some time in the future, I think. It would also
> be the
> > > > place for the IOMMU size parameter.
> > > 
> > > Hmm, now is better than the future? I think that now you can add
> > > something like 'disable_batching_flush' as a common parameter and
> > > change AMD IOMMU to use it.
> > 
> > Ok, I queued the following patch in the AMD IOMMU updates and changed
> > this patch to use iommu_fullflush instead. Is this patch ok? It
> changes
> > the behavior of GART to use lazy flushing by default. But I don't
> think
> > that this is a problem.
> > 
> > commit 9769771290fddcfc0362c5d30242151d4eb1cc46
> > Author: Joerg Roedel <joerg.roedel@amd.com>
> > Date:   Thu Sep 18 15:23:43 2008 +0200
> > 
> >     x86: move GART TLB flushing options to generic code
> >     
> >     The GART currently implements the iommu=[no]fullflush command line
> >     parameters which influence its IO/TLB flushing strategy. This
> patch
> >     makes these parameters generic so that they can be used by the AMD
> IOMMU
> >     too.
> >     
> >     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > 
> > diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> > index c2e00ee..569527e 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -888,6 +888,10 @@ and is between 256 and 4096 characters. It is
> defined in the file
> >  		nomerge
> >  		forcesac
> >  		soft
> > +		fullflush
> > +			Flush IO/TLB at every deallocation
> > +		nofullflush
> > +			Flush IO/TLB only when addresses are reused
> (default)
> >  
> >  
> >  	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
> > diff --git a/Documentation/x86/x86_64/boot-options.txt
> b/Documentation/x86/x86_64/boot-options.txt
> > index 72ffb53..42c887b 100644
> > --- a/Documentation/x86/x86_64/boot-options.txt
> > +++ b/Documentation/x86/x86_64/boot-options.txt
> > @@ -229,8 +229,6 @@ IOMMU (input/output memory management unit)
> >    iommu options only relevant to the AMD GART hardware IOMMU:
> >      <size>             Set the size of the remapping area in bytes.
> >      allowed            Overwrite iommu off workarounds for specific
> chipsets.
> > -    fullflush          Flush IOMMU on each allocation (default).
> > -    nofullflush        Don't use IOMMU fullflush.
> >      leak               Turn on simple iommu leak tracing (only when
> >                         CONFIG_IOMMU_LEAK is on). Default number of
> leak pages
> >                         is 20.
> > diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> > index 23882c4..6dae123 100644
> > --- a/arch/x86/kernel/pci-dma.c
> > +++ b/arch/x86/kernel/pci-dma.c
> > @@ -16,6 +16,15 @@ EXPORT_SYMBOL(dma_ops);
> >  
> >  static int iommu_sac_force __read_mostly;
> >  
> > +/*
> > + * 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 for every mapping. Problem is that doing the lazy flush
> seems
> > + * to trigger bugs with some popular PCI cards, in particular 3ware
> (but
> > + * has been also also seen with Qlogic at least).
> > + */
> > +int iommu_fullflush;
> > +
> >  #ifdef CONFIG_IOMMU_DEBUG
> >  int panic_on_overflow __read_mostly = 1;
> >  int force_iommu __read_mostly = 1;
> > @@ -171,6 +180,10 @@ static __init int iommu_setup(char *p)
> >  		}
> >  		if (!strncmp(p, "nomerge", 7))
> >  			iommu_merge = 0;
> > +		if (!strncmp(p, "fullflush", 8))
> > +			iommu_fullflush = 1;
> > +		if (!strncmp(p, "nofullflush", 11))
> > +			iommu_fullflush = 0;
> >  		if (!strncmp(p, "forcesac", 8))
> >  			iommu_sac_force = 1;
> >  		if (!strncmp(p, "allowdac", 8))
> > diff --git a/arch/x86/kernel/pci-gart_64.c
> b/arch/x86/kernel/pci-gart_64.c
> > index 9739d56..508ef47 100644
> > --- a/arch/x86/kernel/pci-gart_64.c
> > +++ b/arch/x86/kernel/pci-gart_64.c
> > @@ -45,15 +45,6 @@ static unsigned long iommu_pages;	/* .. and in
> pages */
> >  
> >  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 for every mapping. Problem is that doing the lazy flush
> seems
> > - * to trigger bugs with some popular PCI cards, in particular 3ware
> (but
> > - * has been also also seen with Qlogic at least).
> > - */
> > -int iommu_fullflush = 1;
> > -
> >  /* Allocation bitmap for the remapping area: */
> >  static DEFINE_SPINLOCK(iommu_bitmap_lock);
> >  /* Guarded by iommu_bitmap_lock: */
> > @@ -901,10 +892,6 @@ void __init gart_parse_options(char *p)
> >  #endif
> >  	if (isdigit(*p) && get_option(&p, &arg))
> >  		iommu_size = arg;
> > -	if (!strncmp(p, "fullflush", 8))
> > -		iommu_fullflush = 1;
> > -	if (!strncmp(p, "nofullflush", 11))
> > -		iommu_fullflush = 0;
> >  	if (!strncmp(p, "noagp", 5))
> >  		no_agp = 1;
> >  	if (!strncmp(p, "noaperture", 10))
> > diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
> > index 546ad31..bcebc37 100644
> > --- a/include/asm-x86/iommu.h
> > +++ b/include/asm-x86/iommu.h
> > @@ -7,6 +7,7 @@ extern struct dma_mapping_ops nommu_dma_ops;
> >  extern int force_iommu, no_iommu;
> >  extern int iommu_detected;
> >  extern int dmar_disabled;
> > +extern int iommu_fullflush;
> >  
> >  extern unsigned long iommu_num_pages(unsigned long addr, unsigned
> long len);
> >  
> > 
> > -- 
> >            |           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
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[David Woodhouse]

On Sun, 2008-09-21 at 11:05 +0200, Joerg Roedel wrote:
> can you add an entry to the MAINTAINERs file for Intel IOMMU? I found
> it difficult to find the responsible person for this thread.

I was trying to avoid doing that until I've actually acquired enough
clue for it to be meaningful. I'll be home from Portland next week, and
working to achieve that, with the shiny new hardware that arrived just
before I left.

I'm also planning to create a separate git tree for iommu work, where we
can all have direct access. It doesn't really live in the x86 tree.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH 03/23] AMD IOMMU: implement lazy IO/TLB flushing

[Muli Ben-Yehuda]

On Fri, Sep 19, 2008 at 07:43:39PM +0200, Joerg Roedel wrote:
> Hi All,
> 
> FUJITA mentioned that I forgot to discuss this patch with you guys, the
> implementers and maintainers for Intel VT-d and Calgary IOMMU drivers. I
> would like to hear your opinion on that patch. He is right with that.
> The patch is already in tip/iommu but can easily be reverted if there
> are fundamental objections.
> The patch basically moves the iommu=fullflush and iommu=nofullflush
> option from the GART code to pci-dma.c. So we can use these parameters
> in other IOMMU implementations too. Since Intel VT-d is implementing
> also lazy IO/TLB flushing it would benefit from this generic parameter
> too. I am not so sure about Calgary, but we have other parameters for
> iommu= which doesn't affect all hardware IOMMUs.

Calgary can't use fullflush, but in general the more unified our IOMMU
options, the better for the users. It's a pain to have to remember
which option applies to which IOMMU implementation.

Cheers,
Muli
-- 
Workshop on I/O Virtualization (WIOV '08)
Co-located with OSDI '08, Dec 2008, San Diego, CA
http://www.usenix.org/wiov08

[PATCH 04/23] AMD IOMMU: add branch hints to completion wait checks

[Joerg Roedel]

This patch adds branch hints to the cecks if a completion_wait is
necessary. The completion_waits in the mapping paths are unlikly because
they will only happen on software implementations of AMD IOMMU which
don't exists today or with lazy IO/TLB flushing when the allocator wraps
around the address space. With lazy IO/TLB flushing the completion_wait
in the unmapping path is unlikely too.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 0569098..7e9e4e7 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -876,7 +876,7 @@ static dma_addr_t map_single(struct device *dev, phys_addr_t paddr,
 	if (addr == bad_dma_address)
 		goto out;
 
-	if (iommu->need_sync)
+	if (unlikely(iommu->need_sync))
 		iommu_completion_wait(iommu);
 
 out:
@@ -905,7 +905,7 @@ static void unmap_single(struct device *dev, dma_addr_t dma_addr,
 
 	__unmap_single(iommu, domain->priv, dma_addr, size, dir);
 
-	if (iommu->need_sync)
+	if (unlikely(iommu->need_sync))
 		iommu_completion_wait(iommu);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
@@ -968,7 +968,7 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 			goto unmap;
 	}
 
-	if (iommu->need_sync)
+	if (unlikely(iommu->need_sync))
 		iommu_completion_wait(iommu);
 
 out:
@@ -1014,7 +1014,7 @@ static void unmap_sg(struct device *dev, struct scatterlist *sglist,
 		s->dma_address = s->dma_length = 0;
 	}
 
-	if (iommu->need_sync)
+	if (unlikely(iommu->need_sync))
 		iommu_completion_wait(iommu);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
@@ -1061,7 +1061,7 @@ static void *alloc_coherent(struct device *dev, size_t size,
 		goto out;
 	}
 
-	if (iommu->need_sync)
+	if (unlikely(iommu->need_sync))
 		iommu_completion_wait(iommu);
 
 out:
@@ -1093,7 +1093,7 @@ static void free_coherent(struct device *dev, size_t size,
 
 	__unmap_single(iommu, domain->priv, dma_addr, size, DMA_BIDIRECTIONAL);
 
-	if (iommu->need_sync)
+	if (unlikely(iommu->need_sync))
 		iommu_completion_wait(iommu);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
-- 
1.5.6.4

[PATCH 05/23] AMD IOMMU: align alloc_coherent addresses properly

[Joerg Roedel]

The API definition for dma_alloc_coherent states that the bus address
has to be aligned to the next power of 2 boundary greater than the
allocation size. This is violated by AMD IOMMU so far and this patch
fixes it.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 7e9e4e7..fdec963 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -383,7 +383,8 @@ static unsigned long dma_mask_to_pages(unsigned long mask)
  */
 static unsigned long dma_ops_alloc_addresses(struct device *dev,
 					     struct dma_ops_domain *dom,
-					     unsigned int pages)
+					     unsigned int pages,
+					     unsigned long align_mask)
 {
 	unsigned long limit = dma_mask_to_pages(*dev->dma_mask);
 	unsigned long address;
@@ -400,10 +401,10 @@ static unsigned long dma_ops_alloc_addresses(struct device *dev,
 	}
 
 	address = iommu_area_alloc(dom->bitmap, limit, dom->next_bit, pages,
-			0 , boundary_size, 0);
+				   0 , boundary_size, align_mask);
 	if (address == -1) {
 		address = iommu_area_alloc(dom->bitmap, limit, 0, pages,
-				0, boundary_size, 0);
+				0, boundary_size, align_mask);
 		dom->need_flush = true;
 	}
 
@@ -787,17 +788,22 @@ static dma_addr_t __map_single(struct device *dev,
 			       struct dma_ops_domain *dma_dom,
 			       phys_addr_t paddr,
 			       size_t size,
-			       int dir)
+			       int dir,
+			       bool align)
 {
 	dma_addr_t offset = paddr & ~PAGE_MASK;
 	dma_addr_t address, start;
 	unsigned int pages;
+	unsigned long align_mask = 0;
 	int i;
 
 	pages = iommu_num_pages(paddr, size);
 	paddr &= PAGE_MASK;
 
-	address = dma_ops_alloc_addresses(dev, dma_dom, pages);
+	if (align)
+		align_mask = (1UL << get_order(size)) - 1;
+
+	address = dma_ops_alloc_addresses(dev, dma_dom, pages, align_mask);
 	if (unlikely(address == bad_dma_address))
 		goto out;
 
@@ -872,7 +878,7 @@ static dma_addr_t map_single(struct device *dev, phys_addr_t paddr,
 		return (dma_addr_t)paddr;
 
 	spin_lock_irqsave(&domain->lock, flags);
-	addr = __map_single(dev, iommu, domain->priv, paddr, size, dir);
+	addr = __map_single(dev, iommu, domain->priv, paddr, size, dir, false);
 	if (addr == bad_dma_address)
 		goto out;
 
@@ -959,7 +965,7 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 		paddr = sg_phys(s);
 
 		s->dma_address = __map_single(dev, iommu, domain->priv,
-					      paddr, s->length, dir);
+					      paddr, s->length, dir, false);
 
 		if (s->dma_address) {
 			s->dma_length = s->length;
@@ -1053,7 +1059,7 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	spin_lock_irqsave(&domain->lock, flags);
 
 	*dma_addr = __map_single(dev, iommu, domain->priv, paddr,
-				 size, DMA_BIDIRECTIONAL);
+				 size, DMA_BIDIRECTIONAL, true);
 
 	if (*dma_addr == bad_dma_address) {
 		free_pages((unsigned long)virt_addr, get_order(size));
-- 
1.5.6.4

[PATCH 06/23] AMD IOMMU: add event buffer allocation

[Joerg Roedel]

This patch adds the allocation of a event buffer for each AMD IOMMU in
the system. The hardware will log events like device page faults or
other errors to this buffer once this is enabled.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c  |   29 +++++++++++++++++++++++++++++
 include/asm-x86/amd_iommu_types.h |    9 +++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index da631ab..a902eb6 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -418,6 +418,30 @@ static void __init free_command_buffer(struct amd_iommu *iommu)
 	free_pages((unsigned long)iommu->cmd_buf, get_order(CMD_BUFFER_SIZE));
 }
 
+/* allocates the memory where the IOMMU will log its events to */
+static u8 * __init alloc_event_buffer(struct amd_iommu *iommu)
+{
+	u64 entry;
+	iommu->evt_buf = (u8 *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+						get_order(EVT_BUFFER_SIZE));
+
+	if (iommu->evt_buf == NULL)
+		return NULL;
+
+	entry = (u64)virt_to_phys(iommu->evt_buf) | EVT_LEN_MASK;
+	memcpy_toio(iommu->mmio_base + MMIO_EVT_BUF_OFFSET,
+		    &entry, sizeof(entry));
+
+	iommu->evt_buf_size = EVT_BUFFER_SIZE;
+
+	return iommu->evt_buf;
+}
+
+static void __init free_event_buffer(struct amd_iommu *iommu)
+{
+	free_pages((unsigned long)iommu->evt_buf, get_order(EVT_BUFFER_SIZE));
+}
+
 /* sets a specific bit in the device table entry. */
 static void set_dev_entry_bit(u16 devid, u8 bit)
 {
@@ -623,6 +647,7 @@ static int __init init_iommu_devices(struct amd_iommu *iommu)
 static void __init free_iommu_one(struct amd_iommu *iommu)
 {
 	free_command_buffer(iommu);
+	free_event_buffer(iommu);
 	iommu_unmap_mmio_space(iommu);
 }
 
@@ -662,6 +687,10 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	if (!iommu->cmd_buf)
 		return -ENOMEM;
 
+	iommu->evt_buf = alloc_event_buffer(iommu);
+	if (!iommu->evt_buf)
+		return -ENOMEM;
+
 	init_iommu_from_pci(iommu);
 	init_iommu_from_acpi(iommu, h);
 	init_iommu_devices(iommu);
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 82a26fb..eabc316 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -116,6 +116,10 @@
 #define MMIO_CMD_SIZE_SHIFT 56
 #define MMIO_CMD_SIZE_512 (0x9ULL << MMIO_CMD_SIZE_SHIFT)
 
+/* constants for event buffer handling */
+#define EVT_BUFFER_SIZE		8192 /* 512 entries */
+#define EVT_LEN_MASK		(0x9ULL << 56)
+
 #define PAGE_MODE_1_LEVEL 0x01
 #define PAGE_MODE_2_LEVEL 0x02
 #define PAGE_MODE_3_LEVEL 0x03
@@ -243,6 +247,11 @@ struct amd_iommu {
 	/* size of command buffer */
 	u32 cmd_buf_size;
 
+	/* event buffer virtual address */
+	u8 *evt_buf;
+	/* size of event buffer */
+	u32 evt_buf_size;
+
 	/* if one, we need to send a completion wait command */
 	int need_sync;
 
-- 
1.5.6.4

[PATCH 07/23] AMD IOMMU: save pci segment from ACPI tables

[Joerg Roedel]

This patch adds the pci_seg field to the amd_iommu structure and fills
it with the corresponding value from the ACPI table.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c  |    1 +
 include/asm-x86/amd_iommu_types.h |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index a902eb6..aa26e37 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -677,6 +677,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	 */
 	iommu->devid = h->devid;
 	iommu->cap_ptr = h->cap_ptr;
+	iommu->pci_seg = h->pci_seg;
 	iommu->mmio_phys = h->mmio_phys;
 	iommu->mmio_base = iommu_map_mmio_space(h->mmio_phys);
 	if (!iommu->mmio_base)
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index eabc316..eb4d47a 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -232,6 +232,9 @@ struct amd_iommu {
 	/* capabilities of that IOMMU read from ACPI */
 	u32 cap;
 
+	/* pci domain of this IOMMU */
+	u16 pci_seg;
+
 	/* first device this IOMMU handles. read from PCI */
 	u16 first_device;
 	/* last device this IOMMU handles. read from PCI */
-- 
1.5.6.4

[PATCH 08/23] AMD IOMMU: save pci_dev instead of devid

[Joerg Roedel]

We need the pci_dev later anyways to enable MSI for the IOMMU hardware.
So remove the devid pointing to the BDF and replace it with the pci_dev
structure where the IOMMU is implemented.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c  |   25 ++++++++++++++++---------
 include/asm-x86/amd_iommu_types.h |    5 +++--
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index aa26e37..da619fc 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -243,9 +243,12 @@ static void __init iommu_feature_disable(struct amd_iommu *iommu, u8 bit)
 /* Function to enable the hardware */
 void __init iommu_enable(struct amd_iommu *iommu)
 {
-	printk(KERN_INFO "AMD IOMMU: Enabling IOMMU at ");
-	print_devid(iommu->devid, 0);
-	printk(" cap 0x%hx\n", iommu->cap_ptr);
+	printk(KERN_INFO "AMD IOMMU: Enabling IOMMU "
+	       "at %02x:%02x.%x cap 0x%hx\n",
+	       iommu->dev->bus->number,
+	       PCI_SLOT(iommu->dev->devfn),
+	       PCI_FUNC(iommu->dev->devfn),
+	       iommu->cap_ptr);
 
 	iommu_feature_enable(iommu, CONTROL_IOMMU_EN);
 }
@@ -512,15 +515,14 @@ static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m)
  */
 static void __init init_iommu_from_pci(struct amd_iommu *iommu)
 {
-	int bus = PCI_BUS(iommu->devid);
-	int dev = PCI_SLOT(iommu->devid);
-	int fn  = PCI_FUNC(iommu->devid);
 	int cap_ptr = iommu->cap_ptr;
 	u32 range;
 
-	iommu->cap = read_pci_config(bus, dev, fn, cap_ptr+MMIO_CAP_HDR_OFFSET);
+	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_CAP_HDR_OFFSET,
+			      &iommu->cap);
+	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_RANGE_OFFSET,
+			      &range);
 
-	range = read_pci_config(bus, dev, fn, cap_ptr+MMIO_RANGE_OFFSET);
 	iommu->first_device = calc_devid(MMIO_GET_BUS(range),
 					 MMIO_GET_FD(range));
 	iommu->last_device = calc_devid(MMIO_GET_BUS(range),
@@ -675,7 +677,10 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	/*
 	 * Copy data from ACPI table entry to the iommu struct
 	 */
-	iommu->devid = h->devid;
+	iommu->dev = pci_get_bus_and_slot(PCI_BUS(h->devid), h->devid & 0xff);
+	if (!iommu->dev)
+		return 1;
+
 	iommu->cap_ptr = h->cap_ptr;
 	iommu->pci_seg = h->pci_seg;
 	iommu->mmio_phys = h->mmio_phys;
@@ -696,6 +701,8 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	init_iommu_from_acpi(iommu, h);
 	init_iommu_devices(iommu);
 
+	pci_enable_device(iommu->dev);
+
 	return 0;
 }
 
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index eb4d47a..58c80f0 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -215,8 +215,9 @@ struct amd_iommu {
 	/* locks the accesses to the hardware */
 	spinlock_t lock;
 
-	/* device id of this IOMMU */
-	u16 devid;
+	/* Pointer to PCI device of this IOMMU */
+	struct pci_dev *dev;
+
 	/*
 	 * Capability pointer. There could be more than one IOMMU per PCI
 	 * device function if there are more than one AMD IOMMU capability
-- 
1.5.6.4

[PATCH 09/23] AMD IOMMU: add MSI interrupt support

[Joerg Roedel]

The AMD IOMMU can generate interrupts for various reasons. This patch
adds the basic interrupt enabling infrastructure to the driver.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/Kconfig                  |    1 +
 arch/x86/kernel/amd_iommu.c       |   11 ++++
 arch/x86/kernel/amd_iommu_init.c  |   99 ++++++++++++++++++++++++++++++++++++-
 include/asm-x86/amd_iommu.h       |    3 +
 include/asm-x86/amd_iommu_types.h |    7 +++
 5 files changed, 120 insertions(+), 1 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5258240..9c2898c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -567,6 +567,7 @@ config CALGARY_IOMMU_ENABLED_BY_DEFAULT
 config AMD_IOMMU
 	bool "AMD IOMMU support"
 	select SWIOTLB
+	select PCI_MSI
 	depends on X86_64 && PCI && ACPI
 	help
 	  With this option you can enable support for AMD IOMMU hardware in
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index fdec963..3caef6b 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -51,6 +51,17 @@ static int iommu_has_npcache(struct amd_iommu *iommu)
 
 /****************************************************************************
  *
+ * Interrupt handling functions
+ *
+ ****************************************************************************/
+
+irqreturn_t amd_iommu_int_handler(int irq, void *data)
+{
+	return IRQ_NONE;
+}
+
+/****************************************************************************
+ *
  * IOMMU command queuing functions
  *
  ****************************************************************************/
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index da619fc..ce3130d 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -22,6 +22,8 @@
 #include <linux/gfp.h>
 #include <linux/list.h>
 #include <linux/sysdev.h>
+#include <linux/interrupt.h>
+#include <linux/msi.h>
 #include <asm/pci-direct.h>
 #include <asm/amd_iommu_types.h>
 #include <asm/amd_iommu.h>
@@ -516,17 +518,20 @@ static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m)
 static void __init init_iommu_from_pci(struct amd_iommu *iommu)
 {
 	int cap_ptr = iommu->cap_ptr;
-	u32 range;
+	u32 range, misc;
 
 	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_CAP_HDR_OFFSET,
 			      &iommu->cap);
 	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_RANGE_OFFSET,
 			      &range);
+	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_MISC_OFFSET,
+			      &misc);
 
 	iommu->first_device = calc_devid(MMIO_GET_BUS(range),
 					 MMIO_GET_FD(range));
 	iommu->last_device = calc_devid(MMIO_GET_BUS(range),
 					MMIO_GET_LD(range));
+	iommu->evt_msi_num = MMIO_MSI_NUM(misc);
 }
 
 /*
@@ -697,6 +702,8 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	if (!iommu->evt_buf)
 		return -ENOMEM;
 
+	iommu->int_enabled = false;
+
 	init_iommu_from_pci(iommu);
 	init_iommu_from_acpi(iommu, h);
 	init_iommu_devices(iommu);
@@ -744,6 +751,95 @@ static int __init init_iommu_all(struct acpi_table_header *table)
 
 /****************************************************************************
  *
+ * The following functions initialize the MSI interrupts for all IOMMUs
+ * in the system. Its a bit challenging because there could be multiple
+ * IOMMUs per PCI BDF but we can call pci_enable_msi(x) only once per
+ * pci_dev.
+ *
+ ****************************************************************************/
+
+static int __init iommu_setup_msix(struct amd_iommu *iommu)
+{
+	struct amd_iommu *curr;
+	struct msix_entry entries[32]; /* only 32 supported by AMD IOMMU */
+	int nvec = 0, i;
+
+	list_for_each_entry(curr, &amd_iommu_list, list) {
+		if (curr->dev == iommu->dev) {
+			entries[nvec].entry = curr->evt_msi_num;
+			entries[nvec].vector = 0;
+			curr->int_enabled = true;
+			nvec++;
+		}
+	}
+
+	if (pci_enable_msix(iommu->dev, entries, nvec)) {
+		pci_disable_msix(iommu->dev);
+		return 1;
+	}
+
+	for (i = 0; i < nvec; ++i) {
+		int r = request_irq(entries->vector, amd_iommu_int_handler,
+				    IRQF_SAMPLE_RANDOM,
+				    "AMD IOMMU",
+				    NULL);
+		if (r)
+			goto out_free;
+	}
+
+	return 0;
+
+out_free:
+	for (i -= 1; i >= 0; --i)
+		free_irq(entries->vector, NULL);
+
+	pci_disable_msix(iommu->dev);
+
+	return 1;
+}
+
+static int __init iommu_setup_msi(struct amd_iommu *iommu)
+{
+	int r;
+	struct amd_iommu *curr;
+
+	list_for_each_entry(curr, &amd_iommu_list, list) {
+		if (curr->dev == iommu->dev)
+			curr->int_enabled = true;
+	}
+
+
+	if (pci_enable_msi(iommu->dev))
+		return 1;
+
+	r = request_irq(iommu->dev->irq, amd_iommu_int_handler,
+			IRQF_SAMPLE_RANDOM,
+			"AMD IOMMU",
+			NULL);
+
+	if (r) {
+		pci_disable_msi(iommu->dev);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int __init iommu_init_msi(struct amd_iommu *iommu)
+{
+	if (iommu->int_enabled)
+		return 0;
+
+	if (pci_find_capability(iommu->dev, PCI_CAP_ID_MSIX))
+		return iommu_setup_msix(iommu);
+	else if (pci_find_capability(iommu->dev, PCI_CAP_ID_MSI))
+		return iommu_setup_msi(iommu);
+
+	return 1;
+}
+
+/****************************************************************************
+ *
  * The next functions belong to the third pass of parsing the ACPI
  * table. In this last pass the memory mapping requirements are
  * gathered (like exclusion and unity mapping reanges).
@@ -863,6 +959,7 @@ static void __init enable_iommus(void)
 
 	list_for_each_entry(iommu, &amd_iommu_list, list) {
 		iommu_set_exclusion_range(iommu);
+		iommu_init_msi(iommu);
 		iommu_enable(iommu);
 	}
 }
diff --git a/include/asm-x86/amd_iommu.h b/include/asm-x86/amd_iommu.h
index 783f43e..041d0db 100644
--- a/include/asm-x86/amd_iommu.h
+++ b/include/asm-x86/amd_iommu.h
@@ -20,10 +20,13 @@
 #ifndef ASM_X86__AMD_IOMMU_H
 #define ASM_X86__AMD_IOMMU_H
 
+#include <linux/irqreturn.h>
+
 #ifdef CONFIG_AMD_IOMMU
 extern int amd_iommu_init(void);
 extern int amd_iommu_init_dma_ops(void);
 extern void amd_iommu_detect(void);
+extern irqreturn_t amd_iommu_int_handler(int irq, void *data);
 #else
 static inline int amd_iommu_init(void) { return -ENODEV; }
 static inline void amd_iommu_detect(void) { }
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 58c80f0..6ed8ffa 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -37,6 +37,7 @@
 /* Capability offsets used by the driver */
 #define MMIO_CAP_HDR_OFFSET	0x00
 #define MMIO_RANGE_OFFSET	0x0c
+#define MMIO_MISC_OFFSET	0x10
 
 /* Masks, shifts and macros to parse the device range capability */
 #define MMIO_RANGE_LD_MASK	0xff000000
@@ -48,6 +49,7 @@
 #define MMIO_GET_LD(x)  (((x) & MMIO_RANGE_LD_MASK) >> MMIO_RANGE_LD_SHIFT)
 #define MMIO_GET_FD(x)  (((x) & MMIO_RANGE_FD_MASK) >> MMIO_RANGE_FD_SHIFT)
 #define MMIO_GET_BUS(x) (((x) & MMIO_RANGE_BUS_MASK) >> MMIO_RANGE_BUS_SHIFT)
+#define MMIO_MSI_NUM(x)	((x) & 0x1f)
 
 /* Flag masks for the AMD IOMMU exclusion range */
 #define MMIO_EXCL_ENABLE_MASK 0x01ULL
@@ -255,10 +257,15 @@ struct amd_iommu {
 	u8 *evt_buf;
 	/* size of event buffer */
 	u32 evt_buf_size;
+	/* MSI number for event interrupt */
+	u16 evt_msi_num;
 
 	/* if one, we need to send a completion wait command */
 	int need_sync;
 
+	/* true if interrupts for this IOMMU are already enabled */
+	bool int_enabled;
+
 	/* default dma_ops domain for that IOMMU */
 	struct dma_ops_domain *default_dom;
 };
-- 
1.5.6.4

[PATCH 10/23] AMD IOMMU: add event handling code

[Joerg Roedel]

This patch adds code for polling and printing out events generated by
the AMD IOMMU.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c       |   87 ++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/amd_iommu_init.c  |    1 -
 include/asm-x86/amd_iommu_types.h |   22 +++++++++
 3 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 3caef6b..71e20ad 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -55,9 +55,94 @@ static int iommu_has_npcache(struct amd_iommu *iommu)
  *
  ****************************************************************************/
 
+static void iommu_print_event(void *__evt)
+{
+	u32 *event = __evt;
+	int type  = (event[1] >> EVENT_TYPE_SHIFT)  & EVENT_TYPE_MASK;
+	int devid = (event[0] >> EVENT_DEVID_SHIFT) & EVENT_DEVID_MASK;
+	int domid = (event[1] >> EVENT_DOMID_SHIFT) & EVENT_DOMID_MASK;
+	int flags = (event[1] >> EVENT_FLAGS_SHIFT) & EVENT_FLAGS_MASK;
+	u64 address = (u64)(((u64)event[3]) << 32) | event[2];
+
+	printk(KERN_ERR "AMD IOMMU: Event logged [");
+
+	switch (type) {
+	case EVENT_TYPE_ILL_DEV:
+		printk("ILLEGAL_DEV_TABLE_ENTRY device=%02x:%02x.%x "
+		       "address=0x%016llx flags=0x%04x]\n",
+		       PCI_BUS(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+		       address, flags);
+		break;
+	case EVENT_TYPE_IO_FAULT:
+		printk("IO_PAGE_FAULT device=%02x:%02x.%x "
+		       "domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+		       PCI_BUS(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+		       domid, address, flags);
+		break;
+	case EVENT_TYPE_DEV_TAB_ERR:
+		printk("DEV_TAB_HARDWARE_ERROR device=%02x:%02x.%x "
+		       "address=0x%016llx flags=0x%04x]\n",
+		       PCI_BUS(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+		       address, flags);
+		break;
+	case EVENT_TYPE_PAGE_TAB_ERR:
+		printk("PAGE_TAB_HARDWARE_ERROR device=%02x:%02x.%x "
+		       "domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+		       PCI_BUS(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+		       domid, address, flags);
+		break;
+	case EVENT_TYPE_ILL_CMD:
+		printk("ILLEGAL_COMMAND_ERROR address=0x%016llx]\n", address);
+		break;
+	case EVENT_TYPE_CMD_HARD_ERR:
+		printk("COMMAND_HARDWARE_ERROR address=0x%016llx "
+		       "flags=0x%04x]\n", address, flags);
+		break;
+	case EVENT_TYPE_IOTLB_INV_TO:
+		printk("IOTLB_INV_TIMEOUT device=%02x:%02x.%x "
+		       "address=0x%016llx]\n",
+		       PCI_BUS(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+		       address);
+		break;
+	case EVENT_TYPE_INV_DEV_REQ:
+		printk("INVALID_DEVICE_REQUEST device=%02x:%02x.%x "
+		       "address=0x%016llx flags=0x%04x]\n",
+		       PCI_BUS(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+		       address, flags);
+		break;
+	default:
+		printk(KERN_ERR "UNKNOWN type=0x%02x]\n", type);
+	}
+}
+
+static void iommu_poll_events(struct amd_iommu *iommu)
+{
+	u32 head, tail;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iommu->lock, flags);
+
+	head = readl(iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
+	tail = readl(iommu->mmio_base + MMIO_EVT_TAIL_OFFSET);
+
+	while (head != tail) {
+		iommu_print_event(iommu->evt_buf + head);
+		head = (head + EVENT_ENTRY_SIZE) % iommu->evt_buf_size;
+	}
+
+	writel(head, iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
+
+	spin_unlock_irqrestore(&iommu->lock, flags);
+}
+
 irqreturn_t amd_iommu_int_handler(int irq, void *data)
 {
-	return IRQ_NONE;
+	struct amd_iommu *iommu;
+
+	list_for_each_entry(iommu, &amd_iommu_list, list)
+		iommu_poll_events(iommu);
+
+	return IRQ_HANDLED;
 }
 
 /****************************************************************************
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index ce3130d..07709a9 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -32,7 +32,6 @@
 /*
  * definitions for the ACPI scanning code
  */
-#define PCI_BUS(x) (((x) >> 8) & 0xff)
 #define IVRS_HEADER_LENGTH 48
 
 #define ACPI_IVHD_TYPE                  0x10
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 6ed8ffa..1c9d8d4 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -71,6 +71,25 @@
 /* MMIO status bits */
 #define MMIO_STATUS_COM_WAIT_INT_MASK	0x04
 
+/* event logging constants */
+#define EVENT_ENTRY_SIZE	0x10
+#define EVENT_TYPE_SHIFT	28
+#define EVENT_TYPE_MASK		0xf
+#define EVENT_TYPE_ILL_DEV	0x1
+#define EVENT_TYPE_IO_FAULT	0x2
+#define EVENT_TYPE_DEV_TAB_ERR	0x3
+#define EVENT_TYPE_PAGE_TAB_ERR	0x4
+#define EVENT_TYPE_ILL_CMD	0x5
+#define EVENT_TYPE_CMD_HARD_ERR	0x6
+#define EVENT_TYPE_IOTLB_INV_TO	0x7
+#define EVENT_TYPE_INV_DEV_REQ	0x8
+#define EVENT_DEVID_MASK	0xffff
+#define EVENT_DEVID_SHIFT	0
+#define EVENT_DOMID_MASK	0xffff
+#define EVENT_DOMID_SHIFT	0
+#define EVENT_FLAGS_MASK	0xfff
+#define EVENT_FLAGS_SHIFT	0x10
+
 /* feature control bits */
 #define CONTROL_IOMMU_EN        0x00ULL
 #define CONTROL_HT_TUN_EN       0x01ULL
@@ -165,6 +184,9 @@
 
 #define MAX_DOMAIN_ID 65536
 
+/* FIXME: move this macro to <linux/pci.h> */
+#define PCI_BUS(x) (((x) >> 8) & 0xff)
+
 /*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
-- 
1.5.6.4

[PATCH 11/23] AMD IOMMU: enable event logging

[Joerg Roedel]

The code to log IOMMU events is in place now. So enable event logging
with this patch.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 07709a9..5182132 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -254,6 +254,13 @@ void __init iommu_enable(struct amd_iommu *iommu)
 	iommu_feature_enable(iommu, CONTROL_IOMMU_EN);
 }
 
+/* Function to enable IOMMU event logging and event interrupts */
+void __init iommu_enable_event_logging(struct amd_iommu *iommu)
+{
+	iommu_feature_enable(iommu, CONTROL_EVT_LOG_EN);
+	iommu_feature_enable(iommu, CONTROL_EVT_INT_EN);
+}
+
 /*
  * mapping and unmapping functions for the IOMMU MMIO space. Each AMD IOMMU in
  * the system has one.
@@ -959,6 +966,7 @@ static void __init enable_iommus(void)
 	list_for_each_entry(iommu, &amd_iommu_list, list) {
 		iommu_set_exclusion_range(iommu);
 		iommu_init_msi(iommu);
+		iommu_enable_event_logging(iommu);
 		iommu_enable(iommu);
 	}
 }
-- 
1.5.6.4

[PATCH 12/23] AMD IOMMU: allow IO page faults from devices

[Joerg Roedel]

There is a bit in the device entry to suppress all IO page faults
generated by a device. This bit was set until now because there was no
event logging. Now that there is event logging this patch allows IO page
faults from devices to see them in the kernel log too.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 5182132..d8c1a45 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -951,7 +951,6 @@ static void init_device_table(void)
 	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
 		set_dev_entry_bit(devid, DEV_ENTRY_VALID);
 		set_dev_entry_bit(devid, DEV_ENTRY_TRANSLATION);
-		set_dev_entry_bit(devid, DEV_ENTRY_NO_PAGE_FAULT);
 	}
 }
 
-- 
1.5.6.4

[PATCH 13/23] AMD IOMMU: add dma_supported callback

[Joerg Roedel]

This function determines if the AMD IOMMU implementation is responsible
for a given device. So the DMA layer can get this information from the
driver.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 71e20ad..0410be1 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1205,6 +1205,30 @@ free_mem:
 }
 
 /*
+ * This function is called by the DMA layer to find out if we can handle a
+ * particular device. It is part of the dma_ops.
+ */
+static int amd_iommu_dma_supported(struct device *dev, u64 mask)
+{
+	u16 bdf;
+	struct pci_dev *pcidev;
+
+	/* No device or no PCI device */
+	if (!dev || dev->bus != &pci_bus_type)
+		return 0;
+
+	pcidev = to_pci_dev(dev);
+
+	bdf = calc_devid(pcidev->bus->number, pcidev->devfn);
+
+	/* Out of our scope? */
+	if (bdf > amd_iommu_last_bdf)
+		return 0;
+
+	return 1;
+}
+
+/*
  * The function for pre-allocating protection domains.
  *
  * If the driver core informs the DMA layer if a driver grabs a device
@@ -1247,6 +1271,7 @@ static struct dma_mapping_ops amd_iommu_dma_ops = {
 	.unmap_single = unmap_single,
 	.map_sg = map_sg,
 	.unmap_sg = unmap_sg,
+	.dma_supported = amd_iommu_dma_supported,
 };
 
 /*
-- 
1.5.6.4

[PATCH 14/23] AMD IOMMU: don't assing preallocated protection domains to devices

[Joerg Roedel]

In isolation mode the protection domains for the devices are
preallocated and preassigned. This is bad if a device should be passed
to a virtualization guest because the IOMMU code does not know if it is
in use by a driver. This patch changes the code to assign the device to
the preallocated domain only if there were dma mapping requests for it.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c       |   43 ++++++++++++++++++++++++++++++++----
 include/asm-x86/amd_iommu_types.h |    6 +++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 0410be1..09d5e92 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -33,6 +33,10 @@
 
 static DEFINE_RWLOCK(amd_iommu_devtable_lock);
 
+/* A list of preallocated protection domains */
+static LIST_HEAD(iommu_pd_list);
+static DEFINE_SPINLOCK(iommu_pd_list_lock);
+
 /*
  * general struct to manage commands send to an IOMMU
  */
@@ -663,6 +667,7 @@ static struct dma_ops_domain *dma_ops_domain_alloc(struct amd_iommu *iommu,
 	dma_dom->next_bit = 0;
 
 	dma_dom->need_flush = false;
+	dma_dom->target_dev = 0xffff;
 
 	/* Intialize the exclusion range if necessary */
 	if (iommu->exclusion_start &&
@@ -769,6 +774,33 @@ static bool check_device(struct device *dev)
 }
 
 /*
+ * In this function the list of preallocated protection domains is traversed to
+ * find the domain for a specific device
+ */
+static struct dma_ops_domain *find_protection_domain(u16 devid)
+{
+	struct dma_ops_domain *entry, *ret = NULL;
+	unsigned long flags;
+
+	if (list_empty(&iommu_pd_list))
+		return NULL;
+
+	spin_lock_irqsave(&iommu_pd_list_lock, flags);
+
+	list_for_each_entry(entry, &iommu_pd_list, list) {
+		if (entry->target_dev == devid) {
+			ret = entry;
+			list_del(&ret->list);
+			break;
+		}
+	}
+
+	spin_unlock_irqrestore(&iommu_pd_list_lock, flags);
+
+	return ret;
+}
+
+/*
  * In the dma_ops path we only have the struct device. This function
  * finds the corresponding IOMMU, the protection domain and the
  * requestor id for a given device.
@@ -803,9 +835,11 @@ static int get_device_resources(struct device *dev,
 	*iommu = amd_iommu_rlookup_table[*bdf];
 	if (*iommu == NULL)
 		return 0;
-	dma_dom = (*iommu)->default_dom;
 	*domain = domain_for_device(*bdf);
 	if (*domain == NULL) {
+		dma_dom = find_protection_domain(*bdf);
+		if (!dma_dom)
+			dma_dom = (*iommu)->default_dom;
 		*domain = &dma_dom->domain;
 		set_device_domain(*iommu, *domain, *bdf);
 		printk(KERN_INFO "AMD IOMMU: Using protection domain %d for "
@@ -1257,10 +1291,9 @@ void prealloc_protection_domains(void)
 		if (!dma_dom)
 			continue;
 		init_unity_mappings_for_device(dma_dom, devid);
-		set_device_domain(iommu, &dma_dom->domain, devid);
-		printk(KERN_INFO "AMD IOMMU: Allocated domain %d for device ",
-		       dma_dom->domain.id);
-		print_devid(devid, 1);
+		dma_dom->target_dev = devid;
+
+		list_add_tail(&dma_dom->list, &iommu_pd_list);
 	}
 }
 
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 1c9d8d4..56dc6bd 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -227,6 +227,12 @@ struct dma_ops_domain {
 
 	/* This will be set to true when TLB needs to be flushed */
 	bool need_flush;
+
+	/*
+	 * if this is a preallocated domain, keep the device for which it was
+	 * preallocated in this variable
+	 */
+	u16 target_dev;
 };
 
 /*
-- 
1.5.6.4

[PATCH 15/23] AMD IOMMU: some set_device_domain cleanups

[Joerg Roedel]

Remove some magic numbers and split the pte_root using standard
functions.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c       |    9 +++++----
 include/asm-x86/amd_iommu_types.h |    3 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 09d5e92..8c97360 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -739,12 +739,13 @@ static void set_device_domain(struct amd_iommu *iommu,
 
 	u64 pte_root = virt_to_phys(domain->pt_root);
 
-	pte_root |= (domain->mode & 0x07) << 9;
-	pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | 2;
+	pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
+		    << DEV_ENTRY_MODE_SHIFT;
+	pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;
 
 	write_lock_irqsave(&amd_iommu_devtable_lock, flags);
-	amd_iommu_dev_table[devid].data[0] = pte_root;
-	amd_iommu_dev_table[devid].data[1] = pte_root >> 32;
+	amd_iommu_dev_table[devid].data[0] = lower_32_bits(pte_root);
+	amd_iommu_dev_table[devid].data[1] = upper_32_bits(pte_root);
 	amd_iommu_dev_table[devid].data[2] = domain->id;
 
 	amd_iommu_pd_table[devid] = domain;
diff --git a/include/asm-x86/amd_iommu_types.h b/include/asm-x86/amd_iommu_types.h
index 56dc6bd..b308586 100644
--- a/include/asm-x86/amd_iommu_types.h
+++ b/include/asm-x86/amd_iommu_types.h
@@ -130,6 +130,8 @@
 #define DEV_ENTRY_NMI_PASS      0xba
 #define DEV_ENTRY_LINT0_PASS    0xbe
 #define DEV_ENTRY_LINT1_PASS    0xbf
+#define DEV_ENTRY_MODE_MASK	0x07
+#define DEV_ENTRY_MODE_SHIFT	0x09
 
 /* constants to configure the command buffer */
 #define CMD_BUFFER_SIZE    8192
@@ -159,6 +161,7 @@
 #define IOMMU_MAP_SIZE_L3 (1ULL << 39)
 
 #define IOMMU_PTE_P  (1ULL << 0)
+#define IOMMU_PTE_TV (1ULL << 1)
 #define IOMMU_PTE_U  (1ULL << 59)
 #define IOMMU_PTE_FC (1ULL << 60)
 #define IOMMU_PTE_IR (1ULL << 61)
-- 
1.5.6.4

[PATCH 16/23] AMD IOMMU: avoid unnecessary low zone allocation in alloc_coherent

[Joerg Roedel]

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

x86's common alloc_coherent (dma_alloc_coherent in dma-mapping.h) sets
up the gfp flag according to the device dma_mask but AMD IOMMU doesn't
need it for devices that the IOMMU can do virtual mappings for. This
patch avoids unnecessary low zone allocation.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 8c97360..963b8bb 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1173,6 +1173,9 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	if (!check_device(dev))
 		return NULL;
 
+	if (!get_device_resources(dev, &iommu, &domain, &devid))
+		flag &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
+
 	virt_addr = (void *)__get_free_pages(flag, get_order(size));
 	if (!virt_addr)
 		return 0;
@@ -1180,8 +1183,6 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	memset(virt_addr, 0, size);
 	paddr = virt_to_phys(virt_addr);
 
-	get_device_resources(dev, &iommu, &domain, &devid);
-
 	if (!iommu || !domain) {
 		*dma_addr = (dma_addr_t)paddr;
 		return virt_addr;
-- 
1.5.6.4

[PATCH 17/23] AMD IOMMU: replace memset with __GFP_ZERO in alloc_coherent

[Joerg Roedel]

Remove the memset to 0 and use __GFP_ZERO at allocation time instead.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 963b8bb..65aed6e 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1176,11 +1176,11 @@ static void *alloc_coherent(struct device *dev, size_t size,
 	if (!get_device_resources(dev, &iommu, &domain, &devid))
 		flag &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
 
+	flag |= __GFP_ZERO;
 	virt_addr = (void *)__get_free_pages(flag, get_order(size));
 	if (!virt_addr)
 		return 0;
 
-	memset(virt_addr, 0, size);
 	paddr = virt_to_phys(virt_addr);
 
 	if (!iommu || !domain) {
-- 
1.5.6.4

[PATCH 18/23] AMD IOMMU: simplify dma_mask_to_pages

[Joerg Roedel]

The current calculation is very complicated. This patch replaces it with
a much simpler version.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 65aed6e..e38719a 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -472,8 +472,7 @@ static int init_unity_mappings_for_device(struct dma_ops_domain *dma_dom,
  ****************************************************************************/
 static unsigned long dma_mask_to_pages(unsigned long mask)
 {
-	return (mask >> PAGE_SHIFT) +
-		(PAGE_ALIGN(mask & ~PAGE_MASK) >> PAGE_SHIFT);
+	return PAGE_ALIGN(mask) >> PAGE_SHIFT;
 }
 
 /*
-- 
1.5.6.4

Re: [PATCH 18/23] AMD IOMMU: simplify dma_mask_to_pages

[FUJITA Tomonori]

On Wed, 17 Sep 2008 18:52:52 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> The current calculation is very complicated. This patch replaces it with
> a much simpler version.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kernel/amd_iommu.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)

I think that you can use iommu_device_max_index() for what
dma_mask_to_pages does.

Re: [PATCH 18/23] AMD IOMMU: simplify dma_mask_to_pages

[Joerg Roedel]

On Thu, Sep 18, 2008 at 04:20:20AM +0900, FUJITA Tomonori wrote:
> On Wed, 17 Sep 2008 18:52:52 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > The current calculation is very complicated. This patch replaces it with
> > a much simpler version.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  arch/x86/kernel/amd_iommu.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> I think that you can use iommu_device_max_index() for what
> dma_mask_to_pages does.

Hmm, in theory yes. But iommu_device_max_index() returns a size in bytes
and not in pages. So I still need to do the page shift somewhere. This
eliminates somewhat the benefit in this particular case.

Joerg

-- 
           |           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

Re: [PATCH 18/23] AMD IOMMU: simplify dma_mask_to_pages

[FUJITA Tomonori]

On Thu, 18 Sep 2008 09:32:32 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> On Thu, Sep 18, 2008 at 04:20:20AM +0900, FUJITA Tomonori wrote:
> > On Wed, 17 Sep 2008 18:52:52 +0200
> > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > 
> > > The current calculation is very complicated. This patch replaces it with
> > > a much simpler version.
> > > 
> > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > ---
> > >  arch/x86/kernel/amd_iommu.c |    3 +--
> > >  1 files changed, 1 insertions(+), 2 deletions(-)
> > 
> > I think that you can use iommu_device_max_index() for what
> > dma_mask_to_pages does.
> 
> Hmm, in theory yes. But iommu_device_max_index() returns a size in bytes
> and not in pages.

It doesn't return a size in bytes. You could use like:

limit = iommu_device_max_index(dom->aperture_size >> PAGE_SHIFT, 0,
				dma_get_mask(dev) >> PAGE_SHIFT);


Anyway, it's not urgent at all. I'll send a patch for this after your
patchset is merged.

Re: [PATCH 18/23] AMD IOMMU: simplify dma_mask_to_pages

[Joerg Roedel]

On Fri, Sep 19, 2008 at 12:57:36AM +0900, FUJITA Tomonori wrote:
> On Thu, 18 Sep 2008 09:32:32 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > On Thu, Sep 18, 2008 at 04:20:20AM +0900, FUJITA Tomonori wrote:
> > > On Wed, 17 Sep 2008 18:52:52 +0200
> > > Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > 
> > > > The current calculation is very complicated. This patch replaces it with
> > > > a much simpler version.
> > > > 
> > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > > > ---
> > > >  arch/x86/kernel/amd_iommu.c |    3 +--
> > > >  1 files changed, 1 insertions(+), 2 deletions(-)
> > > 
> > > I think that you can use iommu_device_max_index() for what
> > > dma_mask_to_pages does.
> > 
> > Hmm, in theory yes. But iommu_device_max_index() returns a size in bytes
> > and not in pages.
> 
> It doesn't return a size in bytes. You could use like:
> 
> limit = iommu_device_max_index(dom->aperture_size >> PAGE_SHIFT, 0,
> 				dma_get_mask(dev) >> PAGE_SHIFT);

Hmm, ok, makes sense if we remove this line too:

	limit = limit < size ? limit : size;

This is the logic in iommu_device_max_index().

> Anyway, it's not urgent at all. I'll send a patch for this after your
> patchset is merged.

Ok, fine.

Joerg

-- 
           |           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

[PATCH 19/23] AMD IOMMU: free domain bitmap with its allocation order

[Joerg Roedel]

The amd_iommu_pd_alloc_bitmap is allocated with a calculated order and
freed with order 1. This is not a bug since the calculated order always
evaluates to 1, but its unclean code. So replace the 1 with the
calculation in the release path.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index d8c1a45..a307cf6 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -1145,7 +1145,8 @@ out:
 	return ret;
 
 free:
-	free_pages((unsigned long)amd_iommu_pd_alloc_bitmap, 1);
+	free_pages((unsigned long)amd_iommu_pd_alloc_bitmap,
+		   get_order(MAX_DOMAIN_ID/8));
 
 	free_pages((unsigned long)amd_iommu_pd_table,
 		   get_order(rlookup_table_size));
-- 
1.5.6.4

[PATCH 20/23] AMD IOMMU: remove unnecessary cast to u64 in the init code

[Joerg Roedel]

From: Joerg Roedel <jroedel@lemmy.amd.com>

The ctrl variable is only u32 and readl also returns a 32 bit value. So
the cast to u64 is pointless. Remove it with this patch.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index a307cf6..b876499 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -236,7 +236,7 @@ static void __init iommu_feature_disable(struct amd_iommu *iommu, u8 bit)
 {
 	u32 ctrl;
 
-	ctrl = (u64)readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
+	ctrl = readl(iommu->mmio_base + MMIO_CONTROL_OFFSET);
 	ctrl &= ~(1 << bit);
 	writel(ctrl, iommu->mmio_base + MMIO_CONTROL_OFFSET);
 }
-- 
1.5.6.4

[PATCH 21/23] AMD IOMMU: calculate IVHD size with a function

[Joerg Roedel]

From: Joerg Roedel <jroedel@lemmy.amd.com>

The current calculation of the IVHD entry size is hard to read. So move
this code to a seperate function to make it more clear what this
calculation does.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index b876499..5303404 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -298,6 +298,14 @@ static void __init iommu_unmap_mmio_space(struct amd_iommu *iommu)
  ****************************************************************************/
 
 /*
+ * This function calculates the length of a given IVHD entry
+ */
+static inline int ivhd_entry_length(u8 *ivhd)
+{
+	return 0x04 << (*ivhd >> 6);
+}
+
+/*
  * This function reads the last device id the IOMMU has to handle from the PCI
  * capability header for this IOMMU
  */
@@ -341,7 +349,7 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
 		default:
 			break;
 		}
-		p += 0x04 << (*p >> 6);
+		p += ivhd_entry_length(p);
 	}
 
 	WARN_ON(p != end);
@@ -642,7 +650,7 @@ static void __init init_iommu_from_acpi(struct amd_iommu *iommu,
 			break;
 		}
 
-		p += 0x04 << (e->type >> 6);
+		p += ivhd_entry_length(p);
 	}
 }
 
-- 
1.5.6.4

[PATCH 22/23] AMD IOMMU: use cmd_buf_size when freeing the command buffer

[Joerg Roedel]

From: Joerg Roedel <jroedel@lemmy.amd.com>

The command buffer release function uses the CMD_BUF_SIZE macro for
get_order. Replace this with iommu->cmd_buf_size which is more reliable
about the actual size of the buffer.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu_init.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 5303404..d0ea4d3 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -434,7 +434,8 @@ static u8 * __init alloc_command_buffer(struct amd_iommu *iommu)
 
 static void __init free_command_buffer(struct amd_iommu *iommu)
 {
-	free_pages((unsigned long)iommu->cmd_buf, get_order(CMD_BUFFER_SIZE));
+	free_pages((unsigned long)iommu->cmd_buf,
+		   get_order(iommu->cmd_buf_size));
 }
 
 /* allocates the memory where the IOMMU will log its events to */
-- 
1.5.6.4

[PATCH 23/23] add AMD IOMMU tree to MAINTAINERS file

[Joerg Roedel]

From: Joerg Roedel <jroedel@lemmy.amd.com>

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 MAINTAINERS |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 38713f9..a649ea5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -387,6 +387,7 @@ AMD IOMMU (AMD-VI)
 P:	Joerg Roedel
 M:	joerg.roedel@amd.com
 L:	iommu@lists.linux-foundation.org
+T:	git://git.kernel.org/pub/scm/linux/kernel/git/joro/linux-2.6-iommu.git
 S:	Supported
 
 AMD MICROCODE UPDATE SUPPORT
-- 
1.5.6.4