[RFC,2/7] iommu: Add share domain interface in iommu for spimdev

Message ID 20180801102221.5308-3-nek.in.cn@gmail.com
State New
Headers show
Series
  • A General Accelerator Framework, WarpDrive
Related show

Commit Message

Kenneth Lee Aug. 1, 2018, 10:22 a.m.
From: Kenneth Lee <liguozhu@hisilicon.com>


This patch add sharing interface for a iommu_group. The new interface:

	iommu_group_share_domain()
	iommu_group_unshare_domain()

can be used by some virtual iommu_group (such as iommu_group for spimdev)
to share their parent's iommu_group.

When the domain of the group is shared, it cannot be changed before
unshared.  In the future, notification can be added if update is required.

Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>

---
 drivers/iommu/iommu.c | 28 +++++++++++++++++++++++++++-
 include/linux/iommu.h |  2 ++
 2 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Tian, Kevin Aug. 2, 2018, 3:17 a.m. | #1
> From: Kenneth Lee

> Sent: Wednesday, August 1, 2018 6:22 PM

> 

> From: Kenneth Lee <liguozhu@hisilicon.com>

> 

> This patch add sharing interface for a iommu_group. The new interface:

> 

> 	iommu_group_share_domain()

> 	iommu_group_unshare_domain()

> 

> can be used by some virtual iommu_group (such as iommu_group for

> spimdev)

> to share their parent's iommu_group.

> 

> When the domain of the group is shared, it cannot be changed before

> unshared.  In the future, notification can be added if update is required.


Is it necessary or just asking VFIO to use parent domain directly? 

> 

> Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>

> ---

>  drivers/iommu/iommu.c | 28 +++++++++++++++++++++++++++-

>  include/linux/iommu.h |  2 ++

>  2 files changed, 29 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c

> index 63b37563db7e..a832aafe660d 100644

> --- a/drivers/iommu/iommu.c

> +++ b/drivers/iommu/iommu.c

> @@ -54,6 +54,9 @@ struct iommu_group {

>  	int id;

>  	struct iommu_domain *default_domain;

>  	struct iommu_domain *domain;

> +	atomic_t domain_shared_ref; /* Number of user of current domain.

> +				     * The domain cannot be modified if ref >

> 0

> +				     */

>  };

> 

>  struct group_device {

> @@ -353,6 +356,7 @@ struct iommu_group *iommu_group_alloc(void)

>  		return ERR_PTR(ret);

>  	}

>  	group->id = ret;

> +	atomic_set(&group->domain_shared_ref, 0);

> 

>  	ret = kobject_init_and_add(&group->kobj, &iommu_group_ktype,

>  				   NULL, "%d", group->id);

> @@ -482,6 +486,25 @@ int iommu_group_set_name(struct iommu_group

> *group, const char *name)

>  }

>  EXPORT_SYMBOL_GPL(iommu_group_set_name);

> 

> +struct iommu_domain *iommu_group_share_domain(struct

> iommu_group *group)

> +{

> +	/* the domain can be shared only when the default domain is used

> */

> +	/* todo: more shareable check */

> +	if (group->domain != group->default_domain)

> +		return ERR_PTR(-EINVAL);

> +

> +	atomic_inc(&group->domain_shared_ref);

> +	return group->domain;

> +}

> +EXPORT_SYMBOL_GPL(iommu_group_share_domain);

> +

> +void iommu_group_unshare_domain(struct iommu_group *group)

> +{

> +	atomic_dec(&group->domain_shared_ref);

> +	WARN_ON(atomic_read(&group->domain_shared_ref) < 0);

> +}

> +EXPORT_SYMBOL_GPL(iommu_group_unshare_domain);

> +

>  static int iommu_group_create_direct_mappings(struct iommu_group

> *group,

>  					      struct device *dev)

>  {

> @@ -1401,7 +1424,8 @@ static int __iommu_attach_group(struct

> iommu_domain *domain,

>  {

>  	int ret;

> 

> -	if (group->default_domain && group->domain != group-

> >default_domain)

> +	if ((group->default_domain && group->domain != group-

> >default_domain) ||

> +	     atomic_read(&group->domain_shared_ref) > 0)

>  		return -EBUSY;

> 

>  	ret = __iommu_group_for_each_dev(group, domain,

> @@ -1438,6 +1462,8 @@ static void __iommu_detach_group(struct

> iommu_domain *domain,

>  {

>  	int ret;

> 

> +	WARN_ON(atomic_read(&group->domain_shared_ref) > 0);

> +

>  	if (!group->default_domain) {

>  		__iommu_group_for_each_dev(group, domain,

>  					   iommu_group_do_detach_device);

> diff --git a/include/linux/iommu.h b/include/linux/iommu.h

> index 19938ee6eb31..278d60e3ec39 100644

> --- a/include/linux/iommu.h

> +++ b/include/linux/iommu.h

> @@ -349,6 +349,8 @@ extern int iommu_domain_get_attr(struct

> iommu_domain *domain, enum iommu_attr,

>  				 void *data);

>  extern int iommu_domain_set_attr(struct iommu_domain *domain, enum

> iommu_attr,

>  				 void *data);

> +extern struct iommu_domain *iommu_group_share_domain(struct

> iommu_group *group);

> +extern void iommu_group_unshare_domain(struct iommu_group *group);

> 

>  /* Window handling function prototypes */

>  extern int iommu_domain_window_enable(struct iommu_domain

> *domain, u32 wnd_nr,

> --

> 2.17.1
Tian, Kevin Aug. 2, 2018, 4:39 a.m. | #2
> From: Kenneth Lee

> Sent: Thursday, August 2, 2018 12:16 PM

> 

> On Thu, Aug 02, 2018 at 03:17:03AM +0000, Tian, Kevin wrote:

> > > From: Kenneth Lee

> > > Sent: Wednesday, August 1, 2018 6:22 PM

> > >

> > > From: Kenneth Lee <liguozhu@hisilicon.com>

> > >

> > > This patch add sharing interface for a iommu_group. The new interface:

> > >

> > > 	iommu_group_share_domain()

> > > 	iommu_group_unshare_domain()

> > >

> > > can be used by some virtual iommu_group (such as iommu_group for

> > > spimdev)

> > > to share their parent's iommu_group.

> > >

> > > When the domain of the group is shared, it cannot be changed before

> > > unshared.  In the future, notification can be added if update is required.

> >

> > Is it necessary or just asking VFIO to use parent domain directly?

> >

> Even we add to VFIO, the iommu still need to be changed. We can move

> the type1

> part to VFIO if we have agreement in RFC stage.


We have similar consideration in IOMMU aware mdev series. Now
we are inclined to avoid faked entity within IOMMU layer - leave to
caller to figure out right physical entity for any IOMMU ops. That 
might be leveraged here too after we get a new version.

Thanks
Kevin

Patch

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 63b37563db7e..a832aafe660d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -54,6 +54,9 @@  struct iommu_group {
 	int id;
 	struct iommu_domain *default_domain;
 	struct iommu_domain *domain;
+	atomic_t domain_shared_ref; /* Number of user of current domain.
+				     * The domain cannot be modified if ref > 0
+				     */
 };
 
 struct group_device {
@@ -353,6 +356,7 @@  struct iommu_group *iommu_group_alloc(void)
 		return ERR_PTR(ret);
 	}
 	group->id = ret;
+	atomic_set(&group->domain_shared_ref, 0);
 
 	ret = kobject_init_and_add(&group->kobj, &iommu_group_ktype,
 				   NULL, "%d", group->id);
@@ -482,6 +486,25 @@  int iommu_group_set_name(struct iommu_group *group, const char *name)
 }
 EXPORT_SYMBOL_GPL(iommu_group_set_name);
 
+struct iommu_domain *iommu_group_share_domain(struct iommu_group *group)
+{
+	/* the domain can be shared only when the default domain is used */
+	/* todo: more shareable check */
+	if (group->domain != group->default_domain)
+		return ERR_PTR(-EINVAL);
+
+	atomic_inc(&group->domain_shared_ref);
+	return group->domain;
+}
+EXPORT_SYMBOL_GPL(iommu_group_share_domain);
+
+void iommu_group_unshare_domain(struct iommu_group *group)
+{
+	atomic_dec(&group->domain_shared_ref);
+	WARN_ON(atomic_read(&group->domain_shared_ref) < 0);
+}
+EXPORT_SYMBOL_GPL(iommu_group_unshare_domain);
+
 static int iommu_group_create_direct_mappings(struct iommu_group *group,
 					      struct device *dev)
 {
@@ -1401,7 +1424,8 @@  static int __iommu_attach_group(struct iommu_domain *domain,
 {
 	int ret;
 
-	if (group->default_domain && group->domain != group->default_domain)
+	if ((group->default_domain && group->domain != group->default_domain) ||
+	     atomic_read(&group->domain_shared_ref) > 0)
 		return -EBUSY;
 
 	ret = __iommu_group_for_each_dev(group, domain,
@@ -1438,6 +1462,8 @@  static void __iommu_detach_group(struct iommu_domain *domain,
 {
 	int ret;
 
+	WARN_ON(atomic_read(&group->domain_shared_ref) > 0);
+
 	if (!group->default_domain) {
 		__iommu_group_for_each_dev(group, domain,
 					   iommu_group_do_detach_device);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 19938ee6eb31..278d60e3ec39 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -349,6 +349,8 @@  extern int iommu_domain_get_attr(struct iommu_domain *domain, enum iommu_attr,
 				 void *data);
 extern int iommu_domain_set_attr(struct iommu_domain *domain, enum iommu_attr,
 				 void *data);
+extern struct iommu_domain *iommu_group_share_domain(struct iommu_group *group);
+extern void iommu_group_unshare_domain(struct iommu_group *group);
 
 /* Window handling function prototypes */
 extern int iommu_domain_window_enable(struct iommu_domain *domain, u32 wnd_nr,