PASID (Process Address Space ID) is a PCIe extension to tag the DMA transactions out of a physical device, and most modern IOMMU hardware have supported PASID granular address translation. So a PASID-capable devices can be attached to multiple hwpts (a.k.a. domains), each attachment is tagged with a PASID.
This series first adds a missing iommu API to replace domain for a pasid, then adds iommufd APIs for device drivers to attach/replace/detach pasid to/from hwpt per userspace's request, and adds selftest to validate the iommufd APIs.
pasid attach/replace is mandatory on Intel VT-d given the PASID table locates in the physical address space hence must be managed by the kernel, both for supporting vSVA and coming SIOV. But it's optional on ARM/AMD which allow configuring the PASID/CD table either in host physical address space or nested on top of an GPA address space. This series only add VT-d support as the minimal requirement.
Complete code can be found in below link:
https://github.com/yiliu1765/iommufd/tree/iommufd_pasid
Regards, Yi Liu
Kevin Tian (1): iommufd: Support attach/replace hwpt per pasid
Lu Baolu (2): iommu: Introduce a replace API for device pasid iommu/vt-d: Add set_dev_pasid callback for nested domain
Yi Liu (5): iommufd: replace attach_fn with a structure iommufd/selftest: Add set_dev_pasid and remove_dev_pasid in mock iommu iommufd/selftest: Add a helper to get test device iommufd/selftest: Add test ops to test pasid attach/detach iommufd/selftest: Add coverage for iommufd pasid attach/detach
drivers/iommu/intel/nested.c | 47 +++++ drivers/iommu/iommu-priv.h | 2 + drivers/iommu/iommu.c | 73 ++++++-- drivers/iommu/iommufd/Makefile | 1 + drivers/iommu/iommufd/device.c | 42 +++-- drivers/iommu/iommufd/iommufd_private.h | 16 ++ drivers/iommu/iommufd/iommufd_test.h | 24 +++ drivers/iommu/iommufd/pasid.c | 152 ++++++++++++++++ drivers/iommu/iommufd/selftest.c | 158 ++++++++++++++-- include/linux/iommufd.h | 6 + tools/testing/selftests/iommu/iommufd.c | 172 ++++++++++++++++++ .../selftests/iommu/iommufd_fail_nth.c | 28 ++- tools/testing/selftests/iommu/iommufd_utils.h | 78 ++++++++ 13 files changed, 756 insertions(+), 43 deletions(-) create mode 100644 drivers/iommu/iommufd/pasid.c
From: Lu Baolu baolu.lu@linux.intel.com
Provide a high-level API to allow replacements of one domain with another for specific pasid of a device. This is similar to iommu_group_replace_domain() and it is also expected to be used only by IOMMUFD.
Signed-off-by: Lu Baolu baolu.lu@linux.intel.com Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommu-priv.h | 2 ++ drivers/iommu/iommu.c | 73 ++++++++++++++++++++++++++++++-------- 2 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h index 2024a2313348..5c32637f6325 100644 --- a/drivers/iommu/iommu-priv.h +++ b/drivers/iommu/iommu-priv.h @@ -19,6 +19,8 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
int iommu_group_replace_domain(struct iommu_group *group, struct iommu_domain *new_domain); +int iommu_replace_device_pasid(struct iommu_domain *domain, + struct device *dev, ioasid_t pasid);
int iommu_device_register_bus(struct iommu_device *iommu, const struct iommu_ops *ops, struct bus_type *bus, diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index e53c9e659ae6..b47581c7b1c6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -3382,6 +3382,27 @@ static void __iommu_remove_group_pasid(struct iommu_group *group, } }
+static int __iommu_group_attach_pasid(struct iommu_domain *domain, + struct iommu_group *group, ioasid_t pasid) +{ + void *curr; + int ret; + + lockdep_assert_held(&group->mutex); + + curr = xa_cmpxchg(&group->pasid_array, pasid, NULL, domain, GFP_KERNEL); + if (curr) + return xa_err(curr) ? : -EBUSY; + + ret = __iommu_set_group_pasid(domain, group, pasid); + if (ret) { + __iommu_remove_group_pasid(group, pasid); + xa_erase(&group->pasid_array, pasid); + } + + return ret; +} + /* * iommu_attach_device_pasid() - Attach a domain to pasid of device * @domain: the iommu domain. @@ -3394,7 +3415,6 @@ int iommu_attach_device_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid) { struct iommu_group *group; - void *curr; int ret;
if (!domain->ops->set_dev_pasid) @@ -3405,19 +3425,9 @@ int iommu_attach_device_pasid(struct iommu_domain *domain, return -ENODEV;
mutex_lock(&group->mutex); - curr = xa_cmpxchg(&group->pasid_array, pasid, NULL, domain, GFP_KERNEL); - if (curr) { - ret = xa_err(curr) ? : -EBUSY; - goto out_unlock; - } - - ret = __iommu_set_group_pasid(domain, group, pasid); - if (ret) { - __iommu_remove_group_pasid(group, pasid); - xa_erase(&group->pasid_array, pasid); - } -out_unlock: + ret = __iommu_group_attach_pasid(domain, group, pasid); mutex_unlock(&group->mutex); + iommu_group_put(group);
return ret; @@ -3433,8 +3443,8 @@ EXPORT_SYMBOL_GPL(iommu_attach_device_pasid); * The @domain must have been attached to @pasid of the @dev with * iommu_attach_device_pasid(). */ -void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev, - ioasid_t pasid) +void iommu_detach_device_pasid(struct iommu_domain *domain, + struct device *dev, ioasid_t pasid) { struct iommu_group *group = iommu_group_get(dev);
@@ -3447,6 +3457,39 @@ void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev, } EXPORT_SYMBOL_GPL(iommu_detach_device_pasid);
+/** + * iommu_replace_device_pasid - replace the domain that a pasid is attached to + * @domain: new IOMMU domain to replace with + * @dev: the physical device + * @pasid: pasid that will be attached to the new domain + * + * This API allows the pasid to switch domains. Return 0 on success, or an + * error. The pasid will roll back to use the old domain if failure. The + * caller could call iommu_detach_device_pasid() before free the old domain + * in order to avoid use-after-free case. + */ +int iommu_replace_device_pasid(struct iommu_domain *domain, + struct device *dev, ioasid_t pasid) +{ + struct iommu_group *group = dev->iommu_group; + int ret; + + if (!domain) + return -EINVAL; + + if (!group) + return -ENODEV; + + mutex_lock(&group->mutex); + __iommu_remove_group_pasid(group, pasid); + xa_erase(&group->pasid_array, pasid); + ret = __iommu_group_attach_pasid(domain, group, pasid); + mutex_unlock(&group->mutex); + + return ret; +} +EXPORT_SYMBOL_NS_GPL(iommu_replace_device_pasid, IOMMUFD_INTERNAL); + /* * iommu_get_domain_for_dev_pasid() - Retrieve domain for @pasid of @dev * @dev: the queried device
On 9/26/23 5:26 PM, Yi Liu wrote:
From: Lu Baolu baolu.lu@linux.intel.com
Provide a high-level API to allow replacements of one domain with another for specific pasid of a device. This is similar to iommu_group_replace_domain() and it is also expected to be used only by IOMMUFD.
Signed-off-by: Lu Baolu baolu.lu@linux.intel.com Signed-off-by: Yi Liu yi.l.liu@intel.com
drivers/iommu/iommu-priv.h | 2 ++ drivers/iommu/iommu.c | 73 ++++++++++++++++++++++++++++++-------- 2 files changed, 60 insertions(+), 15 deletions(-)
[...]
@@ -3433,8 +3443,8 @@ EXPORT_SYMBOL_GPL(iommu_attach_device_pasid);
- The @domain must have been attached to @pasid of the @dev with
- iommu_attach_device_pasid().
*/ -void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev,
ioasid_t pasid)+void iommu_detach_device_pasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid)
Above change is irrelevant.
{ struct iommu_group *group = iommu_group_get(dev); @@ -3447,6 +3457,39 @@ void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev, } EXPORT_SYMBOL_GPL(iommu_detach_device_pasid); +/**
- iommu_replace_device_pasid - replace the domain that a pasid is attached to
- @domain: new IOMMU domain to replace with
- @dev: the physical device
- @pasid: pasid that will be attached to the new domain
- This API allows the pasid to switch domains. Return 0 on success, or an
- error. The pasid will roll back to use the old domain if failure. The
- caller could call iommu_detach_device_pasid() before free the old domain
- in order to avoid use-after-free case.
The comment does not match the actual behavior of the code. We need to discuss and agree on which state the PASID should park in if replacing the domain fails.
- */
+int iommu_replace_device_pasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid)+{
- struct iommu_group *group = dev->iommu_group;
- int ret;
- if (!domain)
return -EINVAL;- if (!group)
return -ENODEV;- mutex_lock(&group->mutex);
- __iommu_remove_group_pasid(group, pasid);
- xa_erase(&group->pasid_array, pasid);
- ret = __iommu_group_attach_pasid(domain, group, pasid);
- mutex_unlock(&group->mutex);
- return ret;
+} +EXPORT_SYMBOL_NS_GPL(iommu_replace_device_pasid, IOMMUFD_INTERNAL);
- /*
- iommu_get_domain_for_dev_pasid() - Retrieve domain for @pasid of @dev
- @dev: the queried device
Best regards, baolu
From: Baolu Lu baolu.lu@linux.intel.com Sent: Wednesday, September 27, 2023 10:05 AM
On 9/26/23 5:26 PM, Yi Liu wrote:
+/**
- iommu_replace_device_pasid - replace the domain that a pasid is
attached to
- @domain: new IOMMU domain to replace with
- @dev: the physical device
- @pasid: pasid that will be attached to the new domain
- This API allows the pasid to switch domains. Return 0 on success, or an
- error. The pasid will roll back to use the old domain if failure. The
- caller could call iommu_detach_device_pasid() before free the old
domain
- in order to avoid use-after-free case.
The comment does not match the actual behavior of the code. We need to discuss and agree on which state the PASID should park in if replacing the domain fails.
there is no legacy as in group_set_domain and iommufd is the only caller of this new function. So IMHO the description above is the right thing to do and the code should be updated to match it.
Most of the core logic before conducting the actual device attach/ replace operation can be shared with pasid attach/replace. Create a new structure so more information (e.g. pasid) can be later added along with the attach_fn.
Signed-off-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommufd/device.c | 35 ++++++++++++++++--------- drivers/iommu/iommufd/iommufd_private.h | 8 ++++++ 2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 645ab5d290fe..4fa4153c5df7 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -597,8 +597,11 @@ iommufd_device_do_replace(struct iommufd_device *idev, return ERR_PTR(rc); }
-typedef struct iommufd_hw_pagetable *(*attach_fn)( - struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt); +static struct iommufd_hw_pagetable *do_attach(struct iommufd_device *idev, + struct iommufd_hw_pagetable *hwpt, struct attach_data *data) +{ + return data->attach_fn(idev, hwpt); +}
/* * When automatically managing the domains we search for a compatible domain in @@ -608,7 +611,7 @@ typedef struct iommufd_hw_pagetable *(*attach_fn)( static struct iommufd_hw_pagetable * iommufd_device_auto_get_domain(struct iommufd_device *idev, struct iommufd_ioas *ioas, u32 *pt_id, - attach_fn do_attach) + struct attach_data *data) { /* * iommufd_hw_pagetable_attach() is called by @@ -617,7 +620,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev, * to use the immediate_attach path as it supports drivers that can't * directly allocate a domain. */ - bool immediate_attach = do_attach == iommufd_device_do_attach; + bool immediate_attach = data->attach_fn == iommufd_device_do_attach; struct iommufd_hw_pagetable *destroy_hwpt; struct iommufd_hw_pagetable *hwpt;
@@ -633,7 +636,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
if (!iommufd_lock_obj(&hwpt->obj)) continue; - destroy_hwpt = (*do_attach)(idev, hwpt); + destroy_hwpt = do_attach(idev, hwpt, data); if (IS_ERR(destroy_hwpt)) { iommufd_put_object(&hwpt->obj); /* @@ -660,7 +663,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev, }
if (!immediate_attach) { - destroy_hwpt = (*do_attach)(idev, hwpt); + destroy_hwpt = do_attach(idev, hwpt, data); if (IS_ERR(destroy_hwpt)) goto out_abort; } else { @@ -681,8 +684,8 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev, return destroy_hwpt; }
-static int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id, - attach_fn do_attach) +int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id, + struct attach_data *data) { struct iommufd_hw_pagetable *destroy_hwpt; struct iommufd_object *pt_obj; @@ -696,7 +699,7 @@ static int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id, struct iommufd_hw_pagetable *hwpt = container_of(pt_obj, struct iommufd_hw_pagetable, obj);
- destroy_hwpt = (*do_attach)(idev, hwpt); + destroy_hwpt = do_attach(idev, hwpt, data); if (IS_ERR(destroy_hwpt)) goto out_put_pt_obj; break; @@ -706,7 +709,7 @@ static int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id, container_of(pt_obj, struct iommufd_ioas, obj);
destroy_hwpt = iommufd_device_auto_get_domain(idev, ioas, pt_id, - do_attach); + data); if (IS_ERR(destroy_hwpt)) goto out_put_pt_obj; break; @@ -742,8 +745,11 @@ static int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id, int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id) { int rc; + struct attach_data data = { + .attach_fn = &iommufd_device_do_attach, + };
- rc = iommufd_device_change_pt(idev, pt_id, &iommufd_device_do_attach); + rc = iommufd_device_change_pt(idev, pt_id, &data); if (rc) return rc;
@@ -773,8 +779,11 @@ EXPORT_SYMBOL_NS_GPL(iommufd_device_attach, IOMMUFD); */ int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id) { - return iommufd_device_change_pt(idev, pt_id, - &iommufd_device_do_replace); + struct attach_data data = { + .attach_fn = &iommufd_device_do_replace, + }; + + return iommufd_device_change_pt(idev, pt_id, &data); } EXPORT_SYMBOL_NS_GPL(iommufd_device_replace, IOMMUFD);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 1bd412cff2d6..f1fe4120c3b1 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -353,6 +353,14 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd); int iommufd_set_dev_data(struct iommufd_ucmd *ucmd); int iommufd_unset_dev_data(struct iommufd_ucmd *ucmd);
+struct attach_data { + union { + struct iommufd_hw_pagetable *(*attach_fn)( + struct iommufd_device *idev, + struct iommufd_hw_pagetable *hwpt); + }; +}; + struct iommufd_access { struct iommufd_object obj; struct iommufd_ctx *ictx;
On 9/26/23 5:26 PM, Yi Liu wrote:
Most of the core logic before conducting the actual device attach/ replace operation can be shared with pasid attach/replace. Create a new structure so more information (e.g. pasid) can be later added along with the attach_fn.
Signed-off-by: Kevin Tiankevin.tian@intel.com Signed-off-by: Yi Liuyi.l.liu@intel.com
drivers/iommu/iommufd/device.c | 35 ++++++++++++++++--------- drivers/iommu/iommufd/iommufd_private.h | 8 ++++++ 2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 645ab5d290fe..4fa4153c5df7 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -597,8 +597,11 @@ iommufd_device_do_replace(struct iommufd_device *idev, return ERR_PTR(rc); } -typedef struct iommufd_hw_pagetable *(*attach_fn)(
- struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt);
+static struct iommufd_hw_pagetable *do_attach(struct iommufd_device *idev,
struct iommufd_hw_pagetable *hwpt, struct attach_data *data)+{
- return data->attach_fn(idev, hwpt);
+}
I assume that this change was made because we need to pass the pasid value to the attach_fn() callback.
If so, how about passing it directly to attach_fn() function?
typedef struct iommufd_hw_pagetable *(*attach_fn)( struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt, ioasid_t pasid);
In no pasid case, use IOMMU_NO_PASID.
Best regards, baolu
From: Baolu Lu baolu.lu@linux.intel.com Sent: Wednesday, September 27, 2023 10:18 AM
On 9/26/23 5:26 PM, Yi Liu wrote:
-typedef struct iommufd_hw_pagetable *(*attach_fn)(
- struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt);
+static struct iommufd_hw_pagetable *do_attach(struct iommufd_device
*idev,
struct iommufd_hw_pagetable *hwpt, struct attach_data*data)
+{
- return data->attach_fn(idev, hwpt);
+}
I assume that this change was made because we need to pass the pasid value to the attach_fn() callback.
If so, how about passing it directly to attach_fn() function?
typedef struct iommufd_hw_pagetable *(*attach_fn)( struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt, ioasid_t pasid);
In no pasid case, use IOMMU_NO_PASID.
that is one option, but also means the existing device attach_fn() needs to accept an unused pasid parameter which doesn't read good.
So this patch chooses to handle the pasid necessity in an outer wrapper and contain the pasid awareness only in pasid related attach_fn.
From: Kevin Tian kevin.tian@intel.com
This introduces three APIs for device drivers to manage pasid attach/ replace/detach.
int iommufd_device_pasid_attach(struct iommufd_device *idev, u32 pasid, u32 *pt_id); int iommufd_device_pasid_replace(struct iommufd_device *idev, u32 pasid, u32 *pt_id); void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid);
pasid operations have different implications when comparing to device operations:
- No connection to iommufd_group since pasid is a device capability and can be enabled only in singleton group;
- no reserved region per pasid otherwise SVA architecture is already broken (CPU address space doesn't count device reserved regions);
- accordingly no sw_msi trick;
- immediated_attach is not supported, expecting that arm-smmu driver will already remove that requirement before supporting this pasid operation. This avoids unnecessary change in iommufd_hw_pagetable_alloc() to carry the pasid from device.c.
With above differences, this puts all pasid related logics into a new pasid.c file.
Cache coherency enforcement is still applied to pasid operations since it is about memory accesses post page table walking (no matter the walk is per RID or per PASID).
Since the attach is per PASID, this introduces a pasid_hwpts xarray to track the per-pasid attach data.
Signed-off-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommufd/Makefile | 1 + drivers/iommu/iommufd/device.c | 9 +- drivers/iommu/iommufd/iommufd_private.h | 8 ++ drivers/iommu/iommufd/pasid.c | 152 ++++++++++++++++++++++++ include/linux/iommufd.h | 6 + 5 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 drivers/iommu/iommufd/pasid.c
diff --git a/drivers/iommu/iommufd/Makefile b/drivers/iommu/iommufd/Makefile index 8aeba81800c5..be83044a5101 100644 --- a/drivers/iommu/iommufd/Makefile +++ b/drivers/iommu/iommufd/Makefile @@ -6,6 +6,7 @@ iommufd-y := \ ioas.o \ main.o \ pages.o \ + pasid.o \ vfio_compat.o
iommufd-$(CONFIG_IOMMUFD_TEST) += selftest.o diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 4fa4153c5df7..f88f31368eec 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -136,6 +136,7 @@ void iommufd_device_destroy(struct iommufd_object *obj) struct iommufd_device *idev = container_of(obj, struct iommufd_device, obj);
+ WARN_ON(!xa_empty(&idev->pasid_hwpts)); if (idev->has_user_data) dev_iommu_ops(idev->dev)->unset_dev_user_data(idev->dev); iommu_device_release_dma_owner(idev->dev); @@ -218,6 +219,8 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx, /* igroup refcount moves into iommufd_device */ idev->igroup = igroup;
+ xa_init(&idev->pasid_hwpts); + /* * If the caller fails after this success it must call * iommufd_unbind_device() which is safe since we hold this refcount. @@ -600,7 +603,9 @@ iommufd_device_do_replace(struct iommufd_device *idev, static struct iommufd_hw_pagetable *do_attach(struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt, struct attach_data *data) { - return data->attach_fn(idev, hwpt); + return data->pasid == IOMMU_PASID_INVALID ? + data->attach_fn(idev, hwpt) : + data->pasid_attach_fn(idev, data->pasid, hwpt); }
/* @@ -747,6 +752,7 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id) int rc; struct attach_data data = { .attach_fn = &iommufd_device_do_attach, + .pasid = IOMMU_PASID_INVALID };
rc = iommufd_device_change_pt(idev, pt_id, &data); @@ -781,6 +787,7 @@ int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id) { struct attach_data data = { .attach_fn = &iommufd_device_do_replace, + .pasid = IOMMU_PASID_INVALID };
return iommufd_device_change_pt(idev, pt_id, &data); diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index f1fe4120c3b1..97f0e8f28d69 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -336,6 +336,7 @@ struct iommufd_device { struct list_head group_item; /* always the physical device */ struct device *dev; + struct xarray pasid_hwpts; bool enforce_cache_coherency; bool has_user_data; }; @@ -358,9 +359,16 @@ struct attach_data { struct iommufd_hw_pagetable *(*attach_fn)( struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt); + struct iommufd_hw_pagetable *(*pasid_attach_fn)( + struct iommufd_device *idev, u32 pasid, + struct iommufd_hw_pagetable *hwpt); }; + u32 pasid; };
+int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id, + struct attach_data *data); + struct iommufd_access { struct iommufd_object obj; struct iommufd_ctx *ictx; diff --git a/drivers/iommu/iommufd/pasid.c b/drivers/iommu/iommufd/pasid.c new file mode 100644 index 000000000000..bdfdf5b2a48d --- /dev/null +++ b/drivers/iommu/iommufd/pasid.c @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2023, Intel Corporation + */ +#include <linux/iommufd.h> +#include <linux/iommu.h> +#include "../iommu-priv.h" + +#include "iommufd_private.h" + +static int __iommufd_device_pasid_do_attach(struct iommufd_device *idev, + u32 pasid, + struct iommufd_hw_pagetable *hwpt, + bool replace) +{ + int rc; + + /* + * Try to upgrade the domain we have. This is also required for + * pasid attach since pasid only matters for identifying a hwpt + * while cache coherency is about memory access semantics post + * walking hwpt. + * + * Only for kernel-managed hwpt. + */ + if (idev->enforce_cache_coherency && !hwpt->user_managed) { + rc = iommufd_hw_pagetable_enforce_cc(hwpt); + if (rc) + return rc; + } + + if (!replace) + rc = iommu_attach_device_pasid(hwpt->domain, idev->dev, pasid); + else + rc = iommu_replace_device_pasid(hwpt->domain, idev->dev, pasid); + if (rc) + return rc; + + refcount_inc(&hwpt->obj.users); + xa_store(&idev->pasid_hwpts, pasid, hwpt, GFP_KERNEL); + return 0; +} + +static struct iommufd_hw_pagetable * +iommufd_device_pasid_do_attach(struct iommufd_device *idev, u32 pasid, + struct iommufd_hw_pagetable *hwpt) +{ + struct iommufd_hw_pagetable *old_hwpt; + int rc; + + old_hwpt = xa_load(&idev->pasid_hwpts, pasid); + if (old_hwpt) { + /* Attach does not allow overwrite */ + if (old_hwpt == hwpt) + return NULL; + else + return ERR_PTR(-EINVAL); + } + + rc = __iommufd_device_pasid_do_attach(idev, pasid, hwpt, false); + return rc ? ERR_PTR(rc) : NULL; +} + +static struct iommufd_hw_pagetable * +iommufd_device_pasid_do_replace(struct iommufd_device *idev, u32 pasid, + struct iommufd_hw_pagetable *hwpt) +{ + struct iommufd_hw_pagetable *old_hwpt; + int rc; + + old_hwpt = xa_load(&idev->pasid_hwpts, pasid); + if (!old_hwpt) + return ERR_PTR(-EINVAL); + + if (hwpt == old_hwpt) + return NULL; + + rc = __iommufd_device_pasid_do_attach(idev, pasid, hwpt, true); + /* Caller must destroy old_hwpt */ + return rc ? ERR_PTR(rc) : old_hwpt; +} + +/** + * iommufd_device_pasid_attach - Connect a {device, pasid} to an iommu_domain + * @idev: device to attach + * @pasid: pasid to attach + * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE + * Output the IOMMUFD_OBJ_HW_PAGETABLE ID + * + * This connects a pasid of the device to an iommu_domain. Once this + * completes the device could do DMA with the pasid. + * + * This function is undone by calling iommufd_device_detach_pasid(). + */ +int iommufd_device_pasid_attach(struct iommufd_device *idev, + u32 pasid, u32 *pt_id) +{ + struct attach_data data = { + .pasid_attach_fn = &iommufd_device_pasid_do_attach, + .pasid = pasid + }; + + return iommufd_device_change_pt(idev, pt_id, &data); +} +EXPORT_SYMBOL_NS_GPL(iommufd_device_pasid_attach, IOMMUFD); + +/** + * iommufd_device_pasid_replace- Change the {device, pasid}'s iommu_domain + * @idev: device to change + * @pasid: pasid to change + * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE + * Output the IOMMUFD_OBJ_HW_PAGETABLE ID + * + * This is the same as + * iommufd_device_pasid_detach(); + * iommufd_device_pasid_attach(); + * + * If it fails then no change is made to the attachment. The iommu driver may + * implement this so there is no disruption in translation. This can only be + * called if iommufd_device_pasid_attach() has already succeeded. + */ +int iommufd_device_pasid_replace(struct iommufd_device *idev, + u32 pasid, u32 *pt_id) +{ + struct attach_data data = { + .pasid_attach_fn = &iommufd_device_pasid_do_replace, + .pasid = pasid + }; + + return iommufd_device_change_pt(idev, pt_id, &data); +} +EXPORT_SYMBOL_NS_GPL(iommufd_device_pasid_replace, IOMMUFD); + +/** + * iommufd_device_pasid_detach - Disconnect a {device, pasid} to an iommu_domain + * @idev: device to detach + * @pasid: pasid to detach + * + * Undo iommufd_device_pasid_attach(). This disconnects the idev/pasid from + * the previously attached pt_id. + */ +void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid) +{ + struct iommufd_hw_pagetable *hwpt; + + hwpt = xa_load(&idev->pasid_hwpts, pasid); + if (!hwpt) + return; + xa_erase(&idev->pasid_hwpts, pasid); + iommu_detach_device_pasid(hwpt->domain, idev->dev, pasid); + iommufd_hw_pagetable_put(idev->ictx, hwpt); +} +EXPORT_SYMBOL_NS_GPL(iommufd_device_pasid_detach, IOMMUFD); diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index ffc3a949f837..0b007c376306 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -26,6 +26,12 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id); int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id); void iommufd_device_detach(struct iommufd_device *idev);
+int iommufd_device_pasid_attach(struct iommufd_device *idev, + u32 pasid, u32 *pt_id); +int iommufd_device_pasid_replace(struct iommufd_device *idev, + u32 pasid, u32 *pt_id); +void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid); + struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev); u32 iommufd_device_to_id(struct iommufd_device *idev);
On 9/26/23 5:26 PM, Yi Liu wrote:
From: Kevin Tiankevin.tian@intel.com
This introduces three APIs for device drivers to manage pasid attach/ replace/detach.
int iommufd_device_pasid_attach(struct iommufd_device *idev, u32 pasid, u32 *pt_id); int iommufd_device_pasid_replace(struct iommufd_device *idev, u32 pasid, u32 *pt_id); void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid);
I am a bit puzzled. Do we really need both attach and replace interfaces to install a hwpt onto a pasid on device? The IOMMUFD already tracks the connections between hwpt and {device, pasid}, so it could easily call the right iommu interfaces (attach vs. replace). Perhaps I overlooked previous discussion on this.
Best regards, baolu
From: Baolu Lu baolu.lu@linux.intel.com Sent: Wednesday, September 27, 2023 10:49 AM
On 9/26/23 5:26 PM, Yi Liu wrote:
From: Kevin Tiankevin.tian@intel.com
This introduces three APIs for device drivers to manage pasid attach/ replace/detach.
int iommufd_device_pasid_attach(struct iommufd_device *idev, u32 pasid, u32 *pt_id); int iommufd_device_pasid_replace(struct iommufd_device *idev, u32 pasid, u32 *pt_id); void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid);I am a bit puzzled. Do we really need both attach and replace interfaces to install a hwpt onto a pasid on device? The IOMMUFD already tracks the connections between hwpt and {device, pasid}, so it could easily call the right iommu interfaces (attach vs. replace). Perhaps I overlooked previous discussion on this.
attach means a transition from non-present to present.
replace can support changing a present entry atomically if iommu driver support it.
the necessity of supporting both applies to both RID and RID+PASID.
On Wed, Sep 27, 2023 at 10:49:29AM +0800, Baolu Lu wrote:
On 9/26/23 5:26 PM, Yi Liu wrote:
From: Kevin Tiankevin.tian@intel.com
This introduces three APIs for device drivers to manage pasid attach/ replace/detach.
int iommufd_device_pasid_attach(struct iommufd_device *idev, u32 pasid, u32 *pt_id); int iommufd_device_pasid_replace(struct iommufd_device *idev, u32 pasid, u32 *pt_id); void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid);I am a bit puzzled. Do we really need both attach and replace interfaces to install a hwpt onto a pasid on device? The IOMMUFD already tracks the connections between hwpt and {device, pasid}, so it could easily call the right iommu interfaces (attach vs. replace). Perhaps I overlooked previous discussion on this.
It was a decision that attach will fail if something is already attached..
But for this API we could go the way of the iommu code and have only 'set' and 'unset' as the two operations.
Jason
From: Jason Gunthorpe jgg@nvidia.com Sent: Wednesday, September 27, 2023 11:44 PM
On Wed, Sep 27, 2023 at 10:49:29AM +0800, Baolu Lu wrote:
On 9/26/23 5:26 PM, Yi Liu wrote:
From: Kevin Tiankevin.tian@intel.com
This introduces three APIs for device drivers to manage pasid attach/ replace/detach.
int iommufd_device_pasid_attach(struct iommufd_device *idev, u32 pasid, u32 *pt_id); int iommufd_device_pasid_replace(struct iommufd_device *idev, u32 pasid, u32 *pt_id); void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid);I am a bit puzzled. Do we really need both attach and replace interfaces to install a hwpt onto a pasid on device? The IOMMUFD already tracks the connections between hwpt and {device, pasid}, so it could easily call the right iommu interfaces (attach vs. replace). Perhaps I overlooked previous discussion on this.
It was a decision that attach will fail if something is already attached..
But for this API we could go the way of the iommu code and have only 'set' and 'unset' as the two operations.
I'm not sure the benefit of doing so. Instead it makes the caller side vfio more confusing by using attach/replace/detach for device vs. using set/unset for pasid?
The two callbacks are needed to make pasid_attach/detach path complete for mock device. A nop is enough for set_dev_pasid, a domain type check in the remove_dev_pasid is also helpful.
Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommufd/selftest.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c index 117776d236dc..200c88387cd1 100644 --- a/drivers/iommu/iommufd/selftest.c +++ b/drivers/iommu/iommufd/selftest.c @@ -431,6 +431,31 @@ static void mock_domain_unset_dev_user_data(struct device *dev) mdev->dev_data = 0; }
+static void mock_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid) +{ + struct iommu_domain *domain; + + /* Domain type specific cleanup: */ + domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0); + if (domain) { + switch (domain->type) { + case IOMMU_DOMAIN_NESTED: + case IOMMU_DOMAIN_UNMANAGED: + break; + default: + /* should never reach here */ + WARN_ON(1); + break; + } + } +} + +static int mock_domain_set_dev_pasid_nop(struct iommu_domain *domain, + struct device *dev, ioasid_t pasid) +{ + return 0; +} + static const struct iommu_ops mock_ops = { .owner = THIS_MODULE, .pgsize_bitmap = MOCK_IO_PAGE_SIZE, @@ -443,6 +468,7 @@ static const struct iommu_ops mock_ops = { .probe_device = mock_probe_device, .set_dev_user_data = mock_domain_set_dev_user_data, .unset_dev_user_data = mock_domain_unset_dev_user_data, + .remove_dev_pasid = mock_iommu_remove_dev_pasid, .default_domain_ops = &(struct iommu_domain_ops){ .free = mock_domain_free, @@ -450,6 +476,7 @@ static const struct iommu_ops mock_ops = { .map_pages = mock_domain_map_pages, .unmap_pages = mock_domain_unmap_pages, .iova_to_phys = mock_domain_iova_to_phys, + .set_dev_pasid = mock_domain_set_dev_pasid_nop, }, };
@@ -500,6 +527,7 @@ static struct iommu_domain_ops domain_nested_ops = { .free = mock_domain_free, .attach_dev = mock_domain_nop_attach, .cache_invalidate_user = mock_domain_cache_invalidate_user, + .set_dev_pasid = mock_domain_set_dev_pasid_nop, };
static inline struct iommufd_hw_pagetable *
There is need to get the selftest device (sobj->type == TYPE_IDEV) in multiple places, so have a helper to for it.
Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommufd/selftest.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c index 200c88387cd1..1f2ce4ffee4e 100644 --- a/drivers/iommu/iommufd/selftest.c +++ b/drivers/iommu/iommufd/selftest.c @@ -668,29 +668,39 @@ static int iommufd_test_mock_domain(struct iommufd_ucmd *ucmd, return rc; }
-/* Replace the mock domain with a manually allocated hw_pagetable */ -static int iommufd_test_mock_domain_replace(struct iommufd_ucmd *ucmd, - unsigned int device_id, u32 pt_id, - struct iommu_test_cmd *cmd) +static struct selftest_obj * +iommufd_test_get_self_test_device(struct iommufd_ctx *ictx, u32 id) { struct iommufd_object *dev_obj; struct selftest_obj *sobj; - int rc;
/* * Prefer to use the OBJ_SELFTEST because the destroy_rwsem will ensure * it doesn't race with detach, which is not allowed. */ - dev_obj = - iommufd_get_object(ucmd->ictx, device_id, IOMMUFD_OBJ_SELFTEST); + dev_obj = iommufd_get_object(ictx, id, IOMMUFD_OBJ_SELFTEST); if (IS_ERR(dev_obj)) - return PTR_ERR(dev_obj); + return (struct selftest_obj *)dev_obj;
sobj = container_of(dev_obj, struct selftest_obj, obj); if (sobj->type != TYPE_IDEV) { - rc = -EINVAL; - goto out_dev_obj; + iommufd_put_object(dev_obj); + return ERR_PTR(-EINVAL); } + return sobj; +} + +/* Replace the mock domain with a manually allocated hw_pagetable */ +static int iommufd_test_mock_domain_replace(struct iommufd_ucmd *ucmd, + unsigned int device_id, u32 pt_id, + struct iommu_test_cmd *cmd) +{ + struct selftest_obj *sobj; + int rc; + + sobj = iommufd_test_get_self_test_device(ucmd->ictx, device_id); + if (IS_ERR(sobj)) + return PTR_ERR(sobj);
rc = iommufd_device_replace(sobj->idev.idev, &pt_id); if (rc) @@ -700,7 +710,7 @@ static int iommufd_test_mock_domain_replace(struct iommufd_ucmd *ucmd, rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
out_dev_obj: - iommufd_put_object(dev_obj); + iommufd_put_object(&sobj->obj); return rc; }
This adds 4 test ops for pasid attach/replace/detach testing. There are ops to attach/detach pasid, and also op to check the attached domain of a pasid.
Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommufd/iommufd_test.h | 24 +++++++ drivers/iommu/iommufd/selftest.c | 98 ++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+)
diff --git a/drivers/iommu/iommufd/iommufd_test.h b/drivers/iommu/iommufd/iommufd_test.h index 65a363f5e81e..e2ab61251ef9 100644 --- a/drivers/iommu/iommufd/iommufd_test.h +++ b/drivers/iommu/iommufd/iommufd_test.h @@ -21,6 +21,10 @@ enum { IOMMU_TEST_OP_ACCESS_REPLACE_IOAS, IOMMU_TEST_OP_MD_CHECK_IOTLB, IOMMU_TEST_OP_DEV_CHECK_DATA, + IOMMU_TEST_OP_PASID_ATTACH, + IOMMU_TEST_OP_PASID_REPLACE, + IOMMU_TEST_OP_PASID_DETACH, + IOMMU_TEST_OP_PASID_CHECK_DOMAIN, };
enum { @@ -109,6 +113,26 @@ struct iommu_test_cmd { struct { __u32 val; } check_dev_data; + struct { + __u32 pasid; + __u32 pt_id; + /* @id is stdev_id for IOMMU_TEST_OP_PASID_ATTACH */ + } pasid_attach; + struct { + __u32 pasid; + __u32 pt_id; + /* @id is stdev_id for IOMMU_TEST_OP_PASID_ATTACH */ + } pasid_replace; + struct { + __u32 pasid; + /* @id is stdev_id for IOMMU_TEST_OP_PASID_DETACH */ + } pasid_detach; + struct { + __u32 pasid; + __u32 hwpt_id; + __u64 out_result_ptr; + /* @id is stdev_id for IOMMU_TEST_OP_HWPT_GET_DOMAIN */ + } pasid_check; }; __u32 last; }; diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c index 1f2ce4ffee4e..bfc81615e9f9 100644 --- a/drivers/iommu/iommufd/selftest.c +++ b/drivers/iommu/iommufd/selftest.c @@ -1238,6 +1238,96 @@ static_assert((unsigned int)MOCK_ACCESS_RW_WRITE == IOMMUFD_ACCESS_RW_WRITE); static_assert((unsigned int)MOCK_ACCESS_RW_SLOW_PATH == __IOMMUFD_ACCESS_RW_SLOW_PATH);
+static int iommufd_test_pasid_attach(struct iommufd_ucmd *ucmd, + struct iommu_test_cmd *cmd) +{ + struct selftest_obj *sobj; + int rc; + + sobj = iommufd_test_get_self_test_device(ucmd->ictx, cmd->id); + if (IS_ERR(sobj)) + return PTR_ERR(sobj); + + rc = iommufd_device_pasid_attach(sobj->idev.idev, + cmd->pasid_attach.pasid, + &cmd->pasid_attach.pt_id); + iommufd_put_object(&sobj->obj); + return rc; +} + +static int iommufd_test_pasid_replace(struct iommufd_ucmd *ucmd, + struct iommu_test_cmd *cmd) +{ + struct selftest_obj *sobj; + int rc; + + sobj = iommufd_test_get_self_test_device(ucmd->ictx, cmd->id); + if (IS_ERR(sobj)) + return PTR_ERR(sobj); + + rc = iommufd_device_pasid_replace(sobj->idev.idev, + cmd->pasid_attach.pasid, + &cmd->pasid_attach.pt_id); + iommufd_put_object(&sobj->obj); + return rc; +} + +static int iommufd_test_pasid_detach(struct iommufd_ucmd *ucmd, + struct iommu_test_cmd *cmd) +{ + struct selftest_obj *sobj; + + sobj = iommufd_test_get_self_test_device(ucmd->ictx, cmd->id); + if (IS_ERR(sobj)) + return PTR_ERR(sobj); + + iommufd_device_pasid_detach(sobj->idev.idev, + cmd->pasid_detach.pasid); + iommufd_put_object(&sobj->obj); + return 0; +} + +static int iommufd_test_pasid_check_domain(struct iommufd_ucmd *ucmd, + struct iommu_test_cmd *cmd) +{ + struct iommu_domain *attached_domain, *expect_domain = NULL; + struct iommufd_hw_pagetable *hwpt = NULL; + struct selftest_obj *sobj; + struct mock_dev *mdev; + bool result; + int rc = 0; + + sobj = iommufd_test_get_self_test_device(ucmd->ictx, cmd->id); + if (IS_ERR(sobj)) + return PTR_ERR(sobj); + + mdev = sobj->idev.mock_dev; + + attached_domain = iommu_get_domain_for_dev_pasid(&mdev->dev, + cmd->pasid_check.pasid, 0); + if (IS_ERR(attached_domain)) + attached_domain = NULL; + + if (cmd->pasid_check.hwpt_id) { + hwpt = iommufd_get_hwpt(ucmd, cmd->pasid_check.hwpt_id); + if (IS_ERR(hwpt)) { + rc = PTR_ERR(hwpt); + goto out_put_dev; + } + expect_domain = hwpt->domain; + } + + result = (attached_domain == expect_domain) ? 1 : 0; + if (copy_to_user(u64_to_user_ptr(cmd->pasid_check.out_result_ptr), + &result, sizeof(result))) + rc = -EFAULT; + if (hwpt) + iommufd_put_object(&hwpt->obj); +out_put_dev: + iommufd_put_object(&sobj->obj); + return rc; +} + void iommufd_selftest_destroy(struct iommufd_object *obj) { struct selftest_obj *sobj = container_of(obj, struct selftest_obj, obj); @@ -1309,6 +1399,14 @@ int iommufd_test(struct iommufd_ucmd *ucmd) return -EINVAL; iommufd_test_memory_limit = cmd->memory_limit.limit; return 0; + case IOMMU_TEST_OP_PASID_ATTACH: + return iommufd_test_pasid_attach(ucmd, cmd); + case IOMMU_TEST_OP_PASID_REPLACE: + return iommufd_test_pasid_replace(ucmd, cmd); + case IOMMU_TEST_OP_PASID_DETACH: + return iommufd_test_pasid_detach(ucmd, cmd); + case IOMMU_TEST_OP_PASID_CHECK_DOMAIN: + return iommufd_test_pasid_check_domain(ucmd, cmd); default: return -EOPNOTSUPP; }
This tests iommufd pasid attach/replace/detach.
Signed-off-by: Yi Liu yi.l.liu@intel.com --- tools/testing/selftests/iommu/iommufd.c | 172 ++++++++++++++++++ .../selftests/iommu/iommufd_fail_nth.c | 28 ++- tools/testing/selftests/iommu/iommufd_utils.h | 78 ++++++++ 3 files changed, 274 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c index 7cf06a4635d8..92e20a18bbea 100644 --- a/tools/testing/selftests/iommu/iommufd.c +++ b/tools/testing/selftests/iommu/iommufd.c @@ -2022,4 +2022,176 @@ TEST_F(vfio_compat_mock_domain, huge_map) } }
+FIXTURE(iommufd_device_pasid) +{ + int fd; + uint32_t ioas_id; + uint32_t hwpt_id; + uint32_t stdev_id; + uint32_t device_id; +}; + +FIXTURE_SETUP(iommufd_device_pasid) +{ + self->fd = open("/dev/iommu", O_RDWR); + ASSERT_NE(-1, self->fd); + test_ioctl_ioas_alloc(&self->ioas_id); + + test_cmd_mock_domain(self->ioas_id, &self->stdev_id, + &self->hwpt_id, &self->device_id); +} + +FIXTURE_TEARDOWN(iommufd_device_pasid) +{ + teardown_iommufd(self->fd, _metadata); +} + +TEST_F(iommufd_device_pasid, pasid_attach) +{ + if (self->device_id) { + struct iommu_hwpt_selftest data = { + .iotlb = IOMMU_TEST_IOTLB_DEFAULT, + }; + uint32_t nested_hwpt_id[2] = {}; + uint32_t parent_hwpt_id = 0; + uint32_t pasid = 100; + bool result; + + /* Allocate two nested hwpts sharing one common parent hwpt */ + test_cmd_hwpt_alloc(self->device_id, self->ioas_id, + IOMMU_HWPT_ALLOC_NEST_PARENT, + &parent_hwpt_id); + + test_cmd_hwpt_alloc_nested(self->device_id, parent_hwpt_id, 0, + &nested_hwpt_id[0], + IOMMU_HWPT_TYPE_SELFTEST, + &data, sizeof(data)); + test_cmd_hwpt_alloc_nested(self->device_id, parent_hwpt_id, 0, + &nested_hwpt_id[1], + IOMMU_HWPT_TYPE_SELFTEST, + &data, sizeof(data)); + + /* + * Attach ioas to pasid 100, should succeed, domain should + * be valid. + */ + test_cmd_pasid_attach(pasid, self->ioas_id); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, self->hwpt_id, + &result)); + EXPECT_EQ(1, result); + + /* + * Try attach pasid 100 with self->ioas_id, should succeed + * as it is the same with existing hwpt. + */ + test_cmd_pasid_attach(pasid, self->ioas_id); + + /* + * Try attach pasid 100 with another hwpt, should FAIL + * as attach does not allow overwrite, use REPLACE instead. + */ + test_err_cmd_pasid_attach(EINVAL, pasid, nested_hwpt_id[0]); + + /* + * Detach hwpt from pasid 100, and check if the pasid 100 + * has null domain. Should be done before the next attach. + */ + test_cmd_pasid_detach(pasid); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, 0, &result)); + EXPECT_EQ(1, result); + + /* + * Attach nested hwpt to pasid 100, should succeed, domain + * should be valid. + */ + test_cmd_pasid_attach(pasid, nested_hwpt_id[0]); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, nested_hwpt_id[0], + &result)); + EXPECT_EQ(1, result); + + /* + * Detach hwpt from pasid 100, and check if the pasid 100 + * has null domain + */ + test_cmd_pasid_detach(pasid); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, 0, &result)); + EXPECT_EQ(1, result); + + /* Replace tests */ + pasid = 200; + + /* + * Replace pasid 200 without attaching it first, should + * fail with -EINVAL. + */ + test_err_cmd_pasid_replace(EINVAL, pasid, parent_hwpt_id); + + /* + * Attach a s2 hwpt to pasid 200, should succeed, domain should + * be valid. + */ + test_cmd_pasid_attach(pasid, parent_hwpt_id); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, parent_hwpt_id, + &result)); + EXPECT_EQ(1, result); + + /* + * Replace pasid 200 with self->ioas_id, should succeed, + * and have valid domain. + */ + test_cmd_pasid_replace(pasid, self->ioas_id); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, self->hwpt_id, + &result)); + EXPECT_EQ(1, result); + + /* + * Replace a nested hwpt for pasid 200, should succeed, + * and have valid domain. + */ + test_cmd_pasid_replace(pasid, nested_hwpt_id[0]); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, nested_hwpt_id[0], + &result)); + EXPECT_EQ(1, result); + + /* + * Replace with another nested hwpt for pasid 200, should + * succeed, and have valid domain. + */ + test_cmd_pasid_replace(pasid, nested_hwpt_id[1]); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, nested_hwpt_id[1], + &result)); + EXPECT_EQ(1, result); + + /* + * Detach hwpt from pasid 200, and check if the pasid 200 + * has null domain. + */ + test_cmd_pasid_detach(pasid); + ASSERT_EQ(0, + test_cmd_pasid_check_domain(self->fd, self->stdev_id, + pasid, 0, &result)); + EXPECT_EQ(1, result); + + test_ioctl_destroy(nested_hwpt_id[0]); + test_ioctl_destroy(nested_hwpt_id[1]); + test_ioctl_destroy(parent_hwpt_id); + } +} + TEST_HARNESS_MAIN diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c index d3f47f262c04..797f2b3103fc 100644 --- a/tools/testing/selftests/iommu/iommufd_fail_nth.c +++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c @@ -206,12 +206,16 @@ FIXTURE(basic_fail_nth) { int fd; uint32_t access_id; + uint32_t stdev_id; + uint32_t pasid; };
FIXTURE_SETUP(basic_fail_nth) { self->fd = -1; self->access_id = 0; + self->stdev_id = 0; + self->pasid = 0; //test should use a non-zero value }
FIXTURE_TEARDOWN(basic_fail_nth) @@ -223,6 +227,8 @@ FIXTURE_TEARDOWN(basic_fail_nth) rc = _test_cmd_destroy_access(self->access_id); assert(rc == 0); } + if (self->pasid && self->stdev_id) + _test_cmd_pasid_detach(self->fd, self->stdev_id, self->pasid); teardown_iommufd(self->fd, _metadata); }
@@ -579,7 +585,6 @@ TEST_FAIL_NTH(basic_fail_nth, device) struct iommu_test_hw_info info; uint32_t ioas_id; uint32_t ioas_id2; - uint32_t stdev_id; uint32_t idev_id; uint32_t hwpt_id; __u64 iova; @@ -608,7 +613,7 @@ TEST_FAIL_NTH(basic_fail_nth, device)
fail_nth_enable();
- if (_test_cmd_mock_domain(self->fd, ioas_id, &stdev_id, NULL, + if (_test_cmd_mock_domain(self->fd, ioas_id, &self->stdev_id, NULL, &idev_id)) return -1;
@@ -619,11 +624,26 @@ TEST_FAIL_NTH(basic_fail_nth, device) IOMMU_HWPT_TYPE_DEFAULT, 0, 0)) return -1;
- if (_test_cmd_mock_domain_replace(self->fd, stdev_id, ioas_id2, NULL)) + if (_test_cmd_mock_domain_replace(self->fd, self->stdev_id, ioas_id2, NULL)) + return -1; + + if (_test_cmd_mock_domain_replace(self->fd, self->stdev_id, hwpt_id, NULL)) return -1;
- if (_test_cmd_mock_domain_replace(self->fd, stdev_id, hwpt_id, NULL)) + self->pasid = 200; + + /* Tests for pasid attach/replace/detach */ + if (_test_cmd_pasid_attach(self->fd, self->stdev_id, self->pasid, ioas_id)) return -1; + + if (_test_cmd_pasid_replace(self->fd, self->stdev_id, self->pasid, ioas_id2)) + return -1; + + if (_test_cmd_pasid_detach(self->fd, self->stdev_id, self->pasid)) + return -1; + + self->pasid = 0; + return 0; }
diff --git a/tools/testing/selftests/iommu/iommufd_utils.h b/tools/testing/selftests/iommu/iommufd_utils.h index b75f168fca46..40d954a4cbc1 100644 --- a/tools/testing/selftests/iommu/iommufd_utils.h +++ b/tools/testing/selftests/iommu/iommufd_utils.h @@ -551,3 +551,81 @@ static int _test_cmd_unset_dev_data(int fd, __u32 device_id) #define test_err_unset_dev_data(_errno, device_id) \ EXPECT_ERRNO(_errno, \ _test_cmd_unset_dev_data(self->fd, device_id)) + +static int _test_cmd_pasid_attach(int fd, __u32 stdev_id, __u32 pasid, __u32 pt_id) +{ + struct iommu_test_cmd test_attach = { + .size = sizeof(test_attach), + .op = IOMMU_TEST_OP_PASID_ATTACH, + .id = stdev_id, + .pasid_attach = { + .pasid = pasid, + .pt_id = pt_id, + }, + }; + + return ioctl(fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_PASID_ATTACH), &test_attach); +} + +#define test_cmd_pasid_attach(pasid, hwpt_id) \ + ASSERT_EQ(0, _test_cmd_pasid_attach(self->fd, self->stdev_id, pasid, hwpt_id)) + +#define test_err_cmd_pasid_attach(_errno, pasid, hwpt_id) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_pasid_attach(self->fd, self->stdev_id, pasid, hwpt_id)) + +static int _test_cmd_pasid_replace(int fd, __u32 stdev_id, __u32 pasid, __u32 pt_id) +{ + struct iommu_test_cmd test_replace = { + .size = sizeof(test_replace), + .op = IOMMU_TEST_OP_PASID_REPLACE, + .id = stdev_id, + .pasid_replace = { + .pasid = pasid, + .pt_id = pt_id, + }, + }; + + return ioctl(fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_PASID_REPLACE), &test_replace); +} + +#define test_cmd_pasid_replace(pasid, hwpt_id) \ + ASSERT_EQ(0, _test_cmd_pasid_replace(self->fd, self->stdev_id, pasid, hwpt_id)) + +#define test_err_cmd_pasid_replace(_errno, pasid, hwpt_id) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_pasid_replace(self->fd, self->stdev_id, pasid, hwpt_id)) + +static int _test_cmd_pasid_detach(int fd, __u32 stdev_id, __u32 pasid) +{ + struct iommu_test_cmd test_detach = { + .size = sizeof(test_detach), + .op = IOMMU_TEST_OP_PASID_DETACH, + .id = stdev_id, + .pasid_detach = { + .pasid = pasid, + }, + }; + + return ioctl(fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_PASID_DETACH), &test_detach); +} + +#define test_cmd_pasid_detach(pasid) \ + ASSERT_EQ(0, _test_cmd_pasid_detach(self->fd, self->stdev_id, pasid)) + +static int test_cmd_pasid_check_domain(int fd, __u32 stdev_id, __u32 pasid, + __u32 hwpt_id, bool *result) +{ + struct iommu_test_cmd test_pasid_check = { + .size = sizeof(test_pasid_check), + .op = IOMMU_TEST_OP_PASID_CHECK_DOMAIN, + .id = stdev_id, + .pasid_check = { + .pasid = pasid, + .hwpt_id = hwpt_id, + .out_result_ptr = (__u64)result, + }, + }; + + return ioctl(fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_PASID_CHECK_DOMAIN), &test_pasid_check); +}
From: Lu Baolu baolu.lu@linux.intel.com
This allows the upper layers to set a nested type domain to a PASID of a device if the PASID feature is supported by the IOMMU hardware.
Signed-off-by: Lu Baolu baolu.lu@linux.intel.com Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c index 4853fee216d9..dc99ea1abf68 100644 --- a/drivers/iommu/intel/nested.c +++ b/drivers/iommu/intel/nested.c @@ -63,6 +63,52 @@ static int intel_nested_attach_dev(struct iommu_domain *domain, return 0; }
+static int intel_nested_set_dev_pasid(struct iommu_domain *domain, + struct device *dev, ioasid_t pasid) +{ + struct device_domain_info *info = dev_iommu_priv_get(dev); + struct dmar_domain *dmar_domain = to_dmar_domain(domain); + struct intel_iommu *iommu = info->iommu; + struct dev_pasid_info *dev_pasid; + unsigned long flags; + int ret = 0; + + if (!pasid_supported(iommu)) + return -EOPNOTSUPP; + + if (iommu->agaw < dmar_domain->s2_domain->agaw) + return -EINVAL; + + ret = prepare_domain_attach_device(&dmar_domain->s2_domain->domain, dev); + if (ret) + return ret; + + dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL); + if (!dev_pasid) + return -ENOMEM; + + ret = domain_attach_iommu(dmar_domain, iommu); + if (ret) + goto err_free; + + ret = intel_pasid_setup_nested(iommu, dev, pasid, dmar_domain); + if (ret) + goto err_detach_iommu; + + dev_pasid->dev = dev; + dev_pasid->pasid = pasid; + spin_lock_irqsave(&dmar_domain->lock, flags); + list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids); + spin_unlock_irqrestore(&dmar_domain->lock, flags); + + return 0; +err_detach_iommu: + domain_detach_iommu(dmar_domain, iommu); +err_free: + kfree(dev_pasid); + return ret; +} + static void intel_nested_domain_free(struct iommu_domain *domain) { kfree(to_dmar_domain(domain)); @@ -127,6 +173,7 @@ static int intel_nested_cache_invalidate_user(struct iommu_domain *domain,
static const struct iommu_domain_ops intel_nested_domain_ops = { .attach_dev = intel_nested_attach_dev, + .set_dev_pasid = intel_nested_set_dev_pasid, .free = intel_nested_domain_free, .cache_invalidate_user = intel_nested_cache_invalidate_user, };
From: Liu, Yi L yi.l.liu@intel.com Sent: Tuesday, September 26, 2023 5:27 PM
From: Lu Baolu baolu.lu@linux.intel.com
This allows the upper layers to set a nested type domain to a PASID of a device if the PASID feature is supported by the IOMMU hardware.
Signed-off-by: Lu Baolu baolu.lu@linux.intel.com Signed-off-by: Yi Liu yi.l.liu@intel.com
drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
pasid can be attached to either user hwpt or kernel hwpt.
We should also introduce a set_dev_pasid callback for non-nest domain ops.
On 2023/9/27 15:52, Tian, Kevin wrote:
From: Liu, Yi Lyi.l.liu@intel.com Sent: Tuesday, September 26, 2023 5:27 PM
From: Lu Baolubaolu.lu@linux.intel.com
This allows the upper layers to set a nested type domain to a PASID of a device if the PASID feature is supported by the IOMMU hardware.
Signed-off-by: Lu Baolubaolu.lu@linux.intel.com Signed-off-by: Yi Liuyi.l.liu@intel.com
drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
pasid can be attached to either user hwpt or kernel hwpt.
We should also introduce a set_dev_pasid callback for non-nest domain ops.
We already have the code in Linus' tree. The idxd driver uses it for kernel DMA with pasid.
Best regards, baolu
From: Baolu Lu baolu.lu@linux.intel.com Sent: Wednesday, September 27, 2023 3:58 PM
On 2023/9/27 15:52, Tian, Kevin wrote:
From: Liu, Yi Lyi.l.liu@intel.com Sent: Tuesday, September 26, 2023 5:27 PM
From: Lu Baolubaolu.lu@linux.intel.com
This allows the upper layers to set a nested type domain to a PASID of a device if the PASID feature is supported by the IOMMU hardware.
Signed-off-by: Lu Baolubaolu.lu@linux.intel.com Signed-off-by: Yi Liuyi.l.liu@intel.com
drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
pasid can be attached to either user hwpt or kernel hwpt.
We should also introduce a set_dev_pasid callback for non-nest domain
ops.
We already have the code in Linus' tree. The idxd driver uses it for kernel DMA with pasid.
ah, I forgot. 😊 probably can mention it in the commit message.
On 2023/9/27 16:09, Tian, Kevin wrote:
From: Baolu Lu baolu.lu@linux.intel.com Sent: Wednesday, September 27, 2023 3:58 PM
On 2023/9/27 15:52, Tian, Kevin wrote:
From: Liu, Yi Lyi.l.liu@intel.com Sent: Tuesday, September 26, 2023 5:27 PM
From: Lu Baolubaolu.lu@linux.intel.com
This allows the upper layers to set a nested type domain to a PASID of a device if the PASID feature is supported by the IOMMU hardware.
Signed-off-by: Lu Baolubaolu.lu@linux.intel.com Signed-off-by: Yi Liuyi.l.liu@intel.com
drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
pasid can be attached to either user hwpt or kernel hwpt.
We should also introduce a set_dev_pasid callback for non-nest domain
ops.
We already have the code in Linus' tree. The idxd driver uses it for kernel DMA with pasid.
ah, I forgot. 😊 probably can mention it in the commit message.
sure.
linux-kselftest-mirror@lists.linaro.org
-
Baolu Lu -
Jason Gunthorpe -
Tian, Kevin -
Yi Liu