On 04/29/2015 12:16 PM, David Gibson wrote:

On Sat, Apr 25, 2015 at 10:14:33PM +1000, Alexey Kardashevskiy wrote:

quoted

This is to make extended ownership and multiple groups support patches
simpler for review.

This should cause no behavioural change.

Um.. this doesn't appear to be true.  Previously removing a group from
an enabled container would fail with EBUSY, now it forces a disable.

This is the original tce_iommu_detach_group() where I cannot find EBUSY you 
are referring to; it did and does enforce disable. What do I miss here?

static void tce_iommu_detach_group(void *iommu_data,
                 struct iommu_group *iommu_group)
{
         struct tce_container *container = iommu_data;
         struct iommu_table *tbl = iommu_group_get_iommudata(iommu_group);

         BUG_ON(!tbl);
         mutex_lock(&container->lock);
         if (tbl != container->tbl) {
                 pr_warn("tce_vfio: detaching group #%u, expected group is 
#%u\n",
                                 iommu_group_id(iommu_group),
                                 iommu_group_id(tbl->it_group));
         } else {
                 if (container->enabled) {
                         pr_warn("tce_vfio: detaching group #%u from 
enabled container, forcing disable\n",
                                         iommu_group_id(tbl->it_group));
                         tce_iommu_disable(container);
                 }

                 /* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
                                 iommu_group_id(iommu_group), iommu_group); */
                 container->tbl = NULL;
                 tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
                 iommu_release_ownership(tbl);
         }
         mutex_unlock(&container->lock);
}

quoted

Signed-off-by: Alexey Kardashevskiy <redacted>
[aw: for the vfio related changes]
Acked-by: Alex Williamson <redacted>
Reviewed-by: David Gibson <redacted>
---
  drivers/vfio/vfio_iommu_spapr_tce.c | 40 ++++++++++++++++++++++---------------
  1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 115d5e6..0fbe03e 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c

@@ -460,16 +460,21 @@ static int tce_iommu_attach_group(void *iommu_data,
  				iommu_group_id(container->tbl->it_group),
  				iommu_group_id(iommu_group));
  		ret = -EBUSY;
-	} else if (container->enabled) {
+		goto unlock_exit;
+	}
+
+	if (container->enabled) {
  		pr_err("tce_vfio: attaching group #%u to enabled container\n",
  				iommu_group_id(iommu_group));
  		ret = -EBUSY;
-	} else {
-		ret = iommu_take_ownership(tbl);
-		if (!ret)
-			container->tbl = tbl;
+		goto unlock_exit;
  	}

+	ret = iommu_take_ownership(tbl);
+	if (!ret)
+		container->tbl = tbl;
+
+unlock_exit:
  	mutex_unlock(&container->lock);

  	return ret;

@@ -487,19 +492,22 @@ static void tce_iommu_detach_group(void *iommu_data,
  		pr_warn("tce_vfio: detaching group #%u, expected group is #%u\n",
  				iommu_group_id(iommu_group),
  				iommu_group_id(tbl->it_group));
-	} else {
-		if (container->enabled) {
-			pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n",
-					iommu_group_id(tbl->it_group));
-			tce_iommu_disable(container);
-		}
+		goto unlock_exit;
+	}

-		/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
-				iommu_group_id(iommu_group), iommu_group); */
-		container->tbl = NULL;
-		tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
-		iommu_release_ownership(tbl);
+	if (container->enabled) {
+		pr_warn("tce_vfio: detaching group #%u from enabled container, forcing disable\n",
+				iommu_group_id(tbl->it_group));
+		tce_iommu_disable(container);
  	}
+
+	/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
+	   iommu_group_id(iommu_group), iommu_group); */
+	container->tbl = NULL;
+	tce_iommu_clear(container, tbl, tbl->it_offset, tbl->it_size);
+	iommu_release_ownership(tbl);
+
+unlock_exit:
  	mutex_unlock(&container->lock);
  }

-- 
Alexey