Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1200895 > unrolled thread

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

Started byJoerg Roedel <joro@8bytes.org>
First post2015-08-05 17:30 +0200
Last post2015-08-07 13:10 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:30 +0200
    Re: [PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids Alex Williamson <alex.williamson@redhat.com> - 2015-08-06 21:30 +0200
      Re: [PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids Joerg Roedel <joro@8bytes.org> - 2015-08-07 13:10 +0200

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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:30 +0200
Subject[PATCH 01/26] iommu/vt-d: Keep track of per-iommu domain ids
Message-ID<pU8NK-5jj-75@gated-at.bofh.it>
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1201989

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-08-06 21:30 +0200
Message-ID<pUz1w-1yi-19@gated-at.bofh.it>
In reply to#1200895
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);
>  



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1202586

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-07 13:10 +0200
Message-ID<pUNHc-64q-9@gated-at.bofh.it>
In reply to#1201989
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web