RE: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-06-02 Thread Tian, Kevin
> From: Jean-Philippe Brucker 
> Sent: Wednesday, May 25, 2022 3:30 PM
> 
> On Wed, May 25, 2022 at 02:04:49AM +, Tian, Kevin wrote:
> > > From: Jean-Philippe Brucker 
> > > Sent: Tuesday, May 24, 2022 6:58 PM
> > >
> > > On Tue, May 24, 2022 at 10:22:28AM +, Tian, Kevin wrote:
> > > > > From: Lu Baolu 
> > > > > Sent: Thursday, May 19, 2022 3:21 PM
> > > > >
> > > > > The existing iommu SVA interfaces are implemented by calling the SVA
> > > > > specific iommu ops provided by the IOMMU drivers. There's no need
> for
> > > > > any SVA specific ops in iommu_ops vector anymore as we can achieve
> > > > > this through the generic attach/detach_dev_pasid domain ops.
> > > >
> > > > set/block_pasid_dev, to be consistent.
> > > >
> > > > > +
> > > > > + mutex_lock(_sva_lock);
> > > > > + /* Search for an existing domain. */
> > > > > + domain = iommu_get_domain_for_dev_pasid(dev, mm-
> >pasid);
> > > > > + if (domain) {
> > > > > + sva_domain = to_sva_domain(domain);
> > > > > + refcount_inc(_domain->bond.users);
> > > > > + goto out_success;
> > > > > + }
> > > > > +
> > > >
> > > > why would one device/pasid be bound to a mm more than once?
> > >
> > > Device drivers can call bind() multiple times for the same device and mm,
> > > for example if one process wants to open multiple accelerator queues.
> > >
> >
> > Is it clearer to have a sva_bond_get/put() pair instead of calling
> > bind() multiple times here?
> 
> I don't think it's clearer, and it would force device drivers to keep
> track of {dev, mm} pairs, when the IOMMU subsystem already does that.
> At the moment a device driver calls
> 
>   bond = iommu_sva_bind_device(dev, mm)
> 
> for each ADI that it wants to assign to userspace. If a process happens to
> want multiple ADIs on one device, then the {dev, mm} parameters are the
> same and bind() returns the same bond. Since the IOMMU driver needs to
> track these anyway, it might as well refcount them.
> 

My impression was that when an interface returns an object
then further reference to this object is usually directly acquired
on the object hence requires the caller to track it. But not a
strong opinion here if others all agree to favor simplicity for
the caller side.

Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-25 Thread Jean-Philippe Brucker
On Wed, May 25, 2022 at 02:04:49AM +, Tian, Kevin wrote:
> > From: Jean-Philippe Brucker 
> > Sent: Tuesday, May 24, 2022 6:58 PM
> > 
> > On Tue, May 24, 2022 at 10:22:28AM +, Tian, Kevin wrote:
> > > > From: Lu Baolu 
> > > > Sent: Thursday, May 19, 2022 3:21 PM
> > > >
> > > > The existing iommu SVA interfaces are implemented by calling the SVA
> > > > specific iommu ops provided by the IOMMU drivers. There's no need for
> > > > any SVA specific ops in iommu_ops vector anymore as we can achieve
> > > > this through the generic attach/detach_dev_pasid domain ops.
> > >
> > > set/block_pasid_dev, to be consistent.
> > >
> > > > +
> > > > +   mutex_lock(_sva_lock);
> > > > +   /* Search for an existing domain. */
> > > > +   domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid);
> > > > +   if (domain) {
> > > > +   sva_domain = to_sva_domain(domain);
> > > > +   refcount_inc(_domain->bond.users);
> > > > +   goto out_success;
> > > > +   }
> > > > +
> > >
> > > why would one device/pasid be bound to a mm more than once?
> > 
> > Device drivers can call bind() multiple times for the same device and mm,
> > for example if one process wants to open multiple accelerator queues.
> > 
> 
> Is it clearer to have a sva_bond_get/put() pair instead of calling
> bind() multiple times here? 

I don't think it's clearer, and it would force device drivers to keep
track of {dev, mm} pairs, when the IOMMU subsystem already does that.
At the moment a device driver calls

bond = iommu_sva_bind_device(dev, mm)

for each ADI that it wants to assign to userspace. If a process happens to
want multiple ADIs on one device, then the {dev, mm} parameters are the
same and bind() returns the same bond. Since the IOMMU driver needs to
track these anyway, it might as well refcount them.

Thanks,
Jean


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-24 Thread Tian, Kevin
> From: Jean-Philippe Brucker 
> Sent: Tuesday, May 24, 2022 6:58 PM
> 
> On Tue, May 24, 2022 at 10:22:28AM +, Tian, Kevin wrote:
> > > From: Lu Baolu 
> > > Sent: Thursday, May 19, 2022 3:21 PM
> > >
> > > The existing iommu SVA interfaces are implemented by calling the SVA
> > > specific iommu ops provided by the IOMMU drivers. There's no need for
> > > any SVA specific ops in iommu_ops vector anymore as we can achieve
> > > this through the generic attach/detach_dev_pasid domain ops.
> >
> > set/block_pasid_dev, to be consistent.
> >
> > > +
> > > + mutex_lock(_sva_lock);
> > > + /* Search for an existing domain. */
> > > + domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid);
> > > + if (domain) {
> > > + sva_domain = to_sva_domain(domain);
> > > + refcount_inc(_domain->bond.users);
> > > + goto out_success;
> > > + }
> > > +
> >
> > why would one device/pasid be bound to a mm more than once?
> 
> Device drivers can call bind() multiple times for the same device and mm,
> for example if one process wants to open multiple accelerator queues.
> 

Is it clearer to have a sva_bond_get/put() pair instead of calling
bind() multiple times here? 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-24 Thread Jean-Philippe Brucker
On Tue, May 24, 2022 at 10:22:28AM +, Tian, Kevin wrote:
> > From: Lu Baolu 
> > Sent: Thursday, May 19, 2022 3:21 PM
> > 
> > The existing iommu SVA interfaces are implemented by calling the SVA
> > specific iommu ops provided by the IOMMU drivers. There's no need for
> > any SVA specific ops in iommu_ops vector anymore as we can achieve
> > this through the generic attach/detach_dev_pasid domain ops.
> 
> set/block_pasid_dev, to be consistent.
> 
> > +
> > +   mutex_lock(_sva_lock);
> > +   /* Search for an existing domain. */
> > +   domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid);
> > +   if (domain) {
> > +   sva_domain = to_sva_domain(domain);
> > +   refcount_inc(_domain->bond.users);
> > +   goto out_success;
> > +   }
> > +
> 
> why would one device/pasid be bound to a mm more than once?

Device drivers can call bind() multiple times for the same device and mm,
for example if one process wants to open multiple accelerator queues.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-24 Thread Tian, Kevin
> From: Lu Baolu 
> Sent: Thursday, May 19, 2022 3:21 PM
> 
> The existing iommu SVA interfaces are implemented by calling the SVA
> specific iommu ops provided by the IOMMU drivers. There's no need for
> any SVA specific ops in iommu_ops vector anymore as we can achieve
> this through the generic attach/detach_dev_pasid domain ops.

set/block_pasid_dev, to be consistent.

> +
> + mutex_lock(_sva_lock);
> + /* Search for an existing domain. */
> + domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid);
> + if (domain) {
> + sva_domain = to_sva_domain(domain);
> + refcount_inc(_domain->bond.users);
> + goto out_success;
> + }
> +

why would one device/pasid be bound to a mm more than once?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-22 Thread Baolu Lu

On 2022/5/20 19:28, Jean-Philippe Brucker wrote:

On Fri, May 20, 2022 at 02:38:12PM +0800, Baolu Lu wrote:

On 2022/5/20 00:39, Jean-Philippe Brucker wrote:

+struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct 
*mm)
+{
+   struct iommu_sva_domain *sva_domain;
+   struct iommu_domain *domain;
+   ioasid_t max_pasid = 0;
+   int ret = -EINVAL;
+
+   /* Allocate mm->pasid if necessary. */
+   if (!dev->iommu->iommu_dev->pasids)
+   return ERR_PTR(-EOPNOTSUPP);
+
+   if (dev_is_pci(dev)) {
+   max_pasid = pci_max_pasids(to_pci_dev(dev));
+   if (max_pasid < 0)
+   return ERR_PTR(max_pasid);
+   } else {
+   ret = device_property_read_u32(dev, "pasid-num-bits",
+  _pasid);
+   if (ret)
+   return ERR_PTR(ret);
+   max_pasid = (1UL << max_pasid);
+   }

The IOMMU driver needs this PASID width information earlier, when creating
the PASID table (in .probe_device(), .attach_dev()). Since we're moving it
to the IOMMU core to avoid code duplication, it should be done earlier and
stored in dev->iommu

Yes, really. How about below changes?

 From f1382579e8a15ca49acdf758d38fd36451ea174d Mon Sep 17 00:00:00 2001
From: Lu Baolu
Date: Mon, 28 Feb 2022 15:01:35 +0800
Subject: [PATCH 1/1] iommu: Add pasids field in struct dev_iommu

Use this field to save the number of PASIDs that a device is able to
consume. It is a generic attribute of a device and lifting it into the
per-device dev_iommu struct could help to avoid the boilerplate code
in various IOMMU drivers.

Signed-off-by: Lu Baolu
---
  drivers/iommu/iommu.c | 15 +++
  include/linux/iommu.h |  2 ++
  2 files changed, 17 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e49c5a5b8cc1..6b731171d42f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -20,6 +20,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -194,6 +195,8 @@ EXPORT_SYMBOL_GPL(iommu_device_unregister);
  static struct dev_iommu *dev_iommu_get(struct device *dev)
  {
struct dev_iommu *param = dev->iommu;
+   u32 max_pasids = 0;
+   int ret;

if (param)
return param;
@@ -202,6 +205,18 @@ static struct dev_iommu *dev_iommu_get(struct device
*dev)
if (!param)
return NULL;

+   if (dev_is_pci(dev)) {
+   ret = pci_max_pasids(to_pci_dev(dev));
+   if (ret > 0)
+   max_pasids = ret;
+   } else {
+   ret = device_property_read_u32(dev, "pasid-num-bits",
+  _pasids);
+   if (!ret)
+   max_pasids = (1UL << max_pasids);
+   }
+   param->pasids = max_pasids;
+

we could also do a min() with the IOMMU PASID size here


mutex_init(>lock);
dev->iommu = param;
return param;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 45f274b2640d..d4296136ba75 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -371,6 +371,7 @@ struct iommu_fault_param {
   * @fwspec:IOMMU fwspec data
   * @iommu_dev: IOMMU device this device is linked to
   * @priv:  IOMMU Driver private data
+ * @pasids: number of supported PASIDs

'max_pasids' to stay consistent?


Both done. How about below changes?

From 008c73b9c0ad51a4a70a18d60361a76c28a63342 Mon Sep 17 00:00:00 2001
From: Lu Baolu 
Date: Mon, 28 Feb 2022 15:01:35 +0800
Subject: [PATCH 1/1] iommu: Add max_pasids field in struct dev_iommu

Use this field to save the number of PASIDs that a device is able to
consume. It is a generic attribute of a device and lifting it into the
per-device dev_iommu struct could help to avoid the boilerplate code
in various IOMMU drivers.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/iommu.c | 26 ++
 include/linux/iommu.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 847ad47a2dfd..365d0f2b7f55 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -218,6 +219,30 @@ static void dev_iommu_free(struct device *dev)
kfree(param);
 }

+static u32 dev_iommu_get_max_pasids(struct device *dev)
+{
+   u32 max_pasids = dev->iommu->iommu_dev->max_pasids;
+   u32 num_bits;
+   int ret;
+
+   if (!max_pasids)
+   return 0;
+
+   if (dev_is_pci(dev)) {
+   ret = pci_max_pasids(to_pci_dev(dev));
+   if (ret < 0)
+   return 0;
+
+   return min_t(u32, max_pasids, ret);
+   }
+
+   ret = device_property_read_u32(dev, "pasid-num-bits", _bits);
+   if (ret)
+   return 0;
+
+   

Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-20 Thread Jean-Philippe Brucker
On Fri, May 20, 2022 at 02:38:12PM +0800, Baolu Lu wrote:
> On 2022/5/20 00:39, Jean-Philippe Brucker wrote:
> > > +struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct 
> > > mm_struct *mm)
> > > +{
> > > + struct iommu_sva_domain *sva_domain;
> > > + struct iommu_domain *domain;
> > > + ioasid_t max_pasid = 0;
> > > + int ret = -EINVAL;
> > > +
> > > + /* Allocate mm->pasid if necessary. */
> > > + if (!dev->iommu->iommu_dev->pasids)
> > > + return ERR_PTR(-EOPNOTSUPP);
> > > +
> > > + if (dev_is_pci(dev)) {
> > > + max_pasid = pci_max_pasids(to_pci_dev(dev));
> > > + if (max_pasid < 0)
> > > + return ERR_PTR(max_pasid);
> > > + } else {
> > > + ret = device_property_read_u32(dev, "pasid-num-bits",
> > > +_pasid);
> > > + if (ret)
> > > + return ERR_PTR(ret);
> > > + max_pasid = (1UL << max_pasid);
> > > + }
> > The IOMMU driver needs this PASID width information earlier, when creating
> > the PASID table (in .probe_device(), .attach_dev()). Since we're moving it
> > to the IOMMU core to avoid code duplication, it should be done earlier and
> > stored in dev->iommu
> 
> Yes, really. How about below changes?
> 
> From f1382579e8a15ca49acdf758d38fd36451ea174d Mon Sep 17 00:00:00 2001
> From: Lu Baolu 
> Date: Mon, 28 Feb 2022 15:01:35 +0800
> Subject: [PATCH 1/1] iommu: Add pasids field in struct dev_iommu
> 
> Use this field to save the number of PASIDs that a device is able to
> consume. It is a generic attribute of a device and lifting it into the
> per-device dev_iommu struct could help to avoid the boilerplate code
> in various IOMMU drivers.
> 
> Signed-off-by: Lu Baolu 
> ---
>  drivers/iommu/iommu.c | 15 +++
>  include/linux/iommu.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index e49c5a5b8cc1..6b731171d42f 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -194,6 +195,8 @@ EXPORT_SYMBOL_GPL(iommu_device_unregister);
>  static struct dev_iommu *dev_iommu_get(struct device *dev)
>  {
>   struct dev_iommu *param = dev->iommu;
> + u32 max_pasids = 0;
> + int ret;
> 
>   if (param)
>   return param;
> @@ -202,6 +205,18 @@ static struct dev_iommu *dev_iommu_get(struct device
> *dev)
>   if (!param)
>   return NULL;
> 
> + if (dev_is_pci(dev)) {
> + ret = pci_max_pasids(to_pci_dev(dev));
> + if (ret > 0)
> + max_pasids = ret;
> + } else {
> + ret = device_property_read_u32(dev, "pasid-num-bits",
> +_pasids);
> + if (!ret)
> + max_pasids = (1UL << max_pasids);
> + }
> + param->pasids = max_pasids;
> +

we could also do a min() with the IOMMU PASID size here

>   mutex_init(>lock);
>   dev->iommu = param;
>   return param;
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 45f274b2640d..d4296136ba75 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -371,6 +371,7 @@ struct iommu_fault_param {
>   * @fwspec:   IOMMU fwspec data
>   * @iommu_dev:IOMMU device this device is linked to
>   * @priv: IOMMU Driver private data
> + * @pasids:   number of supported PASIDs

'max_pasids' to stay consistent?

Thanks,
Jean

>   *
>   * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
>   *   struct iommu_group  *iommu_group;
> @@ -382,6 +383,7 @@ struct dev_iommu {
>   struct iommu_fwspec *fwspec;
>   struct iommu_device *iommu_dev;
>   void*priv;
> + u32 pasids;
>  };
> 
>  int iommu_device_register(struct iommu_device *iommu,
> -- 
> 2.25.1
> 
> Best regards,
> baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-20 Thread Baolu Lu

On 2022/5/20 00:39, Jean-Philippe Brucker wrote:

+struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct 
*mm)
+{
+   struct iommu_sva_domain *sva_domain;
+   struct iommu_domain *domain;
+   ioasid_t max_pasid = 0;
+   int ret = -EINVAL;
+
+   /* Allocate mm->pasid if necessary. */
+   if (!dev->iommu->iommu_dev->pasids)
+   return ERR_PTR(-EOPNOTSUPP);
+
+   if (dev_is_pci(dev)) {
+   max_pasid = pci_max_pasids(to_pci_dev(dev));
+   if (max_pasid < 0)
+   return ERR_PTR(max_pasid);
+   } else {
+   ret = device_property_read_u32(dev, "pasid-num-bits",
+  _pasid);
+   if (ret)
+   return ERR_PTR(ret);
+   max_pasid = (1UL << max_pasid);
+   }

The IOMMU driver needs this PASID width information earlier, when creating
the PASID table (in .probe_device(), .attach_dev()). Since we're moving it
to the IOMMU core to avoid code duplication, it should be done earlier and
stored in dev->iommu


Yes, really. How about below changes?

From f1382579e8a15ca49acdf758d38fd36451ea174d Mon Sep 17 00:00:00 2001
From: Lu Baolu 
Date: Mon, 28 Feb 2022 15:01:35 +0800
Subject: [PATCH 1/1] iommu: Add pasids field in struct dev_iommu

Use this field to save the number of PASIDs that a device is able to
consume. It is a generic attribute of a device and lifting it into the
per-device dev_iommu struct could help to avoid the boilerplate code
in various IOMMU drivers.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/iommu.c | 15 +++
 include/linux/iommu.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e49c5a5b8cc1..6b731171d42f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -194,6 +195,8 @@ EXPORT_SYMBOL_GPL(iommu_device_unregister);
 static struct dev_iommu *dev_iommu_get(struct device *dev)
 {
struct dev_iommu *param = dev->iommu;
+   u32 max_pasids = 0;
+   int ret;

if (param)
return param;
@@ -202,6 +205,18 @@ static struct dev_iommu *dev_iommu_get(struct 
device *dev)

if (!param)
return NULL;

+   if (dev_is_pci(dev)) {
+   ret = pci_max_pasids(to_pci_dev(dev));
+   if (ret > 0)
+   max_pasids = ret;
+   } else {
+   ret = device_property_read_u32(dev, "pasid-num-bits",
+  _pasids);
+   if (!ret)
+   max_pasids = (1UL << max_pasids);
+   }
+   param->pasids = max_pasids;
+
mutex_init(>lock);
dev->iommu = param;
return param;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 45f274b2640d..d4296136ba75 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -371,6 +371,7 @@ struct iommu_fault_param {
  * @fwspec: IOMMU fwspec data
  * @iommu_dev:  IOMMU device this device is linked to
  * @priv:   IOMMU Driver private data
+ * @pasids: number of supported PASIDs
  *
  * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
  * struct iommu_group  *iommu_group;
@@ -382,6 +383,7 @@ struct dev_iommu {
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
void*priv;
+   u32 pasids;
 };

 int iommu_device_register(struct iommu_device *iommu,
--
2.25.1

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-19 Thread Jean-Philippe Brucker
On Thu, May 19, 2022 at 03:20:43PM +0800, Lu Baolu wrote:
> The existing iommu SVA interfaces are implemented by calling the SVA
> specific iommu ops provided by the IOMMU drivers. There's no need for
> any SVA specific ops in iommu_ops vector anymore as we can achieve
> this through the generic attach/detach_dev_pasid domain ops.
> 
> This refactors the IOMMU SVA interfaces implementation by using the
> set/block_pasid_dev ops and align them with the concept of the SVA
> iommu domain. Put the new SVA code in the sva related file in order
> to make it self-contained.
> 
> Signed-off-by: Lu Baolu 
> ---
>  include/linux/iommu.h |  48 --
>  drivers/iommu/iommu-sva-lib.h |   1 +
>  drivers/iommu/iommu-sva-lib.c | 113 
>  drivers/iommu/iommu.c | 119 --
>  4 files changed, 170 insertions(+), 111 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e8cf82d46ce1..d9ac5ebe5bbb 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -635,6 +635,7 @@ struct iommu_fwspec {
>   */
>  struct iommu_sva {
>   struct device   *dev;
> + refcount_t  users;
>  };
>  
>  int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> @@ -677,11 +678,6 @@ int iommu_dev_enable_feature(struct device *dev, enum 
> iommu_dev_features f);
>  int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
>  bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features 
> f);
>  
> -struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> - struct mm_struct *mm);
> -void iommu_sva_unbind_device(struct iommu_sva *handle);
> -u32 iommu_sva_get_pasid(struct iommu_sva *handle);
> -
>  int iommu_device_use_default_domain(struct device *dev);
>  void iommu_device_unuse_default_domain(struct device *dev);
>  
> @@ -693,6 +689,8 @@ int iommu_set_device_pasid(struct iommu_domain *domain, 
> struct device *dev,
>  ioasid_t pasid);
>  void iommu_block_device_pasid(struct iommu_domain *domain, struct device 
> *dev,
> ioasid_t pasid);
> +struct iommu_domain *
> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid);
>  #else /* CONFIG_IOMMU_API */
>  
>  struct iommu_ops {};
> @@ -1023,21 +1021,6 @@ iommu_dev_disable_feature(struct device *dev, enum 
> iommu_dev_features feat)
>   return -ENODEV;
>  }
>  
> -static inline struct iommu_sva *
> -iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
> -{
> - return NULL;
> -}
> -
> -static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
> -{
> -}
> -
> -static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
> -{
> - return IOMMU_PASID_INVALID;
> -}
> -
>  static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
>  {
>   return NULL;
> @@ -1077,6 +1060,12 @@ static inline void iommu_block_device_pasid(struct 
> iommu_domain *domain,
>   struct device *dev, ioasid_t pasid)
>  {
>  }
> +
> +static inline struct iommu_domain *
> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
> +{
> + return NULL;
> +}
>  #endif /* CONFIG_IOMMU_API */
>  
>  /**
> @@ -1108,6 +1097,10 @@ iommu_sva_alloc_domain(struct bus_type *bus, struct 
> mm_struct *mm);
>  void iommu_sva_free_domain(struct iommu_domain *domain);
>  int iommu_sva_set_domain(struct iommu_domain *domain, struct device *dev,
>ioasid_t pasid);
> +struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> + struct mm_struct *mm);
> +void iommu_sva_unbind_device(struct iommu_sva *handle);
> +u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>  #else /* CONFIG_IOMMU_SVA */
>  static inline struct iommu_domain *
>  iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm)
> @@ -1124,6 +1117,21 @@ static inline int iommu_sva_set_domain(struct 
> iommu_domain *domain,
>  {
>   return -EINVAL;
>  }
> +
> +static inline struct iommu_sva *
> +iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
> +{
> + return NULL;
> +}
> +
> +static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
> +{
> +}
> +
> +static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
> +{
> + return IOMMU_PASID_INVALID;
> +}
>  #endif /* CONFIG_IOMMU_SVA */
>  
>  #endif /* __LINUX_IOMMU_H */
> diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
> index 1be21e6b93ec..ebab5a8cb126 100644
> --- a/drivers/iommu/iommu-sva-lib.h
> +++ b/drivers/iommu/iommu-sva-lib.h
> @@ -20,6 +20,7 @@ struct iopf_queue;
>  struct iommu_sva_domain {
>   struct iommu_domain domain;
>   struct mm_struct*mm;
> + struct iommu_svabond;
>  };
>  
>  #define to_sva_domain(d) 

[PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-19 Thread Lu Baolu
The existing iommu SVA interfaces are implemented by calling the SVA
specific iommu ops provided by the IOMMU drivers. There's no need for
any SVA specific ops in iommu_ops vector anymore as we can achieve
this through the generic attach/detach_dev_pasid domain ops.

This refactors the IOMMU SVA interfaces implementation by using the
set/block_pasid_dev ops and align them with the concept of the SVA
iommu domain. Put the new SVA code in the sva related file in order
to make it self-contained.

Signed-off-by: Lu Baolu 
---
 include/linux/iommu.h |  48 --
 drivers/iommu/iommu-sva-lib.h |   1 +
 drivers/iommu/iommu-sva-lib.c | 113 
 drivers/iommu/iommu.c | 119 --
 4 files changed, 170 insertions(+), 111 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e8cf82d46ce1..d9ac5ebe5bbb 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -635,6 +635,7 @@ struct iommu_fwspec {
  */
 struct iommu_sva {
struct device   *dev;
+   refcount_t  users;
 };
 
 int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
@@ -677,11 +678,6 @@ int iommu_dev_enable_feature(struct device *dev, enum 
iommu_dev_features f);
 int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
 bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
 
-struct iommu_sva *iommu_sva_bind_device(struct device *dev,
-   struct mm_struct *mm);
-void iommu_sva_unbind_device(struct iommu_sva *handle);
-u32 iommu_sva_get_pasid(struct iommu_sva *handle);
-
 int iommu_device_use_default_domain(struct device *dev);
 void iommu_device_unuse_default_domain(struct device *dev);
 
@@ -693,6 +689,8 @@ int iommu_set_device_pasid(struct iommu_domain *domain, 
struct device *dev,
   ioasid_t pasid);
 void iommu_block_device_pasid(struct iommu_domain *domain, struct device *dev,
  ioasid_t pasid);
+struct iommu_domain *
+iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid);
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -1023,21 +1021,6 @@ iommu_dev_disable_feature(struct device *dev, enum 
iommu_dev_features feat)
return -ENODEV;
 }
 
-static inline struct iommu_sva *
-iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
-{
-   return NULL;
-}
-
-static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
-{
-}
-
-static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
-{
-   return IOMMU_PASID_INVALID;
-}
-
 static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
 {
return NULL;
@@ -1077,6 +1060,12 @@ static inline void iommu_block_device_pasid(struct 
iommu_domain *domain,
struct device *dev, ioasid_t pasid)
 {
 }
+
+static inline struct iommu_domain *
+iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
+{
+   return NULL;
+}
 #endif /* CONFIG_IOMMU_API */
 
 /**
@@ -1108,6 +1097,10 @@ iommu_sva_alloc_domain(struct bus_type *bus, struct 
mm_struct *mm);
 void iommu_sva_free_domain(struct iommu_domain *domain);
 int iommu_sva_set_domain(struct iommu_domain *domain, struct device *dev,
 ioasid_t pasid);
+struct iommu_sva *iommu_sva_bind_device(struct device *dev,
+   struct mm_struct *mm);
+void iommu_sva_unbind_device(struct iommu_sva *handle);
+u32 iommu_sva_get_pasid(struct iommu_sva *handle);
 #else /* CONFIG_IOMMU_SVA */
 static inline struct iommu_domain *
 iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm)
@@ -1124,6 +1117,21 @@ static inline int iommu_sva_set_domain(struct 
iommu_domain *domain,
 {
return -EINVAL;
 }
+
+static inline struct iommu_sva *
+iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
+{
+   return NULL;
+}
+
+static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
+{
+}
+
+static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
+{
+   return IOMMU_PASID_INVALID;
+}
 #endif /* CONFIG_IOMMU_SVA */
 
 #endif /* __LINUX_IOMMU_H */
diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
index 1be21e6b93ec..ebab5a8cb126 100644
--- a/drivers/iommu/iommu-sva-lib.h
+++ b/drivers/iommu/iommu-sva-lib.h
@@ -20,6 +20,7 @@ struct iopf_queue;
 struct iommu_sva_domain {
struct iommu_domain domain;
struct mm_struct*mm;
+   struct iommu_svabond;
 };
 
 #define to_sva_domain(d) container_of_safe(d, struct iommu_sva_domain, domain)
diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c
index 210c376f6043..568e0f64edac 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva-lib.c
@@ -4,6 +4,8 @@
  */
 #include 
 #include 
+#include 
+#include 
 
 #include