[PATCH 00/26] iommu/vt-d: Clean up device<->domain attachment

[PATCH 00/26] iommu/vt-d: Clean up device<->domain attachment

[Joerg Roedel]

Hi,

here is a (bigger than I expected) patch-set which cleans up
the code to attach and detach domains to iommus in the Intel
VT-d driver.

In particular, the patch-set does:

	* Remove special cases around the handling of
	  various domain types and align their handling
	  where possible

	* Rework the data structures for the domain<->iommu
	  relation to better match with its usage. This
	  allowed to get rid of a couple of search loops.

	* Make the domain attachment and detachment path
	  to/from an iommu more symmetric. This makes the
	  code easier to understand and maintain.

	* Rework and simplify the locking around the
	  domain<->iommu attachment/detachment path.

A few rough edges and special cases are still left, but I
expect that these will be removed with the conversion to
default domains.

I tested the code with some additional debug code to make
sure that domain and domain-id allocation/deallocation works
as expected.

As test I booted a kernel with the patches (with and
without iommu=pt) and ran a KVM guest with devices assigned.
No lockdep warning popped up and the debug output was also
fine. But of course this is no guarantee that there are no
issues left, so I am happy about feedback. Please review!

Thanks,

	Joerg

Joerg Roedel (26):
  iommu/vt-d: Keep track of per-iommu domain ids
  iommu/vt-d: Add access functions for iommu->domains
  iommu/vt-d: Split up iommu->domains array
  iommu/vt-d: Get rid of iommu_attach_vm_domain()
  iommu/vt-d: Calculate translation in domain_context_mapping_one
  iommu/vt-d: Simplify domain_context_mapping_one
  iommu/vt-d: Pass dmar_domain directly into iommu_flush_iotlb_psi
  iommu/vt-d: Don't pre-allocate domain ids for si_domain
  iommu/vt-d: Kill dmar_domain->id
  iommu/vt-d: Replace iommu_bmp with a refcount
  iommu/vt-d: Simplify io/tlb flushing in intel_iommu_unmap
  iommu/vt-d: Simplify domain_remove_one_dev_info()
  iommu/vt-d: Simplify domain_remove_dev_info()
  iommu/vt-d: Move context-mapping  into dmar_insert_dev_info
  iommu/vt-d: Rename dmar_insert_dev_info()
  iommu/vt-d: Rename domain_remove_one_dev_info()
  iommu/vt-d: Rename iommu_detach_dependent_devices()
  iommu/vt-d: Pass an iommu pointer to domain_init()
  iommu/vt-d: Establish domain<->iommu link in dmar_insert_one_dev_info
  iommu/vt-d: Unify domain->iommu attach/detachment
  iommu/vt-d: Only call domain_remove_one_dev_info to detach old domain
  iommu/vt-d: Get rid of domain->iommu_lock
  iommu/vt-d: Remove dmar_global_lock from device_notifier
  iommu/vt-d: Pass device_domain_info to __dmar_remove_one_dev_info
  iommu/vt-d: Only insert alias dev_info if there is an alias
  iommu/vt-d: Avoid duplicate device_domain_info structures

 drivers/iommu/intel-iommu.c | 664 ++++++++++++++++++++------------------------
 include/linux/intel-iommu.h |   2 +-
 2 files changed, 295 insertions(+), 371 deletions(-)

-- 
1.9.1

[PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Instead of searching in the domain array for already
allocated domain ids, keep track of them explicitly.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 51 +++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0649b94..c3c5167 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -378,6 +378,9 @@ struct dmar_domain {
 	DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
 					/* bitmap of iommus this domain uses*/
 
+	u16	iommu_did[DMAR_UNITS_SUPPORTED];
+					/* Domain ids per IOMMU */
+
 	struct list_head devices;	/* all devices' list */
 	struct iova_domain iovad;	/* iova's that belong to this domain */
 
@@ -1543,11 +1546,13 @@ static int iommu_init_domains(struct intel_iommu *iommu)
 	}
 
 	/*
-	 * if Caching mode is set, then invalid translations are tagged
-	 * with domainid 0. Hence we need to pre-allocate it.
+	 * If Caching mode is set, then invalid translations are tagged
+	 * with domain-id 0, hence we need to pre-allocate it. We also
+	 * use domain-id 0 as a marker for non-allocated domain-id, so
+	 * make sure it is not used for a real domain.
 	 */
-	if (cap_caching_mode(iommu->cap))
-		set_bit(0, iommu->domain_ids);
+	set_bit(0, iommu->domain_ids);
+
 	return 0;
 }
 
@@ -1560,9 +1565,10 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 		for_each_set_bit(i, iommu->domain_ids, cap_ndoms(iommu->cap)) {
 			/*
 			 * Domain id 0 is reserved for invalid translation
-			 * if hardware supports caching mode.
+			 * if hardware supports caching mode and used as
+			 * a non-allocated marker.
 			 */
-			if (cap_caching_mode(iommu->cap) && i == 0)
+			if (i == 0)
 				continue;
 
 			domain = iommu->domains[i];
@@ -1624,6 +1630,7 @@ static int __iommu_attach_domain(struct dmar_domain *domain,
 	if (num < ndomains) {
 		set_bit(num, iommu->domain_ids);
 		iommu->domains[num] = domain;
+		domain->iommu_did[iommu->seq_id] = num;
 	} else {
 		num = -ENOSPC;
 	}
@@ -1650,12 +1657,10 @@ static int iommu_attach_vm_domain(struct dmar_domain *domain,
 				  struct intel_iommu *iommu)
 {
 	int num;
-	unsigned long ndomains;
 
-	ndomains = cap_ndoms(iommu->cap);
-	for_each_set_bit(num, iommu->domain_ids, ndomains)
-		if (iommu->domains[num] == domain)
-			return num;
+	num = domain->iommu_did[iommu->seq_id];
+	if (num)
+		return num;
 
 	return __iommu_attach_domain(domain, iommu);
 }
@@ -1664,22 +1669,17 @@ static void iommu_detach_domain(struct dmar_domain *domain,
 				struct intel_iommu *iommu)
 {
 	unsigned long flags;
-	int num, ndomains;
+	int num;
 
 	spin_lock_irqsave(&iommu->lock, flags);
-	if (domain_type_is_vm_or_si(domain)) {
-		ndomains = cap_ndoms(iommu->cap);
-		for_each_set_bit(num, iommu->domain_ids, ndomains) {
-			if (iommu->domains[num] == domain) {
-				clear_bit(num, iommu->domain_ids);
-				iommu->domains[num] = NULL;
-				break;
-			}
-		}
-	} else {
-		clear_bit(domain->id, iommu->domain_ids);
-		iommu->domains[domain->id] = NULL;
-	}
+
+	num = domain->iommu_did[iommu->seq_id];
+
+	WARN_ON(num == 0);
+
+	clear_bit(num, iommu->domain_ids);
+	iommu->domains[num]		 = NULL;
+
 	spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
@@ -1708,6 +1708,7 @@ static int domain_detach_iommu(struct dmar_domain *domain,
 	if (test_and_clear_bit(iommu->seq_id, domain->iommu_bmp)) {
 		count = --domain->iommu_count;
 		domain_update_iommu_cap(domain);
+		domain->iommu_did[iommu->seq_id] = 0;
 	}
 	spin_unlock_irqrestore(&domain->iommu_lock, flags);
 
-- 
1.9.1

Re: [PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids

[Alex Williamson]

On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Instead of searching in the domain array for already
> allocated domain ids, keep track of them explicitly.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/intel-iommu.c | 51 +++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 0649b94..c3c5167 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -378,6 +378,9 @@ struct dmar_domain {
>  	DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
>  					/* bitmap of iommus this domain uses*/
>  
> +	u16	iommu_did[DMAR_UNITS_SUPPORTED];
> +					/* Domain ids per IOMMU */

Maybe a spec reference here to justify u16 that the 2.3 vt-d spec only
supports up to 16bit domain IDs.

> +
>  	struct list_head devices;	/* all devices' list */
>  	struct iova_domain iovad;	/* iova's that belong to this domain */
>  
> @@ -1543,11 +1546,13 @@ static int iommu_init_domains(struct intel_iommu *iommu)
>  	}
>  
>  	/*
> -	 * if Caching mode is set, then invalid translations are tagged
> -	 * with domainid 0. Hence we need to pre-allocate it.
> +	 * If Caching mode is set, then invalid translations are tagged
> +	 * with domain-id 0, hence we need to pre-allocate it. We also
> +	 * use domain-id 0 as a marker for non-allocated domain-id, so
> +	 * make sure it is not used for a real domain.
>  	 */
> -	if (cap_caching_mode(iommu->cap))
> -		set_bit(0, iommu->domain_ids);
> +	set_bit(0, iommu->domain_ids);
> +
>  	return 0;
>  }
>  
> @@ -1560,9 +1565,10 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
>  		for_each_set_bit(i, iommu->domain_ids, cap_ndoms(iommu->cap)) {
>  			/*
>  			 * Domain id 0 is reserved for invalid translation
> -			 * if hardware supports caching mode.
> +			 * if hardware supports caching mode and used as
> +			 * a non-allocated marker.
>  			 */
> -			if (cap_caching_mode(iommu->cap) && i == 0)
> +			if (i == 0)
>  				continue;
>  
>  			domain = iommu->domains[i];
> @@ -1624,6 +1630,7 @@ static int __iommu_attach_domain(struct dmar_domain *domain,
>  	if (num < ndomains) {
>  		set_bit(num, iommu->domain_ids);
>  		iommu->domains[num] = domain;
> +		domain->iommu_did[iommu->seq_id] = num;
>  	} else {
>  		num = -ENOSPC;
>  	}
> @@ -1650,12 +1657,10 @@ static int iommu_attach_vm_domain(struct dmar_domain *domain,
>  				  struct intel_iommu *iommu)
>  {
>  	int num;
> -	unsigned long ndomains;
>  
> -	ndomains = cap_ndoms(iommu->cap);
> -	for_each_set_bit(num, iommu->domain_ids, ndomains)
> -		if (iommu->domains[num] == domain)
> -			return num;
> +	num = domain->iommu_did[iommu->seq_id];
> +	if (num)
> +		return num;
>  
>  	return __iommu_attach_domain(domain, iommu);
>  }
> @@ -1664,22 +1669,17 @@ static void iommu_detach_domain(struct dmar_domain *domain,
>  				struct intel_iommu *iommu)
>  {
>  	unsigned long flags;
> -	int num, ndomains;
> +	int num;
>  
>  	spin_lock_irqsave(&iommu->lock, flags);
> -	if (domain_type_is_vm_or_si(domain)) {
> -		ndomains = cap_ndoms(iommu->cap);
> -		for_each_set_bit(num, iommu->domain_ids, ndomains) {
> -			if (iommu->domains[num] == domain) {
> -				clear_bit(num, iommu->domain_ids);
> -				iommu->domains[num] = NULL;
> -				break;
> -			}
> -		}
> -	} else {
> -		clear_bit(domain->id, iommu->domain_ids);
> -		iommu->domains[domain->id] = NULL;
> -	}
> +
> +	num = domain->iommu_did[iommu->seq_id];
> +
> +	WARN_ON(num == 0);

Return -ENODEV and let the caller decide if that's an error?  Then we
could safely call this over all all iommus.

> +
> +	clear_bit(num, iommu->domain_ids);
> +	iommu->domains[num]		 = NULL;
> +
>  	spin_unlock_irqrestore(&iommu->lock, flags);
>  }
>  
> @@ -1708,6 +1708,7 @@ static int domain_detach_iommu(struct dmar_domain *domain,
>  	if (test_and_clear_bit(iommu->seq_id, domain->iommu_bmp)) {
>  		count = --domain->iommu_count;
>  		domain_update_iommu_cap(domain);
> +		domain->iommu_did[iommu->seq_id] = 0;
>  	}
>  	spin_unlock_irqrestore(&domain->iommu_lock, flags);
>  

Re: [PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids

[Joerg Roedel]

On Thu, Aug 06, 2015 at 01:19:52PM -0600, Alex Williamson wrote:
> On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> > From: Joerg Roedel <jroedel@suse.de>
> > 
> > Instead of searching in the domain array for already
> > allocated domain ids, keep track of them explicitly.
> > 
> > Signed-off-by: Joerg Roedel <jroedel@suse.de>
> > ---
> >  drivers/iommu/intel-iommu.c | 51 +++++++++++++++++++++++----------------------
> >  1 file changed, 26 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > index 0649b94..c3c5167 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -378,6 +378,9 @@ struct dmar_domain {
> >  	DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
> >  					/* bitmap of iommus this domain uses*/
> >  
> > +	u16	iommu_did[DMAR_UNITS_SUPPORTED];
> > +					/* Domain ids per IOMMU */
> 
> Maybe a spec reference here to justify u16 that the 2.3 vt-d spec only
> supports up to 16bit domain IDs.

Makes sense, I added a reference to the VT-d spec, section 9.3.

> > +	num = domain->iommu_did[iommu->seq_id];
> > +
> > +	WARN_ON(num == 0);
> 
> Return -ENODEV and let the caller decide if that's an error?  Then we
> could safely call this over all all iommus.

Changed that too, but not -ENODEV but just a plain return. The callers
do not care about errors anyway.


	Joerg

[PATCH 02/26] iommu/vt-d: Add access functions for iommu->domains

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This makes it easier to change the layout of the data
structure later.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c3c5167..e6a5966 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -567,6 +567,17 @@ __setup("intel_iommu=", intel_iommu_setup);
 static struct kmem_cache *iommu_domain_cache;
 static struct kmem_cache *iommu_devinfo_cache;
 
+static struct dmar_domain* get_iommu_domain(struct intel_iommu *iommu, u16 did)
+{
+	return iommu->domains[did];
+}
+
+static void set_iommu_domain(struct intel_iommu *iommu, u16 did,
+			     struct dmar_domain *domain)
+{
+	iommu->domains[did] = domain;
+}
+
 static inline void *alloc_pgtable_page(int node)
 {
 	struct page *page;
@@ -1461,7 +1472,8 @@ static void iommu_flush_iotlb_psi(struct intel_iommu *iommu, u16 did,
 	 * flush. However, device IOTLB doesn't need to be flushed in this case.
 	 */
 	if (!cap_caching_mode(iommu->cap) || !map)
-		iommu_flush_dev_iotlb(iommu->domains[did], addr, mask);
+		iommu_flush_dev_iotlb(get_iommu_domain(iommu, did),
+				      addr, mask);
 }
 
 static void iommu_disable_protect_mem_regions(struct intel_iommu *iommu)
@@ -1571,7 +1583,7 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 			if (i == 0)
 				continue;
 
-			domain = iommu->domains[i];
+			domain = get_iommu_domain(iommu, i);
 			clear_bit(i, iommu->domain_ids);
 			if (domain_detach_iommu(domain, iommu) == 0 &&
 			    !domain_type_is_vm(domain))
@@ -1629,7 +1641,7 @@ static int __iommu_attach_domain(struct dmar_domain *domain,
 	num = find_first_zero_bit(iommu->domain_ids, ndomains);
 	if (num < ndomains) {
 		set_bit(num, iommu->domain_ids);
-		iommu->domains[num] = domain;
+		set_iommu_domain(iommu, num, domain);
 		domain->iommu_did[iommu->seq_id] = num;
 	} else {
 		num = -ENOSPC;
@@ -1678,7 +1690,7 @@ static void iommu_detach_domain(struct dmar_domain *domain,
 	WARN_ON(num == 0);
 
 	clear_bit(num, iommu->domain_ids);
-	iommu->domains[num]		 = NULL;
+	set_iommu_domain(iommu, num, NULL);
 
 	spin_unlock_irqrestore(&iommu->lock, flags);
 }
@@ -4828,7 +4840,7 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
                 */
                ndomains = cap_ndoms(iommu->cap);
                for_each_set_bit(num, iommu->domain_ids, ndomains) {
-                       if (iommu->domains[num] == dmar_domain)
+                       if (get_iommu_domain(iommu, num) == dmar_domain)
                                iommu_flush_iotlb_psi(iommu, num, start_pfn,
 						     npages, !freelist, 0);
 	       }
-- 
1.9.1

[PATCH 03/26] iommu/vt-d: Split up iommu->domains array

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This array is indexed by the domain-id and contains the
pointers to the domains attached to this iommu. Modern
systems support 65536 domain ids, so that this array has a
size of 512kb, per iommu.

This is a huge waste of space, as the array is usually
sparsely populated. This patch makes the array
two-dimensional and allocates the memory for the domain
pointers on-demand.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 54 ++++++++++++++++++++++++++++++++++++---------
 include/linux/intel-iommu.h |  2 +-
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e6a5966..7f2e6c8 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -569,13 +569,32 @@ static struct kmem_cache *iommu_devinfo_cache;
 
 static struct dmar_domain* get_iommu_domain(struct intel_iommu *iommu, u16 did)
 {
-	return iommu->domains[did];
+	struct dmar_domain **domains;
+	int idx = did >> 8;
+
+	domains = iommu->domains[idx];
+	if (!domains)
+		return NULL;
+
+	return domains[did & 0xff];
 }
 
 static void set_iommu_domain(struct intel_iommu *iommu, u16 did,
 			     struct dmar_domain *domain)
 {
-	iommu->domains[did] = domain;
+	struct dmar_domain **domains;
+	int idx = did >> 8;
+
+	if (!iommu->domains[idx]) {
+		size_t size = 256 * sizeof(struct dmar_domain *);
+		iommu->domains[idx] = kzalloc(size, GFP_ATOMIC);
+	}
+
+	domains = iommu->domains[idx];
+	if (WARN_ON(!domains))
+		return;
+	else
+		domains[did & 0xff] = domain;
 }
 
 static inline void *alloc_pgtable_page(int node)
@@ -1528,35 +1547,43 @@ static void iommu_disable_translation(struct intel_iommu *iommu)
 
 static int iommu_init_domains(struct intel_iommu *iommu)
 {
-	unsigned long ndomains;
-	unsigned long nlongs;
+	u32 ndomains, nlongs;
+	size_t size;
 
 	ndomains = cap_ndoms(iommu->cap);
-	pr_debug("%s: Number of Domains supported <%ld>\n",
+	pr_debug("%s: Number of Domains supported <%d>\n",
 		 iommu->name, ndomains);
 	nlongs = BITS_TO_LONGS(ndomains);
 
 	spin_lock_init(&iommu->lock);
 
-	/* TBD: there might be 64K domains,
-	 * consider other allocation for future chip
-	 */
 	iommu->domain_ids = kcalloc(nlongs, sizeof(unsigned long), GFP_KERNEL);
 	if (!iommu->domain_ids) {
 		pr_err("%s: Allocating domain id array failed\n",
 		       iommu->name);
 		return -ENOMEM;
 	}
-	iommu->domains = kcalloc(ndomains, sizeof(struct dmar_domain *),
-			GFP_KERNEL);
-	if (!iommu->domains) {
+
+	size = ((ndomains >> 8) + 1) * sizeof(struct dmar_domain **);
+	iommu->domains = kzalloc(size, GFP_KERNEL);
+
+	if (iommu->domains) {
+		size = 256 * sizeof(struct dmar_domain *);
+		iommu->domains[0] = kzalloc(size, GFP_KERNEL);
+	}
+
+	if (!iommu->domains || !iommu->domains[0]) {
 		pr_err("%s: Allocating domain array failed\n",
 		       iommu->name);
 		kfree(iommu->domain_ids);
+		kfree(iommu->domains);
 		iommu->domain_ids = NULL;
+		iommu->domains    = NULL;
 		return -ENOMEM;
 	}
 
+
+
 	/*
 	 * If Caching mode is set, then invalid translations are tagged
 	 * with domain-id 0, hence we need to pre-allocate it. We also
@@ -1598,6 +1625,11 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 static void free_dmar_iommu(struct intel_iommu *iommu)
 {
 	if ((iommu->domains) && (iommu->domain_ids)) {
+		int elems = (cap_ndoms(iommu->cap) >> 8) + 1;
+		int i;
+
+		for (i = 0; i < elems; i++)
+			kfree(iommu->domains[i]);
 		kfree(iommu->domains);
 		kfree(iommu->domain_ids);
 		iommu->domains = NULL;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index d9a366d..6240063 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -344,7 +344,7 @@ struct intel_iommu {
 
 #ifdef CONFIG_INTEL_IOMMU
 	unsigned long 	*domain_ids; /* bitmap of domains */
-	struct dmar_domain **domains; /* ptr to domains */
+	struct dmar_domain ***domains; /* ptr to domains */
 	spinlock_t	lock; /* protect context, domain ids */
 	struct root_entry *root_entry; /* virtual address */
 
-- 
1.9.1

Re: [PATCH 03/26] iommu/vt-d: Split up iommu->domains array

[Alex Williamson]

On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> This array is indexed by the domain-id and contains the
> pointers to the domains attached to this iommu. Modern
> systems support 65536 domain ids, so that this array has a
> size of 512kb, per iommu.
> 
> This is a huge waste of space, as the array is usually
> sparsely populated. This patch makes the array
> two-dimensional and allocates the memory for the domain
> pointers on-demand.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/intel-iommu.c | 54 ++++++++++++++++++++++++++++++++++++---------
>  include/linux/intel-iommu.h |  2 +-
>  2 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index e6a5966..7f2e6c8 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -569,13 +569,32 @@ static struct kmem_cache *iommu_devinfo_cache;
>  
>  static struct dmar_domain* get_iommu_domain(struct intel_iommu *iommu, u16 did)
>  {
> -	return iommu->domains[did];
> +	struct dmar_domain **domains;
> +	int idx = did >> 8;
> +
> +	domains = iommu->domains[idx];
> +	if (!domains)
> +		return NULL;
> +
> +	return domains[did & 0xff];
>  }
>  
>  static void set_iommu_domain(struct intel_iommu *iommu, u16 did,
>  			     struct dmar_domain *domain)
>  {
> -	iommu->domains[did] = domain;
> +	struct dmar_domain **domains;
> +	int idx = did >> 8;
> +
> +	if (!iommu->domains[idx]) {
> +		size_t size = 256 * sizeof(struct dmar_domain *);
> +		iommu->domains[idx] = kzalloc(size, GFP_ATOMIC);
> +	}
> +
> +	domains = iommu->domains[idx];
> +	if (WARN_ON(!domains))
> +		return;
> +	else
> +		domains[did & 0xff] = domain;
>  }

I'm tempted to suggest using pages here since we're dealing with 2k
second level arrays, but maybe caring about pointers per page just makes
that ugly.

>  
>  static inline void *alloc_pgtable_page(int node)
> @@ -1528,35 +1547,43 @@ static void iommu_disable_translation(struct intel_iommu *iommu)
>  
>  static int iommu_init_domains(struct intel_iommu *iommu)
>  {
> -	unsigned long ndomains;
> -	unsigned long nlongs;
> +	u32 ndomains, nlongs;
> +	size_t size;
>  
>  	ndomains = cap_ndoms(iommu->cap);
> -	pr_debug("%s: Number of Domains supported <%ld>\n",
> +	pr_debug("%s: Number of Domains supported <%d>\n",
>  		 iommu->name, ndomains);
>  	nlongs = BITS_TO_LONGS(ndomains);
>  
>  	spin_lock_init(&iommu->lock);
>  
> -	/* TBD: there might be 64K domains,
> -	 * consider other allocation for future chip
> -	 */
>  	iommu->domain_ids = kcalloc(nlongs, sizeof(unsigned long), GFP_KERNEL);
>  	if (!iommu->domain_ids) {
>  		pr_err("%s: Allocating domain id array failed\n",
>  		       iommu->name);
>  		return -ENOMEM;
>  	}
> -	iommu->domains = kcalloc(ndomains, sizeof(struct dmar_domain *),
> -			GFP_KERNEL);
> -	if (!iommu->domains) {
> +
> +	size = ((ndomains >> 8) + 1) * sizeof(struct dmar_domain **);
> +	iommu->domains = kzalloc(size, GFP_KERNEL);
> +
> +	if (iommu->domains) {
> +		size = 256 * sizeof(struct dmar_domain *);
> +		iommu->domains[0] = kzalloc(size, GFP_KERNEL);
> +	}
> +
> +	if (!iommu->domains || !iommu->domains[0]) {
>  		pr_err("%s: Allocating domain array failed\n",
>  		       iommu->name);
>  		kfree(iommu->domain_ids);
> +		kfree(iommu->domains);
>  		iommu->domain_ids = NULL;
> +		iommu->domains    = NULL;
>  		return -ENOMEM;
>  	}
>  
> +
> +
>  	/*
>  	 * If Caching mode is set, then invalid translations are tagged
>  	 * with domain-id 0, hence we need to pre-allocate it. We also
> @@ -1598,6 +1625,11 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
>  static void free_dmar_iommu(struct intel_iommu *iommu)
>  {
>  	if ((iommu->domains) && (iommu->domain_ids)) {
> +		int elems = (cap_ndoms(iommu->cap) >> 8) + 1;
> +		int i;
> +
> +		for (i = 0; i < elems; i++)
> +			kfree(iommu->domains[i]);
>  		kfree(iommu->domains);
>  		kfree(iommu->domain_ids);
>  		iommu->domains = NULL;
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index d9a366d..6240063 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -344,7 +344,7 @@ struct intel_iommu {
>  
>  #ifdef CONFIG_INTEL_IOMMU
>  	unsigned long 	*domain_ids; /* bitmap of domains */
> -	struct dmar_domain **domains; /* ptr to domains */
> +	struct dmar_domain ***domains; /* ptr to domains */
>  	spinlock_t	lock; /* protect context, domain ids */
>  	struct root_entry *root_entry; /* virtual address */
>  

Re: [PATCH 03/26] iommu/vt-d: Split up iommu->domains array

[Joerg Roedel]

On Thu, Aug 06, 2015 at 01:20:09PM -0600, Alex Williamson wrote:
> On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> >  static void set_iommu_domain(struct intel_iommu *iommu, u16 did,
> >  			     struct dmar_domain *domain)
> >  {
> > -	iommu->domains[did] = domain;
> > +	struct dmar_domain **domains;
> > +	int idx = did >> 8;
> > +
> > +	if (!iommu->domains[idx]) {
> > +		size_t size = 256 * sizeof(struct dmar_domain *);
> > +		iommu->domains[idx] = kzalloc(size, GFP_ATOMIC);
> > +	}
> > +
> > +	domains = iommu->domains[idx];
> > +	if (WARN_ON(!domains))
> > +		return;
> > +	else
> > +		domains[did & 0xff] = domain;
> >  }
> 
> I'm tempted to suggest using pages here since we're dealing with 2k
> second level arrays, but maybe caring about pointers per page just makes
> that ugly.

The benefit would be that we avoid the slab-overhead while allocating.
But since the VT-d driver is needed on platforms with different
page-sizes, the handling would be indeed more complicated because we
have to split the index at a different point then depending on the
architecture.


	Joerg

[PATCH 04/26] iommu/vt-d: Get rid of iommu_attach_vm_domain()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The special case for VM domains is not needed, as other
domains could be attached to the iommu in the same way. So
get rid of this special case.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 7f2e6c8..58fc4bb 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1669,8 +1669,13 @@ static int __iommu_attach_domain(struct dmar_domain *domain,
 	int num;
 	unsigned long ndomains;
 
+	num = domain->iommu_did[iommu->seq_id];
+	if (num)
+		return num;
+
 	ndomains = cap_ndoms(iommu->cap);
-	num = find_first_zero_bit(iommu->domain_ids, ndomains);
+	num	 = find_first_zero_bit(iommu->domain_ids, ndomains);
+
 	if (num < ndomains) {
 		set_bit(num, iommu->domain_ids);
 		set_iommu_domain(iommu, num, domain);
@@ -1679,6 +1684,9 @@ static int __iommu_attach_domain(struct dmar_domain *domain,
 		num = -ENOSPC;
 	}
 
+	if (num < 0)
+		pr_err("%s: No free domain ids\n", iommu->name);
+
 	return num;
 }
 
@@ -1691,24 +1699,10 @@ static int iommu_attach_domain(struct dmar_domain *domain,
 	spin_lock_irqsave(&iommu->lock, flags);
 	num = __iommu_attach_domain(domain, iommu);
 	spin_unlock_irqrestore(&iommu->lock, flags);
-	if (num < 0)
-		pr_err("%s: No free domain ids\n", iommu->name);
 
 	return num;
 }
 
-static int iommu_attach_vm_domain(struct dmar_domain *domain,
-				  struct intel_iommu *iommu)
-{
-	int num;
-
-	num = domain->iommu_did[iommu->seq_id];
-	if (num)
-		return num;
-
-	return __iommu_attach_domain(domain, iommu);
-}
-
 static void iommu_detach_domain(struct dmar_domain *domain,
 				struct intel_iommu *iommu)
 {
@@ -1944,7 +1938,7 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 
 	if (domain_type_is_vm_or_si(domain)) {
 		if (domain_type_is_vm(domain)) {
-			id = iommu_attach_vm_domain(domain, iommu);
+			id = __iommu_attach_domain(domain, iommu);
 			if (id < 0) {
 				spin_unlock_irqrestore(&iommu->lock, flags);
 				pr_err("%s: No free domain ids\n", iommu->name);
-- 
1.9.1

[PATCH 05/26] iommu/vt-d: Calculate translation in domain_context_mapping_one

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

There is no reason to pass the translation type through
multiple layers. It can also be determined in the
domain_context_mapping_one function directly.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 50 ++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 58fc4bb..1a934f8 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -364,7 +364,8 @@ static inline int first_pte_in_page(struct dma_pte *pte)
 static struct dmar_domain *si_domain;
 static int hw_pass_through = 1;
 
-/* domain represents a virtual machine, more than one devices
+/*
+ * Domain represents a virtual machine, more than one devices
  * across iommus may be owned in one domain, e.g. kvm guest.
  */
 #define DOMAIN_FLAG_VIRTUAL_MACHINE	(1 << 0)
@@ -638,6 +639,11 @@ static inline int domain_type_is_vm(struct dmar_domain *domain)
 	return domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE;
 }
 
+static inline int domain_type_is_si(struct dmar_domain *domain)
+{
+	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
+}
+
 static inline int domain_type_is_vm_or_si(struct dmar_domain *domain)
 {
 	return domain->flags & (DOMAIN_FLAG_VIRTUAL_MACHINE |
@@ -1904,21 +1910,24 @@ static void domain_exit(struct dmar_domain *domain)
 
 static int domain_context_mapping_one(struct dmar_domain *domain,
 				      struct intel_iommu *iommu,
-				      u8 bus, u8 devfn, int translation)
+				      u8 bus, u8 devfn)
 {
+	int translation = CONTEXT_TT_MULTI_LEVEL;
+	struct device_domain_info *info = NULL;
 	struct context_entry *context;
 	unsigned long flags;
 	struct dma_pte *pgd;
 	int id;
 	int agaw;
-	struct device_domain_info *info = NULL;
+
+	translation = CONTEXT_TT_MULTI_LEVEL;
+	if (hw_pass_through && domain_type_is_si(domain))
+		translation = CONTEXT_TT_PASS_THROUGH;
 
 	pr_debug("Set context mapping for %02x:%02x.%d\n",
 		bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
 
 	BUG_ON(!domain->pgd);
-	BUG_ON(translation != CONTEXT_TT_PASS_THROUGH &&
-	       translation != CONTEXT_TT_MULTI_LEVEL);
 
 	spin_lock_irqsave(&iommu->lock, flags);
 	context = iommu_context_addr(iommu, bus, devfn, 1);
@@ -2010,7 +2019,6 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 struct domain_context_mapping_data {
 	struct dmar_domain *domain;
 	struct intel_iommu *iommu;
-	int translation;
 };
 
 static int domain_context_mapping_cb(struct pci_dev *pdev,
@@ -2019,13 +2027,11 @@ static int domain_context_mapping_cb(struct pci_dev *pdev,
 	struct domain_context_mapping_data *data = opaque;
 
 	return domain_context_mapping_one(data->domain, data->iommu,
-					  PCI_BUS_NUM(alias), alias & 0xff,
-					  data->translation);
+					  PCI_BUS_NUM(alias), alias & 0xff);
 }
 
 static int
-domain_context_mapping(struct dmar_domain *domain, struct device *dev,
-		       int translation)
+domain_context_mapping(struct dmar_domain *domain, struct device *dev)
 {
 	struct intel_iommu *iommu;
 	u8 bus, devfn;
@@ -2036,12 +2042,10 @@ domain_context_mapping(struct dmar_domain *domain, struct device *dev,
 		return -ENODEV;
 
 	if (!dev_is_pci(dev))
-		return domain_context_mapping_one(domain, iommu, bus, devfn,
-						  translation);
+		return domain_context_mapping_one(domain, iommu, bus, devfn);
 
 	data.domain = domain;
 	data.iommu = iommu;
-	data.translation = translation;
 
 	return pci_for_each_dma_alias(to_pci_dev(dev),
 				      &domain_context_mapping_cb, &data);
@@ -2508,7 +2512,7 @@ static int iommu_prepare_identity_map(struct device *dev,
 		goto error;
 
 	/* context entry init */
-	ret = domain_context_mapping(domain, dev, CONTEXT_TT_MULTI_LEVEL);
+	ret = domain_context_mapping(domain, dev);
 	if (ret)
 		goto error;
 
@@ -2621,8 +2625,7 @@ static int identity_mapping(struct device *dev)
 	return 0;
 }
 
-static int domain_add_dev_info(struct dmar_domain *domain,
-			       struct device *dev, int translation)
+static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
 {
 	struct dmar_domain *ndomain;
 	struct intel_iommu *iommu;
@@ -2637,7 +2640,7 @@ static int domain_add_dev_info(struct dmar_domain *domain,
 	if (ndomain != domain)
 		return -EBUSY;
 
-	ret = domain_context_mapping(domain, dev, translation);
+	ret = domain_context_mapping(domain, dev);
 	if (ret) {
 		domain_remove_one_dev_info(domain, dev);
 		return ret;
@@ -2782,9 +2785,7 @@ static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw
 	if (!iommu_should_identity_map(dev, 1))
 		return 0;
 
-	ret = domain_add_dev_info(si_domain, dev,
-				  hw ? CONTEXT_TT_PASS_THROUGH :
-				       CONTEXT_TT_MULTI_LEVEL);
+	ret = domain_add_dev_info(si_domain, dev);
 	if (!ret)
 		pr_info("%s identity mapping for device %s\n",
 			hw ? "Hardware" : "Software", dev_name(dev));
@@ -3311,7 +3312,7 @@ static struct dmar_domain *__get_valid_domain_for_dev(struct device *dev)
 
 	/* make sure context mapping is ok */
 	if (unlikely(!domain_context_mapped(dev))) {
-		ret = domain_context_mapping(domain, dev, CONTEXT_TT_MULTI_LEVEL);
+		ret = domain_context_mapping(domain, dev);
 		if (ret) {
 			pr_err("Domain context map for %s failed\n",
 			       dev_name(dev));
@@ -3366,10 +3367,7 @@ static int iommu_no_mapping(struct device *dev)
 		 */
 		if (iommu_should_identity_map(dev, 0)) {
 			int ret;
-			ret = domain_add_dev_info(si_domain, dev,
-						  hw_pass_through ?
-						  CONTEXT_TT_PASS_THROUGH :
-						  CONTEXT_TT_MULTI_LEVEL);
+			ret = domain_add_dev_info(si_domain, dev);
 			if (!ret) {
 				pr_info("64bit %s uses identity mapping\n",
 					dev_name(dev));
@@ -4786,7 +4784,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 		dmar_domain->agaw--;
 	}
 
-	return domain_add_dev_info(dmar_domain, dev, CONTEXT_TT_MULTI_LEVEL);
+	return domain_add_dev_info(dmar_domain, dev);
 }
 
 static void intel_iommu_detach_device(struct iommu_domain *domain,
-- 
1.9.1

Re: [PATCH 05/26] iommu/vt-d: Calculate translation in domain_context_mapping_one

[Alex Williamson]

On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> There is no reason to pass the translation type through
> multiple layers. It can also be determined in the
> domain_context_mapping_one function directly.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/intel-iommu.c | 50 ++++++++++++++++++++++-----------------------
>  1 file changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 58fc4bb..1a934f8 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -364,7 +364,8 @@ static inline int first_pte_in_page(struct dma_pte *pte)
>  static struct dmar_domain *si_domain;
>  static int hw_pass_through = 1;
>  
> -/* domain represents a virtual machine, more than one devices
> +/*
> + * Domain represents a virtual machine, more than one devices
>   * across iommus may be owned in one domain, e.g. kvm guest.
>   */
>  #define DOMAIN_FLAG_VIRTUAL_MACHINE	(1 << 0)
> @@ -638,6 +639,11 @@ static inline int domain_type_is_vm(struct dmar_domain *domain)
>  	return domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE;
>  }
>  
> +static inline int domain_type_is_si(struct dmar_domain *domain)
> +{
> +	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
> +}
> +
>  static inline int domain_type_is_vm_or_si(struct dmar_domain *domain)
>  {
>  	return domain->flags & (DOMAIN_FLAG_VIRTUAL_MACHINE |
> @@ -1904,21 +1910,24 @@ static void domain_exit(struct dmar_domain *domain)
>  
>  static int domain_context_mapping_one(struct dmar_domain *domain,
>  				      struct intel_iommu *iommu,
> -				      u8 bus, u8 devfn, int translation)
> +				      u8 bus, u8 devfn)
>  {
> +	int translation = CONTEXT_TT_MULTI_LEVEL;
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +	struct device_domain_info *info = NULL;
>  	struct context_entry *context;
>  	unsigned long flags;
>  	struct dma_pte *pgd;
>  	int id;
>  	int agaw;
> -	struct device_domain_info *info = NULL;
> +
> +	translation = CONTEXT_TT_MULTI_LEVEL;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm sure this is already part of your cleanup.

> +	if (hw_pass_through && domain_type_is_si(domain))
> +		translation = CONTEXT_TT_PASS_THROUGH;
>  
>  	pr_debug("Set context mapping for %02x:%02x.%d\n",
>  		bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
>  
>  	BUG_ON(!domain->pgd);
> -	BUG_ON(translation != CONTEXT_TT_PASS_THROUGH &&
> -	       translation != CONTEXT_TT_MULTI_LEVEL);
>  
>  	spin_lock_irqsave(&iommu->lock, flags);
>  	context = iommu_context_addr(iommu, bus, devfn, 1);
> @@ -2010,7 +2019,6 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
>  struct domain_context_mapping_data {
>  	struct dmar_domain *domain;
>  	struct intel_iommu *iommu;
> -	int translation;
>  };
>  
>  static int domain_context_mapping_cb(struct pci_dev *pdev,
> @@ -2019,13 +2027,11 @@ static int domain_context_mapping_cb(struct pci_dev *pdev,
>  	struct domain_context_mapping_data *data = opaque;
>  
>  	return domain_context_mapping_one(data->domain, data->iommu,
> -					  PCI_BUS_NUM(alias), alias & 0xff,
> -					  data->translation);
> +					  PCI_BUS_NUM(alias), alias & 0xff);
>  }
>  
>  static int
> -domain_context_mapping(struct dmar_domain *domain, struct device *dev,
> -		       int translation)
> +domain_context_mapping(struct dmar_domain *domain, struct device *dev)
>  {
>  	struct intel_iommu *iommu;
>  	u8 bus, devfn;
> @@ -2036,12 +2042,10 @@ domain_context_mapping(struct dmar_domain *domain, struct device *dev,
>  		return -ENODEV;
>  
>  	if (!dev_is_pci(dev))
> -		return domain_context_mapping_one(domain, iommu, bus, devfn,
> -						  translation);
> +		return domain_context_mapping_one(domain, iommu, bus, devfn);
>  
>  	data.domain = domain;
>  	data.iommu = iommu;
> -	data.translation = translation;
>  
>  	return pci_for_each_dma_alias(to_pci_dev(dev),
>  				      &domain_context_mapping_cb, &data);
> @@ -2508,7 +2512,7 @@ static int iommu_prepare_identity_map(struct device *dev,
>  		goto error;
>  
>  	/* context entry init */
> -	ret = domain_context_mapping(domain, dev, CONTEXT_TT_MULTI_LEVEL);
> +	ret = domain_context_mapping(domain, dev);
>  	if (ret)
>  		goto error;
>  
> @@ -2621,8 +2625,7 @@ static int identity_mapping(struct device *dev)
>  	return 0;
>  }
>  
> -static int domain_add_dev_info(struct dmar_domain *domain,
> -			       struct device *dev, int translation)
> +static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
>  {
>  	struct dmar_domain *ndomain;
>  	struct intel_iommu *iommu;
> @@ -2637,7 +2640,7 @@ static int domain_add_dev_info(struct dmar_domain *domain,
>  	if (ndomain != domain)
>  		return -EBUSY;
>  
> -	ret = domain_context_mapping(domain, dev, translation);
> +	ret = domain_context_mapping(domain, dev);
>  	if (ret) {
>  		domain_remove_one_dev_info(domain, dev);
>  		return ret;
> @@ -2782,9 +2785,7 @@ static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw
>  	if (!iommu_should_identity_map(dev, 1))
>  		return 0;
>  
> -	ret = domain_add_dev_info(si_domain, dev,
> -				  hw ? CONTEXT_TT_PASS_THROUGH :
> -				       CONTEXT_TT_MULTI_LEVEL);
> +	ret = domain_add_dev_info(si_domain, dev);
>  	if (!ret)
>  		pr_info("%s identity mapping for device %s\n",
>  			hw ? "Hardware" : "Software", dev_name(dev));
> @@ -3311,7 +3312,7 @@ static struct dmar_domain *__get_valid_domain_for_dev(struct device *dev)
>  
>  	/* make sure context mapping is ok */
>  	if (unlikely(!domain_context_mapped(dev))) {
> -		ret = domain_context_mapping(domain, dev, CONTEXT_TT_MULTI_LEVEL);
> +		ret = domain_context_mapping(domain, dev);
>  		if (ret) {
>  			pr_err("Domain context map for %s failed\n",
>  			       dev_name(dev));
> @@ -3366,10 +3367,7 @@ static int iommu_no_mapping(struct device *dev)
>  		 */
>  		if (iommu_should_identity_map(dev, 0)) {
>  			int ret;
> -			ret = domain_add_dev_info(si_domain, dev,
> -						  hw_pass_through ?
> -						  CONTEXT_TT_PASS_THROUGH :
> -						  CONTEXT_TT_MULTI_LEVEL);
> +			ret = domain_add_dev_info(si_domain, dev);
>  			if (!ret) {
>  				pr_info("64bit %s uses identity mapping\n",
>  					dev_name(dev));
> @@ -4786,7 +4784,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
>  		dmar_domain->agaw--;
>  	}
>  
> -	return domain_add_dev_info(dmar_domain, dev, CONTEXT_TT_MULTI_LEVEL);
> +	return domain_add_dev_info(dmar_domain, dev);
>  }
>  
>  static void intel_iommu_detach_device(struct iommu_domain *domain,

Re: [PATCH 05/26] iommu/vt-d: Calculate translation in domain_context_mapping_one

[Joerg Roedel]

On Thu, Aug 06, 2015 at 01:20:01PM -0600, Alex Williamson wrote:
> On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> > +
> > +	translation = CONTEXT_TT_MULTI_LEVEL;
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> I'm sure this is already part of your cleanup.

True, thanks. I removed that line.


	Joerg

[PATCH 06/26] iommu/vt-d: Simplify domain_context_mapping_one

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Get rid of the special cases for VM domains vs. non-VM
domains and simplify the code further to just handle the
hardware passthrough vs. page-table case.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 60 ++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 1a934f8..fdb51ec 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1940,52 +1940,44 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 		return 0;
 	}
 
-	context_clear_entry(context);
-
-	id = domain->id;
 	pgd = domain->pgd;
 
-	if (domain_type_is_vm_or_si(domain)) {
-		if (domain_type_is_vm(domain)) {
-			id = __iommu_attach_domain(domain, iommu);
-			if (id < 0) {
-				spin_unlock_irqrestore(&iommu->lock, flags);
-				pr_err("%s: No free domain ids\n", iommu->name);
-				return -EFAULT;
-			}
-		}
-
-		/* Skip top levels of page tables for
-		 * iommu which has less agaw than default.
-		 * Unnecessary for PT mode.
-		 */
-		if (translation != CONTEXT_TT_PASS_THROUGH) {
-			for (agaw = domain->agaw; agaw != iommu->agaw; agaw--) {
-				pgd = phys_to_virt(dma_pte_addr(pgd));
-				if (!dma_pte_present(pgd)) {
-					spin_unlock_irqrestore(&iommu->lock, flags);
-					return -ENOMEM;
-				}
-			}
-		}
+	id = __iommu_attach_domain(domain, iommu);
+	if (id < 0) {
+		spin_unlock_irqrestore(&iommu->lock, flags);
+		pr_err("%s: No free domain ids\n", iommu->name);
+		return -EFAULT;
 	}
 
+	context_clear_entry(context);
 	context_set_domain_id(context, id);
 
+	/*
+	 * Skip top levels of page tables for iommu which has less agaw
+	 * than default.  Unnecessary for PT mode.
+	 */
 	if (translation != CONTEXT_TT_PASS_THROUGH) {
+		for (agaw = domain->agaw; agaw != iommu->agaw; agaw--) {
+			pgd = phys_to_virt(dma_pte_addr(pgd));
+			if (!dma_pte_present(pgd)) {
+				spin_unlock_irqrestore(&iommu->lock, flags);
+				return -ENOMEM;
+			}
+		}
+
 		info = iommu_support_dev_iotlb(domain, iommu, bus, devfn);
 		translation = info ? CONTEXT_TT_DEV_IOTLB :
 				     CONTEXT_TT_MULTI_LEVEL;
-	}
-	/*
-	 * In pass through mode, AW must be programmed to indicate the largest
-	 * AGAW value supported by hardware. And ASR is ignored by hardware.
-	 */
-	if (unlikely(translation == CONTEXT_TT_PASS_THROUGH))
-		context_set_address_width(context, iommu->msagaw);
-	else {
+
 		context_set_address_root(context, virt_to_phys(pgd));
 		context_set_address_width(context, iommu->agaw);
+	} else {
+		/*
+		 * In pass through mode, AW must be programmed to
+		 * indicate the largest AGAW value supported by
+		 * hardware. And ASR is ignored by hardware.
+		 */
+		context_set_address_width(context, iommu->msagaw);
 	}
 
 	context_set_translation_type(context, translation);
-- 
1.9.1

[PATCH 07/26] iommu/vt-d: Pass dmar_domain directly into iommu_flush_iotlb_psi

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This function can figure out the domain-id to use itself
from the iommu_did array. This is more reliable over
different domain types and brings us one step further to
remove the domain->id field.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index fdb51ec..31e8100 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1469,11 +1469,14 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
-static void iommu_flush_iotlb_psi(struct intel_iommu *iommu, u16 did,
-				  unsigned long pfn, unsigned int pages, int ih, int map)
+static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
+				  struct dmar_domain *domain,
+				  unsigned long pfn, unsigned int pages,
+				  int ih, int map)
 {
 	unsigned int mask = ilog2(__roundup_pow_of_two(pages));
 	uint64_t addr = (uint64_t)pfn << VTD_PAGE_SHIFT;
+	u16 did = domain->iommu_did[iommu->seq_id];
 
 	BUG_ON(pages == 0);
 
@@ -3420,7 +3423,9 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 
 	/* it's a non-present to present mapping. Only flush if caching mode */
 	if (cap_caching_mode(iommu->cap))
-		iommu_flush_iotlb_psi(iommu, domain->id, mm_to_dma_pfn(iova->pfn_lo), size, 0, 1);
+		iommu_flush_iotlb_psi(iommu, domain,
+				      mm_to_dma_pfn(iova->pfn_lo),
+				      size, 0, 1);
 	else
 		iommu_flush_write_buffer(iommu);
 
@@ -3471,7 +3476,7 @@ static void flush_unmaps(void)
 
 			/* On real hardware multiple invalidations are expensive */
 			if (cap_caching_mode(iommu->cap))
-				iommu_flush_iotlb_psi(iommu, domain->id,
+				iommu_flush_iotlb_psi(iommu, domain,
 					iova->pfn_lo, iova_size(iova),
 					!deferred_flush[i].freelist[j], 0);
 			else {
@@ -3555,7 +3560,7 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr)
 	freelist = domain_unmap(domain, start_pfn, last_pfn);
 
 	if (intel_iommu_strict) {
-		iommu_flush_iotlb_psi(iommu, domain->id, start_pfn,
+		iommu_flush_iotlb_psi(iommu, domain, start_pfn,
 				      last_pfn - start_pfn + 1, !freelist, 0);
 		/* free iova */
 		__free_iova(&domain->iovad, iova);
@@ -3713,7 +3718,7 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 
 	/* it's a non-present to present mapping. Only flush if caching mode */
 	if (cap_caching_mode(iommu->cap))
-		iommu_flush_iotlb_psi(iommu, domain->id, start_vpfn, size, 0, 1);
+		iommu_flush_iotlb_psi(iommu, domain, start_vpfn, size, 0, 1);
 	else
 		iommu_flush_write_buffer(iommu);
 
@@ -4419,7 +4424,7 @@ static int intel_iommu_memory_notifier(struct notifier_block *nb,
 
 			rcu_read_lock();
 			for_each_active_iommu(iommu, drhd)
-				iommu_flush_iotlb_psi(iommu, si_domain->id,
+				iommu_flush_iotlb_psi(iommu, si_domain,
 					iova->pfn_lo, iova_size(iova),
 					!freelist, 0);
 			rcu_read_unlock();
@@ -4849,17 +4854,18 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 	npages = last_pfn - start_pfn + 1;
 
 	for_each_set_bit(iommu_id, dmar_domain->iommu_bmp, g_num_of_iommus) {
-               iommu = g_iommus[iommu_id];
-
-               /*
-                * find bit position of dmar_domain
-                */
-               ndomains = cap_ndoms(iommu->cap);
-               for_each_set_bit(num, iommu->domain_ids, ndomains) {
-                       if (get_iommu_domain(iommu, num) == dmar_domain)
-                               iommu_flush_iotlb_psi(iommu, num, start_pfn,
-						     npages, !freelist, 0);
-	       }
+		iommu = g_iommus[iommu_id];
+
+		/*
+		 * find bit position of dmar_domain
+		 */
+		ndomains = cap_ndoms(iommu->cap);
+		for_each_set_bit(num, iommu->domain_ids, ndomains) {
+			if (get_iommu_domain(iommu, num) == dmar_domain)
+				iommu_flush_iotlb_psi(iommu, dmar_domain,
+						      start_pfn, npages,
+						      !freelist, 0);
+		}
 
 	}
 
-- 
1.9.1

[PATCH 08/26] iommu/vt-d: Don't pre-allocate domain ids for si_domain

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

There is no reason for this special handling of the
si_domain. The per-iommu domain-id can be allocated
on-demand like for any other domain. So remove the
pre-allocation code.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 31e8100..35a918e 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2556,37 +2556,18 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width);
 
 static int __init si_domain_init(int hw)
 {
-	struct dmar_drhd_unit *drhd;
-	struct intel_iommu *iommu;
 	int nid, ret = 0;
-	bool first = true;
 
 	si_domain = alloc_domain(DOMAIN_FLAG_STATIC_IDENTITY);
 	if (!si_domain)
 		return -EFAULT;
 
-	for_each_active_iommu(iommu, drhd) {
-		ret = iommu_attach_domain(si_domain, iommu);
-		if (ret < 0) {
-			domain_exit(si_domain);
-			return -EFAULT;
-		} else if (first) {
-			si_domain->id = ret;
-			first = false;
-		} else if (si_domain->id != ret) {
-			domain_exit(si_domain);
-			return -EFAULT;
-		}
-		domain_attach_iommu(si_domain, iommu);
-	}
-
 	if (md_domain_init(si_domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
 		domain_exit(si_domain);
 		return -EFAULT;
 	}
 
-	pr_debug("Identity mapping domain is domain %d\n",
-		 si_domain->id);
+	pr_debug("Identity mapping domain allocated\n");
 
 	if (hw)
 		return 0;
@@ -4195,13 +4176,6 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
 	iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
 	iommu_enable_translation(iommu);
 
-	if (si_domain) {
-		ret = iommu_attach_domain(si_domain, iommu);
-		if (ret < 0 || si_domain->id != ret)
-			goto disable_iommu;
-		domain_attach_iommu(si_domain, iommu);
-	}
-
 	iommu_disable_protect_mem_regions(iommu);
 	return 0;
 
-- 
1.9.1

[PATCH 09/26] iommu/vt-d: Kill dmar_domain->id

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This field is now obsolete because all places use the
per-iommu domain-ids. Kill the remaining uses of this field
and remove it.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 35a918e..25b2ba7 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -374,7 +374,6 @@ static int hw_pass_through = 1;
 #define DOMAIN_FLAG_STATIC_IDENTITY	(1 << 1)
 
 struct dmar_domain {
-	int	id;			/* domain id */
 	int	nid;			/* node id */
 	DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
 					/* bitmap of iommus this domain uses*/
@@ -1653,8 +1652,6 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
 
 static struct dmar_domain *alloc_domain(int flags)
 {
-	/* domain id for virtual machine, it won't be set in context */
-	static atomic_t vm_domid = ATOMIC_INIT(0);
 	struct dmar_domain *domain;
 
 	domain = alloc_domain_mem();
@@ -1666,8 +1663,6 @@ static struct dmar_domain *alloc_domain(int flags)
 	domain->flags = flags;
 	spin_lock_init(&domain->iommu_lock);
 	INIT_LIST_HEAD(&domain->devices);
-	if (flags & DOMAIN_FLAG_VIRTUAL_MACHINE)
-		domain->id = atomic_inc_return(&vm_domid);
 
 	return domain;
 }
@@ -2390,8 +2385,7 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 	domain = alloc_domain(0);
 	if (!domain)
 		return NULL;
-	domain->id = iommu_attach_domain(domain, iommu);
-	if (domain->id < 0) {
+	if (iommu_attach_domain(domain, iommu) < 0) {
 		free_domain_mem(domain);
 		return NULL;
 	}
@@ -2444,8 +2438,7 @@ static int iommu_domain_identity_map(struct dmar_domain *domain,
 		return -ENOMEM;
 	}
 
-	pr_debug("Mapping reserved region %llx-%llx for domain %d\n",
-		 start, end, domain->id);
+	pr_debug("Mapping reserved region %llx-%llx\n", start, end);
 	/*
 	 * RMRR range might have overlap with physical memory range,
 	 * clear it first
-- 
1.9.1

[PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This replaces the dmar_domain->iommu_bmp with a similar
reference count array. This allows us to keep track of how
many devices behind each iommu are attached to the domain.

This is necessary for further simplifications and
optimizations to the iommu<->domain attachment code.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 82 +++++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 25b2ba7..c46afb6 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -373,10 +373,16 @@ static int hw_pass_through = 1;
 /* si_domain contains mulitple devices */
 #define DOMAIN_FLAG_STATIC_IDENTITY	(1 << 1)
 
+#define for_each_domain_iommu(idx, domain)			\
+	for (idx = 0; idx < g_num_of_iommus; idx++)		\
+		if (domain->iommu_refcnt[idx])
+
 struct dmar_domain {
 	int	nid;			/* node id */
-	DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
-					/* bitmap of iommus this domain uses*/
+
+	int		iommu_refcnt[DMAR_UNITS_SUPPORTED];
+					/* Refcount of devices per iommu */
+
 
 	u16	iommu_did[DMAR_UNITS_SUPPORTED];
 					/* Domain ids per IOMMU */
@@ -697,7 +703,9 @@ static struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
 
 	/* si_domain and vm domain should not get here. */
 	BUG_ON(domain_type_is_vm_or_si(domain));
-	iommu_id = find_first_bit(domain->iommu_bmp, g_num_of_iommus);
+	for_each_domain_iommu(iommu_id, domain)
+		break;
+
 	if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
 		return NULL;
 
@@ -713,7 +721,7 @@ static void domain_update_iommu_coherency(struct dmar_domain *domain)
 
 	domain->iommu_coherency = 1;
 
-	for_each_set_bit(i, domain->iommu_bmp, g_num_of_iommus) {
+	for_each_domain_iommu(i, domain) {
 		found = true;
 		if (!ecap_coherent(g_iommus[i]->ecap)) {
 			domain->iommu_coherency = 0;
@@ -1605,25 +1613,26 @@ static int iommu_init_domains(struct intel_iommu *iommu)
 
 static void disable_dmar_iommu(struct intel_iommu *iommu)
 {
-	struct dmar_domain *domain;
-	int i;
+	struct device_domain_info *info, *tmp;
 
-	if ((iommu->domains) && (iommu->domain_ids)) {
-		for_each_set_bit(i, iommu->domain_ids, cap_ndoms(iommu->cap)) {
-			/*
-			 * Domain id 0 is reserved for invalid translation
-			 * if hardware supports caching mode and used as
-			 * a non-allocated marker.
-			 */
-			if (i == 0)
-				continue;
+	if (!iommu->domains || !iommu->domain_ids)
+		return;
 
-			domain = get_iommu_domain(iommu, i);
-			clear_bit(i, iommu->domain_ids);
-			if (domain_detach_iommu(domain, iommu) == 0 &&
-			    !domain_type_is_vm(domain))
-				domain_exit(domain);
-		}
+	list_for_each_entry_safe(info, tmp, &device_domain_list, global) {
+		struct dmar_domain *domain;
+
+		if (info->iommu != iommu)
+			continue;
+
+		if (!info->dev || !info->domain)
+			continue;
+
+		domain = info->domain;
+
+		domain_remove_one_dev_info(domain, info->dev);
+
+		if (!domain_type_is_vm_or_si(domain))
+			domain_exit(domain);
 	}
 
 	if (iommu->gcmd & DMA_GCMD_TE)
@@ -1731,10 +1740,10 @@ static void domain_attach_iommu(struct dmar_domain *domain,
 	unsigned long flags;
 
 	spin_lock_irqsave(&domain->iommu_lock, flags);
-	if (!test_and_set_bit(iommu->seq_id, domain->iommu_bmp)) {
-		domain->iommu_count++;
-		if (domain->iommu_count == 1)
-			domain->nid = iommu->node;
+	domain->iommu_refcnt[iommu->seq_id] += 1;
+	domain->iommu_count += 1;
+	if (domain->iommu_refcnt[iommu->seq_id] == 1) {
+		domain->nid = iommu->node;
 		domain_update_iommu_cap(domain);
 	}
 	spin_unlock_irqrestore(&domain->iommu_lock, flags);
@@ -1747,8 +1756,9 @@ static int domain_detach_iommu(struct dmar_domain *domain,
 	int count = INT_MAX;
 
 	spin_lock_irqsave(&domain->iommu_lock, flags);
-	if (test_and_clear_bit(iommu->seq_id, domain->iommu_bmp)) {
-		count = --domain->iommu_count;
+	domain->iommu_refcnt[iommu->seq_id] -= 1;
+	count = --domain->iommu_count;
+	if (domain->iommu_refcnt[iommu->seq_id] == 0) {
 		domain_update_iommu_cap(domain);
 		domain->iommu_did[iommu->seq_id] = 0;
 	}
@@ -1873,9 +1883,8 @@ static int domain_init(struct dmar_domain *domain, int guest_width)
 
 static void domain_exit(struct dmar_domain *domain)
 {
-	struct dmar_drhd_unit *drhd;
-	struct intel_iommu *iommu;
 	struct page *freelist = NULL;
+	int i;
 
 	/* Domain 0 is reserved, so dont process it */
 	if (!domain)
@@ -1895,10 +1904,8 @@ static void domain_exit(struct dmar_domain *domain)
 
 	/* clear attached or cached domains */
 	rcu_read_lock();
-	for_each_active_iommu(iommu, drhd)
-		if (domain_type_is_vm(domain) ||
-		    test_bit(iommu->seq_id, domain->iommu_bmp))
-			iommu_detach_domain(domain, iommu);
+	for_each_domain_iommu(i, domain)
+		iommu_detach_domain(domain, g_iommus[i]);
 	rcu_read_unlock();
 
 	dma_free_pagelist(freelist);
@@ -4610,9 +4617,10 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 				continue;
 		}
 
-		/* if there is no other devices under the same iommu
-		 * owned by this domain, clear this iommu in iommu_bmp
-		 * update iommu count and coherency
+		/*
+		 * If there is no other devices under the same iommu owned by
+		 * this domain, clear this iommu in iommu_refcnt update iommu
+		 * count and coherency.
 		 */
 		if (info->iommu == iommu)
 			found = true;
@@ -4820,7 +4828,7 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 
 	npages = last_pfn - start_pfn + 1;
 
-	for_each_set_bit(iommu_id, dmar_domain->iommu_bmp, g_num_of_iommus) {
+	for_each_domain_iommu(iommu_id, dmar_domain) {
 		iommu = g_iommus[iommu_id];
 
 		/*
-- 
1.9.1

Re: [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount

[Alex Williamson]

On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> This replaces the dmar_domain->iommu_bmp with a similar
> reference count array. This allows us to keep track of how
> many devices behind each iommu are attached to the domain.
> 
> This is necessary for further simplifications and
> optimizations to the iommu<->domain attachment code.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/intel-iommu.c | 82 +++++++++++++++++++++++++--------------------
>  1 file changed, 45 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 25b2ba7..c46afb6 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -373,10 +373,16 @@ static int hw_pass_through = 1;
>  /* si_domain contains mulitple devices */
>  #define DOMAIN_FLAG_STATIC_IDENTITY	(1 << 1)
>  
> +#define for_each_domain_iommu(idx, domain)			\
> +	for (idx = 0; idx < g_num_of_iommus; idx++)		\
> +		if (domain->iommu_refcnt[idx])
> +
>  struct dmar_domain {
>  	int	nid;			/* node id */
> -	DECLARE_BITMAP(iommu_bmp, DMAR_UNITS_SUPPORTED);
> -					/* bitmap of iommus this domain uses*/
> +
> +	int		iommu_refcnt[DMAR_UNITS_SUPPORTED];
> +					/* Refcount of devices per iommu */

unsigned?

> +
>  
>  	u16	iommu_did[DMAR_UNITS_SUPPORTED];
>  					/* Domain ids per IOMMU */
> @@ -697,7 +703,9 @@ static struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
>  
>  	/* si_domain and vm domain should not get here. */
>  	BUG_ON(domain_type_is_vm_or_si(domain));
> -	iommu_id = find_first_bit(domain->iommu_bmp, g_num_of_iommus);
> +	for_each_domain_iommu(iommu_id, domain)
> +		break;
> +
>  	if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
>  		return NULL;
>  
> @@ -713,7 +721,7 @@ static void domain_update_iommu_coherency(struct dmar_domain *domain)
>  
>  	domain->iommu_coherency = 1;
>  
> -	for_each_set_bit(i, domain->iommu_bmp, g_num_of_iommus) {
> +	for_each_domain_iommu(i, domain) {
>  		found = true;
>  		if (!ecap_coherent(g_iommus[i]->ecap)) {
>  			domain->iommu_coherency = 0;
> @@ -1605,25 +1613,26 @@ static int iommu_init_domains(struct intel_iommu *iommu)
>  
>  static void disable_dmar_iommu(struct intel_iommu *iommu)
>  {
> -	struct dmar_domain *domain;
> -	int i;
> +	struct device_domain_info *info, *tmp;
>  
> -	if ((iommu->domains) && (iommu->domain_ids)) {
> -		for_each_set_bit(i, iommu->domain_ids, cap_ndoms(iommu->cap)) {
> -			/*
> -			 * Domain id 0 is reserved for invalid translation
> -			 * if hardware supports caching mode and used as
> -			 * a non-allocated marker.
> -			 */
> -			if (i == 0)
> -				continue;
> +	if (!iommu->domains || !iommu->domain_ids)
> +		return;
>  
> -			domain = get_iommu_domain(iommu, i);
> -			clear_bit(i, iommu->domain_ids);
> -			if (domain_detach_iommu(domain, iommu) == 0 &&
> -			    !domain_type_is_vm(domain))
> -				domain_exit(domain);
> -		}
> +	list_for_each_entry_safe(info, tmp, &device_domain_list, global) {
> +		struct dmar_domain *domain;
> +
> +		if (info->iommu != iommu)
> +			continue;
> +
> +		if (!info->dev || !info->domain)
> +			continue;
> +
> +		domain = info->domain;
> +
> +		domain_remove_one_dev_info(domain, info->dev);
> +
> +		if (!domain_type_is_vm_or_si(domain))
> +			domain_exit(domain);
>  	}
>  
>  	if (iommu->gcmd & DMA_GCMD_TE)
> @@ -1731,10 +1740,10 @@ static void domain_attach_iommu(struct dmar_domain *domain,
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&domain->iommu_lock, flags);
> -	if (!test_and_set_bit(iommu->seq_id, domain->iommu_bmp)) {
> -		domain->iommu_count++;
> -		if (domain->iommu_count == 1)
> -			domain->nid = iommu->node;
> +	domain->iommu_refcnt[iommu->seq_id] += 1;
> +	domain->iommu_count += 1;
> +	if (domain->iommu_refcnt[iommu->seq_id] == 1) {
> +		domain->nid = iommu->node;
>  		domain_update_iommu_cap(domain);
>  	}
>  	spin_unlock_irqrestore(&domain->iommu_lock, flags);
> @@ -1747,8 +1756,9 @@ static int domain_detach_iommu(struct dmar_domain *domain,
>  	int count = INT_MAX;
>  
>  	spin_lock_irqsave(&domain->iommu_lock, flags);
> -	if (test_and_clear_bit(iommu->seq_id, domain->iommu_bmp)) {
> -		count = --domain->iommu_count;
> +	domain->iommu_refcnt[iommu->seq_id] -= 1;
> +	count = --domain->iommu_count;
> +	if (domain->iommu_refcnt[iommu->seq_id] == 0) {
>  		domain_update_iommu_cap(domain);
>  		domain->iommu_did[iommu->seq_id] = 0;
>  	}
> @@ -1873,9 +1883,8 @@ static int domain_init(struct dmar_domain *domain, int guest_width)
>  
>  static void domain_exit(struct dmar_domain *domain)
>  {
> -	struct dmar_drhd_unit *drhd;
> -	struct intel_iommu *iommu;
>  	struct page *freelist = NULL;
> +	int i;
>  
>  	/* Domain 0 is reserved, so dont process it */
>  	if (!domain)
> @@ -1895,10 +1904,8 @@ static void domain_exit(struct dmar_domain *domain)
>  
>  	/* clear attached or cached domains */
>  	rcu_read_lock();
> -	for_each_active_iommu(iommu, drhd)
> -		if (domain_type_is_vm(domain) ||
> -		    test_bit(iommu->seq_id, domain->iommu_bmp))
> -			iommu_detach_domain(domain, iommu);
> +	for_each_domain_iommu(i, domain)
> +		iommu_detach_domain(domain, g_iommus[i]);

Yay!

>  	rcu_read_unlock();
>  
>  	dma_free_pagelist(freelist);
> @@ -4610,9 +4617,10 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
>  				continue;
>  		}
>  
> -		/* if there is no other devices under the same iommu
> -		 * owned by this domain, clear this iommu in iommu_bmp
> -		 * update iommu count and coherency
> +		/*
> +		 * If there is no other devices under the same iommu owned by
> +		 * this domain, clear this iommu in iommu_refcnt update iommu
> +		 * count and coherency.
>  		 */
>  		if (info->iommu == iommu)
>  			found = true;
> @@ -4820,7 +4828,7 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
>  
>  	npages = last_pfn - start_pfn + 1;
>  
> -	for_each_set_bit(iommu_id, dmar_domain->iommu_bmp, g_num_of_iommus) {
> +	for_each_domain_iommu(iommu_id, dmar_domain) {
>  		iommu = g_iommus[iommu_id];
>  
>  		/*

Re: [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount

[Joerg Roedel]

On Thu, Aug 06, 2015 at 01:19:57PM -0600, Alex Williamson wrote:
> On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> > +
> > +	int		iommu_refcnt[DMAR_UNITS_SUPPORTED];
> > +					/* Refcount of devices per iommu */
> 
> unsigned?

Right, makes more sense. Changed that too, thanks.


	Joerg

[PATCH 11/26] iommu/vt-d: Simplify io/tlb flushing in intel_iommu_unmap

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

We don't need to do an expensive search for domain-ids
anymore, as we keep track of per-iommu domain-ids.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c46afb6..ae2343c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4811,7 +4811,7 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 	struct intel_iommu *iommu;
 	unsigned long start_pfn, last_pfn;
 	unsigned int npages;
-	int iommu_id, num, ndomains, level = 0;
+	int iommu_id, level = 0;
 
 	/* Cope with horrid API which requires us to unmap more than the
 	   size argument if it happens to be a large-page mapping. */
@@ -4831,17 +4831,8 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 	for_each_domain_iommu(iommu_id, dmar_domain) {
 		iommu = g_iommus[iommu_id];
 
-		/*
-		 * find bit position of dmar_domain
-		 */
-		ndomains = cap_ndoms(iommu->cap);
-		for_each_set_bit(num, iommu->domain_ids, ndomains) {
-			if (get_iommu_domain(iommu, num) == dmar_domain)
-				iommu_flush_iotlb_psi(iommu, dmar_domain,
-						      start_pfn, npages,
-						      !freelist, 0);
-		}
-
+		iommu_flush_iotlb_psi(g_iommus[iommu_id], dmar_domain,
+				      start_pfn, npages, !freelist, 0);
 	}
 
 	dma_free_pagelist(freelist);
-- 
1.9.1

[PATCH 12/26] iommu/vt-d: Simplify domain_remove_one_dev_info()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Simplify this function as much as possible with the new
iommu_refcnt field.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 50 +++++++++++++++------------------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ae2343c..f59b4ef 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4587,52 +4587,34 @@ static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
 static void domain_remove_one_dev_info(struct dmar_domain *domain,
 				       struct device *dev)
 {
-	struct device_domain_info *info, *tmp;
+	struct device_domain_info *info;
 	struct intel_iommu *iommu;
 	unsigned long flags;
-	bool found = false;
 	u8 bus, devfn;
 
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
 		return;
 
-	spin_lock_irqsave(&device_domain_lock, flags);
-	list_for_each_entry_safe(info, tmp, &domain->devices, link) {
-		if (info->iommu == iommu && info->bus == bus &&
-		    info->devfn == devfn) {
-			unlink_domain_info(info);
-			spin_unlock_irqrestore(&device_domain_lock, flags);
-
-			iommu_disable_dev_iotlb(info);
-			iommu_detach_dev(iommu, info->bus, info->devfn);
-			iommu_detach_dependent_devices(iommu, dev);
-			free_devinfo_mem(info);
-
-			spin_lock_irqsave(&device_domain_lock, flags);
-
-			if (found)
-				break;
-			else
-				continue;
-		}
+	info = dev->archdata.iommu;
 
-		/*
-		 * If there is no other devices under the same iommu owned by
-		 * this domain, clear this iommu in iommu_refcnt update iommu
-		 * count and coherency.
-		 */
-		if (info->iommu == iommu)
-			found = true;
-	}
+	if (WARN_ON(!info))
+		return;
 
+	spin_lock_irqsave(&device_domain_lock, flags);
+	unlink_domain_info(info);
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
-	if (found == 0) {
-		domain_detach_iommu(domain, iommu);
-		if (!domain_type_is_vm_or_si(domain))
-			iommu_detach_domain(domain, iommu);
-	}
+	iommu_disable_dev_iotlb(info);
+	iommu_detach_dev(iommu, info->bus, info->devfn);
+	iommu_detach_dependent_devices(iommu, dev);
+	free_devinfo_mem(info);
+	domain_detach_iommu(domain, iommu);
+
+	spin_lock_irqsave(&domain->iommu_lock, flags);
+	if (!domain->iommu_refcnt[iommu->seq_id])
+		iommu_detach_domain(domain, iommu);
+	spin_unlock_irqrestore(&domain->iommu_lock, flags);
 }
 
 static int md_domain_init(struct dmar_domain *domain, int guest_width)
-- 
1.9.1

[PATCH 13/26] iommu/vt-d: Simplify domain_remove_dev_info()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Just call domain_remove_one_dev_info() for all devices in
the domain instead of reimplementing the functionality.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f59b4ef..265b02e 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2251,25 +2251,9 @@ static inline void unlink_domain_info(struct device_domain_info *info)
 static void domain_remove_dev_info(struct dmar_domain *domain)
 {
 	struct device_domain_info *info, *tmp;
-	unsigned long flags;
-
-	spin_lock_irqsave(&device_domain_lock, flags);
-	list_for_each_entry_safe(info, tmp, &domain->devices, link) {
-		unlink_domain_info(info);
-		spin_unlock_irqrestore(&device_domain_lock, flags);
-
-		iommu_disable_dev_iotlb(info);
-		iommu_detach_dev(info->iommu, info->bus, info->devfn);
-
-		if (domain_type_is_vm(domain)) {
-			iommu_detach_dependent_devices(info->iommu, info->dev);
-			domain_detach_iommu(domain, info->iommu);
-		}
 
-		free_devinfo_mem(info);
-		spin_lock_irqsave(&device_domain_lock, flags);
-	}
-	spin_unlock_irqrestore(&device_domain_lock, flags);
+	list_for_each_entry_safe(info, tmp, &domain->devices, link)
+		domain_remove_one_dev_info(domain, info->dev);
 }
 
 /*
-- 
1.9.1

[PATCH 14/26] iommu/vt-d: Move context-mapping into dmar_insert_dev_info

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Do the context-mapping of devices from a single place in the
call-path and clean up the other call-sites.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 265b02e..0d4b700 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2325,6 +2325,12 @@ static struct dmar_domain *dmar_insert_dev_info(struct intel_iommu *iommu,
 		dev->archdata.iommu = info;
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
+	if (dev && domain_context_mapping(domain, dev)) {
+		pr_err("Domain context map for %s failed\n", dev_name(dev));
+		domain_remove_one_dev_info(domain, dev);
+		return NULL;
+	}
+
 	return domain;
 }
 
@@ -2337,11 +2343,11 @@ static int get_last_alias(struct pci_dev *pdev, u16 alias, void *opaque)
 /* domain is initialized */
 static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 {
+	struct device_domain_info *info = NULL;
 	struct dmar_domain *domain, *tmp;
 	struct intel_iommu *iommu;
-	struct device_domain_info *info;
-	u16 dma_alias;
 	unsigned long flags;
+	u16 dma_alias;
 	u8 bus, devfn;
 
 	domain = find_domain(dev);
@@ -2490,11 +2496,6 @@ static int iommu_prepare_identity_map(struct device *dev,
 	if (ret)
 		goto error;
 
-	/* context entry init */
-	ret = domain_context_mapping(domain, dev);
-	if (ret)
-		goto error;
-
 	return 0;
 
  error:
@@ -2590,7 +2591,6 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
 	struct dmar_domain *ndomain;
 	struct intel_iommu *iommu;
 	u8 bus, devfn;
-	int ret;
 
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
@@ -2600,12 +2600,6 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
 	if (ndomain != domain)
 		return -EBUSY;
 
-	ret = domain_context_mapping(domain, dev);
-	if (ret) {
-		domain_remove_one_dev_info(domain, dev);
-		return ret;
-	}
-
 	return 0;
 }
 
@@ -3261,7 +3255,6 @@ static struct iova *intel_alloc_iova(struct device *dev,
 static struct dmar_domain *__get_valid_domain_for_dev(struct device *dev)
 {
 	struct dmar_domain *domain;
-	int ret;
 
 	domain = get_domain_for_dev(dev, DEFAULT_DOMAIN_ADDRESS_WIDTH);
 	if (!domain) {
@@ -3270,16 +3263,6 @@ static struct dmar_domain *__get_valid_domain_for_dev(struct device *dev)
 		return NULL;
 	}
 
-	/* make sure context mapping is ok */
-	if (unlikely(!domain_context_mapped(dev))) {
-		ret = domain_context_mapping(domain, dev);
-		if (ret) {
-			pr_err("Domain context map for %s failed\n",
-			       dev_name(dev));
-			return NULL;
-		}
-	}
-
 	return domain;
 }
 
-- 
1.9.1

[PATCH 15/26] iommu/vt-d: Rename dmar_insert_dev_info()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Rename this function to dmar_insert_one_dev_info() to match
the name better with its counter part function
domain_remove_one_dev_info().

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0d4b700..8e87208 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2284,10 +2284,10 @@ dmar_search_domain_by_dev_info(int segment, int bus, int devfn)
 	return NULL;
 }
 
-static struct dmar_domain *dmar_insert_dev_info(struct intel_iommu *iommu,
-						int bus, int devfn,
-						struct device *dev,
-						struct dmar_domain *domain)
+static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
+						    int bus, int devfn,
+						    struct device *dev,
+						    struct dmar_domain *domain)
 {
 	struct dmar_domain *found = NULL;
 	struct device_domain_info *info;
@@ -2394,8 +2394,8 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 
 	/* register PCI DMA alias device */
 	if (dev_is_pci(dev)) {
-		tmp = dmar_insert_dev_info(iommu, PCI_BUS_NUM(dma_alias),
-					   dma_alias & 0xff, NULL, domain);
+		tmp = dmar_insert_one_dev_info(iommu, PCI_BUS_NUM(dma_alias),
+					       dma_alias & 0xff, NULL, domain);
 
 		if (!tmp || tmp != domain) {
 			domain_exit(domain);
@@ -2407,7 +2407,7 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 	}
 
 found_domain:
-	tmp = dmar_insert_dev_info(iommu, bus, devfn, dev, domain);
+	tmp = dmar_insert_one_dev_info(iommu, bus, devfn, dev, domain);
 
 	if (!tmp || tmp != domain) {
 		domain_exit(domain);
@@ -2596,7 +2596,7 @@ static int domain_add_dev_info(struct dmar_domain *domain, struct device *dev)
 	if (!iommu)
 		return -ENODEV;
 
-	ndomain = dmar_insert_dev_info(iommu, bus, devfn, dev, domain);
+	ndomain = dmar_insert_one_dev_info(iommu, bus, devfn, dev, domain);
 	if (ndomain != domain)
 		return -EBUSY;
 
-- 
1.9.1

[PATCH 16/26] iommu/vt-d: Rename domain_remove_one_dev_info()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Rename the function to dmar_remove_one_dev_info to match is
name better with its dmar_insert_one_dev_info counterpart.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8e87208..e033bb0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -470,8 +470,8 @@ static long list_size;
 
 static void domain_exit(struct dmar_domain *domain);
 static void domain_remove_dev_info(struct dmar_domain *domain);
-static void domain_remove_one_dev_info(struct dmar_domain *domain,
-				       struct device *dev);
+static void dmar_remove_one_dev_info(struct dmar_domain *domain,
+				     struct device *dev);
 static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
 					   struct device *dev);
 static int domain_detach_iommu(struct dmar_domain *domain,
@@ -1629,7 +1629,7 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 
 		domain = info->domain;
 
-		domain_remove_one_dev_info(domain, info->dev);
+		dmar_remove_one_dev_info(domain, info->dev);
 
 		if (!domain_type_is_vm_or_si(domain))
 			domain_exit(domain);
@@ -2253,7 +2253,7 @@ static void domain_remove_dev_info(struct dmar_domain *domain)
 	struct device_domain_info *info, *tmp;
 
 	list_for_each_entry_safe(info, tmp, &domain->devices, link)
-		domain_remove_one_dev_info(domain, info->dev);
+		dmar_remove_one_dev_info(domain, info->dev);
 }
 
 /*
@@ -2327,7 +2327,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 
 	if (dev && domain_context_mapping(domain, dev)) {
 		pr_err("Domain context map for %s failed\n", dev_name(dev));
-		domain_remove_one_dev_info(domain, dev);
+		dmar_remove_one_dev_info(domain, dev);
 		return NULL;
 	}
 
@@ -3298,7 +3298,7 @@ static int iommu_no_mapping(struct device *dev)
 			 * 32 bit DMA is removed from si_domain and fall back
 			 * to non-identity mapping.
 			 */
-			domain_remove_one_dev_info(si_domain, dev);
+			dmar_remove_one_dev_info(si_domain, dev);
 			pr_info("32bit %s uses non-identity mapping\n",
 				dev_name(dev));
 			return 0;
@@ -4305,7 +4305,7 @@ static int device_notifier(struct notifier_block *nb,
 		return 0;
 
 	down_read(&dmar_global_lock);
-	domain_remove_one_dev_info(domain, dev);
+	dmar_remove_one_dev_info(domain, dev);
 	if (!domain_type_is_vm_or_si(domain) && list_empty(&domain->devices))
 		domain_exit(domain);
 	up_read(&dmar_global_lock);
@@ -4551,8 +4551,8 @@ static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
 	pci_for_each_dma_alias(to_pci_dev(dev), &iommu_detach_dev_cb, iommu);
 }
 
-static void domain_remove_one_dev_info(struct dmar_domain *domain,
-				       struct device *dev)
+static void dmar_remove_one_dev_info(struct dmar_domain *domain,
+				     struct device *dev)
 {
 	struct device_domain_info *info;
 	struct intel_iommu *iommu;
@@ -4663,7 +4663,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 		old_domain = find_domain(dev);
 		if (old_domain) {
 			if (domain_type_is_vm_or_si(dmar_domain))
-				domain_remove_one_dev_info(old_domain, dev);
+				dmar_remove_one_dev_info(old_domain, dev);
 			else
 				domain_remove_dev_info(old_domain);
 
@@ -4711,7 +4711,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 static void intel_iommu_detach_device(struct iommu_domain *domain,
 				      struct device *dev)
 {
-	domain_remove_one_dev_info(to_dmar_domain(domain), dev);
+	dmar_remove_one_dev_info(to_dmar_domain(domain), dev);
 }
 
 static int intel_iommu_map(struct iommu_domain *domain,
-- 
1.9.1

[PATCH 17/26] iommu/vt-d: Rename iommu_detach_dependent_devices()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Rename this function and the ones further down its
call-chain to domain_context_clear_*. In particular this
means:

	iommu_detach_dependent_devices -> domain_context_clear
		   iommu_detach_dev_cb -> domain_context_clear_one_cb
		      iommu_detach_dev -> domain_context_clear_one

These names match a lot better with its
domain_context_mapping counterparts.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e033bb0..08391a6 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -472,8 +472,8 @@ static void domain_exit(struct dmar_domain *domain);
 static void domain_remove_dev_info(struct dmar_domain *domain);
 static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 				     struct device *dev);
-static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
-					   struct device *dev);
+static void domain_context_clear(struct intel_iommu *iommu,
+				 struct device *dev);
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu);
 
@@ -2228,7 +2228,7 @@ static inline int domain_pfn_mapping(struct dmar_domain *domain, unsigned long i
 	return __domain_mapping(domain, iov_pfn, NULL, phys_pfn, nr_pages, prot);
 }
 
-static void iommu_detach_dev(struct intel_iommu *iommu, u8 bus, u8 devfn)
+static void domain_context_clear_one(struct intel_iommu *iommu, u8 bus, u8 devfn)
 {
 	if (!iommu)
 		return;
@@ -4528,11 +4528,11 @@ out_free_dmar:
 	return ret;
 }
 
-static int iommu_detach_dev_cb(struct pci_dev *pdev, u16 alias, void *opaque)
+static int domain_context_clear_one_cb(struct pci_dev *pdev, u16 alias, void *opaque)
 {
 	struct intel_iommu *iommu = opaque;
 
-	iommu_detach_dev(iommu, PCI_BUS_NUM(alias), alias & 0xff);
+	domain_context_clear_one(iommu, PCI_BUS_NUM(alias), alias & 0xff);
 	return 0;
 }
 
@@ -4542,13 +4542,12 @@ static int iommu_detach_dev_cb(struct pci_dev *pdev, u16 alias, void *opaque)
  * devices, unbinding the driver from any one of them will possibly leave
  * the others unable to operate.
  */
-static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
-					   struct device *dev)
+static void domain_context_clear(struct intel_iommu *iommu, struct device *dev)
 {
 	if (!iommu || !dev || !dev_is_pci(dev))
 		return;
 
-	pci_for_each_dma_alias(to_pci_dev(dev), &iommu_detach_dev_cb, iommu);
+	pci_for_each_dma_alias(to_pci_dev(dev), &domain_context_clear_one_cb, iommu);
 }
 
 static void dmar_remove_one_dev_info(struct dmar_domain *domain,
@@ -4573,8 +4572,7 @@ static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
 	iommu_disable_dev_iotlb(info);
-	iommu_detach_dev(iommu, info->bus, info->devfn);
-	iommu_detach_dependent_devices(iommu, dev);
+	domain_context_clear(iommu, dev);
 	free_devinfo_mem(info);
 	domain_detach_iommu(domain, iommu);
 
-- 
1.9.1

[PATCH 18/26] iommu/vt-d: Pass an iommu pointer to domain_init()

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This allows to do domain->iommu attachment after domain_init
has run.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 08391a6..90c3fa5 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1829,9 +1829,9 @@ static inline int guestwidth_to_adjustwidth(int gaw)
 	return agaw;
 }
 
-static int domain_init(struct dmar_domain *domain, int guest_width)
+static int domain_init(struct dmar_domain *domain, struct intel_iommu *iommu,
+		       int guest_width)
 {
-	struct intel_iommu *iommu;
 	int adjust_width, agaw;
 	unsigned long sagaw;
 
@@ -1840,7 +1840,6 @@ static int domain_init(struct dmar_domain *domain, int guest_width)
 	domain_reserve_special_ranges(domain);
 
 	/* calculate AGAW */
-	iommu = domain_get_iommu(domain);
 	if (guest_width > cap_mgaw(iommu->cap))
 		guest_width = cap_mgaw(iommu->cap);
 	domain->gaw = guest_width;
@@ -2387,7 +2386,7 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 		return NULL;
 	}
 	domain_attach_iommu(domain, iommu);
-	if (domain_init(domain, gaw)) {
+	if (domain_init(domain, iommu, gaw)) {
 		domain_exit(domain);
 		return NULL;
 	}
-- 
1.9.1

[PATCH 19/26] iommu/vt-d: Establish domain<->iommu link in dmar_insert_one_dev_info

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This makes domain attachment more synchronous with domain
deattachment. The domain<->iommu link is released in
dmar_remove_one_dev_info.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 90c3fa5..27762d7 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1916,14 +1916,16 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 				      struct intel_iommu *iommu,
 				      u8 bus, u8 devfn)
 {
+	u16 did = domain->iommu_did[iommu->seq_id];
 	int translation = CONTEXT_TT_MULTI_LEVEL;
 	struct device_domain_info *info = NULL;
 	struct context_entry *context;
 	unsigned long flags;
 	struct dma_pte *pgd;
-	int id;
 	int agaw;
 
+	WARN_ON(did == 0);
+
 	translation = CONTEXT_TT_MULTI_LEVEL;
 	if (hw_pass_through && domain_type_is_si(domain))
 		translation = CONTEXT_TT_PASS_THROUGH;
@@ -1946,15 +1948,8 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 
 	pgd = domain->pgd;
 
-	id = __iommu_attach_domain(domain, iommu);
-	if (id < 0) {
-		spin_unlock_irqrestore(&iommu->lock, flags);
-		pr_err("%s: No free domain ids\n", iommu->name);
-		return -EFAULT;
-	}
-
 	context_clear_entry(context);
-	context_set_domain_id(context, id);
+	context_set_domain_id(context, did);
 
 	/*
 	 * Skip top levels of page tables for iommu which has less agaw
@@ -2000,15 +1995,13 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 					   (((u16)bus) << 8) | devfn,
 					   DMA_CCMD_MASK_NOBIT,
 					   DMA_CCMD_DEVICE_INVL);
-		iommu->flush.flush_iotlb(iommu, id, 0, 0, DMA_TLB_DSI_FLUSH);
+		iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
 	} else {
 		iommu_flush_write_buffer(iommu);
 	}
 	iommu_enable_dev_iotlb(info);
 	spin_unlock_irqrestore(&iommu->lock, flags);
 
-	domain_attach_iommu(domain, iommu);
-
 	return 0;
 }
 
@@ -2318,6 +2311,12 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 		return found;
 	}
 
+	if (iommu_attach_domain(domain, iommu) < 0) {
+		spin_unlock_irqrestore(&device_domain_lock, flags);
+		return NULL;
+	}
+	domain_attach_iommu(domain, iommu);
+
 	list_add(&info->link, &domain->devices);
 	list_add(&info->global, &device_domain_list);
 	if (dev)
@@ -2381,11 +2380,6 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 	domain = alloc_domain(0);
 	if (!domain)
 		return NULL;
-	if (iommu_attach_domain(domain, iommu) < 0) {
-		free_domain_mem(domain);
-		return NULL;
-	}
-	domain_attach_iommu(domain, iommu);
 	if (domain_init(domain, iommu, gaw)) {
 		domain_exit(domain);
 		return NULL;
-- 
1.9.1

[PATCH 20/26] iommu/vt-d: Unify domain->iommu attach/detachment

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Move the code to attach/detach domains to iommus and vice
verce into a single function to make sure there are no
dangling references.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 124 +++++++++++++++++---------------------------
 1 file changed, 49 insertions(+), 75 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 27762d7..50b5e6c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1676,89 +1676,64 @@ static struct dmar_domain *alloc_domain(int flags)
 	return domain;
 }
 
-static int __iommu_attach_domain(struct dmar_domain *domain,
-				 struct intel_iommu *iommu)
-{
-	int num;
-	unsigned long ndomains;
-
-	num = domain->iommu_did[iommu->seq_id];
-	if (num)
-		return num;
-
-	ndomains = cap_ndoms(iommu->cap);
-	num	 = find_first_zero_bit(iommu->domain_ids, ndomains);
-
-	if (num < ndomains) {
-		set_bit(num, iommu->domain_ids);
-		set_iommu_domain(iommu, num, domain);
-		domain->iommu_did[iommu->seq_id] = num;
-	} else {
-		num = -ENOSPC;
-	}
-
-	if (num < 0)
-		pr_err("%s: No free domain ids\n", iommu->name);
-
-	return num;
-}
-
-static int iommu_attach_domain(struct dmar_domain *domain,
+/* Must be called with iommu->lock */
+static int domain_attach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu)
 {
-	int num;
-	unsigned long flags;
-
-	spin_lock_irqsave(&iommu->lock, flags);
-	num = __iommu_attach_domain(domain, iommu);
-	spin_unlock_irqrestore(&iommu->lock, flags);
-
-	return num;
-}
-
-static void iommu_detach_domain(struct dmar_domain *domain,
-				struct intel_iommu *iommu)
-{
+	unsigned long ndomains;
 	unsigned long flags;
-	int num;
-
-	spin_lock_irqsave(&iommu->lock, flags);
-
-	num = domain->iommu_did[iommu->seq_id];
-
-	WARN_ON(num == 0);
-
-	clear_bit(num, iommu->domain_ids);
-	set_iommu_domain(iommu, num, NULL);
+	int ret, num;
 
-	spin_unlock_irqrestore(&iommu->lock, flags);
-}
-
-static void domain_attach_iommu(struct dmar_domain *domain,
-			       struct intel_iommu *iommu)
-{
-	unsigned long flags;
+	assert_spin_locked(&iommu->lock);
 
 	spin_lock_irqsave(&domain->iommu_lock, flags);
+
 	domain->iommu_refcnt[iommu->seq_id] += 1;
 	domain->iommu_count += 1;
 	if (domain->iommu_refcnt[iommu->seq_id] == 1) {
-		domain->nid = iommu->node;
+		ndomains = cap_ndoms(iommu->cap);
+		num      = find_first_zero_bit(iommu->domain_ids, ndomains);
+
+		if (num >= ndomains) {
+			pr_err("%s: No free domain ids\n", iommu->name);
+			domain->iommu_refcnt[iommu->seq_id] -= 1;
+			domain->iommu_count -= 1;
+			ret = -ENOSPC;
+			goto out_unlock;
+		}
+
+		set_bit(num, iommu->domain_ids);
+		set_iommu_domain(iommu, num, domain);
+
+		domain->iommu_did[iommu->seq_id] = num;
+		domain->nid			 = iommu->node;
+
 		domain_update_iommu_cap(domain);
 	}
+
+	ret = 0;
+out_unlock:
 	spin_unlock_irqrestore(&domain->iommu_lock, flags);
+
+	return ret;
 }
 
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu)
 {
+	int num, count = INT_MAX;
 	unsigned long flags;
-	int count = INT_MAX;
+
+	assert_spin_locked(&iommu->lock);
 
 	spin_lock_irqsave(&domain->iommu_lock, flags);
 	domain->iommu_refcnt[iommu->seq_id] -= 1;
 	count = --domain->iommu_count;
 	if (domain->iommu_refcnt[iommu->seq_id] == 0) {
+		num = domain->iommu_did[iommu->seq_id];
+		clear_bit(num, iommu->domain_ids);
+		set_iommu_domain(iommu, num, NULL);
+
 		domain_update_iommu_cap(domain);
 		domain->iommu_did[iommu->seq_id] = 0;
 	}
@@ -1883,7 +1858,6 @@ static int domain_init(struct dmar_domain *domain, struct intel_iommu *iommu,
 static void domain_exit(struct dmar_domain *domain)
 {
 	struct page *freelist = NULL;
-	int i;
 
 	/* Domain 0 is reserved, so dont process it */
 	if (!domain)
@@ -1893,20 +1867,16 @@ static void domain_exit(struct dmar_domain *domain)
 	if (!intel_iommu_strict)
 		flush_unmaps_timeout(0);
 
-	/* remove associated devices */
+	/* Remove associated devices and clear attached or cached domains */
+	rcu_read_lock();
 	domain_remove_dev_info(domain);
+	rcu_read_unlock();
 
 	/* destroy iovas */
 	put_iova_domain(&domain->iovad);
 
 	freelist = domain_unmap(domain, 0, DOMAIN_MAX_PFN(domain->gaw));
 
-	/* clear attached or cached domains */
-	rcu_read_lock();
-	for_each_domain_iommu(i, domain)
-		iommu_detach_domain(domain, g_iommus[i]);
-	rcu_read_unlock();
-
 	dma_free_pagelist(freelist);
 
 	free_domain_mem(domain);
@@ -2284,6 +2254,7 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	struct dmar_domain *found = NULL;
 	struct device_domain_info *info;
 	unsigned long flags;
+	int ret;
 
 	info = alloc_devinfo_mem();
 	if (!info)
@@ -2311,11 +2282,14 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 		return found;
 	}
 
-	if (iommu_attach_domain(domain, iommu) < 0) {
+	spin_lock(&iommu->lock);
+	ret = domain_attach_iommu(domain, iommu);
+	spin_unlock(&iommu->lock);
+
+	if (ret) {
 		spin_unlock_irqrestore(&device_domain_lock, flags);
 		return NULL;
 	}
-	domain_attach_iommu(domain, iommu);
 
 	list_add(&info->link, &domain->devices);
 	list_add(&info->global, &device_domain_list);
@@ -4567,12 +4541,10 @@ static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 	iommu_disable_dev_iotlb(info);
 	domain_context_clear(iommu, dev);
 	free_devinfo_mem(info);
-	domain_detach_iommu(domain, iommu);
 
-	spin_lock_irqsave(&domain->iommu_lock, flags);
-	if (!domain->iommu_refcnt[iommu->seq_id])
-		iommu_detach_domain(domain, iommu);
-	spin_unlock_irqrestore(&domain->iommu_lock, flags);
+	spin_lock_irqsave(&iommu->lock, flags);
+	domain_detach_iommu(domain, iommu);
+	spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
 static int md_domain_init(struct dmar_domain *domain, int guest_width)
@@ -4653,10 +4625,12 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 
 		old_domain = find_domain(dev);
 		if (old_domain) {
+			rcu_read_lock();
 			if (domain_type_is_vm_or_si(dmar_domain))
 				dmar_remove_one_dev_info(old_domain, dev);
 			else
 				domain_remove_dev_info(old_domain);
+			rcu_read_unlock();
 
 			if (!domain_type_is_vm_or_si(old_domain) &&
 			     list_empty(&old_domain->devices))
-- 
1.9.1

[PATCH 21/26] iommu/vt-d: Only call domain_remove_one_dev_info to detach old domain

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

There is no need to make a difference here between VM and
non-VM domains, so simplify this code here.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 50b5e6c..eb2a99a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4626,10 +4626,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 		old_domain = find_domain(dev);
 		if (old_domain) {
 			rcu_read_lock();
-			if (domain_type_is_vm_or_si(dmar_domain))
-				dmar_remove_one_dev_info(old_domain, dev);
-			else
-				domain_remove_dev_info(old_domain);
+			dmar_remove_one_dev_info(old_domain, dev);
 			rcu_read_unlock();
 
 			if (!domain_type_is_vm_or_si(old_domain) &&
-- 
1.9.1

[PATCH 22/26] iommu/vt-d: Get rid of domain->iommu_lock

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

When this lock is held the device_domain_lock is also
required to make sure the device_domain_info does not vanish
while in use. So this lock can be removed as it gives no
additional protection.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 87 +++++++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 38 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index eb2a99a..1a9ecfe 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -404,7 +404,6 @@ struct dmar_domain {
 	int		iommu_superpage;/* Level of superpages supported:
 					   0 == 4KiB (no superpages), 1 == 2MiB,
 					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
-	spinlock_t	iommu_lock;	/* protect iommu set in domain */
 	u64		max_addr;	/* maximum mapped address */
 
 	struct iommu_domain domain;	/* generic domain data structure for
@@ -474,6 +473,8 @@ static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 				     struct device *dev);
 static void domain_context_clear(struct intel_iommu *iommu,
 				 struct device *dev);
+static void __dmar_remove_one_dev_info(struct dmar_domain *domain,
+				       struct device *dev);
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu);
 
@@ -1402,24 +1403,23 @@ iommu_support_dev_iotlb (struct dmar_domain *domain, struct intel_iommu *iommu,
 			 u8 bus, u8 devfn)
 {
 	bool found = false;
-	unsigned long flags;
 	struct device_domain_info *info;
 	struct pci_dev *pdev;
 
+	assert_spin_locked(&device_domain_lock);
+
 	if (!ecap_dev_iotlb_support(iommu->ecap))
 		return NULL;
 
 	if (!iommu->qi)
 		return NULL;
 
-	spin_lock_irqsave(&device_domain_lock, flags);
 	list_for_each_entry(info, &domain->devices, link)
 		if (info->iommu == iommu && info->bus == bus &&
 		    info->devfn == devfn) {
 			found = true;
 			break;
 		}
-	spin_unlock_irqrestore(&device_domain_lock, flags);
 
 	if (!found || !info->dev || !dev_is_pci(info->dev))
 		return NULL;
@@ -1614,10 +1614,12 @@ static int iommu_init_domains(struct intel_iommu *iommu)
 static void disable_dmar_iommu(struct intel_iommu *iommu)
 {
 	struct device_domain_info *info, *tmp;
+	unsigned long flags;
 
 	if (!iommu->domains || !iommu->domain_ids)
 		return;
 
+	spin_lock_irqsave(&device_domain_lock, flags);
 	list_for_each_entry_safe(info, tmp, &device_domain_list, global) {
 		struct dmar_domain *domain;
 
@@ -1634,6 +1636,7 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 		if (!domain_type_is_vm_or_si(domain))
 			domain_exit(domain);
 	}
+	spin_unlock_irqrestore(&device_domain_lock, flags);
 
 	if (iommu->gcmd & DMA_GCMD_TE)
 		iommu_disable_translation(iommu);
@@ -1670,7 +1673,6 @@ static struct dmar_domain *alloc_domain(int flags)
 	memset(domain, 0, sizeof(*domain));
 	domain->nid = -1;
 	domain->flags = flags;
-	spin_lock_init(&domain->iommu_lock);
 	INIT_LIST_HEAD(&domain->devices);
 
 	return domain;
@@ -1681,13 +1683,11 @@ static int domain_attach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu)
 {
 	unsigned long ndomains;
-	unsigned long flags;
-	int ret, num;
+	int num;
 
+	assert_spin_locked(&device_domain_lock);
 	assert_spin_locked(&iommu->lock);
 
-	spin_lock_irqsave(&domain->iommu_lock, flags);
-
 	domain->iommu_refcnt[iommu->seq_id] += 1;
 	domain->iommu_count += 1;
 	if (domain->iommu_refcnt[iommu->seq_id] == 1) {
@@ -1698,8 +1698,7 @@ static int domain_attach_iommu(struct dmar_domain *domain,
 			pr_err("%s: No free domain ids\n", iommu->name);
 			domain->iommu_refcnt[iommu->seq_id] -= 1;
 			domain->iommu_count -= 1;
-			ret = -ENOSPC;
-			goto out_unlock;
+			return -ENOSPC;
 		}
 
 		set_bit(num, iommu->domain_ids);
@@ -1711,22 +1710,17 @@ static int domain_attach_iommu(struct dmar_domain *domain,
 		domain_update_iommu_cap(domain);
 	}
 
-	ret = 0;
-out_unlock:
-	spin_unlock_irqrestore(&domain->iommu_lock, flags);
-
-	return ret;
+	return 0;
 }
 
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu)
 {
 	int num, count = INT_MAX;
-	unsigned long flags;
 
+	assert_spin_locked(&device_domain_lock);
 	assert_spin_locked(&iommu->lock);
 
-	spin_lock_irqsave(&domain->iommu_lock, flags);
 	domain->iommu_refcnt[iommu->seq_id] -= 1;
 	count = --domain->iommu_count;
 	if (domain->iommu_refcnt[iommu->seq_id] == 0) {
@@ -1737,7 +1731,6 @@ static int domain_detach_iommu(struct dmar_domain *domain,
 		domain_update_iommu_cap(domain);
 		domain->iommu_did[iommu->seq_id] = 0;
 	}
-	spin_unlock_irqrestore(&domain->iommu_lock, flags);
 
 	return count;
 }
@@ -1892,7 +1885,7 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 	struct context_entry *context;
 	unsigned long flags;
 	struct dma_pte *pgd;
-	int agaw;
+	int ret, agaw;
 
 	WARN_ON(did == 0);
 
@@ -1905,16 +1898,17 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 
 	BUG_ON(!domain->pgd);
 
-	spin_lock_irqsave(&iommu->lock, flags);
+	spin_lock_irqsave(&device_domain_lock, flags);
+	spin_lock(&iommu->lock);
+
+	ret = -ENOMEM;
 	context = iommu_context_addr(iommu, bus, devfn, 1);
-	spin_unlock_irqrestore(&iommu->lock, flags);
 	if (!context)
-		return -ENOMEM;
-	spin_lock_irqsave(&iommu->lock, flags);
-	if (context_present(context)) {
-		spin_unlock_irqrestore(&iommu->lock, flags);
-		return 0;
-	}
+		goto out_unlock;
+
+	ret = 0;
+	if (context_present(context))
+		goto out_unlock;
 
 	pgd = domain->pgd;
 
@@ -1927,11 +1921,10 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 	 */
 	if (translation != CONTEXT_TT_PASS_THROUGH) {
 		for (agaw = domain->agaw; agaw != iommu->agaw; agaw--) {
+			ret = -ENOMEM;
 			pgd = phys_to_virt(dma_pte_addr(pgd));
-			if (!dma_pte_present(pgd)) {
-				spin_unlock_irqrestore(&iommu->lock, flags);
-				return -ENOMEM;
-			}
+			if (!dma_pte_present(pgd))
+				goto out_unlock;
 		}
 
 		info = iommu_support_dev_iotlb(domain, iommu, bus, devfn);
@@ -1970,7 +1963,12 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 		iommu_flush_write_buffer(iommu);
 	}
 	iommu_enable_dev_iotlb(info);
-	spin_unlock_irqrestore(&iommu->lock, flags);
+
+	ret = 0;
+
+out_unlock:
+	spin_unlock(&iommu->lock);
+	spin_unlock_irqrestore(&device_domain_lock, flags);
 
 	return 0;
 }
@@ -2213,9 +2211,12 @@ static inline void unlink_domain_info(struct device_domain_info *info)
 static void domain_remove_dev_info(struct dmar_domain *domain)
 {
 	struct device_domain_info *info, *tmp;
+	unsigned long flags;
 
+	spin_lock_irqsave(&device_domain_lock, flags);
 	list_for_each_entry_safe(info, tmp, &domain->devices, link)
-		dmar_remove_one_dev_info(domain, info->dev);
+		__dmar_remove_one_dev_info(domain, info->dev);
+	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
 /*
@@ -4517,14 +4518,16 @@ static void domain_context_clear(struct intel_iommu *iommu, struct device *dev)
 	pci_for_each_dma_alias(to_pci_dev(dev), &domain_context_clear_one_cb, iommu);
 }
 
-static void dmar_remove_one_dev_info(struct dmar_domain *domain,
-				     struct device *dev)
+static void __dmar_remove_one_dev_info(struct dmar_domain *domain,
+				       struct device *dev)
 {
 	struct device_domain_info *info;
 	struct intel_iommu *iommu;
 	unsigned long flags;
 	u8 bus, devfn;
 
+	assert_spin_locked(&device_domain_lock);
+
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
 		return;
@@ -4534,9 +4537,7 @@ static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 	if (WARN_ON(!info))
 		return;
 
-	spin_lock_irqsave(&device_domain_lock, flags);
 	unlink_domain_info(info);
-	spin_unlock_irqrestore(&device_domain_lock, flags);
 
 	iommu_disable_dev_iotlb(info);
 	domain_context_clear(iommu, dev);
@@ -4547,6 +4548,16 @@ static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 	spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
+static void dmar_remove_one_dev_info(struct dmar_domain *domain,
+				     struct device *dev)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	__dmar_remove_one_dev_info(domain, dev);
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+}
+
 static int md_domain_init(struct dmar_domain *domain, int guest_width)
 {
 	int adjust_width;
-- 
1.9.1

[PATCH 23/26] iommu/vt-d: Remove dmar_global_lock from device_notifier

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The code in the locked section does not touch anything
protected by the dmar_global_lock. Remove it from there.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 1a9ecfe..624e0a2 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4272,11 +4272,9 @@ static int device_notifier(struct notifier_block *nb,
 	if (!domain)
 		return 0;
 
-	down_read(&dmar_global_lock);
 	dmar_remove_one_dev_info(domain, dev);
 	if (!domain_type_is_vm_or_si(domain) && list_empty(&domain->devices))
 		domain_exit(domain);
-	up_read(&dmar_global_lock);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 24/26] iommu/vt-d: Pass device_domain_info to __dmar_remove_one_dev_info

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This struct contains all necessary information for the
function already. Also handle the info->dev == NULL case
while at it.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 624e0a2..31efb9c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -471,10 +471,9 @@ static void domain_exit(struct dmar_domain *domain);
 static void domain_remove_dev_info(struct dmar_domain *domain);
 static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 				     struct device *dev);
+static void __dmar_remove_one_dev_info(struct device_domain_info *info);
 static void domain_context_clear(struct intel_iommu *iommu,
 				 struct device *dev);
-static void __dmar_remove_one_dev_info(struct dmar_domain *domain,
-				       struct device *dev);
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu);
 
@@ -2215,7 +2214,7 @@ static void domain_remove_dev_info(struct dmar_domain *domain)
 
 	spin_lock_irqsave(&device_domain_lock, flags);
 	list_for_each_entry_safe(info, tmp, &domain->devices, link)
-		__dmar_remove_one_dev_info(domain, info->dev);
+		__dmar_remove_one_dev_info(info);
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
@@ -4516,43 +4515,41 @@ static void domain_context_clear(struct intel_iommu *iommu, struct device *dev)
 	pci_for_each_dma_alias(to_pci_dev(dev), &domain_context_clear_one_cb, iommu);
 }
 
-static void __dmar_remove_one_dev_info(struct dmar_domain *domain,
-				       struct device *dev)
+static void __dmar_remove_one_dev_info(struct device_domain_info *info)
 {
-	struct device_domain_info *info;
 	struct intel_iommu *iommu;
 	unsigned long flags;
-	u8 bus, devfn;
 
 	assert_spin_locked(&device_domain_lock);
 
-	iommu = device_to_iommu(dev, &bus, &devfn);
-	if (!iommu)
+	if (WARN_ON(!info))
 		return;
 
-	info = dev->archdata.iommu;
+	iommu = info->iommu;
 
-	if (WARN_ON(!info))
-		return;
+	if (info->dev) {
+		iommu_disable_dev_iotlb(info);
+		domain_context_clear(iommu, info->dev);
+	}
 
 	unlink_domain_info(info);
 
-	iommu_disable_dev_iotlb(info);
-	domain_context_clear(iommu, dev);
-	free_devinfo_mem(info);
-
 	spin_lock_irqsave(&iommu->lock, flags);
-	domain_detach_iommu(domain, iommu);
+	domain_detach_iommu(info->domain, iommu);
 	spin_unlock_irqrestore(&iommu->lock, flags);
+
+	free_devinfo_mem(info);
 }
 
 static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 				     struct device *dev)
 {
+	struct device_domain_info *info;
 	unsigned long flags;
 
 	spin_lock_irqsave(&device_domain_lock, flags);
-	__dmar_remove_one_dev_info(domain, dev);
+	info = dev->archdata.iommu;
+	__dmar_remove_one_dev_info(info);
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
-- 
1.9.1

[PATCH 25/26] iommu/vt-d: Only insert alias dev_info if there is an alias

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

For devices without an PCI alias there will be two
device_domain_info structures added. Prevent that by
checking if the alias is different from the device.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 31efb9c..3882072 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2318,8 +2318,8 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 	struct device_domain_info *info = NULL;
 	struct dmar_domain *domain, *tmp;
 	struct intel_iommu *iommu;
+	u16 req_id, dma_alias;
 	unsigned long flags;
-	u16 dma_alias;
 	u8 bus, devfn;
 
 	domain = find_domain(dev);
@@ -2330,6 +2330,8 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 	if (!iommu)
 		return NULL;
 
+	req_id = ((u16)bus << 8) | devfn;
+
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
 
@@ -2360,7 +2362,7 @@ static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 	}
 
 	/* register PCI DMA alias device */
-	if (dev_is_pci(dev)) {
+	if (req_id != dma_alias && dev_is_pci(dev)) {
 		tmp = dmar_insert_one_dev_info(iommu, PCI_BUS_NUM(dma_alias),
 					       dma_alias & 0xff, NULL, domain);
 
-- 
1.9.1

[PATCH 26/26] iommu/vt-d: Avoid duplicate device_domain_info structures

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

When a 'struct device_domain_info' is created as an alias
for another device, this struct will not be re-used when the
real device is encountered. Fix that to avoid duplicate
device_domain_info structures being added.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3882072..5cd3d36 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2269,12 +2269,16 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	spin_lock_irqsave(&device_domain_lock, flags);
 	if (dev)
 		found = find_domain(dev);
-	else {
+
+	if (!found) {
 		struct device_domain_info *info2;
 		info2 = dmar_search_domain_by_dev_info(iommu->segment, bus, devfn);
-		if (info2)
-			found = info2->domain;
+		if (info2) {
+			found      = info2->domain;
+			info2->dev = dev;
+		}
 	}
+
 	if (found) {
 		spin_unlock_irqrestore(&device_domain_lock, flags);
 		free_devinfo_mem(info);
-- 
1.9.1

Re: [PATCH 00/26] iommu/vt-d: Clean up device<->domain attachment

[Alex Williamson]

On Wed, 2015-08-05 at 17:18 +0200, Joerg Roedel wrote:
> Hi,
> 
> here is a (bigger than I expected) patch-set which cleans up
> the code to attach and detach domains to iommus in the Intel
> VT-d driver.
> 
> In particular, the patch-set does:
> 
> 	* Remove special cases around the handling of
> 	  various domain types and align their handling
> 	  where possible
> 
> 	* Rework the data structures for the domain<->iommu
> 	  relation to better match with its usage. This
> 	  allowed to get rid of a couple of search loops.
> 
> 	* Make the domain attachment and detachment path
> 	  to/from an iommu more symmetric. This makes the
> 	  code easier to understand and maintain.
> 
> 	* Rework and simplify the locking around the
> 	  domain<->iommu attachment/detachment path.
> 
> A few rough edges and special cases are still left, but I
> expect that these will be removed with the conversion to
> default domains.
> 
> I tested the code with some additional debug code to make
> sure that domain and domain-id allocation/deallocation works
> as expected.
> 
> As test I booted a kernel with the patches (with and
> without iommu=pt) and ran a KVM guest with devices assigned.
> No lockdep warning popped up and the debug output was also
> fine. But of course this is no guarantee that there are no
> issues left, so I am happy about feedback. Please review!

Nice cleanup, Joerg!  A few minor comments in follow-up to the
individual patches.  Thanks,

Alex

Re: [PATCH 00/26] iommu/vt-d: Clean up device<->domain attachment

[Joerg Roedel]

On Thu, Aug 06, 2015 at 01:19:47PM -0600, Alex Williamson wrote:
> Nice cleanup, Joerg!  A few minor comments in follow-up to the
> individual patches.  Thanks,

Thanks, it was finally worth the headaches I got by trying to understand
the old code :)


	Joerg