diff mbox series

[v2,03/11] iommufd/hw_pagetable: Use domain_alloc_user op for domain allocation

Message ID 20230511143844.22693-4-yi.l.liu@intel.com
State New
Headers
Series iommufd: Add nesting infrastructure |

Commit Message

Yi Liu May 11, 2023, 2:38 p.m. UTC 
  This makes IOMMUFD to use iommu_domain_alloc_user() for iommu_domain
creation as IOMMUFD needs to support iommu_domain allocation with
parameters from userspace in nesting support. If the iommu driver
doesn't provide domain_alloc_user callback then it falls back to use
iommu_domain_alloc().

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Co-developed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/iommufd/hw_pagetable.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Tian, Kevin May 19, 2023, 8:56 a.m. UTC | #1 
> From: Yi Liu <yi.l.liu@intel.com>
> Sent: Thursday, May 11, 2023 10:39 PM
> 
> 
> @@ -88,7 +90,10 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx
> *ictx, struct iommufd_ioas *ioas,
>  	refcount_inc(&ioas->obj.users);
>  	hwpt->ioas = ioas;
> 
> -	hwpt->domain = iommu_domain_alloc(idev->dev->bus);
> +	if (ops->domain_alloc_user)
> +		hwpt->domain = ops->domain_alloc_user(idev->dev, NULL,
> NULL);
> +	else
> +		hwpt->domain = iommu_domain_alloc(idev->dev->bus);

this reminds the comment for @domain_alloc_user() should clarify
that UNMANGED domain type is assumed when no user data is 
provided, to be compatible with iommu_domain_alloc().
Nicolin Chen May 19, 2023, 6:57 p.m. UTC | #2 
On Fri, May 19, 2023 at 08:56:25AM +0000, Tian, Kevin wrote:
 
> > From: Yi Liu <yi.l.liu@intel.com>
> > Sent: Thursday, May 11, 2023 10:39 PM
> >
> >
> > @@ -88,7 +90,10 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx
> > *ictx, struct iommufd_ioas *ioas,
> >       refcount_inc(&ioas->obj.users);
> >       hwpt->ioas = ioas;
> >
> > -     hwpt->domain = iommu_domain_alloc(idev->dev->bus);
> > +     if (ops->domain_alloc_user)
> > +             hwpt->domain = ops->domain_alloc_user(idev->dev, NULL,
> > NULL);
> > +     else
> > +             hwpt->domain = iommu_domain_alloc(idev->dev->bus);
> 
> this reminds the comment for @domain_alloc_user() should clarify
> that UNMANGED domain type is assumed when no user data is
> provided, to be compatible with iommu_domain_alloc().

Yes. Perhaps:

 * @domain_alloc_user: allocate user iommu domain. A valid user_data pointer and
 *                     a parent pointer to a kernel-managed domain are required
 *                     to allocate an IOMMU_DOMAIN_NESTED domain. Otherwise, the
 *                     new domain will be set to an IOMMU_DOMAIN_UNMANAGED type.

Thanks
Nic
Tian, Kevin May 24, 2023, 5:04 a.m. UTC | #3 
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Saturday, May 20, 2023 2:58 AM
> 
> On Fri, May 19, 2023 at 08:56:25AM +0000, Tian, Kevin wrote:
> 
> > > From: Yi Liu <yi.l.liu@intel.com>
> > > Sent: Thursday, May 11, 2023 10:39 PM
> > >
> > >
> > > @@ -88,7 +90,10 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx
> > > *ictx, struct iommufd_ioas *ioas,
> > >       refcount_inc(&ioas->obj.users);
> > >       hwpt->ioas = ioas;
> > >
> > > -     hwpt->domain = iommu_domain_alloc(idev->dev->bus);
> > > +     if (ops->domain_alloc_user)
> > > +             hwpt->domain = ops->domain_alloc_user(idev->dev, NULL,
> > > NULL);
> > > +     else
> > > +             hwpt->domain = iommu_domain_alloc(idev->dev->bus);
> >
> > this reminds the comment for @domain_alloc_user() should clarify
> > that UNMANGED domain type is assumed when no user data is
> > provided, to be compatible with iommu_domain_alloc().
> 
> Yes. Perhaps:
> 
>  * @domain_alloc_user: allocate user iommu domain. A valid user_data
> pointer and
>  *                     a parent pointer to a kernel-managed domain are required
>  *                     to allocate an IOMMU_DOMAIN_NESTED domain. Otherwise,
> the
>  *                     new domain will be set to an IOMMU_DOMAIN_UNMANAGED
> type.
> 

yes, this is clearer.
diff mbox series

Patch

diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index cf2c1504e20d..b6323ad9c32d 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -5,6 +5,7 @@ 
 #include <linux/iommu.h>
 #include <uapi/linux/iommufd.h>
 
+#include "../iommu-priv.h"
 #include "iommufd_private.h"
 
 void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
@@ -74,6 +75,7 @@  struct iommufd_hw_pagetable *
 iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 			   struct iommufd_device *idev, bool immediate_attach)
 {
+	const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
 	struct iommufd_hw_pagetable *hwpt;
 	int rc;
 
@@ -88,7 +90,10 @@  iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 	refcount_inc(&ioas->obj.users);
 	hwpt->ioas = ioas;
 
-	hwpt->domain = iommu_domain_alloc(idev->dev->bus);
+	if (ops->domain_alloc_user)
+		hwpt->domain = ops->domain_alloc_user(idev->dev, NULL, NULL);
+	else
+		hwpt->domain = iommu_domain_alloc(idev->dev->bus);
 	if (!hwpt->domain) {
 		rc = -ENOMEM;
 		goto out_abort;