Nested translation has stage-1 and stage-2 page tables. A stage-1 page
table is managed by user space, and it needs to work with a stage-2 page
table, which is a parent hwpt for the stage-1 hwpt.
iommu core already supports accepting parent iommu_domain and user_data
to allocate an iommu_domain. This makes iommufd_hw_pagetable_alloc() to
accept the parent hwpt and user_data, and relays them to iommu core, to
prepare for supporting hw_pagetable allocation with user_data.
Also, add a parent pointer in struct iommufd_hw_pagetable for taking and
releasing its refcount.
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/device.c | 2 +-
drivers/iommu/iommufd/hw_pagetable.c | 28 ++++++++++++++++++++++---
drivers/iommu/iommufd/iommufd_private.h | 5 ++++-
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 5c352807d946..19cd6df46c6a 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -558,7 +558,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
}
hwpt = iommufd_hw_pagetable_alloc(idev->ictx, ioas, idev,
- immediate_attach);
+ NULL, NULL, immediate_attach);
if (IS_ERR(hwpt)) {
destroy_hwpt = ERR_CAST(hwpt);
goto out_unlock;
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 84b4a11e62f8..16e92a1c150b 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -24,6 +24,8 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
if (hwpt->domain)
iommu_domain_free(hwpt->domain);
+ if (hwpt->parent)
+ refcount_dec(&hwpt->parent->obj.users);
refcount_dec(&hwpt->ioas->obj.users);
}
@@ -46,6 +48,8 @@ int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt)
* @ictx: iommufd context
* @ioas: IOAS to associate the domain with
* @idev: Device to get an iommu_domain for
+ * @parent: Optional parent HWPT to associate with the domain with
+ * @user_data: Optional user_data pointer
* @immediate_attach: True if idev should be attached to the hwpt
*
* Allocate a new iommu_domain and return it as a hw_pagetable. The HWPT
@@ -54,14 +58,20 @@ int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt)
*/
struct iommufd_hw_pagetable *
iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
- struct iommufd_device *idev, bool immediate_attach)
+ struct iommufd_device *idev,
+ struct iommufd_hw_pagetable *parent,
+ void *user_data, bool immediate_attach)
{
const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
+ struct iommu_domain *parent_domain = NULL;
struct iommufd_hw_pagetable *hwpt;
int rc;
lockdep_assert_held(&ioas->mutex);
+ if (parent && !ops->domain_alloc_user)
+ return ERR_PTR(-EOPNOTSUPP);
+
hwpt = iommufd_object_alloc(ictx, hwpt, IOMMUFD_OBJ_HW_PAGETABLE);
if (IS_ERR(hwpt))
return hwpt;
@@ -70,9 +80,15 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
/* Pairs with iommufd_hw_pagetable_destroy() */
refcount_inc(&ioas->obj.users);
hwpt->ioas = ioas;
+ if (parent) {
+ hwpt->parent = parent;
+ parent_domain = parent->domain;
+ refcount_inc(&parent->obj.users);
+ }
if (ops->domain_alloc_user)
- hwpt->domain = ops->domain_alloc_user(idev->dev, NULL, NULL);
+ hwpt->domain = ops->domain_alloc_user(idev->dev,
+ parent_domain, user_data);
else
hwpt->domain = iommu_domain_alloc(idev->dev->bus);
if (!hwpt->domain) {
@@ -80,6 +96,11 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
goto out_abort;
}
+ /* It must be either NESTED or UNMANAGED, depending on parent_domain */
+ if ((parent_domain && hwpt->domain->type != IOMMU_DOMAIN_NESTED) ||
+ (!parent_domain && hwpt->domain->type != IOMMU_DOMAIN_UNMANAGED))
+ goto out_abort;
+
/*
* Set the coherency mode before we do iopt_table_add_domain() as some
* iommus have a per-PTE bit that controls it and need to decide before
@@ -150,7 +171,8 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd)
}
mutex_lock(&ioas->mutex);
- hwpt = iommufd_hw_pagetable_alloc(ucmd->ictx, ioas, idev, false);
+ hwpt = iommufd_hw_pagetable_alloc(ucmd->ictx, ioas, idev,
+ NULL, NULL, false);
mutex_unlock(&ioas->mutex);
if (IS_ERR(hwpt)) {
rc = PTR_ERR(hwpt);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 05b5ad66f716..182c074eecdc 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -243,6 +243,7 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd);
*/
struct iommufd_hw_pagetable {
struct iommufd_object obj;
+ struct iommufd_hw_pagetable *parent;
struct iommufd_ioas *ioas;
struct iommu_domain *domain;
bool auto_domain : 1;
@@ -254,7 +255,9 @@ struct iommufd_hw_pagetable {
struct iommufd_hw_pagetable *
iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
- struct iommufd_device *idev, bool immediate_attach);
+ struct iommufd_device *idev,
+ struct iommufd_hw_pagetable *parent,
+ void *user_data, bool immediate_attach);
int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt);
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev);
--
2.34.1> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Thursday, March 9, 2023 4:09 PM > > + /* It must be either NESTED or UNMANAGED, depending on > parent_domain */ > + if ((parent_domain && hwpt->domain->type != > IOMMU_DOMAIN_NESTED) || > + (!parent_domain && hwpt->domain->type != > IOMMU_DOMAIN_UNMANAGED)) > + goto out_abort; > + WARN_ON()
On Fri, Mar 17, 2023 at 10:23:54AM +0000, Tian, Kevin wrote: > > From: Liu, Yi L <yi.l.liu@intel.com> > > Sent: Thursday, March 9, 2023 4:09 PM > > > > + /* It must be either NESTED or UNMANAGED, depending on > > parent_domain */ > > + if ((parent_domain && hwpt->domain->type != > > IOMMU_DOMAIN_NESTED) || > > + (!parent_domain && hwpt->domain->type != > > IOMMU_DOMAIN_UNMANAGED)) > > + goto out_abort; > > + > > WARN_ON() Wouldn't that be userspace triggerable? It gets to pick the hwpt used. Jason
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Monday, March 20, 2023 8:47 PM > > On Fri, Mar 17, 2023 at 10:23:54AM +0000, Tian, Kevin wrote: > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > Sent: Thursday, March 9, 2023 4:09 PM > > > > > > + /* It must be either NESTED or UNMANAGED, depending on > > > parent_domain */ > > > + if ((parent_domain && hwpt->domain->type != > > > IOMMU_DOMAIN_NESTED) || > > > + (!parent_domain && hwpt->domain->type != > > > IOMMU_DOMAIN_UNMANAGED)) > > > + goto out_abort; > > > + > > > > WARN_ON() > > Wouldn't that be userspace triggerable? It gets to pick the hwpt used. > The domain type is set by iommu driver instead of userspace. A sane driver should pass above check, if it claims to support nested.
On 3/9/23 4:09 PM, Yi Liu wrote:
> Nested translation has stage-1 and stage-2 page tables. A stage-1 page
> table is managed by user space, and it needs to work with a stage-2 page
> table, which is a parent hwpt for the stage-1 hwpt.
>
> iommu core already supports accepting parent iommu_domain and user_data
> to allocate an iommu_domain. This makes iommufd_hw_pagetable_alloc() to
> accept the parent hwpt and user_data, and relays them to iommu core, to
> prepare for supporting hw_pagetable allocation with user_data.
>
> Also, add a parent pointer in struct iommufd_hw_pagetable for taking and
> releasing its refcount.
>
> 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/device.c | 2 +-
> drivers/iommu/iommufd/hw_pagetable.c | 28 ++++++++++++++++++++++---
> drivers/iommu/iommufd/iommufd_private.h | 5 ++++-
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 5c352807d946..19cd6df46c6a 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -558,7 +558,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
> }
>
> hwpt = iommufd_hw_pagetable_alloc(idev->ictx, ioas, idev,
> - immediate_attach);
> + NULL, NULL, immediate_attach);
> if (IS_ERR(hwpt)) {
> destroy_hwpt = ERR_CAST(hwpt);
> goto out_unlock;
> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
> index 84b4a11e62f8..16e92a1c150b 100644
> --- a/drivers/iommu/iommufd/hw_pagetable.c
> +++ b/drivers/iommu/iommufd/hw_pagetable.c
> @@ -24,6 +24,8 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
> if (hwpt->domain)
> iommu_domain_free(hwpt->domain);
>
> + if (hwpt->parent)
> + refcount_dec(&hwpt->parent->obj.users);
> refcount_dec(&hwpt->ioas->obj.users);
> }
>
> @@ -46,6 +48,8 @@ int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt)
> * @ictx: iommufd context
> * @ioas: IOAS to associate the domain with
> * @idev: Device to get an iommu_domain for
> + * @parent: Optional parent HWPT to associate with the domain with
> + * @user_data: Optional user_data pointer
> * @immediate_attach: True if idev should be attached to the hwpt
> *
> * Allocate a new iommu_domain and return it as a hw_pagetable. The HWPT
> @@ -54,14 +58,20 @@ int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt)
> */
> struct iommufd_hw_pagetable *
> iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
> - struct iommufd_device *idev, bool immediate_attach)
> + struct iommufd_device *idev,
> + struct iommufd_hw_pagetable *parent,
> + void *user_data, bool immediate_attach)
> {
> const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
> + struct iommu_domain *parent_domain = NULL;
> struct iommufd_hw_pagetable *hwpt;
> int rc;
>
> lockdep_assert_held(&ioas->mutex);
>
> + if (parent && !ops->domain_alloc_user)
> + return ERR_PTR(-EOPNOTSUPP);
My understanding here is that we are checking whether domain_alloc_user
is required. It seems that as long as the caller inputs a valid
user_data or parent, domain_alloc_user is required. If so,
if ((user_data || parent) && !ops->domain_alloc_user)
return ERR_PTR(-EOPNOTSUPP);
Best regards,
baolu
On Fri, Mar 10, 2023 at 10:10:56AM +0800, Baolu Lu wrote:
> > @@ -54,14 +58,20 @@ int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt)
> > */
> > struct iommufd_hw_pagetable *
> > iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
> > - struct iommufd_device *idev, bool immediate_attach)
> > + struct iommufd_device *idev,
> > + struct iommufd_hw_pagetable *parent,
> > + void *user_data, bool immediate_attach)
> > {
> > const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
> > + struct iommu_domain *parent_domain = NULL;
> > struct iommufd_hw_pagetable *hwpt;
> > int rc;
> > lockdep_assert_held(&ioas->mutex);
> > + if (parent && !ops->domain_alloc_user)
> > + return ERR_PTR(-EOPNOTSUPP);
>
> My understanding here is that we are checking whether domain_alloc_user
> is required. It seems that as long as the caller inputs a valid
> user_data or parent, domain_alloc_user is required. If so,
>
> if ((user_data || parent) && !ops->domain_alloc_user)
> return ERR_PTR(-EOPNOTSUPP);
Yes
Jason
© 2016 - 2025 Red Hat, Inc.