LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] iommu/vt-d: Detach domain *only* from attached iommus
@ 2015-03-04 18:30 Alex Williamson
  2015-03-17 19:08 ` Alex Williamson
  2015-03-23 14:24 ` Joerg Roedel
  0 siblings, 2 replies; 3+ messages in thread
From: Alex Williamson @ 2015-03-04 18:30 UTC (permalink / raw)
  To: iommu, dwmw2; +Cc: joro, linux-kernel, jiang.liu

Device domains never span IOMMU hardware units, which allows the
domain ID space for each IOMMU to be an independent address space.
Therefore we can have multiple, independent domains, each with the
same domain->id, but attached to different hardware units.  This is
also why we need to do a heavy-weight search for VM domains since
they can span multiple IOMMUs hardware units and we don't require a
single global ID to use for all hardware units.

Therefore, if we call iommu_detach_domain() across all active IOMMU
hardware units for a non-VM domain, the result is that we clear domain
IDs that are not associated with our domain, allowing them to be
re-allocated and causing apparent coherency issues when the device
cannot access IOVAs for the intended domain.

This bug was introduced in commit fb170fb4c548 ("iommu/vt-d: Introduce
helper functions to make code symmetric for readability"), but is
significantly exacerbated by the more recent commit 62c22167dd70
("iommu/vt-d: Fix dmar_domain leak in iommu_attach_device") which calls
domain_exit() more frequently to resolve a domain leak.

Fixes: fb170fb4c548 ("iommu/vt-d: Introduce helper functions to make code symmetric for readability")
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: stable@vger.kernel.org # v3.17+
---
 drivers/iommu/intel-iommu.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ae4c1a8..a83c965 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1743,8 +1743,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)
@@ -1764,8 +1764,8 @@ static void domain_exit(struct dmar_domain *domain)
 
 	/* clear attached or cached domains */
 	rcu_read_lock();
-	for_each_active_iommu(iommu, drhd)
-		iommu_detach_domain(domain, iommu);
+	for_each_set_bit(i, domain->iommu_bmp, g_num_of_iommus)
+		iommu_detach_domain(domain, g_iommus[i]);
 	rcu_read_unlock();
 
 	dma_free_pagelist(freelist);


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iommu/vt-d: Detach domain *only* from attached iommus
  2015-03-04 18:30 [PATCH] iommu/vt-d: Detach domain *only* from attached iommus Alex Williamson
@ 2015-03-17 19:08 ` Alex Williamson
  2015-03-23 14:24 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2015-03-17 19:08 UTC (permalink / raw)
  To: iommu; +Cc: dwmw2, joro, linux-kernel, jiang.liu


Ping?  I think this needs to be a v4.0 candidate.  Thanks,

Alex

On Wed, 2015-03-04 at 11:30 -0700, Alex Williamson wrote:
> Device domains never span IOMMU hardware units, which allows the
> domain ID space for each IOMMU to be an independent address space.
> Therefore we can have multiple, independent domains, each with the
> same domain->id, but attached to different hardware units.  This is
> also why we need to do a heavy-weight search for VM domains since
> they can span multiple IOMMUs hardware units and we don't require a
> single global ID to use for all hardware units.
> 
> Therefore, if we call iommu_detach_domain() across all active IOMMU
> hardware units for a non-VM domain, the result is that we clear domain
> IDs that are not associated with our domain, allowing them to be
> re-allocated and causing apparent coherency issues when the device
> cannot access IOVAs for the intended domain.
> 
> This bug was introduced in commit fb170fb4c548 ("iommu/vt-d: Introduce
> helper functions to make code symmetric for readability"), but is
> significantly exacerbated by the more recent commit 62c22167dd70
> ("iommu/vt-d: Fix dmar_domain leak in iommu_attach_device") which calls
> domain_exit() more frequently to resolve a domain leak.
> 
> Fixes: fb170fb4c548 ("iommu/vt-d: Introduce helper functions to make code symmetric for readability")
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: stable@vger.kernel.org # v3.17+
> ---
>  drivers/iommu/intel-iommu.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index ae4c1a8..a83c965 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -1743,8 +1743,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)
> @@ -1764,8 +1764,8 @@ static void domain_exit(struct dmar_domain *domain)
>  
>  	/* clear attached or cached domains */
>  	rcu_read_lock();
> -	for_each_active_iommu(iommu, drhd)
> -		iommu_detach_domain(domain, iommu);
> +	for_each_set_bit(i, domain->iommu_bmp, g_num_of_iommus)
> +		iommu_detach_domain(domain, g_iommus[i]);
>  	rcu_read_unlock();
>  
>  	dma_free_pagelist(freelist);
> 




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iommu/vt-d: Detach domain *only* from attached iommus
  2015-03-04 18:30 [PATCH] iommu/vt-d: Detach domain *only* from attached iommus Alex Williamson
  2015-03-17 19:08 ` Alex Williamson
@ 2015-03-23 14:24 ` Joerg Roedel
  1 sibling, 0 replies; 3+ messages in thread
From: Joerg Roedel @ 2015-03-23 14:24 UTC (permalink / raw)
  To: Alex Williamson; +Cc: iommu, dwmw2, linux-kernel, jiang.liu

On Wed, Mar 04, 2015 at 11:30:10AM -0700, Alex Williamson wrote:
>  drivers/iommu/intel-iommu.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied to iommu/fixes, thanks Alex.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-03-23 14:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-04 18:30 [PATCH] iommu/vt-d: Detach domain *only* from attached iommus Alex Williamson
2015-03-17 19:08 ` Alex Williamson
2015-03-23 14:24 ` Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).