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


Groups > linux.kernel > #1200904 > unrolled thread

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

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

Back to article view | Back to linux.kernel


Contents

  [PATCH 00/26] iommu/vt-d: Clean up device<->domain  attachment Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:30 +0200
    [PATCH 03/26] iommu/vt-d: Split up iommu->domains array Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:30 +0200
      Re: [PATCH 03/26] iommu/vt-d: Split up iommu->domains array Alex Williamson <alex.williamson@redhat.com> - 2015-08-07 00:40 +0200
        Re: [PATCH 03/26] iommu/vt-d: Split up iommu->domains array Joerg Roedel <joro@8bytes.org> - 2015-08-07 13:20 +0200
    [PATCH 14/26] iommu/vt-d: Move context-mapping  into dmar_insert_dev_info Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 11/26] iommu/vt-d: Simplify io/tlb flushing in intel_iommu_unmap Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 13/26] iommu/vt-d: Simplify domain_remove_dev_info() Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
      Re: [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount Alex Williamson <alex.williamson@redhat.com> - 2015-08-06 21:30 +0200
        Re: [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount Joerg Roedel <joro@8bytes.org> - 2015-08-07 13:20 +0200
    [PATCH 12/26] iommu/vt-d: Simplify domain_remove_one_dev_info() Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 16/26] iommu/vt-d: Rename domain_remove_one_dev_info() Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 17/26] iommu/vt-d: Rename iommu_detach_dependent_devices() Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 21/26] iommu/vt-d: Only call domain_remove_one_dev_info to detach old domain Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    [PATCH 23/26] iommu/vt-d: Remove dmar_global_lock from device_notifier Joerg Roedel <joro@8bytes.org> - 2015-08-05 17:40 +0200
    Re: [PATCH 00/26] iommu/vt-d: Clean up device<->domain  attachment Alex Williamson <alex.williamson@redhat.com> - 2015-08-06 23:10 +0200
      Re: [PATCH 00/26] iommu/vt-d: Clean up device<->domain  attachment Joerg Roedel <joro@8bytes.org> - 2015-08-07 13:30 +0200

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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:30 +0200
Subject[PATCH 00/26] iommu/vt-d: Clean up device<->domain attachment
Message-ID<pU8E1-57u-9@gated-at.bofh.it>
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
--
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]


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:30 +0200
Subject[PATCH 03/26] iommu/vt-d: Split up iommu->domains array
Message-ID<pU8NM-5jj-99@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-08-07 00:40 +0200
SubjectRe: [PATCH 03/26] iommu/vt-d: Split up iommu->domains array
Message-ID<pUBZo-5LQ-9@gated-at.bofh.it>
In reply to#1200905
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 */
>  



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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-07 13:20 +0200
SubjectRe: [PATCH 03/26] iommu/vt-d: Split up iommu->domains array
Message-ID<pUNQS-6fD-13@gated-at.bofh.it>
In reply to#1202134
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 14/26] iommu/vt-d: Move context-mapping into dmar_insert_dev_info
Message-ID<pU8Xn-5uJ-1@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 11/26] iommu/vt-d: Simplify io/tlb flushing in intel_iommu_unmap
Message-ID<pU8Xo-5uJ-7@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 13/26] iommu/vt-d: Simplify domain_remove_dev_info()
Message-ID<pU8Xo-5uJ-13@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount
Message-ID<pU8Xn-5uJ-3@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-08-06 21:30 +0200
SubjectRe: [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount
Message-ID<pUz1w-1yi-3@gated-at.bofh.it>
In reply to#1200909
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];
>  
>  		/*



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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-07 13:20 +0200
SubjectRe: [PATCH 10/26] iommu/vt-d: Replace iommu_bmp with a refcount
Message-ID<pUNQS-6fD-15@gated-at.bofh.it>
In reply to#1201984
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 12/26] iommu/vt-d: Simplify domain_remove_one_dev_info()
Message-ID<pU8Xo-5uJ-11@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 16/26] iommu/vt-d: Rename domain_remove_one_dev_info()
Message-ID<pU8Xo-5uJ-15@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 17/26] iommu/vt-d: Rename iommu_detach_dependent_devices()
Message-ID<pU8Xo-5uJ-25@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 21/26] iommu/vt-d: Only call domain_remove_one_dev_info to detach old domain
Message-ID<pU8Xp-5uJ-45@gated-at.bofh.it>
In reply to#1200904
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

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


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

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-05 17:40 +0200
Subject[PATCH 23/26] iommu/vt-d: Remove dmar_global_lock from device_notifier
Message-ID<pU8Xq-5uJ-53@gated-at.bofh.it>
In reply to#1200904
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

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


#1202076

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-08-06 23:10 +0200
Message-ID<pUAAi-3Sz-23@gated-at.bofh.it>
In reply to#1200904
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

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


#1202597

FromJoerg Roedel <joro@8bytes.org>
Date2015-08-07 13:30 +0200
Message-ID<pUO0x-6r7-5@gated-at.bofh.it>
In reply to#1202076
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

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