diff mbox

[kernel,v8,13/31] vfio: powerpc/spapr: powerpc/iommu: Rework IOMMU ownership control

Message ID 1428647473-11738-14-git-send-email-aik@ozlabs.ru (mailing list archive)
State Superseded
Delegated to: Benjamin Herrenschmidt
Headers show

Commit Message

Alexey Kardashevskiy April 10, 2015, 6:30 a.m. UTC
This replaces iommu_take_ownership()/iommu_release_ownership() calls
with the callback calls and it is up to the platform code to call
iommu_take_ownership()/iommu_release_ownership() if needed.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/include/asm/iommu.h    |  4 +--
 arch/powerpc/kernel/iommu.c         | 50 ++++++++++++++++++++++++++++---------
 drivers/vfio/vfio_iommu_spapr_tce.c |  4 +--
 3 files changed, 42 insertions(+), 16 deletions(-)

Comments

David Gibson April 16, 2015, 6 a.m. UTC | #1
On Fri, Apr 10, 2015 at 04:30:55PM +1000, Alexey Kardashevskiy wrote:
> This replaces iommu_take_ownership()/iommu_release_ownership() calls
> with the callback calls and it is up to the platform code to call
> iommu_take_ownership()/iommu_release_ownership() if needed.

I think this commit message is out of date - I don't see any callbacks
here.

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  arch/powerpc/include/asm/iommu.h    |  4 +--
>  arch/powerpc/kernel/iommu.c         | 50 ++++++++++++++++++++++++++++---------
>  drivers/vfio/vfio_iommu_spapr_tce.c |  4 +--
>  3 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> index 667aa1a..b9e50d3 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -225,8 +225,8 @@ extern unsigned long iommu_clear_tce(struct iommu_table *tbl,
>  		unsigned long entry);
>  
>  extern void iommu_flush_tce(struct iommu_table *tbl);
> -extern int iommu_take_ownership(struct iommu_table *tbl);
> -extern void iommu_release_ownership(struct iommu_table *tbl);
> +extern int iommu_take_ownership(struct iommu_table_group *table_group);
> +extern void iommu_release_ownership(struct iommu_table_group *table_group);
>  
>  extern enum dma_data_direction iommu_tce_direction(unsigned long tce);
>  extern unsigned long iommu_direction_to_tce_perm(enum dma_data_direction dir);
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index fd49c8e..7d6089b 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1050,7 +1050,7 @@ int iommu_tce_build(struct iommu_table *tbl, unsigned long entry,
>  }
>  EXPORT_SYMBOL_GPL(iommu_tce_build);
>  
> -int iommu_take_ownership(struct iommu_table *tbl)
> +static int iommu_table_take_ownership(struct iommu_table *tbl)
>  {
>  	unsigned long sz = (tbl->it_size + 7) >> 3;
>  
> @@ -1064,19 +1064,36 @@ int iommu_take_ownership(struct iommu_table *tbl)
>  
>  	memset(tbl->it_map, 0xff, sz);
>  
> -	/*
> -	 * Disable iommu bypass, otherwise the user can DMA to all of
> -	 * our physical memory via the bypass window instead of just
> -	 * the pages that has been explicitly mapped into the iommu
> -	 */
> -	if (tbl->set_bypass)
> -		tbl->set_bypass(tbl, false);

The code to disable bypass is removed, and doesn't seem to be replaced
with anything.  That doesn't look safe.

> +	return 0;
> +}
> +
> +static void iommu_table_release_ownership(struct iommu_table *tbl);
> +
> +int iommu_take_ownership(struct iommu_table_group *table_group)
> +{
> +	int i, j, rc = 0;
> +
> +	for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i) {
> +		struct iommu_table *tbl = &table_group->tables[i];
> +
> +		if (!tbl->it_map)
> +			continue;
> +
> +		rc = iommu_table_take_ownership(tbl);
> +		if (rc) {
> +			for (j = 0; j < i; ++j)
> +				iommu_table_release_ownership(
> +						&table_group->tables[j]);
> +
> +			return rc;
> +		}
> +	}
>  
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(iommu_take_ownership);
>  
> -void iommu_release_ownership(struct iommu_table *tbl)
> +static void iommu_table_release_ownership(struct iommu_table *tbl)
>  {
>  	unsigned long sz = (tbl->it_size + 7) >> 3;
>  
> @@ -1086,9 +1103,18 @@ void iommu_release_ownership(struct iommu_table *tbl)
>  	if (tbl->it_offset == 0)
>  		set_bit(0, tbl->it_map);
>  
> -	/* The kernel owns the device now, we can restore the iommu bypass */
> -	if (tbl->set_bypass)
> -		tbl->set_bypass(tbl, true);
> +}
> +
> +extern void iommu_release_ownership(struct iommu_table_group *table_group)
> +{
> +	int i;
> +
> +	for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i) {
> +		struct iommu_table *tbl = &table_group->tables[i];
> +
> +		if (tbl->it_map)
> +			iommu_table_release_ownership(tbl);
> +	}
>  }
>  EXPORT_SYMBOL_GPL(iommu_release_ownership);
>  
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index d61aad2..9f38351 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -535,7 +535,7 @@ static int tce_iommu_attach_group(void *iommu_data,
>  		goto unlock_exit;
>  	}
>  
> -	ret = iommu_take_ownership(&table_group->tables[0]);
> +	ret = iommu_take_ownership(table_group);
>  	if (!ret)
>  		container->grp = iommu_group;
>  
> @@ -572,7 +572,7 @@ static void tce_iommu_detach_group(void *iommu_data,
>  	table_group = iommu_group_get_iommudata(iommu_group);
>  	BUG_ON(!table_group);
>  
> -	iommu_release_ownership(&table_group->tables[0]);
> +	iommu_release_ownership(table_group);
>  
>  unlock_exit:
>  	mutex_unlock(&container->lock);
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 667aa1a..b9e50d3 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -225,8 +225,8 @@  extern unsigned long iommu_clear_tce(struct iommu_table *tbl,
 		unsigned long entry);
 
 extern void iommu_flush_tce(struct iommu_table *tbl);
-extern int iommu_take_ownership(struct iommu_table *tbl);
-extern void iommu_release_ownership(struct iommu_table *tbl);
+extern int iommu_take_ownership(struct iommu_table_group *table_group);
+extern void iommu_release_ownership(struct iommu_table_group *table_group);
 
 extern enum dma_data_direction iommu_tce_direction(unsigned long tce);
 extern unsigned long iommu_direction_to_tce_perm(enum dma_data_direction dir);
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index fd49c8e..7d6089b 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1050,7 +1050,7 @@  int iommu_tce_build(struct iommu_table *tbl, unsigned long entry,
 }
 EXPORT_SYMBOL_GPL(iommu_tce_build);
 
-int iommu_take_ownership(struct iommu_table *tbl)
+static int iommu_table_take_ownership(struct iommu_table *tbl)
 {
 	unsigned long sz = (tbl->it_size + 7) >> 3;
 
@@ -1064,19 +1064,36 @@  int iommu_take_ownership(struct iommu_table *tbl)
 
 	memset(tbl->it_map, 0xff, sz);
 
-	/*
-	 * Disable iommu bypass, otherwise the user can DMA to all of
-	 * our physical memory via the bypass window instead of just
-	 * the pages that has been explicitly mapped into the iommu
-	 */
-	if (tbl->set_bypass)
-		tbl->set_bypass(tbl, false);
+	return 0;
+}
+
+static void iommu_table_release_ownership(struct iommu_table *tbl);
+
+int iommu_take_ownership(struct iommu_table_group *table_group)
+{
+	int i, j, rc = 0;
+
+	for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i) {
+		struct iommu_table *tbl = &table_group->tables[i];
+
+		if (!tbl->it_map)
+			continue;
+
+		rc = iommu_table_take_ownership(tbl);
+		if (rc) {
+			for (j = 0; j < i; ++j)
+				iommu_table_release_ownership(
+						&table_group->tables[j]);
+
+			return rc;
+		}
+	}
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iommu_take_ownership);
 
-void iommu_release_ownership(struct iommu_table *tbl)
+static void iommu_table_release_ownership(struct iommu_table *tbl)
 {
 	unsigned long sz = (tbl->it_size + 7) >> 3;
 
@@ -1086,9 +1103,18 @@  void iommu_release_ownership(struct iommu_table *tbl)
 	if (tbl->it_offset == 0)
 		set_bit(0, tbl->it_map);
 
-	/* The kernel owns the device now, we can restore the iommu bypass */
-	if (tbl->set_bypass)
-		tbl->set_bypass(tbl, true);
+}
+
+extern void iommu_release_ownership(struct iommu_table_group *table_group)
+{
+	int i;
+
+	for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i) {
+		struct iommu_table *tbl = &table_group->tables[i];
+
+		if (tbl->it_map)
+			iommu_table_release_ownership(tbl);
+	}
 }
 EXPORT_SYMBOL_GPL(iommu_release_ownership);
 
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index d61aad2..9f38351 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -535,7 +535,7 @@  static int tce_iommu_attach_group(void *iommu_data,
 		goto unlock_exit;
 	}
 
-	ret = iommu_take_ownership(&table_group->tables[0]);
+	ret = iommu_take_ownership(table_group);
 	if (!ret)
 		container->grp = iommu_group;
 
@@ -572,7 +572,7 @@  static void tce_iommu_detach_group(void *iommu_data,
 	table_group = iommu_group_get_iommudata(iommu_group);
 	BUG_ON(!table_group);
 
-	iommu_release_ownership(&table_group->tables[0]);
+	iommu_release_ownership(table_group);
 
 unlock_exit:
 	mutex_unlock(&container->lock);