* [PATCH v9 00/21] iommufd support pasid attach/replace
@ 2025-03-13 12:35 Yi Liu
2025-03-13 12:35 ` [PATCH v9 01/21] iommu: Clear handle->domain in detach Yi Liu
` (21 more replies)
0 siblings, 22 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
PASID (Process Address Space ID) is a PCIe extension that tags the DMA
transactions from a physical device. Most modern IOMMU hardware supports
PASID-granular address translation. This allows a PASID-capable device
to be attached to multiple hardware page tables (hwpts, also known as
domains), with each attachment tagged by a PASID.
This series builds on previous series [1]. It begins by adding a missing
IOMMU API to replace the domain for a PASID. Utilizing the IOMMU PASID
attach/replace/detach APIs, this series introduces iommufd APIs for device
drivers to attach, replace, or detach PASIDs to/from hwpts at the request
of userspace. It also enforces PASID compatibility with domain requirements,
allocates PASID-compatible hwpts in iommufd, and includes self-tests to
validate the iommufd APIs.
The complete code is available at the following link [2]. Please note that
the existing iommufd self-test was broken, and a temporary fix patch is at
the top of the branch [2]. If you wish to run the iommufd self-test, please
apply that fix. We apologize for any inconvenience.
The series is based on Jason's for-next branch plus one more patch [3].
https://web.git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?h=for-next&id=a05df03a88bc1088be8e9d958f208d6484691e43
[1] https://lore.kernel.org/linux-iommu/20250226011849.5102-1-yi.l.liu@intel.com/
[2] https://github.com/yiliu1765/iommufd/tree/iommufd_pasid
[3] https://lore.kernel.org/linux-iommu/20250306034842.5950-1-yi.l.liu@intel.com/
Change log:
v9:
- Drop patch 01 of v8 (Nic)
- Add patch 01 and 02 to support reusing iommufd_attach_handle
- Pass @pasid to iommufd_hwpt_paging_alloc() instead of passing IOMMU_NO_PASID
when calling iommufd_hw_pagetable_attach()
- Consolidate the RID and PASID path. The RID and PASID path shares almost all
the path from driver facing API to the place invoking underlying iommu driver
callback. (Jason)
*) Add patch 06, 07, 08, 09, 10 and 11 to use iommufd_attach_handle track
attached hwpt and devices of a group.
*) Patch 11 adds pasid_attach array to track iommufd_attach_handles used in
the RID path and PASID path.
*) Extend the existing attach/detach/replace API to support a pasid parameter
to support PASID.
- Rename "PATCH v8 05/12] iommufd: Mark PASID-compatible domain" to be
"[PATCH v9 13/21] iommufd: Enforce PASID-compatible domain in PASID path"
as this patch now adds pasid_compat flag and its check followed by Jason's
remark.
- Add description that the user that wants to use PASID should first attach
its RID path to pasid-compatible domain.
- Add back patch 11 0f v7 as it not included in any other series now
- Drop the mixed_replace tests of v8 as it is not considered to be iommufd
selftest
- Add r-b tags got in v8. For the patches that have minor changes, I kept the
r-b tags with a "v8 - v9" change log in individual patches. For the patches
that have non-trivial change, I dropped the r-b tags to get more attention
from reviewers.
v8: https://lore.kernel.org/linux-iommu/20250226114032.4591-1-yi.l.liu@intel.com/
- Rebase on top of the dependency patch series
- Check both handle and domain in the iommu_replace_device_pasid_handle()
to support replace between domain and handle.
- Fix a typo in patch 02 of v7 (Kevin)
- r-b tag on patch 01 of v7 (Kevin)
- Add selftest for replace between handle and non-handle case
v7: https://lore.kernel.org/linux-iommu/20250216035228.23831-1-yi.l.liu@intel.com/
- Remove the iommu_attach_handle related refactors, as they have been addressed
by Nic's series.
- Address the comments on patch 01 of v6 in a separate series [1]. Store either
the domain or handle in group->pasid_array, and swap the order of setting
group->pasid_array and invoking the attach operation of IOMMU drivers.
- Introduce iommu_attach_device_pasid_handle() and iommu_replace_device_pasid_handle(),
and remove iommu_replace_device_pasid() since iommufd consistently uses the
_handle() API.
- Add patch 04 to include reserved_iova only for the RID path, as the underlying
helpers are shared by both the RID and PASID paths, but only the RID path needs
to add reserved_iova.
- Remove the iommu_dev.max_pasids check in patch 11 of v6. The mock IOMMU always
supports 20-bit PASIDs, so there is no need to verify PASID support in the mock
IOMMU driver. Additionally, the IOMMU_HWPT_ALLOC_PASID flag does not imply PASID
support, so it should be removed to avoid misleading IOMMU driver programming.
v6: https://lore.kernel.org/linux-iommu/20241219132746.16193-1-yi.l.liu@intel.com/
- Add kdoc to iommufd_device_get_attach_handle() to note the returned handle
should be used with care. (Baolu)
- Reworked the patch 07 and 08 of v5 to avoid domain allocation failure on VT-d
after applying patch 07 of v5.
1) Split out the intel iommu driver IOMMU_HWPT_ALLOC_PASID support out of
patch 08
2) Rework the PASID-compatible domain enforcement by checking the RID domain
and idev->pasid_hwpts under the idev->igroup->lock.
- iommufd_device_pasid_do_attach() returns -EINVAL if there is old hwpt and it's
not the same with new hwpt. This aligns with how the iommufd_device_do_attach()
deals it. Otherwise, attaching the same pasid to the same ioas is going to fail
before the auto_domain loop goes to the correct hwpt. Thsi is not reasonable. So
make this change.
- Enhanced the pasid selftest to have non-pasid-capable device and pasid-capable
device.
- The order of the series is tweaked to be prepare the iommufd for pasid attach,
add pasid attach, add PASID-compat domain enforcement and then add the PASID-compat
hwpt allocation.
- Rebased on top of 6.13-rc3 and some already applied patches.
v5: https://lore.kernel.org/linux-iommu/20241104132513.15890-1-yi.l.liu@intel.com/
- Fix a mistake in patch 02 of v4 (Kevin)
- Move the iommufd_handle helpers to device.c
- Add IOMMU_HWPT_ALLOC_PASID check to enforce pasid-compatible domain for pasid
capable device in iommufd
- Update the iommufd selftest to use IOMMU_HWPT_ALLOC_PASID
v4: https://lore.kernel.org/linux-iommu/20240912131255.13305-1-yi.l.liu@intel.com/
- Replace remove_dev_pasid() by supporting set_dev_pasid() for blocking domain (Kevin)
- This is done by the preparation series "Support attaching PASID to the blocked_domain"
- Misc tweaks to foil the merging of the iommufd iopf series. Three new patches are added:
- iommufd: Always pass iommu_attach_handle to iommu core
- iommufd: Move the iommufd_handle helpers to iommufd_private.h
- iommufd: Refactor __fault_domain_replace_dev() to be a wrapper of iommu_replace_group_handle()
- Renmae patch 03 of v3 to be "iommufd: Support pasid attach/replace"
- Add test case for attaching/replacing iopf-capable hwpt to pasid
v3: https://lore.kernel.org/kvm/20240628090557.50898-1-yi.l.liu@intel.com/
- Split the set_dev_pasid op enhancements for domain replacement to be a
separate series "Make set_dev_pasid op supportting domain replacement" [1].
The below changes are made in the separate series.
*) set_dev_pasid() callback should keep the old config if failed to attach to
a domain. This simplifies the caller a lot as caller does not need to attach
it back to old domain explicitly. This also avoids some corner cases in which
the core may do duplicated domain attachment as described in below link (Jason)
https://lore.kernel.org/linux-iommu/BN9PR11MB52768C98314A95AFCD2FA6478C0F2@BN9PR11MB5276.namprd11.prod.outlook.com/
*) Drop patch 10 of v2 as it's a bug fix and can be submitted separately (Kevin)
*) Rebase on top of Baolu's domain_alloc_paging refactor series (Jason)
- Drop the attach_data which includes attach_fn and pasid, insteadly passing the
pasid through the device attach path. (Jason)
- Add a pasid-num-bits property to mock dev to make pasid selftest work (Kevin)
v2: https://lore.kernel.org/linux-iommu/20240412081516.31168-1-yi.l.liu@intel.com/
- Domain replace for pasid should be handled in set_dev_pasid() callbacks
instead of remove_dev_pasid and call set_dev_pasid afteward in iommu
layer (Jason)
- Make xarray operations more self-contained in iommufd pasid attach/replace/detach
(Jason)
- Tweak the dev_iommu_get_max_pasids() to allow iommu driver to populate the
max_pasids. This makes the iommufd selftest simpler to meet the max_pasids
check in iommu_attach_device_pasid() (Jason)
v1: https://lore.kernel.org/kvm/20231127063428.127436-1-yi.l.liu@intel.com/#r
- Implemnet iommu_replace_device_pasid() to fall back to the original domain
if this replacement failed (Kevin)
- Add check in do_attach() to check corressponding attach_fn per the pasid value.
rfc: https://lore.kernel.org/linux-iommu/20230926092651.17041-1-yi.l.liu@intel.com/
Regards,
Yi Liu
Yi Liu (21):
iommu: Clear handle->domain in detach
iommu: Wrap pasid_array entry creation and setting
iommu: Introduce a replace API for device pasid
iommufd: Pass @pasid through the device attach/replace path
iommufd/device: Only add reserved_iova in non-pasid path
iommufd/device: Replace idev->igroup with local variable
iommufd/device: Check !igroup->hwpt in
iommufd_device_attach_reserved_iova()
iommufd/device: Lift iommufd_attach_handle handling to upper level
helpers
iommufd/device: Use iommufd_attach_handle track attachment
iommufd/device: Replace device_list with device_array
iommufd/device: Move attached device tracking to handle
iommufd/device: Add pasid_attach array to track per-PASID attach
iommufd: Enforce PASID-compatible domain in PASID path
iommufd: Support pasid attach/replace
iommufd: Enforce PASID-compatible domain for RID
iommu/vt-d: Add IOMMU_HWPT_ALLOC_PASID support
iommufd: Allow allocating PASID-compatible domain
iommufd/selftest: Add set_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/iommu.c | 3 +-
drivers/iommu/intel/nested.c | 2 +-
drivers/iommu/iommu-priv.h | 4 +
drivers/iommu/iommu.c | 153 +++++++-
drivers/iommu/iommufd/device.c | 364 ++++++++++++------
drivers/iommu/iommufd/hw_pagetable.c | 23 +-
drivers/iommu/iommufd/iommufd_private.h | 14 +-
drivers/iommu/iommufd/iommufd_test.h | 32 ++
drivers/iommu/iommufd/selftest.c | 228 ++++++++++-
drivers/vfio/iommufd.c | 10 +-
include/linux/iommufd.h | 9 +-
include/uapi/linux/iommufd.h | 3 +
tools/testing/selftests/iommu/iommufd.c | 344 +++++++++++++++++
.../selftests/iommu/iommufd_fail_nth.c | 41 +-
tools/testing/selftests/iommu/iommufd_utils.h | 102 +++++
15 files changed, 1136 insertions(+), 196 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v9 01/21] iommu: Clear handle->domain in detach
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 11:46 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting Yi Liu
` (20 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
The iommu core sets the handle->domain in attach/replace. To be completed,
it should clear handle->domain in detach as well to avoid potential side
effect if caller reuses a handle.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommu.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0ee17893810f..332ecb20c385 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2161,6 +2161,13 @@ static void *iommu_make_pasid_array_entry(struct iommu_domain *domain,
return xa_tag_pointer(domain, IOMMU_PASID_ARRAY_DOMAIN);
}
+static void iommu_pasid_array_entry_clear_handle(void *entry)
+{
+ if (xa_pointer_tag(entry) == IOMMU_PASID_ARRAY_HANDLE)
+ ((struct iommu_attach_handle *)
+ xa_untag_pointer(entry))->domain = NULL;
+}
+
static int __iommu_attach_group(struct iommu_domain *domain,
struct iommu_group *group)
{
@@ -3431,10 +3438,13 @@ void iommu_detach_device_pasid(struct iommu_domain *domain, struct device *dev,
{
/* Caller must be a probed driver on dev */
struct iommu_group *group = dev->iommu_group;
+ void *entry;
mutex_lock(&group->mutex);
__iommu_remove_group_pasid(group, pasid, domain);
- xa_erase(&group->pasid_array, pasid);
+ entry = xa_erase(&group->pasid_array, pasid);
+ WARN_ON(!entry);
+ iommu_pasid_array_entry_clear_handle(entry);
mutex_unlock(&group->mutex);
}
EXPORT_SYMBOL_GPL(iommu_detach_device_pasid);
@@ -3562,9 +3572,13 @@ EXPORT_SYMBOL_NS_GPL(iommu_attach_group_handle, "IOMMUFD_INTERNAL");
void iommu_detach_group_handle(struct iommu_domain *domain,
struct iommu_group *group)
{
+ void *entry;
+
mutex_lock(&group->mutex);
__iommu_group_set_core_domain(group);
- xa_erase(&group->pasid_array, IOMMU_NO_PASID);
+ entry = xa_erase(&group->pasid_array, IOMMU_NO_PASID);
+ WARN_ON(!entry);
+ iommu_pasid_array_entry_clear_handle(entry);
mutex_unlock(&group->mutex);
}
EXPORT_SYMBOL_NS_GPL(iommu_detach_group_handle, "IOMMUFD_INTERNAL");
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
2025-03-13 12:35 ` [PATCH v9 01/21] iommu: Clear handle->domain in detach Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:04 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 03/21] iommu: Introduce a replace API for device pasid Yi Liu
` (19 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
The IOMMU core does not mandate that callers must always provide a new
handle, allowing for the possibility of handle reuse. In the replace
path, the existing handle can be reused. To facilitate this, the core
must ensure that the pasid_array entry is made or updated under xa_lock
to prevent race conditions with callers of iommu_attach_handle_get().
Additionally, this operation should be performed only after the underlying
IOMMU driver has successfully set the domain. This precaution is necessary
to prevent forwarding PRIs to the new domain before it is fully prepared.
To streamline this process, the creation of the pasid_array entry and the
__xa_store() operation are encapsulated into a helper, which is invoked at
the end of the replace operation.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommu.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 332ecb20c385..6a314122a9da 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2168,6 +2168,22 @@ static void iommu_pasid_array_entry_clear_handle(void *entry)
xa_untag_pointer(entry))->domain = NULL;
}
+/* Caller should have reserved memory for __xa_store() */
+static void iommu_group_pasid_store(struct iommu_group *group, ioasid_t pasid,
+ struct iommu_domain *domain,
+ struct iommu_attach_handle *handle)
+{
+ void *entry;
+
+ lockdep_assert_held(&group->mutex);
+
+ xa_lock(&group->pasid_array);
+ entry = iommu_make_pasid_array_entry(domain, handle);
+ WARN_ON(xa_is_err(__xa_store(&group->pasid_array, pasid, entry,
+ GFP_KERNEL)));
+ xa_unlock(&group->pasid_array);
+}
+
static int __iommu_attach_group(struct iommu_domain *domain,
struct iommu_group *group)
{
@@ -3369,7 +3385,6 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
struct iommu_group *group = dev->iommu_group;
struct group_device *device;
const struct iommu_ops *ops;
- void *entry;
int ret;
if (!group)
@@ -3393,8 +3408,6 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
}
}
- entry = iommu_make_pasid_array_entry(domain, handle);
-
/*
* Entry present is a failure case. Use xa_insert() instead of
* xa_reserve().
@@ -3415,8 +3428,7 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
* operation succeeds as we cannot tolerate PRIs becoming concurrently
* queued and then failing attach.
*/
- WARN_ON(xa_is_err(xa_store(&group->pasid_array,
- pasid, entry, GFP_KERNEL)));
+ iommu_group_pasid_store(group, pasid, domain, handle);
out_unlock:
mutex_unlock(&group->mutex);
@@ -3527,14 +3539,12 @@ int iommu_attach_group_handle(struct iommu_domain *domain,
struct iommu_group *group,
struct iommu_attach_handle *handle)
{
- void *entry;
int ret;
if (!handle)
return -EINVAL;
mutex_lock(&group->mutex);
- entry = iommu_make_pasid_array_entry(domain, handle);
ret = xa_insert(&group->pasid_array,
IOMMU_NO_PASID, XA_ZERO_ENTRY, GFP_KERNEL);
if (ret)
@@ -3552,8 +3562,7 @@ int iommu_attach_group_handle(struct iommu_domain *domain,
* operation succeeds as we cannot tolerate PRIs becoming concurrently
* queued and then failing attach.
*/
- WARN_ON(xa_is_err(xa_store(&group->pasid_array,
- IOMMU_NO_PASID, entry, GFP_KERNEL)));
+ iommu_group_pasid_store(group, IOMMU_NO_PASID, domain, handle);
out_unlock:
mutex_unlock(&group->mutex);
@@ -3600,14 +3609,12 @@ int iommu_replace_group_handle(struct iommu_group *group,
struct iommu_domain *new_domain,
struct iommu_attach_handle *handle)
{
- void *curr, *entry;
int ret;
if (!new_domain || !handle)
return -EINVAL;
mutex_lock(&group->mutex);
- entry = iommu_make_pasid_array_entry(new_domain, handle);
ret = xa_reserve(&group->pasid_array, IOMMU_NO_PASID, GFP_KERNEL);
if (ret)
goto err_unlock;
@@ -3616,8 +3623,7 @@ int iommu_replace_group_handle(struct iommu_group *group,
if (ret)
goto err_release;
- curr = xa_store(&group->pasid_array, IOMMU_NO_PASID, entry, GFP_KERNEL);
- WARN_ON(xa_is_err(curr));
+ iommu_group_pasid_store(group, IOMMU_NO_PASID, new_domain, handle);
mutex_unlock(&group->mutex);
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 03/21] iommu: Introduce a replace API for device pasid
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
2025-03-13 12:35 ` [PATCH v9 01/21] iommu: Clear handle->domain in detach Yi Liu
2025-03-13 12:35 ` [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:19 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 04/21] iommufd: Pass @pasid through the device attach/replace path Yi Liu
` (18 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
Provide a high-level API to allow replacements of one domain with another
for specific pasid of a device. This is similar to
iommu_replace_group_handle() and it is expected to be used only by IOMMUFD.
Co-developed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
--
v8 -> v9: Adapted this patch to use iommu_group_pasid_store() which checks
the current pasid array entry and change the handle->domain under
xa_lock().
--
drivers/iommu/iommu-priv.h | 4 ++
drivers/iommu/iommu.c | 111 ++++++++++++++++++++++++++++++++++---
2 files changed, 107 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index b4508423e13b..2985f05d699f 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -43,4 +43,8 @@ void iommu_detach_group_handle(struct iommu_domain *domain,
int iommu_replace_group_handle(struct iommu_group *group,
struct iommu_domain *new_domain,
struct iommu_attach_handle *handle);
+
+int iommu_replace_device_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid,
+ struct iommu_attach_handle *handle);
#endif /* __LINUX_IOMMU_PRIV_H */
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 6a314122a9da..a828c96ad6c5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -513,6 +513,13 @@ static void iommu_deinit_device(struct device *dev)
dev_iommu_free(dev);
}
+static inline struct iommu_domain *pasid_array_entry_to_domain(void *entry)
+{
+ if (xa_pointer_tag(entry) == IOMMU_PASID_ARRAY_DOMAIN)
+ return xa_untag_pointer(entry);
+ return ((struct iommu_attach_handle *)xa_untag_pointer(entry))->domain;
+}
+
DEFINE_MUTEX(iommu_probe_device_lock);
static int __iommu_probe_device(struct device *dev, struct list_head *group_list)
@@ -2170,7 +2177,7 @@ static void iommu_pasid_array_entry_clear_handle(void *entry)
/* Caller should have reserved memory for __xa_store() */
static void iommu_group_pasid_store(struct iommu_group *group, ioasid_t pasid,
- struct iommu_domain *domain,
+ void *curr, struct iommu_domain *domain,
struct iommu_attach_handle *handle)
{
void *entry;
@@ -2179,8 +2186,13 @@ static void iommu_group_pasid_store(struct iommu_group *group, ioasid_t pasid,
xa_lock(&group->pasid_array);
entry = iommu_make_pasid_array_entry(domain, handle);
+ if (curr == entry)
+ goto out_unlock;
+
WARN_ON(xa_is_err(__xa_store(&group->pasid_array, pasid, entry,
GFP_KERNEL)));
+
+out_unlock:
xa_unlock(&group->pasid_array);
}
@@ -3334,14 +3346,15 @@ static void iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
}
static int __iommu_set_group_pasid(struct iommu_domain *domain,
- struct iommu_group *group, ioasid_t pasid)
+ struct iommu_group *group, ioasid_t pasid,
+ struct iommu_domain *old)
{
struct group_device *device, *last_gdev;
int ret;
for_each_group_device(group, device) {
ret = domain->ops->set_dev_pasid(domain, device->dev,
- pasid, NULL);
+ pasid, old);
if (ret)
goto err_revert;
}
@@ -3353,7 +3366,15 @@ static int __iommu_set_group_pasid(struct iommu_domain *domain,
for_each_group_device(group, device) {
if (device == last_gdev)
break;
- iommu_remove_dev_pasid(device->dev, pasid, domain);
+ /*
+ * If no old domain, undo the succeeded devices/pasid.
+ * Otherwise, rollback the succeeded devices/pasid to the old
+ * domain. And it is a driver bug to fail attaching with a
+ * previously good domain.
+ */
+ if (!old || WARN_ON(old->ops->set_dev_pasid(old, device->dev,
+ pasid, domain)))
+ iommu_remove_dev_pasid(device->dev, pasid, domain);
}
return ret;
}
@@ -3416,7 +3437,7 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
if (ret)
goto out_unlock;
- ret = __iommu_set_group_pasid(domain, group, pasid);
+ ret = __iommu_set_group_pasid(domain, group, pasid, NULL);
if (ret) {
xa_release(&group->pasid_array, pasid);
goto out_unlock;
@@ -3428,7 +3449,7 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
* operation succeeds as we cannot tolerate PRIs becoming concurrently
* queued and then failing attach.
*/
- iommu_group_pasid_store(group, pasid, domain, handle);
+ iommu_group_pasid_store(group, pasid, NULL, domain, handle);
out_unlock:
mutex_unlock(&group->mutex);
@@ -3436,6 +3457,79 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_attach_device_pasid);
+/**
+ * iommu_replace_device_pasid - Replace the domain that a pasid
+ * is attached to
+ * @domain: the new iommu domain
+ * @dev: the attached device.
+ * @pasid: the pasid of the device.
+ * @handle: the attach handle.
+ *
+ * This API allows the pasid to switch domains. The @pasid should have been
+ * attached. Otherwise, this fails.
+ * The pasid will keep the old configuration if replacement failed.
+ * Return 0 on success, or an error.
+ */
+int iommu_replace_device_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid,
+ struct iommu_attach_handle *handle)
+{
+ /* Caller must be a probed driver on dev */
+ struct iommu_group *group = dev->iommu_group;
+ struct iommu_domain *curr_domain;
+ void *curr;
+ int ret;
+
+ if (!group)
+ return -ENODEV;
+
+ if (!domain->ops->set_dev_pasid)
+ return -EOPNOTSUPP;
+
+ if (dev_iommu_ops(dev) != domain->owner ||
+ pasid == IOMMU_NO_PASID || !handle)
+ return -EINVAL;
+
+ mutex_lock(&group->mutex);
+ curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
+ XA_ZERO_ENTRY, GFP_KERNEL);
+ if (xa_is_err(curr)) {
+ ret = xa_err(curr);
+ goto out_unlock;
+ }
+
+ /*
+ * No domain (with or without handle) attached, hence not
+ * a replace case.
+ */
+ if (!curr) {
+ xa_release(&group->pasid_array, pasid);
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ curr_domain = pasid_array_entry_to_domain(curr);
+ ret = 0;
+
+ if (curr_domain != domain) {
+ ret = __iommu_set_group_pasid(domain, group,
+ pasid, curr_domain);
+ if (ret)
+ goto out_unlock;
+ }
+
+ /*
+ * The above xa_cmpxchg() reserved the memory, and the
+ * group->mutex is held, this cannot fail.
+ */
+ iommu_group_pasid_store(group, pasid, curr, domain, handle);
+
+out_unlock:
+ mutex_unlock(&group->mutex);
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(iommu_replace_device_pasid, "IOMMUFD_INTERNAL");
+
/*
* iommu_detach_device_pasid() - Detach the domain from pasid of device
* @domain: the iommu domain.
@@ -3562,7 +3656,7 @@ int iommu_attach_group_handle(struct iommu_domain *domain,
* operation succeeds as we cannot tolerate PRIs becoming concurrently
* queued and then failing attach.
*/
- iommu_group_pasid_store(group, IOMMU_NO_PASID, domain, handle);
+ iommu_group_pasid_store(group, IOMMU_NO_PASID, NULL, domain, handle);
out_unlock:
mutex_unlock(&group->mutex);
@@ -3623,7 +3717,8 @@ int iommu_replace_group_handle(struct iommu_group *group,
if (ret)
goto err_release;
- iommu_group_pasid_store(group, IOMMU_NO_PASID, new_domain, handle);
+ iommu_group_pasid_store(group, IOMMU_NO_PASID, NULL,
+ new_domain, handle);
mutex_unlock(&group->mutex);
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 04/21] iommufd: Pass @pasid through the device attach/replace path
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (2 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 03/21] iommu: Introduce a replace API for device pasid Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 05/21] iommufd/device: Only add reserved_iova in non-pasid path Yi Liu
` (17 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
Most of the core logic before conducting the actual device attach/
replace operation can be shared with pasid attach/replace. So pass
@pasid through the device attach/replace helpers to prepare adding
pasid attach/replace.
So far the @pasid should only be IOMMU_NO_PASID. No functional change.
Signed-off-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
v8 -> v9: Passs @pasid to iommufd_hwpt_paging_alloc() as well instead
of always passing IOMMU_NO_PASID when calling the
iommufd_hw_pagetable_[at|de]tach(). If not, the error handling
path of iommufd_hwpt_paging_alloc() will detach the RID path by
mistake.
---
drivers/iommu/iommufd/device.c | 70 +++++++++++++++----------
drivers/iommu/iommufd/hw_pagetable.c | 13 ++---
drivers/iommu/iommufd/iommufd_private.h | 8 +--
3 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index bd50146e2ad0..3c83fb014dcb 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -483,7 +483,8 @@ static bool iommufd_device_is_attached(struct iommufd_device *idev)
}
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_device *idev)
+ struct iommufd_device *idev,
+ ioasid_t pasid)
{
struct iommufd_attach_handle *handle;
int rc;
@@ -501,6 +502,7 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
}
handle->idev = idev;
+ WARN_ON(pasid != IOMMU_NO_PASID);
rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
&handle->handle);
if (rc)
@@ -517,25 +519,28 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
}
static struct iommufd_attach_handle *
-iommufd_device_get_attach_handle(struct iommufd_device *idev)
+iommufd_device_get_attach_handle(struct iommufd_device *idev, ioasid_t pasid)
{
struct iommu_attach_handle *handle;
lockdep_assert_held(&idev->igroup->lock);
handle =
- iommu_attach_handle_get(idev->igroup->group, IOMMU_NO_PASID, 0);
+ iommu_attach_handle_get(idev->igroup->group, pasid, 0);
if (IS_ERR(handle))
return NULL;
return to_iommufd_handle(handle);
}
static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_device *idev)
+ struct iommufd_device *idev,
+ ioasid_t pasid)
{
struct iommufd_attach_handle *handle;
- handle = iommufd_device_get_attach_handle(idev);
+ WARN_ON(pasid != IOMMU_NO_PASID);
+
+ handle = iommufd_device_get_attach_handle(idev, pasid);
iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
if (hwpt->fault) {
iommufd_auto_response_faults(hwpt, handle);
@@ -545,13 +550,17 @@ static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
}
static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
+ ioasid_t pasid,
struct iommufd_hw_pagetable *hwpt,
struct iommufd_hw_pagetable *old)
{
- struct iommufd_attach_handle *handle, *old_handle =
- iommufd_device_get_attach_handle(idev);
+ struct iommufd_attach_handle *handle, *old_handle;
int rc;
+ WARN_ON(pasid != IOMMU_NO_PASID);
+
+ old_handle = iommufd_device_get_attach_handle(idev, pasid);
+
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
if (!handle)
return -ENOMEM;
@@ -586,7 +595,7 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
}
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_device *idev)
+ struct iommufd_device *idev, ioasid_t pasid)
{
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
int rc;
@@ -612,7 +621,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
* attachment.
*/
if (list_empty(&idev->igroup->device_list)) {
- rc = iommufd_hwpt_attach_device(hwpt, idev);
+ rc = iommufd_hwpt_attach_device(hwpt, idev, pasid);
if (rc)
goto err_unresv;
idev->igroup->hwpt = hwpt;
@@ -630,7 +639,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
}
struct iommufd_hw_pagetable *
-iommufd_hw_pagetable_detach(struct iommufd_device *idev)
+iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
{
struct iommufd_hw_pagetable *hwpt = idev->igroup->hwpt;
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
@@ -638,7 +647,7 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev)
mutex_lock(&idev->igroup->lock);
list_del(&idev->group_item);
if (list_empty(&idev->igroup->device_list)) {
- iommufd_hwpt_detach_device(hwpt, idev);
+ iommufd_hwpt_detach_device(hwpt, idev, pasid);
idev->igroup->hwpt = NULL;
}
if (hwpt_paging)
@@ -650,12 +659,12 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev)
}
static struct iommufd_hw_pagetable *
-iommufd_device_do_attach(struct iommufd_device *idev,
+iommufd_device_do_attach(struct iommufd_device *idev, ioasid_t pasid,
struct iommufd_hw_pagetable *hwpt)
{
int rc;
- rc = iommufd_hw_pagetable_attach(hwpt, idev);
+ rc = iommufd_hw_pagetable_attach(hwpt, idev, pasid);
if (rc)
return ERR_PTR(rc);
return NULL;
@@ -704,7 +713,7 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
}
static struct iommufd_hw_pagetable *
-iommufd_device_do_replace(struct iommufd_device *idev,
+iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
struct iommufd_hw_pagetable *hwpt)
{
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
@@ -738,7 +747,7 @@ iommufd_device_do_replace(struct iommufd_device *idev,
goto err_unlock;
}
- rc = iommufd_hwpt_replace_device(idev, hwpt, old_hwpt);
+ rc = iommufd_hwpt_replace_device(idev, pasid, hwpt, old_hwpt);
if (rc)
goto err_unresv;
@@ -771,7 +780,8 @@ iommufd_device_do_replace(struct iommufd_device *idev,
}
typedef struct iommufd_hw_pagetable *(*attach_fn)(
- struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt);
+ struct iommufd_device *idev, ioasid_t pasid,
+ struct iommufd_hw_pagetable *hwpt);
/*
* When automatically managing the domains we search for a compatible domain in
@@ -779,7 +789,7 @@ typedef struct iommufd_hw_pagetable *(*attach_fn)(
* Automatic domain selection will never pick a manually created domain.
*/
static struct iommufd_hw_pagetable *
-iommufd_device_auto_get_domain(struct iommufd_device *idev,
+iommufd_device_auto_get_domain(struct iommufd_device *idev, ioasid_t pasid,
struct iommufd_ioas *ioas, u32 *pt_id,
attach_fn do_attach)
{
@@ -808,7 +818,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
hwpt = &hwpt_paging->common;
if (!iommufd_lock_obj(&hwpt->obj))
continue;
- destroy_hwpt = (*do_attach)(idev, hwpt);
+ destroy_hwpt = (*do_attach)(idev, pasid, hwpt);
if (IS_ERR(destroy_hwpt)) {
iommufd_put_object(idev->ictx, &hwpt->obj);
/*
@@ -826,8 +836,8 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
goto out_unlock;
}
- hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev, 0,
- immediate_attach, NULL);
+ hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev, pasid,
+ 0, immediate_attach, NULL);
if (IS_ERR(hwpt_paging)) {
destroy_hwpt = ERR_CAST(hwpt_paging);
goto out_unlock;
@@ -835,7 +845,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
hwpt = &hwpt_paging->common;
if (!immediate_attach) {
- destroy_hwpt = (*do_attach)(idev, hwpt);
+ destroy_hwpt = (*do_attach)(idev, pasid, hwpt);
if (IS_ERR(destroy_hwpt))
goto out_abort;
} else {
@@ -856,8 +866,9 @@ 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)
+static int iommufd_device_change_pt(struct iommufd_device *idev,
+ ioasid_t pasid,
+ u32 *pt_id, attach_fn do_attach)
{
struct iommufd_hw_pagetable *destroy_hwpt;
struct iommufd_object *pt_obj;
@@ -872,7 +883,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, pasid, hwpt);
if (IS_ERR(destroy_hwpt))
goto out_put_pt_obj;
break;
@@ -881,8 +892,8 @@ static int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id,
struct iommufd_ioas *ioas =
container_of(pt_obj, struct iommufd_ioas, obj);
- destroy_hwpt = iommufd_device_auto_get_domain(idev, ioas, pt_id,
- do_attach);
+ destroy_hwpt = iommufd_device_auto_get_domain(idev, pasid, ioas,
+ pt_id, do_attach);
if (IS_ERR(destroy_hwpt))
goto out_put_pt_obj;
break;
@@ -919,7 +930,8 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
{
int rc;
- rc = iommufd_device_change_pt(idev, pt_id, &iommufd_device_do_attach);
+ rc = iommufd_device_change_pt(idev, IOMMU_NO_PASID, pt_id,
+ &iommufd_device_do_attach);
if (rc)
return rc;
@@ -949,7 +961,7 @@ 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,
+ return iommufd_device_change_pt(idev, IOMMU_NO_PASID, pt_id,
&iommufd_device_do_replace);
}
EXPORT_SYMBOL_NS_GPL(iommufd_device_replace, "IOMMUFD");
@@ -965,7 +977,7 @@ void iommufd_device_detach(struct iommufd_device *idev)
{
struct iommufd_hw_pagetable *hwpt;
- hwpt = iommufd_hw_pagetable_detach(idev);
+ hwpt = iommufd_hw_pagetable_detach(idev, IOMMU_NO_PASID);
iommufd_hw_pagetable_put(idev->ictx, hwpt);
refcount_dec(&idev->obj.users);
}
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 268315b1d8bc..46babba1ceff 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -90,6 +90,7 @@ iommufd_hwpt_paging_enforce_cc(struct iommufd_hwpt_paging *hwpt_paging)
* @ictx: iommufd context
* @ioas: IOAS to associate the domain with
* @idev: Device to get an iommu_domain for
+ * @pasid: PASID to get an iommu_domain for
* @flags: Flags from userspace
* @immediate_attach: True if idev should be attached to the hwpt
* @user_data: The user provided driver specific data describing the domain to
@@ -105,8 +106,8 @@ iommufd_hwpt_paging_enforce_cc(struct iommufd_hwpt_paging *hwpt_paging)
*/
struct iommufd_hwpt_paging *
iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
- struct iommufd_device *idev, u32 flags,
- bool immediate_attach,
+ struct iommufd_device *idev, ioasid_t pasid,
+ u32 flags, bool immediate_attach,
const struct iommu_user_data *user_data)
{
const u32 valid_flags = IOMMU_HWPT_ALLOC_NEST_PARENT |
@@ -188,7 +189,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
* sequence. Once those drivers are fixed this should be removed.
*/
if (immediate_attach) {
- rc = iommufd_hw_pagetable_attach(hwpt, idev);
+ rc = iommufd_hw_pagetable_attach(hwpt, idev, pasid);
if (rc)
goto out_abort;
}
@@ -201,7 +202,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
out_detach:
if (immediate_attach)
- iommufd_hw_pagetable_detach(idev);
+ iommufd_hw_pagetable_detach(idev, pasid);
out_abort:
iommufd_object_abort_and_destroy(ictx, &hwpt->obj);
return ERR_PTR(rc);
@@ -361,8 +362,8 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd)
ioas = container_of(pt_obj, struct iommufd_ioas, obj);
mutex_lock(&ioas->mutex);
hwpt_paging = iommufd_hwpt_paging_alloc(
- ucmd->ictx, ioas, idev, cmd->flags, false,
- user_data.len ? &user_data : NULL);
+ ucmd->ictx, ioas, idev, IOMMU_NO_PASID, cmd->flags,
+ false, user_data.len ? &user_data : NULL);
if (IS_ERR(hwpt_paging)) {
rc = PTR_ERR(hwpt_paging);
goto out_unlock;
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 246297452a44..488851dab44a 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -366,13 +366,13 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd);
struct iommufd_hwpt_paging *
iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
- struct iommufd_device *idev, u32 flags,
- bool immediate_attach,
+ struct iommufd_device *idev, ioasid_t pasid,
+ u32 flags, bool immediate_attach,
const struct iommu_user_data *user_data);
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_device *idev);
+ struct iommufd_device *idev, ioasid_t pasid);
struct iommufd_hw_pagetable *
-iommufd_hw_pagetable_detach(struct iommufd_device *idev);
+iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid);
void iommufd_hwpt_paging_destroy(struct iommufd_object *obj);
void iommufd_hwpt_paging_abort(struct iommufd_object *obj);
void iommufd_hwpt_nested_destroy(struct iommufd_object *obj);
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 05/21] iommufd/device: Only add reserved_iova in non-pasid path
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (3 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 04/21] iommufd: Pass @pasid through the device attach/replace path Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 19:22 ` Nicolin Chen
2025-03-13 12:35 ` [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable Yi Liu
` (16 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
As the pasid is passed through the attach/replace/detach helpers, it is
necessary to ensure only the non-pasid path adds reserved_iova.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 3c83fb014dcb..be5226d2883e 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -598,6 +598,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev, ioasid_t pasid)
{
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
+ bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
int rc;
mutex_lock(&idev->igroup->lock);
@@ -607,7 +608,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
goto err_unlock;
}
- if (hwpt_paging) {
+ if (attach_resv) {
rc = iommufd_device_attach_reserved_iova(idev, hwpt_paging);
if (rc)
goto err_unlock;
@@ -631,7 +632,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
mutex_unlock(&idev->igroup->lock);
return 0;
err_unresv:
- if (hwpt_paging)
+ if (attach_resv)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
err_unlock:
mutex_unlock(&idev->igroup->lock);
@@ -650,7 +651,7 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
iommufd_hwpt_detach_device(hwpt, idev, pasid);
idev->igroup->hwpt = NULL;
}
- if (hwpt_paging)
+ if (hwpt_paging && pasid == IOMMU_NO_PASID)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
mutex_unlock(&idev->igroup->lock);
@@ -717,6 +718,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
struct iommufd_hw_pagetable *hwpt)
{
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
+ bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
struct iommufd_hwpt_paging *old_hwpt_paging;
struct iommufd_group *igroup = idev->igroup;
struct iommufd_hw_pagetable *old_hwpt;
@@ -741,7 +743,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
}
old_hwpt = igroup->hwpt;
- if (hwpt_paging) {
+ if (attach_resv) {
rc = iommufd_group_do_replace_reserved_iova(igroup, hwpt_paging);
if (rc)
goto err_unlock;
@@ -752,7 +754,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
goto err_unresv;
old_hwpt_paging = find_hwpt_paging(old_hwpt);
- if (old_hwpt_paging &&
+ if (old_hwpt_paging && pasid == IOMMU_NO_PASID &&
(!hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas))
iommufd_group_remove_reserved_iova(igroup, old_hwpt_paging);
@@ -772,7 +774,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
/* Caller must destroy old_hwpt */
return old_hwpt;
err_unresv:
- if (hwpt_paging)
+ if (attach_resv)
iommufd_group_remove_reserved_iova(igroup, hwpt_paging);
err_unlock:
mutex_unlock(&idev->igroup->lock);
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (4 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 05/21] iommufd/device: Only add reserved_iova in non-pasid path Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:25 ` Jason Gunthorpe
2025-03-18 19:24 ` Nicolin Chen
2025-03-13 12:35 ` [PATCH v9 07/21] iommufd/device: Check !igroup->hwpt in iommufd_device_attach_reserved_iova() Yi Liu
` (15 subsequent siblings)
21 siblings, 2 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
With more use of the fields of igroup, use a local vairable instead of
using the idev->igroup heavily.
No functional change expected.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 43 ++++++++++++++++++----------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index be5226d2883e..ac54d734b819 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -448,18 +448,19 @@ static int
iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
struct iommufd_hwpt_paging *hwpt_paging)
{
+ struct iommufd_group *igroup = idev->igroup;
int rc;
- lockdep_assert_held(&idev->igroup->lock);
+ lockdep_assert_held(&igroup->lock);
rc = iopt_table_enforce_dev_resv_regions(&hwpt_paging->ioas->iopt,
idev->dev,
- &idev->igroup->sw_msi_start);
+ &igroup->sw_msi_start);
if (rc)
return rc;
- if (list_empty(&idev->igroup->device_list)) {
- rc = iommufd_group_setup_msi(idev->igroup, hwpt_paging);
+ if (list_empty(&igroup->device_list)) {
+ rc = iommufd_group_setup_msi(igroup, hwpt_paging);
if (rc) {
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt,
idev->dev);
@@ -599,11 +600,12 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
{
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
+ struct iommufd_group *igroup = idev->igroup;
int rc;
- mutex_lock(&idev->igroup->lock);
+ mutex_lock(&igroup->lock);
- if (idev->igroup->hwpt != NULL && idev->igroup->hwpt != hwpt) {
+ if (igroup->hwpt && igroup->hwpt != hwpt) {
rc = -EINVAL;
goto err_unlock;
}
@@ -621,39 +623,40 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
* reserved regions are only updated during individual device
* attachment.
*/
- if (list_empty(&idev->igroup->device_list)) {
+ if (list_empty(&igroup->device_list)) {
rc = iommufd_hwpt_attach_device(hwpt, idev, pasid);
if (rc)
goto err_unresv;
- idev->igroup->hwpt = hwpt;
+ igroup->hwpt = hwpt;
}
refcount_inc(&hwpt->obj.users);
- list_add_tail(&idev->group_item, &idev->igroup->device_list);
- mutex_unlock(&idev->igroup->lock);
+ list_add_tail(&idev->group_item, &igroup->device_list);
+ mutex_unlock(&igroup->lock);
return 0;
err_unresv:
if (attach_resv)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
err_unlock:
- mutex_unlock(&idev->igroup->lock);
+ mutex_unlock(&igroup->lock);
return rc;
}
struct iommufd_hw_pagetable *
iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
{
- struct iommufd_hw_pagetable *hwpt = idev->igroup->hwpt;
+ struct iommufd_group *igroup = idev->igroup;
+ struct iommufd_hw_pagetable *hwpt = igroup->hwpt;
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
- mutex_lock(&idev->igroup->lock);
+ mutex_lock(&igroup->lock);
list_del(&idev->group_item);
- if (list_empty(&idev->igroup->device_list)) {
+ if (list_empty(&igroup->device_list)) {
iommufd_hwpt_detach_device(hwpt, idev, pasid);
- idev->igroup->hwpt = NULL;
+ igroup->hwpt = NULL;
}
if (hwpt_paging && pasid == IOMMU_NO_PASID)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
- mutex_unlock(&idev->igroup->lock);
+ mutex_unlock(&igroup->lock);
/* Caller must destroy hwpt */
return hwpt;
@@ -725,7 +728,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
unsigned int num_devices;
int rc;
- mutex_lock(&idev->igroup->lock);
+ mutex_lock(&igroup->lock);
if (igroup->hwpt == NULL) {
rc = -EINVAL;
@@ -738,7 +741,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
}
if (hwpt == igroup->hwpt) {
- mutex_unlock(&idev->igroup->lock);
+ mutex_unlock(&igroup->lock);
return NULL;
}
@@ -769,7 +772,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
if (num_devices > 1)
WARN_ON(refcount_sub_and_test(num_devices - 1,
&old_hwpt->obj.users));
- mutex_unlock(&idev->igroup->lock);
+ mutex_unlock(&igroup->lock);
/* Caller must destroy old_hwpt */
return old_hwpt;
@@ -777,7 +780,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
if (attach_resv)
iommufd_group_remove_reserved_iova(igroup, hwpt_paging);
err_unlock:
- mutex_unlock(&idev->igroup->lock);
+ mutex_unlock(&igroup->lock);
return ERR_PTR(rc);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 07/21] iommufd/device: Check !igroup->hwpt in iommufd_device_attach_reserved_iova()
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (5 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:27 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 08/21] iommufd/device: Lift iommufd_attach_handle handling to upper level helpers Yi Liu
` (14 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
iommufd_device_attach_reserved_iova() needs to handle the sw_msi when
the first device of the group is attaching to the domain. Currently,
it checks the igroup->device_list empty. However, !igroup->hwpt has the
same effect. Use it can help to further transit the igroup->hwpt to
iommufd_attach_handle. Hence prepare to add PASID support.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index ac54d734b819..a63d90ab3154 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -459,7 +459,7 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
if (rc)
return rc;
- if (list_empty(&igroup->device_list)) {
+ if (!igroup->hwpt) {
rc = iommufd_group_setup_msi(igroup, hwpt_paging);
if (rc) {
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt,
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 08/21] iommufd/device: Lift iommufd_attach_handle handling to upper level helpers
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (6 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 07/21] iommufd/device: Check !igroup->hwpt in iommufd_device_attach_reserved_iova() Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:30 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 09/21] iommufd/device: Use iommufd_attach_handle track attachment Yi Liu
` (13 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
iommufd_attach_handle is allocated when attaching the first device of a
group, and destroyed when the last device is detached. The life circle is
aligned with the igroup->hwpt which is used to track attached hwpt of the
group.
This lifts the handle allocation/destroy to the upper level helpers. It
prepares for using the handle to track hwpt attachment.
No functional change is expected, except for that the replace path reuses
the old handle instead of allocating a new one. This is needed when using
handle to track attached devices of a group. Otherwise, the replace path
would need to move the tracked devices to the new handle.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 66 +++++++++++++++++-----------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index a63d90ab3154..e1f27da65720 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -485,24 +485,19 @@ static bool iommufd_device_is_attached(struct iommufd_device *idev)
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev,
- ioasid_t pasid)
+ ioasid_t pasid,
+ struct iommufd_attach_handle *handle)
{
- struct iommufd_attach_handle *handle;
int rc;
lockdep_assert_held(&idev->igroup->lock);
- handle = kzalloc(sizeof(*handle), GFP_KERNEL);
- if (!handle)
- return -ENOMEM;
-
if (hwpt->fault) {
rc = iommufd_fault_iopf_enable(idev);
if (rc)
- goto out_free_handle;
+ return rc;
}
- handle->idev = idev;
WARN_ON(pasid != IOMMU_NO_PASID);
rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
&handle->handle);
@@ -514,8 +509,6 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
out_disable_iopf:
if (hwpt->fault)
iommufd_fault_iopf_disable(idev);
-out_free_handle:
- kfree(handle);
return rc;
}
@@ -535,63 +528,50 @@ iommufd_device_get_attach_handle(struct iommufd_device *idev, ioasid_t pasid)
static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev,
- ioasid_t pasid)
+ ioasid_t pasid,
+ struct iommufd_attach_handle *handle)
{
- struct iommufd_attach_handle *handle;
-
WARN_ON(pasid != IOMMU_NO_PASID);
- handle = iommufd_device_get_attach_handle(idev, pasid);
iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
if (hwpt->fault) {
iommufd_auto_response_faults(hwpt, handle);
iommufd_fault_iopf_disable(idev);
}
- kfree(handle);
}
static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
ioasid_t pasid,
struct iommufd_hw_pagetable *hwpt,
- struct iommufd_hw_pagetable *old)
+ struct iommufd_hw_pagetable *old,
+ struct iommufd_attach_handle *handle)
{
- struct iommufd_attach_handle *handle, *old_handle;
int rc;
WARN_ON(pasid != IOMMU_NO_PASID);
- old_handle = iommufd_device_get_attach_handle(idev, pasid);
-
- handle = kzalloc(sizeof(*handle), GFP_KERNEL);
- if (!handle)
- return -ENOMEM;
-
if (hwpt->fault && !old->fault) {
rc = iommufd_fault_iopf_enable(idev);
if (rc)
- goto out_free_handle;
+ return rc;
}
- handle->idev = idev;
rc = iommu_replace_group_handle(idev->igroup->group, hwpt->domain,
&handle->handle);
if (rc)
goto out_disable_iopf;
if (old->fault) {
- iommufd_auto_response_faults(hwpt, old_handle);
+ iommufd_auto_response_faults(hwpt, handle);
if (!hwpt->fault)
iommufd_fault_iopf_disable(idev);
}
- kfree(old_handle);
return 0;
out_disable_iopf:
if (hwpt->fault && !old->fault)
iommufd_fault_iopf_disable(idev);
-out_free_handle:
- kfree(handle);
return rc;
}
@@ -601,6 +581,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
struct iommufd_group *igroup = idev->igroup;
+ struct iommufd_attach_handle *handle;
int rc;
mutex_lock(&igroup->lock);
@@ -610,10 +591,19 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
goto err_unlock;
}
+ if (!igroup->hwpt) {
+ handle = kzalloc(sizeof(*handle), GFP_KERNEL);
+ if (!handle) {
+ rc = -ENOMEM;
+ goto err_unlock;
+ }
+ handle->idev = idev;
+ }
+
if (attach_resv) {
rc = iommufd_device_attach_reserved_iova(idev, hwpt_paging);
if (rc)
- goto err_unlock;
+ goto err_free_handle;
}
/*
@@ -624,7 +614,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
* attachment.
*/
if (list_empty(&igroup->device_list)) {
- rc = iommufd_hwpt_attach_device(hwpt, idev, pasid);
+ rc = iommufd_hwpt_attach_device(hwpt, idev, pasid, handle);
if (rc)
goto err_unresv;
igroup->hwpt = hwpt;
@@ -636,6 +626,9 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
err_unresv:
if (attach_resv)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
+err_free_handle:
+ if (!igroup->hwpt)
+ kfree(handle);
err_unlock:
mutex_unlock(&igroup->lock);
return rc;
@@ -647,12 +640,15 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
struct iommufd_group *igroup = idev->igroup;
struct iommufd_hw_pagetable *hwpt = igroup->hwpt;
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
+ struct iommufd_attach_handle *handle;
mutex_lock(&igroup->lock);
list_del(&idev->group_item);
if (list_empty(&igroup->device_list)) {
- iommufd_hwpt_detach_device(hwpt, idev, pasid);
+ handle = iommufd_device_get_attach_handle(idev, pasid);
+ iommufd_hwpt_detach_device(hwpt, idev, pasid, handle);
igroup->hwpt = NULL;
+ kfree(handle);
}
if (hwpt_paging && pasid == IOMMU_NO_PASID)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
@@ -725,6 +721,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
struct iommufd_hwpt_paging *old_hwpt_paging;
struct iommufd_group *igroup = idev->igroup;
struct iommufd_hw_pagetable *old_hwpt;
+ struct iommufd_attach_handle *handle;
unsigned int num_devices;
int rc;
@@ -745,6 +742,9 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
return NULL;
}
+ handle = iommufd_device_get_attach_handle(idev, pasid);
+ WARN_ON(!handle);
+
old_hwpt = igroup->hwpt;
if (attach_resv) {
rc = iommufd_group_do_replace_reserved_iova(igroup, hwpt_paging);
@@ -752,7 +752,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
goto err_unlock;
}
- rc = iommufd_hwpt_replace_device(idev, pasid, hwpt, old_hwpt);
+ rc = iommufd_hwpt_replace_device(idev, pasid, hwpt, old_hwpt, handle);
if (rc)
goto err_unresv;
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 09/21] iommufd/device: Use iommufd_attach_handle track attachment
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (7 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 08/21] iommufd/device: Lift iommufd_attach_handle handling to upper level helpers Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 10/21] iommufd/device: Replace device_list with device_array Yi Liu
` (12 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
Currently, in the RID path, the attached hwpt is stored in igroup->hwpt,
and the attached devices of a group are tracked in igroup->device_list.
However, this setup is insufficient for supporting PASID. PASID allows
a device to be attached on a per-PASID basis, operating in parallel with
the RID path.
To accommodate this, iommufd will require a per-PASID object to track
PASID attachments. This object should include both the attached hwpt and
the device list. The iommufd_attach_handle is a suitable candidate for
this role. It is currently created when the first device of a group is
attached and destroyed when the last device is detached. This lifecycle
aligns well with the requirements for attach tracking.
To initiate the transition, the igroup->hwpt will be moved to the
iommufd_attach_handle. The device_list will be relocated in a separate
commit.
iommufd_device_get_attach_handle() is dropped as iommufd can get handle
from igroup.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 67 +++++++++++--------------
drivers/iommu/iommufd/iommufd_private.h | 5 +-
2 files changed, 34 insertions(+), 38 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index e1f27da65720..9f31fc6acde3 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -23,7 +23,7 @@ static void iommufd_group_release(struct kref *kref)
struct iommufd_group *igroup =
container_of(kref, struct iommufd_group, ref);
- WARN_ON(igroup->hwpt || !list_empty(&igroup->device_list));
+ WARN_ON(igroup->handle || !list_empty(&igroup->device_list));
xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
NULL, GFP_KERNEL);
@@ -459,7 +459,7 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
if (rc)
return rc;
- if (!igroup->hwpt) {
+ if (!igroup->handle) {
rc = iommufd_group_setup_msi(igroup, hwpt_paging);
if (rc) {
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt,
@@ -472,7 +472,6 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
/* The device attach/detach/replace helpers for attach_handle */
-/* Check if idev is attached to igroup->hwpt */
static bool iommufd_device_is_attached(struct iommufd_device *idev)
{
struct iommufd_device *cur;
@@ -512,20 +511,6 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
return rc;
}
-static struct iommufd_attach_handle *
-iommufd_device_get_attach_handle(struct iommufd_device *idev, ioasid_t pasid)
-{
- struct iommu_attach_handle *handle;
-
- lockdep_assert_held(&idev->igroup->lock);
-
- handle =
- iommu_attach_handle_get(idev->igroup->group, pasid, 0);
- if (IS_ERR(handle))
- return NULL;
- return to_iommufd_handle(handle);
-}
-
static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev,
ioasid_t pasid,
@@ -581,17 +566,14 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
struct iommufd_group *igroup = idev->igroup;
+ struct iommufd_hw_pagetable *old_hwpt;
struct iommufd_attach_handle *handle;
int rc;
mutex_lock(&igroup->lock);
- if (igroup->hwpt && igroup->hwpt != hwpt) {
- rc = -EINVAL;
- goto err_unlock;
- }
-
- if (!igroup->hwpt) {
+ handle = igroup->handle;
+ if (!handle) {
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
if (!handle) {
rc = -ENOMEM;
@@ -600,6 +582,13 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
handle->idev = idev;
}
+ old_hwpt = handle->hwpt;
+
+ if (old_hwpt && old_hwpt != hwpt) {
+ rc = -EINVAL;
+ goto err_free_handle;
+ }
+
if (attach_resv) {
rc = iommufd_device_attach_reserved_iova(idev, hwpt_paging);
if (rc)
@@ -617,7 +606,8 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
rc = iommufd_hwpt_attach_device(hwpt, idev, pasid, handle);
if (rc)
goto err_unresv;
- igroup->hwpt = hwpt;
+ handle->hwpt = hwpt;
+ igroup->handle = handle;
}
refcount_inc(&hwpt->obj.users);
list_add_tail(&idev->group_item, &igroup->device_list);
@@ -627,7 +617,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
if (attach_resv)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
err_free_handle:
- if (!igroup->hwpt)
+ if (!old_hwpt)
kfree(handle);
err_unlock:
mutex_unlock(&igroup->lock);
@@ -638,16 +628,18 @@ struct iommufd_hw_pagetable *
iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
{
struct iommufd_group *igroup = idev->igroup;
- struct iommufd_hw_pagetable *hwpt = igroup->hwpt;
- struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
+ struct iommufd_hwpt_paging *hwpt_paging;
struct iommufd_attach_handle *handle;
+ struct iommufd_hw_pagetable *hwpt;
mutex_lock(&igroup->lock);
+ handle = igroup->handle;
+ hwpt = handle->hwpt;
+ hwpt_paging = find_hwpt_paging(hwpt);
list_del(&idev->group_item);
if (list_empty(&igroup->device_list)) {
- handle = iommufd_device_get_attach_handle(idev, pasid);
iommufd_hwpt_detach_device(hwpt, idev, pasid, handle);
- igroup->hwpt = NULL;
+ igroup->handle = NULL;
kfree(handle);
}
if (hwpt_paging && pasid == IOMMU_NO_PASID)
@@ -692,7 +684,8 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
lockdep_assert_held(&igroup->lock);
- old_hwpt_paging = find_hwpt_paging(igroup->hwpt);
+ WARN_ON(!igroup->handle);
+ old_hwpt_paging = find_hwpt_paging(igroup->handle->hwpt);
if (!old_hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas) {
list_for_each_entry(cur, &igroup->device_list, group_item) {
rc = iopt_table_enforce_dev_resv_regions(
@@ -727,7 +720,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
mutex_lock(&igroup->lock);
- if (igroup->hwpt == NULL) {
+ if (!igroup->handle) {
rc = -EINVAL;
goto err_unlock;
}
@@ -737,15 +730,15 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
goto err_unlock;
}
- if (hwpt == igroup->hwpt) {
+ handle = igroup->handle;
+ WARN_ON(!handle);
+ old_hwpt = handle->hwpt;
+
+ if (hwpt == old_hwpt) {
mutex_unlock(&igroup->lock);
return NULL;
}
- handle = iommufd_device_get_attach_handle(idev, pasid);
- WARN_ON(!handle);
-
- old_hwpt = igroup->hwpt;
if (attach_resv) {
rc = iommufd_group_do_replace_reserved_iova(igroup, hwpt_paging);
if (rc)
@@ -761,7 +754,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
(!hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas))
iommufd_group_remove_reserved_iova(igroup, old_hwpt_paging);
- igroup->hwpt = hwpt;
+ handle->hwpt = hwpt;
num_devices = list_count_nodes(&igroup->device_list);
/*
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 488851dab44a..d07ba94e63b3 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -396,12 +396,14 @@ static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
refcount_dec(&hwpt->obj.users);
}
+struct iommufd_attach_handle;
+
struct iommufd_group {
struct kref ref;
struct mutex lock;
struct iommufd_ctx *ictx;
struct iommu_group *group;
- struct iommufd_hw_pagetable *hwpt;
+ struct iommufd_attach_handle *handle;
struct list_head device_list;
struct iommufd_sw_msi_maps required_sw_msi;
phys_addr_t sw_msi_start;
@@ -500,6 +502,7 @@ static inline void iommufd_fault_deliver_restore(struct iommufd_fault *fault,
struct iommufd_attach_handle {
struct iommu_attach_handle handle;
struct iommufd_device *idev;
+ struct iommufd_hw_pagetable *hwpt;
};
/* Convert an iommu attach handle to iommufd handle. */
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 10/21] iommufd/device: Replace device_list with device_array
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (8 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 09/21] iommufd/device: Use iommufd_attach_handle track attachment Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle Yi Liu
` (11 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
igroup->device_list is used to track attached device of a group in the
RID path. Such a tracking is also needed in the PASID path in order to
share common path with the RID path.
While there is only one list_head in the iommufd_device. It cannot work
out if the device has been attached in both RID path and PASID path. To
solve it, replacing the device_list with an xarray. The attached
iommufd_device is stored in the entry indexed by the idev->obj.id.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 57 +++++++++++++++++--------
drivers/iommu/iommufd/iommufd_private.h | 2 +-
2 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 9f31fc6acde3..f34481940b6b 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -23,7 +23,7 @@ static void iommufd_group_release(struct kref *kref)
struct iommufd_group *igroup =
container_of(kref, struct iommufd_group, ref);
- WARN_ON(igroup->handle || !list_empty(&igroup->device_list));
+ WARN_ON(igroup->handle || !xa_empty(&igroup->device_array));
xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
NULL, GFP_KERNEL);
@@ -90,7 +90,7 @@ static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
kref_init(&new_igroup->ref);
mutex_init(&new_igroup->lock);
- INIT_LIST_HEAD(&new_igroup->device_list);
+ xa_init(&new_igroup->device_array);
new_igroup->sw_msi_start = PHYS_ADDR_MAX;
/* group reference moves into new_igroup */
new_igroup->group = group;
@@ -294,6 +294,20 @@ u32 iommufd_device_to_id(struct iommufd_device *idev)
}
EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, "IOMMUFD");
+static int iommufd_group_device_num(struct iommufd_group *igroup)
+{
+ struct iommufd_device *idev;
+ unsigned long index;
+ int count = 0;
+
+ lockdep_assert_held(&igroup->lock);
+
+ if (igroup->handle)
+ xa_for_each(&igroup->device_array, index, idev)
+ count++;
+ return count;
+}
+
/*
* Get a iommufd_sw_msi_map for the msi physical address requested by the irq
* layer. The mapping to IOVA is global to the iommufd file descriptor, every
@@ -474,12 +488,7 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
static bool iommufd_device_is_attached(struct iommufd_device *idev)
{
- struct iommufd_device *cur;
-
- list_for_each_entry(cur, &idev->igroup->device_list, group_item)
- if (cur == idev)
- return true;
- return false;
+ return xa_load(&idev->igroup->device_array, idev->obj.id);
}
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
@@ -584,15 +593,22 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
old_hwpt = handle->hwpt;
+ rc = xa_insert(&igroup->device_array, idev->obj.id, XA_ZERO_ENTRY,
+ GFP_KERNEL);
+ if (rc) {
+ WARN_ON(rc == -EBUSY && !old_hwpt);
+ goto err_free_handle;
+ }
+
if (old_hwpt && old_hwpt != hwpt) {
rc = -EINVAL;
- goto err_free_handle;
+ goto err_release_devid;
}
if (attach_resv) {
rc = iommufd_device_attach_reserved_iova(idev, hwpt_paging);
if (rc)
- goto err_free_handle;
+ goto err_release_devid;
}
/*
@@ -602,7 +618,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
* reserved regions are only updated during individual device
* attachment.
*/
- if (list_empty(&igroup->device_list)) {
+ if (!iommufd_group_device_num(igroup)) {
rc = iommufd_hwpt_attach_device(hwpt, idev, pasid, handle);
if (rc)
goto err_unresv;
@@ -610,12 +626,15 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
igroup->handle = handle;
}
refcount_inc(&hwpt->obj.users);
- list_add_tail(&idev->group_item, &igroup->device_list);
+ WARN_ON(xa_is_err(__xa_store(&igroup->device_array, idev->obj.id,
+ idev, GFP_KERNEL)));
mutex_unlock(&igroup->lock);
return 0;
err_unresv:
if (attach_resv)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
+err_release_devid:
+ xa_release(&igroup->device_array, idev->obj.id);
err_free_handle:
if (!old_hwpt)
kfree(handle);
@@ -636,8 +655,8 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
handle = igroup->handle;
hwpt = handle->hwpt;
hwpt_paging = find_hwpt_paging(hwpt);
- list_del(&idev->group_item);
- if (list_empty(&igroup->device_list)) {
+ xa_erase(&igroup->device_array, idev->obj.id);
+ if (xa_empty(&igroup->device_array)) {
iommufd_hwpt_detach_device(hwpt, idev, pasid, handle);
igroup->handle = NULL;
kfree(handle);
@@ -667,10 +686,11 @@ iommufd_group_remove_reserved_iova(struct iommufd_group *igroup,
struct iommufd_hwpt_paging *hwpt_paging)
{
struct iommufd_device *cur;
+ unsigned long index;
lockdep_assert_held(&igroup->lock);
- list_for_each_entry(cur, &igroup->device_list, group_item)
+ xa_for_each(&igroup->device_array, index, cur)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, cur->dev);
}
@@ -680,6 +700,7 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
{
struct iommufd_hwpt_paging *old_hwpt_paging;
struct iommufd_device *cur;
+ unsigned long index;
int rc;
lockdep_assert_held(&igroup->lock);
@@ -687,7 +708,7 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
WARN_ON(!igroup->handle);
old_hwpt_paging = find_hwpt_paging(igroup->handle->hwpt);
if (!old_hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas) {
- list_for_each_entry(cur, &igroup->device_list, group_item) {
+ xa_for_each(&igroup->device_array, index, cur) {
rc = iopt_table_enforce_dev_resv_regions(
&hwpt_paging->ioas->iopt, cur->dev, NULL);
if (rc)
@@ -756,9 +777,9 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
handle->hwpt = hwpt;
- num_devices = list_count_nodes(&igroup->device_list);
+ num_devices = iommufd_group_device_num(igroup);
/*
- * Move the refcounts held by the device_list to the new hwpt. Retain a
+ * Move the refcounts held by the device_array to the new hwpt. Retain a
* refcount for this thread as the caller will free it.
*/
refcount_add(num_devices, &hwpt->obj.users);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index d07ba94e63b3..387a0fab258c 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -404,9 +404,9 @@ struct iommufd_group {
struct iommufd_ctx *ictx;
struct iommu_group *group;
struct iommufd_attach_handle *handle;
- struct list_head device_list;
struct iommufd_sw_msi_maps required_sw_msi;
phys_addr_t sw_msi_start;
+ struct xarray device_array;
};
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (9 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 10/21] iommufd/device: Replace device_list with device_array Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:34 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 12/21] iommufd/device: Add pasid_attach array to track per-PASID attach Yi Liu
` (10 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
By tracking both attached hwpt and devices, iommufd_attach_handle is
prepared to be used as attach object. This prepares for adding per-PASID
attachment by using handle.
The handle has cached the first attached idev, this is used in the PRI
reading path to retrieve devid. This field is kept although it is a
bit duplicated with the devices tracked in device_array. However, there
is open on the PRI path. Conceptually, the PRI read path should use the
device_array to loop all attached devices to figure out the correct idev.
But the existing code can only work for the groups that have only one
device attached as it needs the PRI flushing of the detach path to fence
the detach path and the PRI read path. If the group has multiple devices
attached, this fence does not work because the detach path only flushes
PRI when the last attached device is detached. We may solve it separately
hence no need to touch it in this commit.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 24 +++++++++++++-----------
drivers/iommu/iommufd/iommufd_private.h | 2 +-
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index f34481940b6b..7d52f304b9bb 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -23,7 +23,7 @@ static void iommufd_group_release(struct kref *kref)
struct iommufd_group *igroup =
container_of(kref, struct iommufd_group, ref);
- WARN_ON(igroup->handle || !xa_empty(&igroup->device_array));
+ WARN_ON(igroup->handle);
xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
NULL, GFP_KERNEL);
@@ -90,7 +90,6 @@ static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
kref_init(&new_igroup->ref);
mutex_init(&new_igroup->lock);
- xa_init(&new_igroup->device_array);
new_igroup->sw_msi_start = PHYS_ADDR_MAX;
/* group reference moves into new_igroup */
new_igroup->group = group;
@@ -303,7 +302,7 @@ static int iommufd_group_device_num(struct iommufd_group *igroup)
lockdep_assert_held(&igroup->lock);
if (igroup->handle)
- xa_for_each(&igroup->device_array, index, idev)
+ xa_for_each(&igroup->handle->device_array, index, idev)
count++;
return count;
}
@@ -488,7 +487,8 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
static bool iommufd_device_is_attached(struct iommufd_device *idev)
{
- return xa_load(&idev->igroup->device_array, idev->obj.id);
+ WARN_ON(!idev->igroup->handle);
+ return xa_load(&idev->igroup->handle->device_array, idev->obj.id);
}
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
@@ -589,11 +589,12 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
goto err_unlock;
}
handle->idev = idev;
+ xa_init(&handle->device_array);
}
old_hwpt = handle->hwpt;
- rc = xa_insert(&igroup->device_array, idev->obj.id, XA_ZERO_ENTRY,
+ rc = xa_insert(&handle->device_array, idev->obj.id, XA_ZERO_ENTRY,
GFP_KERNEL);
if (rc) {
WARN_ON(rc == -EBUSY && !old_hwpt);
@@ -626,7 +627,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
igroup->handle = handle;
}
refcount_inc(&hwpt->obj.users);
- WARN_ON(xa_is_err(__xa_store(&igroup->device_array, idev->obj.id,
+ WARN_ON(xa_is_err(__xa_store(&handle->device_array, idev->obj.id,
idev, GFP_KERNEL)));
mutex_unlock(&igroup->lock);
return 0;
@@ -634,7 +635,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
if (attach_resv)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, idev->dev);
err_release_devid:
- xa_release(&igroup->device_array, idev->obj.id);
+ xa_release(&handle->device_array, idev->obj.id);
err_free_handle:
if (!old_hwpt)
kfree(handle);
@@ -655,8 +656,8 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
handle = igroup->handle;
hwpt = handle->hwpt;
hwpt_paging = find_hwpt_paging(hwpt);
- xa_erase(&igroup->device_array, idev->obj.id);
- if (xa_empty(&igroup->device_array)) {
+ xa_erase(&handle->device_array, idev->obj.id);
+ if (xa_empty(&handle->device_array)) {
iommufd_hwpt_detach_device(hwpt, idev, pasid, handle);
igroup->handle = NULL;
kfree(handle);
@@ -690,7 +691,8 @@ iommufd_group_remove_reserved_iova(struct iommufd_group *igroup,
lockdep_assert_held(&igroup->lock);
- xa_for_each(&igroup->device_array, index, cur)
+ WARN_ON(!igroup->handle);
+ xa_for_each(&igroup->handle->device_array, index, cur)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, cur->dev);
}
@@ -708,7 +710,7 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
WARN_ON(!igroup->handle);
old_hwpt_paging = find_hwpt_paging(igroup->handle->hwpt);
if (!old_hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas) {
- xa_for_each(&igroup->device_array, index, cur) {
+ xa_for_each(&igroup->handle->device_array, index, cur) {
rc = iopt_table_enforce_dev_resv_regions(
&hwpt_paging->ioas->iopt, cur->dev, NULL);
if (rc)
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 387a0fab258c..2c0ff618b916 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -406,7 +406,6 @@ struct iommufd_group {
struct iommufd_attach_handle *handle;
struct iommufd_sw_msi_maps required_sw_msi;
phys_addr_t sw_msi_start;
- struct xarray device_array;
};
/*
@@ -503,6 +502,7 @@ struct iommufd_attach_handle {
struct iommu_attach_handle handle;
struct iommufd_device *idev;
struct iommufd_hw_pagetable *hwpt;
+ struct xarray device_array;
};
/* Convert an iommu attach handle to iommufd handle. */
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 12/21] iommufd/device: Add pasid_attach array to track per-PASID attach
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (10 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-17 7:07 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 13/21] iommufd: Enforce PASID-compatible domain in PASID path Yi Liu
` (9 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
PASIDs of PASID-capable device can be attached to hwpt separately, hence
a pasid array to track per-PASID attachment is necessary. The index
IOMMU_NO_PASID is used by the RID path. Hence drop the igroup->handle.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 78 +++++++++++++++++--------
drivers/iommu/iommufd/iommufd_private.h | 4 +-
2 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 7d52f304b9bb..63b189a5b8ac 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -23,7 +23,7 @@ static void iommufd_group_release(struct kref *kref)
struct iommufd_group *igroup =
container_of(kref, struct iommufd_group, ref);
- WARN_ON(igroup->handle);
+ WARN_ON(!xa_empty(&igroup->pasid_attach));
xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
NULL, GFP_KERNEL);
@@ -90,6 +90,7 @@ static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
kref_init(&new_igroup->ref);
mutex_init(&new_igroup->lock);
+ xa_init(&new_igroup->pasid_attach);
new_igroup->sw_msi_start = PHYS_ADDR_MAX;
/* group reference moves into new_igroup */
new_igroup->group = group;
@@ -293,16 +294,19 @@ u32 iommufd_device_to_id(struct iommufd_device *idev)
}
EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, "IOMMUFD");
-static int iommufd_group_device_num(struct iommufd_group *igroup)
+static int iommufd_group_device_num(struct iommufd_group *igroup,
+ ioasid_t pasid)
{
+ struct iommufd_attach_handle *handle;
struct iommufd_device *idev;
unsigned long index;
int count = 0;
lockdep_assert_held(&igroup->lock);
- if (igroup->handle)
- xa_for_each(&igroup->handle->device_array, index, idev)
+ handle = xa_load(&igroup->pasid_attach, pasid);
+ if (handle)
+ xa_for_each(&handle->device_array, index, idev)
count++;
return count;
}
@@ -472,7 +476,7 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
if (rc)
return rc;
- if (!igroup->handle) {
+ if (!xa_load(&igroup->pasid_attach, IOMMU_NO_PASID)) {
rc = iommufd_group_setup_msi(igroup, hwpt_paging);
if (rc) {
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt,
@@ -485,10 +489,14 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
/* The device attach/detach/replace helpers for attach_handle */
-static bool iommufd_device_is_attached(struct iommufd_device *idev)
+static bool iommufd_device_is_attached(struct iommufd_device *idev,
+ ioasid_t pasid)
{
- WARN_ON(!idev->igroup->handle);
- return xa_load(&idev->igroup->handle->device_array, idev->obj.id);
+ struct iommufd_attach_handle *handle;
+
+ handle = xa_load(&idev->igroup->pasid_attach, pasid);
+ WARN_ON(!handle);
+ return xa_load(&handle->device_array, idev->obj.id);
}
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
@@ -574,19 +582,25 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
{
struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
+ struct iommufd_hw_pagetable *old_hwpt = NULL;
struct iommufd_group *igroup = idev->igroup;
- struct iommufd_hw_pagetable *old_hwpt;
struct iommufd_attach_handle *handle;
int rc;
mutex_lock(&igroup->lock);
- handle = igroup->handle;
+ handle = xa_cmpxchg(&igroup->pasid_attach, pasid, NULL,
+ XA_ZERO_ENTRY, GFP_KERNEL);
+ if (xa_is_err(handle)) {
+ rc = PTR_ERR(handle);
+ goto err_unlock;
+ }
+
if (!handle) {
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
if (!handle) {
rc = -ENOMEM;
- goto err_unlock;
+ goto err_release_pasid;
}
handle->idev = idev;
xa_init(&handle->device_array);
@@ -619,12 +633,13 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
* reserved regions are only updated during individual device
* attachment.
*/
- if (!iommufd_group_device_num(igroup)) {
+ if (!iommufd_group_device_num(igroup, pasid)) {
rc = iommufd_hwpt_attach_device(hwpt, idev, pasid, handle);
if (rc)
goto err_unresv;
handle->hwpt = hwpt;
- igroup->handle = handle;
+ WARN_ON(xa_is_err(xa_store(&igroup->pasid_attach, pasid, handle,
+ GFP_KERNEL)));
}
refcount_inc(&hwpt->obj.users);
WARN_ON(xa_is_err(__xa_store(&handle->device_array, idev->obj.id,
@@ -639,6 +654,9 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
err_free_handle:
if (!old_hwpt)
kfree(handle);
+err_release_pasid:
+ if (!old_hwpt)
+ xa_release(&igroup->pasid_attach, pasid);
err_unlock:
mutex_unlock(&igroup->lock);
return rc;
@@ -653,13 +671,13 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev, ioasid_t pasid)
struct iommufd_hw_pagetable *hwpt;
mutex_lock(&igroup->lock);
- handle = igroup->handle;
+ handle = xa_load(&igroup->pasid_attach, pasid);
hwpt = handle->hwpt;
hwpt_paging = find_hwpt_paging(hwpt);
xa_erase(&handle->device_array, idev->obj.id);
if (xa_empty(&handle->device_array)) {
iommufd_hwpt_detach_device(hwpt, idev, pasid, handle);
- igroup->handle = NULL;
+ xa_erase(&igroup->pasid_attach, pasid);
kfree(handle);
}
if (hwpt_paging && pasid == IOMMU_NO_PASID)
@@ -686,13 +704,15 @@ static void
iommufd_group_remove_reserved_iova(struct iommufd_group *igroup,
struct iommufd_hwpt_paging *hwpt_paging)
{
+ struct iommufd_attach_handle *handle;
struct iommufd_device *cur;
unsigned long index;
lockdep_assert_held(&igroup->lock);
- WARN_ON(!igroup->handle);
- xa_for_each(&igroup->handle->device_array, index, cur)
+ handle = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
+ WARN_ON(!handle);
+ xa_for_each(&handle->device_array, index, cur)
iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt, cur->dev);
}
@@ -701,16 +721,18 @@ iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
struct iommufd_hwpt_paging *hwpt_paging)
{
struct iommufd_hwpt_paging *old_hwpt_paging;
+ struct iommufd_attach_handle *handle;
struct iommufd_device *cur;
unsigned long index;
int rc;
lockdep_assert_held(&igroup->lock);
- WARN_ON(!igroup->handle);
- old_hwpt_paging = find_hwpt_paging(igroup->handle->hwpt);
+ handle = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
+ WARN_ON(!handle);
+ old_hwpt_paging = find_hwpt_paging(handle->hwpt);
if (!old_hwpt_paging || hwpt_paging->ioas != old_hwpt_paging->ioas) {
- xa_for_each(&igroup->handle->device_array, index, cur) {
+ xa_for_each(&handle->device_array, index, cur) {
rc = iopt_table_enforce_dev_resv_regions(
&hwpt_paging->ioas->iopt, cur->dev, NULL);
if (rc)
@@ -743,18 +765,24 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
mutex_lock(&igroup->lock);
- if (!igroup->handle) {
+ handle = xa_cmpxchg(&igroup->pasid_attach, pasid, NULL,
+ XA_ZERO_ENTRY, GFP_KERNEL);
+ if (xa_is_err(handle)) {
+ rc = xa_err(handle);
+ goto err_unlock;
+ }
+
+ if (!handle) {
+ xa_release(&igroup->pasid_attach, pasid);
rc = -EINVAL;
goto err_unlock;
}
- if (!iommufd_device_is_attached(idev)) {
+ if (!iommufd_device_is_attached(idev, pasid)) {
rc = -EINVAL;
goto err_unlock;
}
- handle = igroup->handle;
- WARN_ON(!handle);
old_hwpt = handle->hwpt;
if (hwpt == old_hwpt) {
@@ -779,7 +807,7 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
handle->hwpt = hwpt;
- num_devices = iommufd_group_device_num(igroup);
+ num_devices = iommufd_group_device_num(igroup, pasid);
/*
* Move the refcounts held by the device_array to the new hwpt. Retain a
* refcount for this thread as the caller will free it.
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 2c0ff618b916..f1e9b45ebd79 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -396,16 +396,14 @@ static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
refcount_dec(&hwpt->obj.users);
}
-struct iommufd_attach_handle;
-
struct iommufd_group {
struct kref ref;
struct mutex lock;
struct iommufd_ctx *ictx;
struct iommu_group *group;
- struct iommufd_attach_handle *handle;
struct iommufd_sw_msi_maps required_sw_msi;
phys_addr_t sw_msi_start;
+ struct xarray pasid_attach;
};
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 13/21] iommufd: Enforce PASID-compatible domain in PASID path
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (11 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 12/21] iommufd/device: Add pasid_attach array to track per-PASID attach Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 14/21] iommufd: Support pasid attach/replace Yi Liu
` (8 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
AMD IOMMU requires attaching PASID-compatible domains to PASID-capable
devices. This includes the domains attached to RID and PASIDs. Related
discussions in link [1] and [2]. ARM also has such a requirement, Intel
does not need it, but can live up with it. Hence, iommufd is going to
enforce this requirement as it is not harmful to vendors that do not
need it.
Mark the PASID-compatible domains and enforce it in the PASID path.
[1] https://lore.kernel.org/linux-iommu/20240709182303.GK14050@ziepe.ca/
[2] https://lore.kernel.org/linux-iommu/20240822124433.GD3468552@ziepe.ca/
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
v8 -> v9: Renamed the subject from "iommufd: Mark PASID-compatible domain"
to better describe what the patch has done.
iommufd_hwpt_pasid_compat() is added in this patch instead of
later patch
---
drivers/iommu/iommufd/device.c | 17 +++++++++++++++++
drivers/iommu/iommufd/hw_pagetable.c | 3 +++
drivers/iommu/iommufd/iommufd_private.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 63b189a5b8ac..adeeb0145184 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -499,6 +499,15 @@ static bool iommufd_device_is_attached(struct iommufd_device *idev,
return xa_load(&handle->device_array, idev->obj.id);
}
+static int iommufd_hwpt_pasid_compat(struct iommufd_hw_pagetable *hwpt,
+ struct iommufd_device *idev,
+ ioasid_t pasid)
+{
+ if (pasid != IOMMU_NO_PASID && !hwpt->pasid_compat)
+ return -EINVAL;
+ return 0;
+}
+
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev,
ioasid_t pasid,
@@ -508,6 +517,10 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
lockdep_assert_held(&idev->igroup->lock);
+ rc = iommufd_hwpt_pasid_compat(hwpt, idev, pasid);
+ if (rc)
+ return rc;
+
if (hwpt->fault) {
rc = iommufd_fault_iopf_enable(idev);
if (rc)
@@ -552,6 +565,10 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
WARN_ON(pasid != IOMMU_NO_PASID);
+ rc = iommufd_hwpt_pasid_compat(hwpt, idev, pasid);
+ if (rc)
+ return rc;
+
if (hwpt->fault && !old->fault) {
rc = iommufd_fault_iopf_enable(idev);
if (rc)
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 46babba1ceff..7a86c164f514 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -136,6 +136,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
if (IS_ERR(hwpt_paging))
return ERR_CAST(hwpt_paging);
hwpt = &hwpt_paging->common;
+ hwpt->pasid_compat = flags & IOMMU_HWPT_ALLOC_PASID;
INIT_LIST_HEAD(&hwpt_paging->hwpt_item);
/* Pairs with iommufd_hw_pagetable_destroy() */
@@ -243,6 +244,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
if (IS_ERR(hwpt_nested))
return ERR_CAST(hwpt_nested);
hwpt = &hwpt_nested->common;
+ hwpt->pasid_compat = flags & IOMMU_HWPT_ALLOC_PASID;
refcount_inc(&parent->common.obj.users);
hwpt_nested->parent = parent;
@@ -298,6 +300,7 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
if (IS_ERR(hwpt_nested))
return ERR_CAST(hwpt_nested);
hwpt = &hwpt_nested->common;
+ hwpt->pasid_compat = flags & IOMMU_HWPT_ALLOC_PASID;
hwpt_nested->viommu = viommu;
refcount_inc(&viommu->obj.users);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index f1e9b45ebd79..2d768fcfa398 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -296,6 +296,7 @@ struct iommufd_hw_pagetable {
struct iommufd_object obj;
struct iommu_domain *domain;
struct iommufd_fault *fault;
+ bool pasid_compat : 1;
};
struct iommufd_hwpt_paging {
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 14/21] iommufd: Support pasid attach/replace
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (12 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 13/21] iommufd: Enforce PASID-compatible domain in PASID path Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:35 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID Yi Liu
` (7 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
This extends the below APIs to support PASID. Device drivers to manage pasid
attach/replace/detach.
int iommufd_device_attach(struct iommufd_device *idev,
ioasid_t pasid, u32 *pt_id);
int iommufd_device_replace(struct iommufd_device *idev,
ioasid_t pasid, u32 *pt_id);
void iommufd_device_detach(struct iommufd_device *idev,
ioasid_t pasid);
The pasid operations share underlying attach/replace/detach infrastructure
with the device operations, but still have some different implications:
- 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;
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).
Signed-off-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/device.c | 57 ++++++++++++++++++++------------
drivers/iommu/iommufd/selftest.c | 8 ++---
drivers/vfio/iommufd.c | 10 +++---
include/linux/iommufd.h | 9 +++--
4 files changed, 52 insertions(+), 32 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index adeeb0145184..bceaee7376c2 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -527,9 +527,12 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
return rc;
}
- WARN_ON(pasid != IOMMU_NO_PASID);
- rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
- &handle->handle);
+ if (pasid == IOMMU_NO_PASID)
+ rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
+ &handle->handle);
+ else
+ rc = iommu_attach_device_pasid(hwpt->domain, idev->dev, pasid,
+ &handle->handle);
if (rc)
goto out_disable_iopf;
@@ -546,9 +549,11 @@ static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
ioasid_t pasid,
struct iommufd_attach_handle *handle)
{
- WARN_ON(pasid != IOMMU_NO_PASID);
+ if (pasid == IOMMU_NO_PASID)
+ iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
+ else
+ iommu_detach_device_pasid(hwpt->domain, idev->dev, pasid);
- iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
if (hwpt->fault) {
iommufd_auto_response_faults(hwpt, handle);
iommufd_fault_iopf_disable(idev);
@@ -563,8 +568,6 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
{
int rc;
- WARN_ON(pasid != IOMMU_NO_PASID);
-
rc = iommufd_hwpt_pasid_compat(hwpt, idev, pasid);
if (rc)
return rc;
@@ -575,8 +578,12 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
return rc;
}
- rc = iommu_replace_group_handle(idev->igroup->group, hwpt->domain,
- &handle->handle);
+ if (pasid == IOMMU_NO_PASID)
+ rc = iommu_replace_group_handle(idev->igroup->group,
+ hwpt->domain, &handle->handle);
+ else
+ rc = iommu_replace_device_pasid(hwpt->domain, idev->dev,
+ pasid, &handle->handle);
if (rc)
goto out_disable_iopf;
@@ -981,22 +988,25 @@ static int iommufd_device_change_pt(struct iommufd_device *idev,
}
/**
- * iommufd_device_attach - Connect a device to an iommu_domain
+ * iommufd_device_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_HWPT_PAGING
* Output the IOMMUFD_OBJ_HWPT_PAGING ID
*
- * This connects the device to an iommu_domain, either automatically or manually
- * selected. Once this completes the device could do DMA.
+ * This connects the device/pasid to an iommu_domain, either automatically
+ * or manually selected. Once this completes the device could do DMA with
+ * @pasid. @pasid is IOMMU_NO_PASID if this attach is for no pasid usage.
*
* The caller should return the resulting pt_id back to userspace.
* This function is undone by calling iommufd_device_detach().
*/
-int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
+int iommufd_device_attach(struct iommufd_device *idev, ioasid_t pasid,
+ u32 *pt_id)
{
int rc;
- rc = iommufd_device_change_pt(idev, IOMMU_NO_PASID, pt_id,
+ rc = iommufd_device_change_pt(idev, pasid, pt_id,
&iommufd_device_do_attach);
if (rc)
return rc;
@@ -1011,8 +1021,9 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
EXPORT_SYMBOL_NS_GPL(iommufd_device_attach, "IOMMUFD");
/**
- * iommufd_device_replace - Change the device's iommu_domain
+ * iommufd_device_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_HWPT_PAGING
* Output the IOMMUFD_OBJ_HWPT_PAGING ID
*
@@ -1023,27 +1034,31 @@ EXPORT_SYMBOL_NS_GPL(iommufd_device_attach, "IOMMUFD");
*
* 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_attach() has already succeeded.
+ * called if iommufd_device_attach() has already succeeded. @pasid is
+ * IOMMU_NO_PASID for no pasid usage.
*/
-int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id)
+int iommufd_device_replace(struct iommufd_device *idev, ioasid_t pasid,
+ u32 *pt_id)
{
- return iommufd_device_change_pt(idev, IOMMU_NO_PASID, pt_id,
+ return iommufd_device_change_pt(idev, pasid, pt_id,
&iommufd_device_do_replace);
}
EXPORT_SYMBOL_NS_GPL(iommufd_device_replace, "IOMMUFD");
/**
- * iommufd_device_detach - Disconnect a device to an iommu_domain
+ * iommufd_device_detach - Disconnect a device/device to an iommu_domain
* @idev: device to detach
+ * @pasid: pasid to detach
*
* Undo iommufd_device_attach(). This disconnects the idev from the previously
* attached pt_id. The device returns back to a blocked DMA translation.
+ * @pasid is IOMMU_NO_PASID for no pasid usage.
*/
-void iommufd_device_detach(struct iommufd_device *idev)
+void iommufd_device_detach(struct iommufd_device *idev, ioasid_t pasid)
{
struct iommufd_hw_pagetable *hwpt;
- hwpt = iommufd_hw_pagetable_detach(idev, IOMMU_NO_PASID);
+ hwpt = iommufd_hw_pagetable_detach(idev, pasid);
iommufd_hw_pagetable_put(idev->ictx, hwpt);
refcount_dec(&idev->obj.users);
}
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index d40deb0a4f06..84ad7b639fb3 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -921,7 +921,7 @@ static int iommufd_test_mock_domain(struct iommufd_ucmd *ucmd,
}
sobj->idev.idev = idev;
- rc = iommufd_device_attach(idev, &pt_id);
+ rc = iommufd_device_attach(idev, IOMMU_NO_PASID, &pt_id);
if (rc)
goto out_unbind;
@@ -936,7 +936,7 @@ static int iommufd_test_mock_domain(struct iommufd_ucmd *ucmd,
return 0;
out_detach:
- iommufd_device_detach(idev);
+ iommufd_device_detach(idev, IOMMU_NO_PASID);
out_unbind:
iommufd_device_unbind(idev);
out_mdev:
@@ -970,7 +970,7 @@ static int iommufd_test_mock_domain_replace(struct iommufd_ucmd *ucmd,
goto out_dev_obj;
}
- rc = iommufd_device_replace(sobj->idev.idev, &pt_id);
+ rc = iommufd_device_replace(sobj->idev.idev, IOMMU_NO_PASID, &pt_id);
if (rc)
goto out_dev_obj;
@@ -1603,7 +1603,7 @@ void iommufd_selftest_destroy(struct iommufd_object *obj)
switch (sobj->type) {
case TYPE_IDEV:
- iommufd_device_detach(sobj->idev.idev);
+ iommufd_device_detach(sobj->idev.idev, IOMMU_NO_PASID);
iommufd_device_unbind(sobj->idev.idev);
mock_dev_destroy(sobj->idev.mock_dev);
break;
diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
index 516294fd901b..37e1efa2c7bf 100644
--- a/drivers/vfio/iommufd.c
+++ b/drivers/vfio/iommufd.c
@@ -128,7 +128,7 @@ void vfio_iommufd_physical_unbind(struct vfio_device *vdev)
lockdep_assert_held(&vdev->dev_set->lock);
if (vdev->iommufd_attached) {
- iommufd_device_detach(vdev->iommufd_device);
+ iommufd_device_detach(vdev->iommufd_device, IOMMU_NO_PASID);
vdev->iommufd_attached = false;
}
iommufd_device_unbind(vdev->iommufd_device);
@@ -146,9 +146,11 @@ int vfio_iommufd_physical_attach_ioas(struct vfio_device *vdev, u32 *pt_id)
return -EINVAL;
if (vdev->iommufd_attached)
- rc = iommufd_device_replace(vdev->iommufd_device, pt_id);
+ rc = iommufd_device_replace(vdev->iommufd_device,
+ IOMMU_NO_PASID, pt_id);
else
- rc = iommufd_device_attach(vdev->iommufd_device, pt_id);
+ rc = iommufd_device_attach(vdev->iommufd_device,
+ IOMMU_NO_PASID, pt_id);
if (rc)
return rc;
vdev->iommufd_attached = true;
@@ -163,7 +165,7 @@ void vfio_iommufd_physical_detach_ioas(struct vfio_device *vdev)
if (WARN_ON(!vdev->iommufd_device) || !vdev->iommufd_attached)
return;
- iommufd_device_detach(vdev->iommufd_device);
+ iommufd_device_detach(vdev->iommufd_device, IOMMU_NO_PASID);
vdev->iommufd_attached = false;
}
EXPORT_SYMBOL_GPL(vfio_iommufd_physical_detach_ioas);
diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index 11110c749200..604241ed9cb2 100644
--- a/include/linux/iommufd.h
+++ b/include/linux/iommufd.h
@@ -8,6 +8,7 @@
#include <linux/err.h>
#include <linux/errno.h>
+#include <linux/iommu.h>
#include <linux/refcount.h>
#include <linux/types.h>
#include <linux/xarray.h>
@@ -52,9 +53,11 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
struct device *dev, u32 *id);
void iommufd_device_unbind(struct iommufd_device *idev);
-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_attach(struct iommufd_device *idev, ioasid_t pasid,
+ u32 *pt_id);
+int iommufd_device_replace(struct iommufd_device *idev, ioasid_t pasid,
+ u32 *pt_id);
+void iommufd_device_detach(struct iommufd_device *idev, ioasid_t pasid);
struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev);
u32 iommufd_device_to_id(struct iommufd_device *idev);
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (13 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 14/21] iommufd: Support pasid attach/replace Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:38 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 16/21] iommu/vt-d: Add IOMMU_HWPT_ALLOC_PASID support Yi Liu
` (6 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
Per the definition of IOMMU_HWPT_ALLOC_PASID, iommufd needs to enforce
the RID to use PASID-compatible domain if PASID has been attached, and
vice versa. The PASID path has already enforced it. This adds the
enforcement in the RID path.
This enforcement requires a lock across the RID and PASID attach path,
the idev->igroup->lock is used as both the RID and the PASID path holds
it.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
v8 -> v9: iommufd_hwpt_pasid_compat() is moved to prior patch
I didn't include r-b from Kevin and Jason as this patch has
quite a few change compared with v8. It's mainly due to the
introduction of pasid_attach for both RID and PASID path.
---
drivers/iommu/iommufd/device.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index bceaee7376c2..5db6dc705d44 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -503,8 +503,29 @@ static int iommufd_hwpt_pasid_compat(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev,
ioasid_t pasid)
{
- if (pasid != IOMMU_NO_PASID && !hwpt->pasid_compat)
- return -EINVAL;
+ struct iommufd_group *igroup = idev->igroup;
+
+ lockdep_assert_held(&igroup->lock);
+
+ if (pasid == IOMMU_NO_PASID) {
+ unsigned long start = IOMMU_NO_PASID;
+
+ if (!hwpt->pasid_compat &&
+ xa_find_after(&igroup->pasid_attach,
+ &start, UINT_MAX, XA_PRESENT))
+ return -EINVAL;
+ }
+
+ if (pasid != IOMMU_NO_PASID) {
+ struct iommufd_attach_handle *handle;
+
+ handle = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
+
+ if (!hwpt->pasid_compat ||
+ (handle && handle->hwpt && !handle->hwpt->pasid_compat))
+ return -EINVAL;
+ }
+
return 0;
}
@@ -515,8 +536,6 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
{
int rc;
- lockdep_assert_held(&idev->igroup->lock);
-
rc = iommufd_hwpt_pasid_compat(hwpt, idev, pasid);
if (rc)
return rc;
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 16/21] iommu/vt-d: Add IOMMU_HWPT_ALLOC_PASID support
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (14 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 17/21] iommufd: Allow allocating PASID-compatible domain Yi Liu
` (5 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
Intel iommu driver just treats it as a nop since Intel VT-d does not have
special requirement on domains attached to either the PASID or RID of a
PASID-capable device.
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/intel/iommu.c | 3 ++-
drivers/iommu/intel/nested.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index cc46098f875b..7bc890609b90 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3338,7 +3338,8 @@ intel_iommu_domain_alloc_paging_flags(struct device *dev, u32 flags,
bool first_stage;
if (flags &
- (~(IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING)))
+ (~(IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
+ IOMMU_HWPT_ALLOC_PASID)))
return ERR_PTR(-EOPNOTSUPP);
if (nested_parent && !nested_supported(iommu))
return ERR_PTR(-EOPNOTSUPP);
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index aba92c00b427..6ac5c534bef4 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -198,7 +198,7 @@ intel_iommu_domain_alloc_nested(struct device *dev, struct iommu_domain *parent,
struct dmar_domain *domain;
int ret;
- if (!nested_supported(iommu) || flags)
+ if (!nested_supported(iommu) || flags & ~IOMMU_HWPT_ALLOC_PASID)
return ERR_PTR(-EOPNOTSUPP);
/* Must be nested domain */
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 17/21] iommufd: Allow allocating PASID-compatible domain
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (15 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 16/21] iommu/vt-d: Add IOMMU_HWPT_ALLOC_PASID support Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:39 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 18/21] iommufd/selftest: Add set_dev_pasid in mock iommu Yi Liu
` (4 subsequent siblings)
21 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
The underlying infrastructure has supported the PASID attach and related
enforcement per the requirement of the IOMMU_HWPT_ALLOC_PASID flag. This
extends iommufd to support PASID compatible domain requested by userspace
or the PASID compatible domain allocated in the auto_domain path.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
v8 -> v9: Add description in uapi on how user should start using PASID. It
should attach its RID path to pasid-compat domain before attaching
any PASID.
---
drivers/iommu/iommufd/device.c | 4 +++-
drivers/iommu/iommufd/hw_pagetable.c | 7 ++++---
include/uapi/linux/iommufd.h | 3 +++
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 5db6dc705d44..70da39f5e227 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -929,7 +929,9 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev, ioasid_t pasid,
}
hwpt_paging = iommufd_hwpt_paging_alloc(idev->ictx, ioas, idev, pasid,
- 0, immediate_attach, NULL);
+ pasid != IOMMU_NO_PASID ?
+ IOMMU_HWPT_ALLOC_PASID : 0,
+ immediate_attach, NULL);
if (IS_ERR(hwpt_paging)) {
destroy_hwpt = ERR_CAST(hwpt_paging);
goto out_unlock;
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 7a86c164f514..2384b5db7eea 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -112,7 +112,8 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
{
const u32 valid_flags = IOMMU_HWPT_ALLOC_NEST_PARENT |
IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
- IOMMU_HWPT_FAULT_ID_VALID;
+ IOMMU_HWPT_FAULT_ID_VALID |
+ IOMMU_HWPT_ALLOC_PASID;
const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
struct iommufd_hwpt_paging *hwpt_paging;
struct iommufd_hw_pagetable *hwpt;
@@ -232,7 +233,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
struct iommufd_hw_pagetable *hwpt;
int rc;
- if ((flags & ~IOMMU_HWPT_FAULT_ID_VALID) ||
+ if ((flags & ~(IOMMU_HWPT_FAULT_ID_VALID | IOMMU_HWPT_ALLOC_PASID)) ||
!user_data->len || !ops->domain_alloc_nested)
return ERR_PTR(-EOPNOTSUPP);
if (parent->auto_domain || !parent->nest_parent ||
@@ -288,7 +289,7 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
struct iommufd_hw_pagetable *hwpt;
int rc;
- if (flags & ~IOMMU_HWPT_FAULT_ID_VALID)
+ if (flags & ~(IOMMU_HWPT_FAULT_ID_VALID | IOMMU_HWPT_ALLOC_PASID))
return ERR_PTR(-EOPNOTSUPP);
if (!user_data->len)
return ERR_PTR(-EOPNOTSUPP);
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 78747b24bd0f..75905f59b87f 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -392,6 +392,9 @@ struct iommu_vfio_ioas {
* Any domain attached to the non-PASID part of the
* device must also be flagged, otherwise attaching a
* PASID will blocked.
+ * For the user that wants to attach PASID, it should
+ * attach its RID to a domain flagged. It should not
+ * use the auto-allocated domain by kernel on RID.
* If IOMMU does not support PASID it will return
* error (-EOPNOTSUPP).
*/
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 18/21] iommufd/selftest: Add set_dev_pasid in mock iommu
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (16 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 17/21] iommufd: Allow allocating PASID-compatible domain Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 19/21] iommufd/selftest: Add a helper to get test device Yi Liu
` (3 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
The callback is needed to make pasid_attach/detach path complete for mock
device. A nop is enough for set_dev_pasid.
A MOCK_FLAGS_DEVICE_PASID is added to indicate a pasid-capable mock device
for the pasid test cases. Other test cases will still create a non-pasid
mock device. While the mock iommu always pretends to be pasid-capable.
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/iommufd_test.h | 1 +
drivers/iommu/iommufd/selftest.c | 33 ++++++++++++++++++++++++----
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommufd/iommufd_test.h b/drivers/iommu/iommufd/iommufd_test.h
index a6b7a163f636..bdc979557272 100644
--- a/drivers/iommu/iommufd/iommufd_test.h
+++ b/drivers/iommu/iommufd/iommufd_test.h
@@ -48,6 +48,7 @@ enum {
enum {
MOCK_FLAGS_DEVICE_NO_DIRTY = 1 << 0,
MOCK_FLAGS_DEVICE_HUGE_IOVA = 1 << 1,
+ MOCK_FLAGS_DEVICE_PASID = 1 << 2,
};
enum {
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 84ad7b639fb3..29faf3fac8e6 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -200,8 +200,16 @@ static int mock_domain_nop_attach(struct iommu_domain *domain,
return 0;
}
+static int mock_domain_set_dev_pasid_nop(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid,
+ struct iommu_domain *old)
+{
+ return 0;
+}
+
static const struct iommu_domain_ops mock_blocking_ops = {
.attach_dev = mock_domain_nop_attach,
+ .set_dev_pasid = mock_domain_set_dev_pasid_nop
};
static struct iommu_domain mock_blocking_domain = {
@@ -343,7 +351,7 @@ mock_domain_alloc_nested(struct device *dev, struct iommu_domain *parent,
struct mock_iommu_domain_nested *mock_nested;
struct mock_iommu_domain *mock_parent;
- if (flags)
+ if (flags & ~IOMMU_HWPT_ALLOC_PASID)
return ERR_PTR(-EOPNOTSUPP);
if (!parent || parent->ops != mock_ops.default_domain_ops)
return ERR_PTR(-EINVAL);
@@ -365,7 +373,8 @@ mock_domain_alloc_paging_flags(struct device *dev, u32 flags,
{
bool has_dirty_flag = flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
const u32 PAGING_FLAGS = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
- IOMMU_HWPT_ALLOC_NEST_PARENT;
+ IOMMU_HWPT_ALLOC_NEST_PARENT |
+ IOMMU_HWPT_ALLOC_PASID;
struct mock_dev *mdev = to_mock_dev(dev);
bool no_dirty_ops = mdev->flags & MOCK_FLAGS_DEVICE_NO_DIRTY;
struct mock_iommu_domain *mock;
@@ -585,7 +594,7 @@ mock_viommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,
struct mock_viommu *mock_viommu = to_mock_viommu(viommu);
struct mock_iommu_domain_nested *mock_nested;
- if (flags)
+ if (flags & ~IOMMU_HWPT_ALLOC_PASID)
return ERR_PTR(-EOPNOTSUPP);
mock_nested = __mock_domain_alloc_nested(user_data);
@@ -720,6 +729,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,
},
};
@@ -780,6 +790,7 @@ static struct iommu_domain_ops domain_nested_ops = {
.free = mock_domain_free_nested,
.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 *
@@ -839,11 +850,16 @@ static void mock_dev_release(struct device *dev)
static struct mock_dev *mock_dev_create(unsigned long dev_flags)
{
+ struct property_entry prop[] = {
+ PROPERTY_ENTRY_U32("pasid-num-bits", 20),
+ {},
+ };
struct mock_dev *mdev;
int rc, i;
if (dev_flags &
- ~(MOCK_FLAGS_DEVICE_NO_DIRTY | MOCK_FLAGS_DEVICE_HUGE_IOVA))
+ ~(MOCK_FLAGS_DEVICE_NO_DIRTY |
+ MOCK_FLAGS_DEVICE_HUGE_IOVA | MOCK_FLAGS_DEVICE_PASID))
return ERR_PTR(-EINVAL);
mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
@@ -866,6 +882,14 @@ static struct mock_dev *mock_dev_create(unsigned long dev_flags)
if (rc)
goto err_put;
+ if (dev_flags & MOCK_FLAGS_DEVICE_PASID) {
+ rc = device_create_managed_software_node(&mdev->dev, prop, NULL);
+ if (rc) {
+ dev_err(&mdev->dev, "add pasid-num-bits property failed, rc: %d", rc);
+ goto err_put;
+ }
+ }
+
rc = device_add(&mdev->dev);
if (rc)
goto err_put;
@@ -1724,6 +1748,7 @@ int __init iommufd_test_init(void)
init_completion(&mock_iommu.complete);
mock_iommu_iopf_queue = iopf_queue_alloc("mock-iopfq");
+ mock_iommu.iommu_dev.max_pasids = (1 << 20);
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 19/21] iommufd/selftest: Add a helper to get test device
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (17 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 18/21] iommufd/selftest: Add set_dev_pasid in mock iommu Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 20/21] iommufd/selftest: Add test ops to test pasid attach/detach Yi Liu
` (2 subsequent siblings)
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
There is need to get the selftest device (sobj->type == TYPE_IDEV) in
multiple places, so have a helper to for it.
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
drivers/iommu/iommufd/selftest.c | 36 ++++++++++++++++++++------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 29faf3fac8e6..d5b662d1671b 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -970,39 +970,49 @@ 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_selftest_obj(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 ERR_CAST(dev_obj);
sobj = to_selftest_obj(dev_obj);
if (sobj->type != TYPE_IDEV) {
- rc = -EINVAL;
- goto out_dev_obj;
+ iommufd_put_object(ictx, 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_selftest_obj(ucmd->ictx, device_id);
+ if (IS_ERR(sobj))
+ return PTR_ERR(sobj);
rc = iommufd_device_replace(sobj->idev.idev, IOMMU_NO_PASID, &pt_id);
if (rc)
- goto out_dev_obj;
+ goto out_sobj;
cmd->mock_domain_replace.pt_id = pt_id;
rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
-out_dev_obj:
- iommufd_put_object(ucmd->ictx, dev_obj);
+out_sobj:
+ iommufd_put_object(ucmd->ictx, &sobj->obj);
return rc;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 20/21] iommufd/selftest: Add test ops to test pasid attach/detach
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (18 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 19/21] iommufd/selftest: Add a helper to get test device Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 21/21] iommufd/selftest: Add coverage for iommufd " Yi Liu
2025-03-18 12:41 ` [PATCH v9 00/21] iommufd support pasid attach/replace Jason Gunthorpe
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
This adds 5 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.
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
v8 -> v9: Dropped the code for mixed_replace test
---
drivers/iommu/iommufd/iommufd_test.h | 31 ++++++
drivers/iommu/iommufd/selftest.c | 151 +++++++++++++++++++++++++++
2 files changed, 182 insertions(+)
diff --git a/drivers/iommu/iommufd/iommufd_test.h b/drivers/iommu/iommufd/iommufd_test.h
index bdc979557272..d034b6b8266a 100644
--- a/drivers/iommu/iommufd/iommufd_test.h
+++ b/drivers/iommu/iommufd/iommufd_test.h
@@ -24,6 +24,11 @@ enum {
IOMMU_TEST_OP_MD_CHECK_IOTLB,
IOMMU_TEST_OP_TRIGGER_IOPF,
IOMMU_TEST_OP_DEV_CHECK_CACHE,
+ IOMMU_TEST_OP_PASID_ATTACH,
+ IOMMU_TEST_OP_PASID_REPLACE,
+ IOMMU_TEST_OP_PASID_MIX_REPLACE_HANDLE,
+ IOMMU_TEST_OP_PASID_DETACH,
+ IOMMU_TEST_OP_PASID_CHECK_DOMAIN,
};
enum {
@@ -146,6 +151,32 @@ struct iommu_test_cmd {
__u32 id;
__u32 cache;
} check_dev_cache;
+ struct {
+ __u32 pasid;
+ __u32 pt_id;
+ /* @id is stdev_id
+ * pasid#1024 is for special test, do not use it
+ * in normal case.
+ */
+ } pasid_attach;
+ struct {
+ __u32 pasid;
+ __u32 pt_id;
+ /* @id is stdev_id
+ * pasid#1024 is for special test, do not use it
+ * in normal case.
+ */
+ } pasid_replace;
+ struct {
+ __u32 pasid;
+ /* @id is stdev_id */
+ } pasid_detach;
+ struct {
+ __u32 pasid;
+ __u32 hwpt_id;
+ __u64 out_result_ptr;
+ /* @id is stdev_id */
+ } pasid_check;
};
__u32 last;
};
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index d5b662d1671b..25df7aaadad7 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -200,10 +200,29 @@ static int mock_domain_nop_attach(struct iommu_domain *domain,
return 0;
}
+static bool pasid_1024_attached;
+
static int mock_domain_set_dev_pasid_nop(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid,
struct iommu_domain *old)
{
+ /*
+ * First attach with pasid 1024 succ, second attach would fail.
+ * This is helpful to test the case in which the iommu core needs
+ * to rollback to old domain due to driver failure.
+ */
+ if (pasid == 1024) {
+ if (domain->type == IOMMU_DOMAIN_BLOCKED) {
+ pasid_1024_attached = false;
+ } else if (pasid_1024_attached) {
+ pasid_1024_attached = false;
+ // Fake an error to fail the replacement
+ return -ENOMEM;
+ } else {
+ pasid_1024_attached = true;
+ }
+ }
+
return 0;
}
@@ -1631,6 +1650,129 @@ static int iommufd_test_trigger_iopf(struct iommufd_ucmd *ucmd,
return 0;
}
+static inline struct iommufd_hw_pagetable *
+iommufd_get_hwpt(struct iommufd_ucmd *ucmd, u32 id)
+{
+ struct iommufd_object *pt_obj;
+
+ pt_obj = iommufd_get_object(ucmd->ictx, id, IOMMUFD_OBJ_ANY);
+ if (IS_ERR(pt_obj))
+ return ERR_CAST(pt_obj);
+
+ if (pt_obj->type != IOMMUFD_OBJ_HWPT_NESTED &&
+ pt_obj->type != IOMMUFD_OBJ_HWPT_PAGING) {
+ iommufd_put_object(ucmd->ictx, pt_obj);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return container_of(pt_obj, struct iommufd_hw_pagetable, obj);
+}
+
+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 iommu_attach_handle *handle;
+ struct selftest_obj *sobj;
+ struct mock_dev *mdev;
+ bool result;
+ int rc = 0;
+
+ sobj = iommufd_test_get_selftest_obj(ucmd->ictx, cmd->id);
+ if (IS_ERR(sobj))
+ return PTR_ERR(sobj);
+
+ mdev = sobj->idev.mock_dev;
+
+ handle = iommu_attach_handle_get(mdev->dev.iommu_group,
+ cmd->pasid_check.pasid, 0);
+ if (IS_ERR(handle))
+ attached_domain = NULL;
+ else
+ attached_domain = handle->domain;
+
+ 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(ucmd->ictx, &hwpt->obj);
+out_put_dev:
+ iommufd_put_object(ucmd->ictx, &sobj->obj);
+ return rc;
+}
+
+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_selftest_obj(ucmd->ictx, cmd->id);
+ if (IS_ERR(sobj))
+ return PTR_ERR(sobj);
+
+ rc = iommufd_device_attach(sobj->idev.idev, cmd->pasid_attach.pasid,
+ &cmd->pasid_attach.pt_id);
+ if (rc)
+ goto out_sobj;
+
+ rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+ if (rc)
+ iommufd_device_detach(sobj->idev.idev,
+ cmd->pasid_attach.pasid);
+
+out_sobj:
+ iommufd_put_object(ucmd->ictx, &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_selftest_obj(ucmd->ictx, cmd->id);
+ if (IS_ERR(sobj))
+ return PTR_ERR(sobj);
+
+ rc = iommufd_device_replace(sobj->idev.idev, cmd->pasid_attach.pasid,
+ &cmd->pasid_attach.pt_id);
+ if (rc)
+ goto out_sobj;
+
+ rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+
+out_sobj:
+ iommufd_put_object(ucmd->ictx, &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_selftest_obj(ucmd->ictx, cmd->id);
+ if (IS_ERR(sobj))
+ return PTR_ERR(sobj);
+
+ iommufd_device_detach(sobj->idev.idev, cmd->pasid_detach.pasid);
+ iommufd_put_object(ucmd->ictx, &sobj->obj);
+ return 0;
+}
+
void iommufd_selftest_destroy(struct iommufd_object *obj)
{
struct selftest_obj *sobj = to_selftest_obj(obj);
@@ -1712,6 +1854,14 @@ int iommufd_test(struct iommufd_ucmd *ucmd)
cmd->dirty.flags);
case IOMMU_TEST_OP_TRIGGER_IOPF:
return iommufd_test_trigger_iopf(ucmd, cmd);
+ 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;
}
@@ -1759,6 +1909,7 @@ int __init iommufd_test_init(void)
mock_iommu_iopf_queue = iopf_queue_alloc("mock-iopfq");
mock_iommu.iommu_dev.max_pasids = (1 << 20);
+ pasid_1024_attached = false;
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v9 21/21] iommufd/selftest: Add coverage for iommufd pasid attach/detach
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (19 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 20/21] iommufd/selftest: Add test ops to test pasid attach/detach Yi Liu
@ 2025-03-13 12:35 ` Yi Liu
2025-03-18 12:41 ` [PATCH v9 00/21] iommufd support pasid attach/replace Jason Gunthorpe
21 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-13 12:35 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, yi.l.liu, iommu, nicolinc
This tests iommufd pasid attach/replace/detach.
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
tools/testing/selftests/iommu/iommufd.c | 344 ++++++++++++++++++
.../selftests/iommu/iommufd_fail_nth.c | 41 ++-
tools/testing/selftests/iommu/iommufd_utils.h | 102 ++++++
3 files changed, 480 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 618c03bb6509..c41d15e91983 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -2960,4 +2960,348 @@ TEST_F(iommufd_viommu, vdevice_cache)
}
}
+FIXTURE(iommufd_device_pasid)
+{
+ int fd;
+ uint32_t ioas_id;
+ uint32_t hwpt_id;
+ uint32_t stdev_id;
+ uint32_t device_id;
+ uint32_t no_pasid_stdev_id;
+ uint32_t no_pasid_device_id;
+};
+
+FIXTURE_VARIANT(iommufd_device_pasid)
+{
+ bool pasid_capable;
+};
+
+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_flags(self->ioas_id,
+ MOCK_FLAGS_DEVICE_PASID,
+ &self->stdev_id, &self->hwpt_id,
+ &self->device_id);
+ if (!variant->pasid_capable)
+ test_cmd_mock_domain_flags(self->ioas_id, 0,
+ &self->no_pasid_stdev_id, NULL,
+ &self->no_pasid_device_id);
+}
+
+FIXTURE_TEARDOWN(iommufd_device_pasid)
+{
+ teardown_iommufd(self->fd, _metadata);
+}
+
+FIXTURE_VARIANT_ADD(iommufd_device_pasid, no_pasid)
+{
+ .pasid_capable = false,
+};
+
+FIXTURE_VARIANT_ADD(iommufd_device_pasid, has_pasid)
+{
+ .pasid_capable = true,
+};
+
+TEST_F(iommufd_device_pasid, pasid_attach)
+{
+ struct iommu_hwpt_selftest data = {
+ .iotlb = IOMMU_TEST_IOTLB_DEFAULT,
+ };
+ uint32_t nested_hwpt_id[3] = {};
+ uint32_t parent_hwpt_id = 0;
+ uint32_t fault_id, fault_fd;
+ uint32_t s2_hwpt_id = 0;
+ uint32_t iopf_hwpt_id;
+ uint32_t pasid = 100;
+ uint32_t auto_hwpt;
+ uint32_t viommu_id;
+ 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,
+ IOMMU_HWPT_ALLOC_PASID,
+ &nested_hwpt_id[0],
+ IOMMU_HWPT_DATA_SELFTEST,
+ &data, sizeof(data));
+ test_cmd_hwpt_alloc_nested(self->device_id, parent_hwpt_id,
+ IOMMU_HWPT_ALLOC_PASID,
+ &nested_hwpt_id[1],
+ IOMMU_HWPT_DATA_SELFTEST,
+ &data, sizeof(data));
+
+ /* Faulte related preparation */
+ test_ioctl_fault_alloc(&fault_id, &fault_fd);
+ test_cmd_hwpt_alloc_iopf(self->device_id, parent_hwpt_id, fault_id,
+ IOMMU_HWPT_FAULT_ID_VALID | IOMMU_HWPT_ALLOC_PASID,
+ &iopf_hwpt_id,
+ IOMMU_HWPT_DATA_SELFTEST, &data,
+ sizeof(data));
+
+ /* Allocate a regular nested hwpt based on viommu */
+ test_cmd_viommu_alloc(self->device_id, parent_hwpt_id,
+ IOMMU_VIOMMU_TYPE_SELFTEST,
+ &viommu_id);
+ test_cmd_hwpt_alloc_nested(self->device_id, viommu_id,
+ IOMMU_HWPT_ALLOC_PASID,
+ &nested_hwpt_id[2],
+ IOMMU_HWPT_DATA_SELFTEST, &data,
+ sizeof(data));
+
+ test_cmd_hwpt_alloc(self->device_id, self->ioas_id,
+ IOMMU_HWPT_ALLOC_PASID,
+ &s2_hwpt_id);
+
+ /* Attach RID to non-pasid compat domain, */
+ test_cmd_mock_domain_replace(self->stdev_id, parent_hwpt_id);
+ /* then attach to pasid should fail */
+ test_err_pasid_attach(EINVAL, pasid, s2_hwpt_id, NULL);
+
+ /* Attach RID to pasid compat domain, */
+ test_cmd_mock_domain_replace(self->stdev_id, s2_hwpt_id);
+ /* then attach to pasid should succeed, */
+ test_cmd_pasid_attach(pasid, nested_hwpt_id[0], NULL);
+ /* but attach RID to non-pasid compat domain should fail now. */
+ test_err_mock_domain_replace(EINVAL, self->stdev_id, parent_hwpt_id);
+ test_cmd_pasid_detach(pasid);
+
+ if (!variant->pasid_capable) {
+ /*
+ * PASID-compatible domain can be used by non-PASID-capable
+ * device.
+ */
+ test_cmd_mock_domain_replace(self->no_pasid_stdev_id, nested_hwpt_id[0]);
+ test_cmd_mock_domain_replace(self->no_pasid_stdev_id, self->ioas_id);
+ /*
+ * Attach hwpt to pasid#100 of non-PASID-capable device,
+ * should fail, no matter domain is pasid-comapt or not.
+ */
+ EXPECT_ERRNO(EINVAL,
+ _test_cmd_pasid_attach(self->fd, self->no_pasid_stdev_id,
+ pasid, parent_hwpt_id, NULL));
+ EXPECT_ERRNO(EINVAL,
+ _test_cmd_pasid_attach(self->fd, self->no_pasid_stdev_id,
+ pasid, s2_hwpt_id, NULL));
+ }
+
+ /*
+ * Attach non pasid compat hwpt to pasid-capable device, should
+ * fail, and have null domain.
+ */
+ test_err_pasid_attach(EINVAL, pasid, parent_hwpt_id, NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, 0, &result));
+ EXPECT_EQ(1, result);
+
+ /*
+ * Attach ioas to pasid 100, should succeed, domain should
+ * be valid.
+ */
+ test_cmd_pasid_attach(pasid, self->ioas_id, &auto_hwpt);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, auto_hwpt, &result));
+ EXPECT_EQ(1, result);
+
+ /* Attach to pasid 100 which has been attached, should fail. */
+ test_err_pasid_attach(EBUSY, pasid, self->ioas_id, &auto_hwpt);
+
+ /*
+ * Try attach pasid 100 with another hwpt, should FAIL
+ * as attach does not allow overwrite, use REPLACE instead.
+ */
+ test_err_pasid_attach(EBUSY, pasid, nested_hwpt_id[0], NULL);
+
+ /*
+ * 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], NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, nested_hwpt_id[0],
+ &result));
+ EXPECT_EQ(1, result);
+
+ /* Attach to pasid 100 which has been attached, should fail. */
+ test_err_pasid_attach(EBUSY, pasid, nested_hwpt_id[0], NULL);
+
+ /*
+ * 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, s2_hwpt_id, NULL);
+
+ /*
+ * Attach a s2 hwpt to pasid 200, should succeed, domain should
+ * be valid.
+ */
+ test_cmd_pasid_attach(pasid, s2_hwpt_id, NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, s2_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, &auto_hwpt);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, auto_hwpt,
+ &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], NULL);
+ 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], NULL);
+ 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);
+
+ /* Negative Tests for pasid replace, use pasid 1024 */
+
+ /*
+ * Attach a s2 hwpt to pasid 1024, should succeed, domain should
+ * be valid.
+ */
+ pasid = 1024;
+ test_cmd_pasid_attach(pasid, s2_hwpt_id, NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, s2_hwpt_id,
+ &result));
+ EXPECT_EQ(1, result);
+
+ /*
+ * Replace pasid 1024 with self->ioas_id, should fail,
+ * but have the old valid domain. This is a designed
+ * negative case, normally replace with self->ioas_id
+ * could succeed.
+ */
+ test_err_cmd_pasid_replace(ENOMEM, pasid, self->ioas_id, NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, s2_hwpt_id,
+ &result));
+ EXPECT_EQ(1, result);
+
+ /*
+ * Detach hwpt from pasid 1024, and check if the pasid 1024
+ * 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);
+
+ /* Attach to iopf-capable hwpt */
+
+ /*
+ * Attach an iopf hwpt to pasid 2048, should succeed, domain should
+ * be valid.
+ */
+ pasid = 2048;
+ test_cmd_pasid_attach(pasid, iopf_hwpt_id, NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, iopf_hwpt_id,
+ &result));
+ EXPECT_EQ(1, result);
+
+ /*
+ * Replace with s2_hwpt_id for pasid 2048, should
+ * succeed, and have valid domain.
+ */
+ test_cmd_pasid_replace(pasid, s2_hwpt_id, NULL);
+ ASSERT_EQ(0,
+ test_cmd_pasid_check_domain(self->fd, self->stdev_id,
+ pasid, s2_hwpt_id,
+ &result));
+ EXPECT_EQ(1, result);
+
+ /*
+ * Detach hwpt from pasid 2048, and check if the pasid 2048
+ * 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(iopf_hwpt_id);
+ close(fault_fd);
+ test_ioctl_destroy(fault_id);
+
+ /* Detach the s2_hwpt_id from RID */
+ test_cmd_mock_domain_replace(self->stdev_id, self->ioas_id);
+
+ test_ioctl_destroy(nested_hwpt_id[0]);
+ test_ioctl_destroy(nested_hwpt_id[1]);
+ test_ioctl_destroy(nested_hwpt_id[2]);
+ test_ioctl_destroy(viommu_id);
+ test_ioctl_destroy(parent_hwpt_id);
+ test_ioctl_destroy(s2_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 64b1f8e1b0cf..6bbdc187a986 100644
--- a/tools/testing/selftests/iommu/iommufd_fail_nth.c
+++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c
@@ -209,12 +209,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)
@@ -226,6 +230,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);
}
@@ -623,7 +629,6 @@ TEST_FAIL_NTH(basic_fail_nth, device)
uint32_t fault_hwpt_id;
uint32_t ioas_id;
uint32_t ioas_id2;
- uint32_t stdev_id;
uint32_t idev_id;
uint32_t hwpt_id;
uint32_t viommu_id;
@@ -654,25 +659,29 @@ TEST_FAIL_NTH(basic_fail_nth, device)
fail_nth_enable();
- if (_test_cmd_mock_domain(self->fd, ioas_id, &stdev_id, NULL,
- &idev_id))
+ if (_test_cmd_mock_domain_flags(self->fd, ioas_id,
+ MOCK_FLAGS_DEVICE_PASID,
+ &self->stdev_id, NULL, &idev_id))
return -1;
if (_test_cmd_get_hw_info(self->fd, idev_id, &info, sizeof(info), NULL))
return -1;
- if (_test_cmd_hwpt_alloc(self->fd, idev_id, ioas_id, 0, 0, &hwpt_id,
+ if (_test_cmd_hwpt_alloc(self->fd, idev_id, ioas_id, 0,
+ IOMMU_HWPT_ALLOC_PASID, &hwpt_id,
IOMMU_HWPT_DATA_NONE, 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, stdev_id, hwpt_id, NULL))
+ if (_test_cmd_mock_domain_replace(self->fd, self->stdev_id, hwpt_id, NULL))
return -1;
if (_test_cmd_hwpt_alloc(self->fd, idev_id, ioas_id, 0,
- IOMMU_HWPT_ALLOC_NEST_PARENT, &hwpt_id,
+ IOMMU_HWPT_ALLOC_NEST_PARENT |
+ IOMMU_HWPT_ALLOC_PASID,
+ &hwpt_id,
IOMMU_HWPT_DATA_NONE, 0, 0))
return -1;
@@ -692,6 +701,24 @@ TEST_FAIL_NTH(basic_fail_nth, device)
IOMMU_HWPT_DATA_SELFTEST, &data, sizeof(data)))
return -1;
+ self->pasid = 200;
+
+ /* Tests for pasid attach/replace/detach */
+ if (_test_cmd_pasid_attach(self->fd, self->stdev_id,
+ self->pasid, ioas_id, NULL)) {
+ self->pasid = 0;
+ return -1;
+ }
+
+ if (_test_cmd_pasid_replace(self->fd, self->stdev_id,
+ self->pasid, ioas_id2, NULL))
+ 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 d979f5b0efe8..523ff28e4bc9 100644
--- a/tools/testing/selftests/iommu/iommufd_utils.h
+++ b/tools/testing/selftests/iommu/iommufd_utils.h
@@ -936,3 +936,105 @@ static int _test_cmd_vdevice_alloc(int fd, __u32 viommu_id, __u32 idev_id,
EXPECT_ERRNO(_errno, \
_test_cmd_vdevice_alloc(self->fd, viommu_id, idev_id, \
virt_id, vdev_id))
+
+static int _test_cmd_pasid_attach(int fd, __u32 stdev_id, __u32 pasid,
+ __u32 pt_id, __u32 *out_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,
+ },
+ };
+ int ret;
+
+ ret = ioctl(fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_PASID_ATTACH),
+ &test_attach);
+ if (ret)
+ return ret;
+
+ if (out_pt_id)
+ *out_pt_id = test_attach.pasid_attach.pt_id;
+ return 0;
+}
+
+#define test_cmd_pasid_attach(pasid, hwpt_id, out_pt_id) \
+ ASSERT_EQ(0, _test_cmd_pasid_attach(self->fd, self->stdev_id, \
+ pasid, hwpt_id, out_pt_id))
+
+#define test_err_pasid_attach(_errno, pasid, hwpt_id, out_pt_id) \
+ EXPECT_ERRNO(_errno, \
+ _test_cmd_pasid_attach(self->fd, self->stdev_id, \
+ pasid, hwpt_id, out_pt_id))
+
+static int _test_cmd_pasid_replace(int fd, __u32 stdev_id, __u32 pasid,
+ __u32 pt_id, __u32 *out_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,
+ },
+ };
+ int ret;
+
+ ret = ioctl(fd, _IOMMU_TEST_CMD(IOMMU_TEST_OP_PASID_REPLACE),
+ &test_replace);
+ if (ret)
+ return ret;
+
+ if (out_pt_id)
+ *out_pt_id = test_replace.pasid_replace.pt_id;
+ return 0;
+}
+
+#define test_cmd_pasid_replace(pasid, hwpt_id, out_pt_id) \
+ ASSERT_EQ(0, _test_cmd_pasid_replace(self->fd, self->stdev_id, \
+ pasid, hwpt_id, out_pt_id))
+
+#define test_err_cmd_pasid_replace(_errno, pasid, hwpt_id, out_pt_id) \
+ EXPECT_ERRNO(_errno, \
+ _test_cmd_pasid_replace(self->fd, self->stdev_id, \
+ pasid, hwpt_id, out_pt_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);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v9 12/21] iommufd/device: Add pasid_attach array to track per-PASID attach
2025-03-13 12:35 ` [PATCH v9 12/21] iommufd/device: Add pasid_attach array to track per-PASID attach Yi Liu
@ 2025-03-17 7:07 ` Yi Liu
0 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-17 7:07 UTC (permalink / raw)
To: kevin.tian, jgg; +Cc: joro, baolu.lu, iommu, nicolinc
On 2025/3/13 20:35, Yi Liu wrote:
> PASIDs of PASID-capable device can be attached to hwpt separately, hence
> a pasid array to track per-PASID attachment is necessary. The index
> IOMMU_NO_PASID is used by the RID path. Hence drop the igroup->handle.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/iommu/iommufd/device.c | 78 +++++++++++++++++--------
> drivers/iommu/iommufd/iommufd_private.h | 4 +-
> 2 files changed, 54 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 7d52f304b9bb..63b189a5b8ac 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -23,7 +23,7 @@ static void iommufd_group_release(struct kref *kref)
> struct iommufd_group *igroup =
> container_of(kref, struct iommufd_group, ref);
>
> - WARN_ON(igroup->handle);
> + WARN_ON(!xa_empty(&igroup->pasid_attach));
>
> xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
> NULL, GFP_KERNEL);
> @@ -90,6 +90,7 @@ static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
>
> kref_init(&new_igroup->ref);
> mutex_init(&new_igroup->lock);
> + xa_init(&new_igroup->pasid_attach);
> new_igroup->sw_msi_start = PHYS_ADDR_MAX;
> /* group reference moves into new_igroup */
> new_igroup->group = group;
> @@ -293,16 +294,19 @@ u32 iommufd_device_to_id(struct iommufd_device *idev)
> }
> EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, "IOMMUFD");
>
> -static int iommufd_group_device_num(struct iommufd_group *igroup)
> +static int iommufd_group_device_num(struct iommufd_group *igroup,
> + ioasid_t pasid)
> {
> + struct iommufd_attach_handle *handle;
> struct iommufd_device *idev;
> unsigned long index;
> int count = 0;
>
> lockdep_assert_held(&igroup->lock);
>
> - if (igroup->handle)
> - xa_for_each(&igroup->handle->device_array, index, idev)
> + handle = xa_load(&igroup->pasid_attach, pasid);
> + if (handle)
> + xa_for_each(&handle->device_array, index, idev)
> count++;
> return count;
> }
> @@ -472,7 +476,7 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
> if (rc)
> return rc;
>
> - if (!igroup->handle) {
> + if (!xa_load(&igroup->pasid_attach, IOMMU_NO_PASID)) {
> rc = iommufd_group_setup_msi(igroup, hwpt_paging);
> if (rc) {
> iopt_remove_reserved_iova(&hwpt_paging->ioas->iopt,
> @@ -485,10 +489,14 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
>
> /* The device attach/detach/replace helpers for attach_handle */
>
> -static bool iommufd_device_is_attached(struct iommufd_device *idev)
> +static bool iommufd_device_is_attached(struct iommufd_device *idev,
> + ioasid_t pasid)
> {
> - WARN_ON(!idev->igroup->handle);
> - return xa_load(&idev->igroup->handle->device_array, idev->obj.id);
> + struct iommufd_attach_handle *handle;
> +
> + handle = xa_load(&idev->igroup->pasid_attach, pasid);
> + WARN_ON(!handle);
> + return xa_load(&handle->device_array, idev->obj.id);
> }
>
> static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
> @@ -574,19 +582,25 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
> {
> struct iommufd_hwpt_paging *hwpt_paging = find_hwpt_paging(hwpt);
> bool attach_resv = hwpt_paging && pasid == IOMMU_NO_PASID;
> + struct iommufd_hw_pagetable *old_hwpt = NULL;
> struct iommufd_group *igroup = idev->igroup;
> - struct iommufd_hw_pagetable *old_hwpt;
> struct iommufd_attach_handle *handle;
> int rc;
>
> mutex_lock(&igroup->lock);
>
> - handle = igroup->handle;
> + handle = xa_cmpxchg(&igroup->pasid_attach, pasid, NULL,
> + XA_ZERO_ENTRY, GFP_KERNEL);
> + if (xa_is_err(handle)) {
> + rc = PTR_ERR(handle);
An issue was reported by Dan. This should be rc = xa_err(handle); Feel free
to let me know if a v10 is necessary to fix this.
https://lore.kernel.org/all/202503161104.CANtg5Ng-lkp@intel.com/
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 01/21] iommu: Clear handle->domain in detach
2025-03-13 12:35 ` [PATCH v9 01/21] iommu: Clear handle->domain in detach Yi Liu
@ 2025-03-18 11:46 ` Jason Gunthorpe
2025-03-18 13:30 ` Yi Liu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 11:46 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:12AM -0700, Yi Liu wrote:
> The iommu core sets the handle->domain in attach/replace. To be completed,
> it should clear handle->domain in detach as well to avoid potential side
> effect if caller reuses a handle.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/iommu/iommu.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> + entry = xa_erase(&group->pasid_array, pasid);
> + WARN_ON(!entry);
> + iommu_pasid_array_entry_clear_handle(entry);
Though if you have to respin it I'd probably put the WARN_ON inside
iommu_pasid_array_entry_clear_handle()
if (WARN_ON(!entry))
return;
Then it doesn't oops the kernel immediately after.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting
2025-03-13 12:35 ` [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting Yi Liu
@ 2025-03-18 12:04 ` Jason Gunthorpe
2025-03-18 14:40 ` Yi Liu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:04 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:13AM -0700, Yi Liu wrote:
> The IOMMU core does not mandate that callers must always provide a new
> handle, allowing for the possibility of handle reuse. In the replace
> path, the existing handle can be reused. To facilitate this, the core
> must ensure that the pasid_array entry is made or updated under xa_lock
> to prevent race conditions with callers of
> iommu_attach_handle_get().
I don't think that helps, the access to handle->domain is done unlocked:
static struct iommu_attach_handle *find_fault_handler(struct device *dev,
struct iopf_fault *evt)
{
[..]
if (!attach_handle->domain->iopf_handler)
return NULL;
And so on. So even with this locking change the domain value is unstable.
The driver still has to fence the iopf queue to flush out the domain
references during replace.
We decided the instability of fault delivery during replace is fine,
as it is logically OK for either domain to receive the fault.
What is problematic here is the repeated references to handle->domain
in the fault path without locking during handle reuse. It should be
using READ_ONCE(attach_handle->domain) and it should happen only once.
> Additionally, this operation should be performed only after the underlying
> IOMMU driver has successfully set the domain. This precaution is necessary
> to prevent forwarding PRIs to the new domain before it is fully prepared.
This can't work, we need to change the xarray, then have the driver
do the fencing to flush out the old xarray value from the fault path.
Otherwise the old handle and domain is still floating out there after
replace/attach returns which will UAF the domain pointer.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 03/21] iommu: Introduce a replace API for device pasid
2025-03-13 12:35 ` [PATCH v9 03/21] iommu: Introduce a replace API for device pasid Yi Liu
@ 2025-03-18 12:19 ` Jason Gunthorpe
2025-03-18 13:50 ` Baolu Lu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:19 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:14AM -0700, Yi Liu wrote:
> +int iommu_replace_device_pasid(struct iommu_domain *domain,
> + struct device *dev, ioasid_t pasid,
> + struct iommu_attach_handle *handle)
> +{
> + /* Caller must be a probed driver on dev */
> + struct iommu_group *group = dev->iommu_group;
> + struct iommu_domain *curr_domain;
> + void *curr;
> + int ret;
> +
> + if (!group)
> + return -ENODEV;
> +
> + if (!domain->ops->set_dev_pasid)
> + return -EOPNOTSUPP;
> +
> + if (dev_iommu_ops(dev) != domain->owner ||
> + pasid == IOMMU_NO_PASID || !handle)
> + return -EINVAL;
> +
> + mutex_lock(&group->mutex);
> + curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
> + XA_ZERO_ENTRY, GFP_KERNEL);
> + if (xa_is_err(curr)) {
> + ret = xa_err(curr);
> + goto out_unlock;
> + }
> +
> + /*
> + * No domain (with or without handle) attached, hence not
> + * a replace case.
> + */
> + if (!curr) {
> + xa_release(&group->pasid_array, pasid);
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + curr_domain = pasid_array_entry_to_domain(curr);
> + ret = 0;
> +
> + if (curr_domain != domain) {
> + ret = __iommu_set_group_pasid(domain, group,
> + pasid, curr_domain);
> + if (ret)
> + goto out_unlock;
> + }
> +
> + /*
> + * The above xa_cmpxchg() reserved the memory, and the
> + * group->mutex is held, this cannot fail.
> + */
> + iommu_group_pasid_store(group, pasid, curr, domain, handle);
This has the order problem as well :\
It is really problematic to fix since either placement of the store is
creating error case bugs.
Maybe we need to give up on having the driver fence the PRI hidden
inside attach calls and directly add a new fault fencing op. Then the
core code can sequence the fencing as required to make all the error
cases work and the driver doesn't see the complexity?
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable
2025-03-13 12:35 ` [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable Yi Liu
@ 2025-03-18 12:25 ` Jason Gunthorpe
2025-03-18 19:24 ` Nicolin Chen
1 sibling, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:25 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:17AM -0700, Yi Liu wrote:
> With more use of the fields of igroup, use a local vairable instead of
> using the idev->igroup heavily.
>
> No functional change expected.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/iommu/iommufd/device.c | 43 ++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 20 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 07/21] iommufd/device: Check !igroup->hwpt in iommufd_device_attach_reserved_iova()
2025-03-13 12:35 ` [PATCH v9 07/21] iommufd/device: Check !igroup->hwpt in iommufd_device_attach_reserved_iova() Yi Liu
@ 2025-03-18 12:27 ` Jason Gunthorpe
0 siblings, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:27 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:18AM -0700, Yi Liu wrote:
> iommufd_device_attach_reserved_iova() needs to handle the sw_msi when
> the first device of the group is attaching to the domain. Currently,
> it checks the igroup->device_list empty. However, !igroup->hwpt has the
> same effect. Use it can help to further transit the igroup->hwpt to
> iommufd_attach_handle. Hence prepare to add PASID support.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/iommu/iommufd/device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Though I wonder if this pattern should have been made into a little
inline helper.. igroup_first_attach(igroup, pasid) ?
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 08/21] iommufd/device: Lift iommufd_attach_handle handling to upper level helpers
2025-03-13 12:35 ` [PATCH v9 08/21] iommufd/device: Lift iommufd_attach_handle handling to upper level helpers Yi Liu
@ 2025-03-18 12:30 ` Jason Gunthorpe
0 siblings, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:30 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:19AM -0700, Yi Liu wrote:
> iommufd_attach_handle is allocated when attaching the first device of a
> group, and destroyed when the last device is detached. The life circle is
> aligned with the igroup->hwpt which is used to track attached hwpt of the
> group.
>
> This lifts the handle allocation/destroy to the upper level helpers. It
> prepares for using the handle to track hwpt attachment.
>
> No functional change is expected, except for that the replace path reuses
> the old handle instead of allocating a new one. This is needed when using
> handle to track attached devices of a group. Otherwise, the replace path
> would need to move the tracked devices to the new handle.
I see why you have the earlier patches.. Any chance we can not do
this? Reusing a handle is making any of the handle data unstable on
the unlocked fault path which is really tricky to combine with the
fencing semantic for cleanup.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-13 12:35 ` [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle Yi Liu
@ 2025-03-18 12:34 ` Jason Gunthorpe
2025-03-18 13:25 ` Yi Liu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:34 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:22AM -0700, Yi Liu wrote:
> By tracking both attached hwpt and devices, iommufd_attach_handle is
> prepared to be used as attach object. This prepares for adding per-PASID
> attachment by using handle.
So this is the patch that really requires re-using the handle..
The issue you are trying to solve is to avoid a second memory
allocation when storing the PASID in the xarray?
struct xarray_pasid_entry {
struct iommufd_attach_handle *handle;
struct xarray device_array;
};
?
I think I'd just accept the additional allocation and not try to make
handle re-usable. Every unique attach gets a unique handle allocation.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 14/21] iommufd: Support pasid attach/replace
2025-03-13 12:35 ` [PATCH v9 14/21] iommufd: Support pasid attach/replace Yi Liu
@ 2025-03-18 12:35 ` Jason Gunthorpe
0 siblings, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:35 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:25AM -0700, Yi Liu wrote:
> This extends the below APIs to support PASID. Device drivers to manage pasid
> attach/replace/detach.
>
> int iommufd_device_attach(struct iommufd_device *idev,
> ioasid_t pasid, u32 *pt_id);
> int iommufd_device_replace(struct iommufd_device *idev,
> ioasid_t pasid, u32 *pt_id);
> void iommufd_device_detach(struct iommufd_device *idev,
> ioasid_t pasid);
>
> The pasid operations share underlying attach/replace/detach infrastructure
> with the device operations, but still have some different implications:
>
> - 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;
>
> 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).
>
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/iommu/iommufd/device.c | 57 ++++++++++++++++++++------------
> drivers/iommu/iommufd/selftest.c | 8 ++---
> drivers/vfio/iommufd.c | 10 +++---
> include/linux/iommufd.h | 9 +++--
> 4 files changed, 52 insertions(+), 32 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID
2025-03-13 12:35 ` [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID Yi Liu
@ 2025-03-18 12:38 ` Jason Gunthorpe
2025-03-18 14:09 ` Yi Liu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:38 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:26AM -0700, Yi Liu wrote:
> Per the definition of IOMMU_HWPT_ALLOC_PASID, iommufd needs to enforce
> the RID to use PASID-compatible domain if PASID has been attached, and
> vice versa. The PASID path has already enforced it. This adds the
> enforcement in the RID path.
>
> This enforcement requires a lock across the RID and PASID attach path,
> the idev->igroup->lock is used as both the RID and the PASID path holds
> it.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> v8 -> v9: iommufd_hwpt_pasid_compat() is moved to prior patch
> I didn't include r-b from Kevin and Jason as this patch has
> quite a few change compared with v8. It's mainly due to the
> introduction of pasid_attach for both RID and PASID path.
> ---
> drivers/iommu/iommufd/device.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index bceaee7376c2..5db6dc705d44 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -503,8 +503,29 @@ static int iommufd_hwpt_pasid_compat(struct iommufd_hw_pagetable *hwpt,
> struct iommufd_device *idev,
> ioasid_t pasid)
> {
> - if (pasid != IOMMU_NO_PASID && !hwpt->pasid_compat)
> - return -EINVAL;
> + struct iommufd_group *igroup = idev->igroup;
> +
> + lockdep_assert_held(&igroup->lock);
> +
> + if (pasid == IOMMU_NO_PASID) {
> + unsigned long start = IOMMU_NO_PASID;
> +
> + if (!hwpt->pasid_compat &&
> + xa_find_after(&igroup->pasid_attach,
> + &start, UINT_MAX, XA_PRESENT))
> + return -EINVAL;
> + }
> +
> + if (pasid != IOMMU_NO_PASID) {
just use else
> + struct iommufd_attach_handle *handle;
> +
> + handle = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
> +
> + if (!hwpt->pasid_compat ||
I probably wouldn't put the hwpt test inside this if statement. The
hwpt test is always needed, this if statement is about checking if the
RID is compatible.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 17/21] iommufd: Allow allocating PASID-compatible domain
2025-03-13 12:35 ` [PATCH v9 17/21] iommufd: Allow allocating PASID-compatible domain Yi Liu
@ 2025-03-18 12:39 ` Jason Gunthorpe
0 siblings, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:39 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:28AM -0700, Yi Liu wrote:
> The underlying infrastructure has supported the PASID attach and related
> enforcement per the requirement of the IOMMU_HWPT_ALLOC_PASID flag. This
> extends iommufd to support PASID compatible domain requested by userspace
> or the PASID compatible domain allocated in the auto_domain path.
There is no auto_domain path anymore right?
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 00/21] iommufd support pasid attach/replace
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
` (20 preceding siblings ...)
2025-03-13 12:35 ` [PATCH v9 21/21] iommufd/selftest: Add coverage for iommufd " Yi Liu
@ 2025-03-18 12:41 ` Jason Gunthorpe
2025-03-18 14:37 ` Yi Liu
21 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 12:41 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Thu, Mar 13, 2025 at 05:35:11AM -0700, Yi Liu wrote:
> PASID (Process Address Space ID) is a PCIe extension that tags the DMA
> transactions from a physical device. Most modern IOMMU hardware supports
> PASID-granular address translation. This allows a PASID-capable device
> to be attached to multiple hardware page tables (hwpts, also known as
> domains), with each attachment tagged by a PASID.
I'm tempted punt the error case handling for PRI flushing to another
series as fixing it properly looks a bit big.. Maybe just code this
series to assume PASID attach success for now?
The attach handle locking thing needs fixing, I hope you can just
allocate new attach handles and not change that design, drop the first
two patches.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-18 12:34 ` Jason Gunthorpe
@ 2025-03-18 13:25 ` Yi Liu
2025-03-18 13:26 ` Jason Gunthorpe
0 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-18 13:25 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 20:34, Jason Gunthorpe wrote:
> On Thu, Mar 13, 2025 at 05:35:22AM -0700, Yi Liu wrote:
>> By tracking both attached hwpt and devices, iommufd_attach_handle is
>> prepared to be used as attach object. This prepares for adding per-PASID
>> attachment by using handle.
>
> So this is the patch that really requires re-using the handle..
>
> The issue you are trying to solve is to avoid a second memory
> allocation when storing the PASID in the xarray?
and another is to avoid copying all the tracked devices in the device_array
in the replace path.
> struct xarray_pasid_entry {
> struct iommufd_attach_handle *handle;
> struct xarray device_array;
> };
>
> ?
>
> I think I'd just accept the additional allocation and not try to make
> handle re-usable. Every unique attach gets a unique handle allocation.
If so, we also need to loop the device_array and add the attached devices
to the new device_array of the new handle.
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-18 13:25 ` Yi Liu
@ 2025-03-18 13:26 ` Jason Gunthorpe
2025-03-18 13:50 ` Yi Liu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 13:26 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Tue, Mar 18, 2025 at 09:25:56PM +0800, Yi Liu wrote:
> > struct iommufd_attach_handle *handle;
> > struct xarray device_array;
> > };
> >
> > ?
> >
> > I think I'd just accept the additional allocation and not try to make
> > handle re-usable. Every unique attach gets a unique handle allocation.
>
> If so, we also need to loop the device_array and add the attached devices
> to the new device_array of the new handle.
Why? Just don't use the handle for that, as above, the handle is still
a pointer that you allocate, and this pasid_enty is another pointer
that is allocated once.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 01/21] iommu: Clear handle->domain in detach
2025-03-18 11:46 ` Jason Gunthorpe
@ 2025-03-18 13:30 ` Yi Liu
0 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-18 13:30 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 19:46, Jason Gunthorpe wrote:
> On Thu, Mar 13, 2025 at 05:35:12AM -0700, Yi Liu wrote:
>> The iommu core sets the handle->domain in attach/replace. To be completed,
>> it should clear handle->domain in detach as well to avoid potential side
>> effect if caller reuses a handle.
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>> drivers/iommu/iommu.c | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>
>> + entry = xa_erase(&group->pasid_array, pasid);
>> + WARN_ON(!entry);
>> + iommu_pasid_array_entry_clear_handle(entry);
>
> Though if you have to respin it I'd probably put the WARN_ON inside
> iommu_pasid_array_entry_clear_handle()
>
> if (WARN_ON(!entry))
> return;
>
> Then it doesn't oops the kernel immediately after.
yes. I think at least one more version given the remarks in later patches.
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-18 13:26 ` Jason Gunthorpe
@ 2025-03-18 13:50 ` Yi Liu
2025-03-18 13:56 ` Jason Gunthorpe
0 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-18 13:50 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 21:26, Jason Gunthorpe wrote:
> On Tue, Mar 18, 2025 at 09:25:56PM +0800, Yi Liu wrote:
>>> struct iommufd_attach_handle *handle;
>>> struct xarray device_array;
>>> };
>>>
>>> ?
>>>
>>> I think I'd just accept the additional allocation and not try to make
>>> handle re-usable. Every unique attach gets a unique handle allocation.
>>
>> If so, we also need to loop the device_array and add the attached devices
>> to the new device_array of the new handle.
>
> Why? Just don't use the handle for that, as above, the handle is still
> a pointer that you allocate, and this pasid_enty is another pointer
> that is allocated once.
I see. How about making a new structure dedicate for tracking attached hwpt
and device_array? Like the below. Reusing iommufd_attach_handle structure
is not mandatory.
struct iommufd_attach {
struct iommufd_hw_pagetable *hwpt;
struct xarray device_array;
};
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 03/21] iommu: Introduce a replace API for device pasid
2025-03-18 12:19 ` Jason Gunthorpe
@ 2025-03-18 13:50 ` Baolu Lu
2025-03-18 13:57 ` Jason Gunthorpe
0 siblings, 1 reply; 48+ messages in thread
From: Baolu Lu @ 2025-03-18 13:50 UTC (permalink / raw)
To: Jason Gunthorpe, Yi Liu; +Cc: baolu.lu, kevin.tian, joro, iommu, nicolinc
On 3/18/2025 8:19 PM, Jason Gunthorpe wrote:
> On Thu, Mar 13, 2025 at 05:35:14AM -0700, Yi Liu wrote:
>> +int iommu_replace_device_pasid(struct iommu_domain *domain,
>> + struct device *dev, ioasid_t pasid,
>> + struct iommu_attach_handle *handle)
>> +{
>> + /* Caller must be a probed driver on dev */
>> + struct iommu_group *group = dev->iommu_group;
>> + struct iommu_domain *curr_domain;
>> + void *curr;
>> + int ret;
>> +
>> + if (!group)
>> + return -ENODEV;
>> +
>> + if (!domain->ops->set_dev_pasid)
>> + return -EOPNOTSUPP;
>> +
>> + if (dev_iommu_ops(dev) != domain->owner ||
>> + pasid == IOMMU_NO_PASID || !handle)
>> + return -EINVAL;
>> +
>> + mutex_lock(&group->mutex);
>> + curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
>> + XA_ZERO_ENTRY, GFP_KERNEL);
>> + if (xa_is_err(curr)) {
>> + ret = xa_err(curr);
>> + goto out_unlock;
>> + }
>> +
>> + /*
>> + * No domain (with or without handle) attached, hence not
>> + * a replace case.
>> + */
>> + if (!curr) {
>> + xa_release(&group->pasid_array, pasid);
>> + ret = -EINVAL;
>> + goto out_unlock;
>> + }
>> +
>> + curr_domain = pasid_array_entry_to_domain(curr);
>> + ret = 0;
>> +
>> + if (curr_domain != domain) {
>> + ret = __iommu_set_group_pasid(domain, group,
>> + pasid, curr_domain);
>> + if (ret)
>> + goto out_unlock;
>> + }
>> +
>> + /*
>> + * The above xa_cmpxchg() reserved the memory, and the
>> + * group->mutex is held, this cannot fail.
>> + */
>> + iommu_group_pasid_store(group, pasid, curr, domain, handle);
> This has the order problem as well :\
>
> It is really problematic to fix since either placement of the store is
> creating error case bugs.
>
> Maybe we need to give up on having the driver fence the PRI hidden
> inside attach calls and directly add a new fault fencing op. Then the
> core code can sequence the fencing as required to make all the error
> cases work and the driver doesn't see the complexity?
Good suggestion. A new fault fencing op will make the code much simpler.
Basically, iommu_replace_device_pasid() could be implemented like this:
- Replace the new domain on the hardware.
- Store the new handle in the xarray.
- Fence the PRI.
Before PRI fencing, the outstanding IOPFs might be routed through the
new or old handle, but that doesn't matter, as we discussed before.
After the PRI fencing, all IOPFs will use the new handle, and there will
not be any UAF issues anymore.
Do I understand it right?
Thanks,
baolu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-18 13:50 ` Yi Liu
@ 2025-03-18 13:56 ` Jason Gunthorpe
2025-03-18 14:13 ` Yi Liu
0 siblings, 1 reply; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 13:56 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Tue, Mar 18, 2025 at 09:50:12PM +0800, Yi Liu wrote:
> On 2025/3/18 21:26, Jason Gunthorpe wrote:
> > On Tue, Mar 18, 2025 at 09:25:56PM +0800, Yi Liu wrote:
> > > > struct iommufd_attach_handle *handle;
> > > > struct xarray device_array;
> > > > };
> > > >
> > > > ?
> > > >
> > > > I think I'd just accept the additional allocation and not try to make
> > > > handle re-usable. Every unique attach gets a unique handle allocation.
> > >
> > > If so, we also need to loop the device_array and add the attached devices
> > > to the new device_array of the new handle.
> >
> > Why? Just don't use the handle for that, as above, the handle is still
> > a pointer that you allocate, and this pasid_enty is another pointer
> > that is allocated once.
>
> I see. How about making a new structure dedicate for tracking attached hwpt
> and device_array? Like the below. Reusing iommufd_attach_handle structure
> is not mandatory.
>
> struct iommufd_attach {
> struct iommufd_hw_pagetable *hwpt;
> struct xarray device_array;
> };
Yeah, something like that would make sense. Then the handle is
allocated uniquely for every attach.
I'd also add info to the kdoc's that handle should not be reused as
part of the API requirement.
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 03/21] iommu: Introduce a replace API for device pasid
2025-03-18 13:50 ` Baolu Lu
@ 2025-03-18 13:57 ` Jason Gunthorpe
0 siblings, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 13:57 UTC (permalink / raw)
To: Baolu Lu; +Cc: Yi Liu, kevin.tian, joro, iommu, nicolinc
On Tue, Mar 18, 2025 at 09:50:20PM +0800, Baolu Lu wrote:
> > Maybe we need to give up on having the driver fence the PRI hidden
> > inside attach calls and directly add a new fault fencing op. Then the
> > core code can sequence the fencing as required to make all the error
> > cases work and the driver doesn't see the complexity?
>
> Good suggestion. A new fault fencing op will make the code much simpler.
> Basically, iommu_replace_device_pasid() could be implemented like this:
>
> - Replace the new domain on the hardware.
> - Store the new handle in the xarray.
> - Fence the PRI.
>
> Before PRI fencing, the outstanding IOPFs might be routed through the
> new or old handle, but that doesn't matter, as we discussed before.
> After the PRI fencing, all IOPFs will use the new handle, and there will
> not be any UAF issues anymore.
>
> Do I understand it right?
Yes
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID
2025-03-18 12:38 ` Jason Gunthorpe
@ 2025-03-18 14:09 ` Yi Liu
0 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-18 14:09 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 20:38, Jason Gunthorpe wrote:
> On Thu, Mar 13, 2025 at 05:35:26AM -0700, Yi Liu wrote:
>> Per the definition of IOMMU_HWPT_ALLOC_PASID, iommufd needs to enforce
>> the RID to use PASID-compatible domain if PASID has been attached, and
>> vice versa. The PASID path has already enforced it. This adds the
>> enforcement in the RID path.
>>
>> This enforcement requires a lock across the RID and PASID attach path,
>> the idev->igroup->lock is used as both the RID and the PASID path holds
>> it.
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>> v8 -> v9: iommufd_hwpt_pasid_compat() is moved to prior patch
>> I didn't include r-b from Kevin and Jason as this patch has
>> quite a few change compared with v8. It's mainly due to the
>> introduction of pasid_attach for both RID and PASID path.
>> ---
>> drivers/iommu/iommufd/device.c | 27 +++++++++++++++++++++++----
>> 1 file changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
>> index bceaee7376c2..5db6dc705d44 100644
>> --- a/drivers/iommu/iommufd/device.c
>> +++ b/drivers/iommu/iommufd/device.c
>> @@ -503,8 +503,29 @@ static int iommufd_hwpt_pasid_compat(struct iommufd_hw_pagetable *hwpt,
>> struct iommufd_device *idev,
>> ioasid_t pasid)
>> {
>> - if (pasid != IOMMU_NO_PASID && !hwpt->pasid_compat)
>> - return -EINVAL;
>> + struct iommufd_group *igroup = idev->igroup;
>> +
>> + lockdep_assert_held(&igroup->lock);
>> +
>> + if (pasid == IOMMU_NO_PASID) {
>> + unsigned long start = IOMMU_NO_PASID;
>> +
>> + if (!hwpt->pasid_compat &&
>> + xa_find_after(&igroup->pasid_attach,
>> + &start, UINT_MAX, XA_PRESENT))
>> + return -EINVAL;
>> + }
>> +
>> + if (pasid != IOMMU_NO_PASID) {
>
> just use else
>
>> + struct iommufd_attach_handle *handle;
>> +
>> + handle = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
>> +
>> + if (!hwpt->pasid_compat ||
>
> I probably wouldn't put the hwpt test inside this if statement. The
> hwpt test is always needed, this if statement is about checking if the
> RID is compatible.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
sure. let me add the hwpt test in a separate if.
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-18 13:56 ` Jason Gunthorpe
@ 2025-03-18 14:13 ` Yi Liu
2025-03-18 14:32 ` Jason Gunthorpe
0 siblings, 1 reply; 48+ messages in thread
From: Yi Liu @ 2025-03-18 14:13 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 21:56, Jason Gunthorpe wrote:
> On Tue, Mar 18, 2025 at 09:50:12PM +0800, Yi Liu wrote:
>> On 2025/3/18 21:26, Jason Gunthorpe wrote:
>>> On Tue, Mar 18, 2025 at 09:25:56PM +0800, Yi Liu wrote:
>>>>> struct iommufd_attach_handle *handle;
>>>>> struct xarray device_array;
>>>>> };
>>>>>
>>>>> ?
>>>>>
>>>>> I think I'd just accept the additional allocation and not try to make
>>>>> handle re-usable. Every unique attach gets a unique handle allocation.
>>>>
>>>> If so, we also need to loop the device_array and add the attached devices
>>>> to the new device_array of the new handle.
>>>
>>> Why? Just don't use the handle for that, as above, the handle is still
>>> a pointer that you allocate, and this pasid_enty is another pointer
>>> that is allocated once.
>>
>> I see. How about making a new structure dedicate for tracking attached hwpt
>> and device_array? Like the below. Reusing iommufd_attach_handle structure
>> is not mandatory.
>>
>> struct iommufd_attach {
>> struct iommufd_hw_pagetable *hwpt;
>> struct xarray device_array;
>> };
>
> Yeah, something like that would make sense. Then the handle is
> allocated uniquely for every attach.
yes, just keep it as it is today.
> I'd also add info to the kdoc's that handle should not be reused as
> part of the API requirement.
yes. Maybe we can check the stored handle and new handle to fail the same
handle case. I suppose the attach path is fine if reusing handle since
the detach path should fence it.
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle
2025-03-18 14:13 ` Yi Liu
@ 2025-03-18 14:32 ` Jason Gunthorpe
0 siblings, 0 replies; 48+ messages in thread
From: Jason Gunthorpe @ 2025-03-18 14:32 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On Tue, Mar 18, 2025 at 10:13:41PM +0800, Yi Liu wrote:
> yes. Maybe we can check the stored handle and new handle to fail the same
> handle case. I suppose the attach path is fine if reusing handle since
> the detach path should fence it.
Yes to both
Jason
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 00/21] iommufd support pasid attach/replace
2025-03-18 12:41 ` [PATCH v9 00/21] iommufd support pasid attach/replace Jason Gunthorpe
@ 2025-03-18 14:37 ` Yi Liu
0 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-18 14:37 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 20:41, Jason Gunthorpe wrote:
> On Thu, Mar 13, 2025 at 05:35:11AM -0700, Yi Liu wrote:
>> PASID (Process Address Space ID) is a PCIe extension that tags the DMA
>> transactions from a physical device. Most modern IOMMU hardware supports
>> PASID-granular address translation. This allows a PASID-capable device
>> to be attached to multiple hardware page tables (hwpts, also known as
>> domains), with each attachment tagged by a PASID.
>
> I'm tempted punt the error case handling for PRI flushing to another
> series as fixing it properly looks a bit big.. Maybe just code this
> series to assume PASID attach success for now?
>
> The attach handle locking thing needs fixing, I hope you can just
> allocate new attach handles and not change that design, drop the first
> two patches.
yes. I think the PRI path also needs to consider the multi-device groups
support or detection and the DMA alias stuffs.
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting
2025-03-18 12:04 ` Jason Gunthorpe
@ 2025-03-18 14:40 ` Yi Liu
0 siblings, 0 replies; 48+ messages in thread
From: Yi Liu @ 2025-03-18 14:40 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: kevin.tian, joro, baolu.lu, iommu, nicolinc
On 2025/3/18 20:04, Jason Gunthorpe wrote:
> On Thu, Mar 13, 2025 at 05:35:13AM -0700, Yi Liu wrote:
>> The IOMMU core does not mandate that callers must always provide a new
>> handle, allowing for the possibility of handle reuse. In the replace
>> path, the existing handle can be reused. To facilitate this, the core
>> must ensure that the pasid_array entry is made or updated under xa_lock
>> to prevent race conditions with callers of
>> iommu_attach_handle_get().
>
> I don't think that helps, the access to handle->domain is done unlocked:
>
> static struct iommu_attach_handle *find_fault_handler(struct device *dev,
> struct iopf_fault *evt)
> {
> [..]
> if (!attach_handle->domain->iopf_handler)
> return NULL;
>
> And so on. So even with this locking change the domain value is unstable.
>
> The driver still has to fence the iopf queue to flush out the domain
> references during replace.
>
> We decided the instability of fault delivery during replace is fine,
> as it is logically OK for either domain to receive the fault.
>
> What is problematic here is the repeated references to handle->domain
> in the fault path without locking during handle reuse. It should be
> using READ_ONCE(attach_handle->domain) and it should happen only once.
>
>> Additionally, this operation should be performed only after the underlying
>> IOMMU driver has successfully set the domain. This precaution is necessary
>> to prevent forwarding PRIs to the new domain before it is fully prepared.
>
> This can't work, we need to change the xarray, then have the driver
> do the fencing to flush out the old xarray value from the fault path.
>
> Otherwise the old handle and domain is still floating out there after
> replace/attach returns which will UAF the domain pointer.
yeah, let's go forward with the agreement in patch 11 of this series.
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 05/21] iommufd/device: Only add reserved_iova in non-pasid path
2025-03-13 12:35 ` [PATCH v9 05/21] iommufd/device: Only add reserved_iova in non-pasid path Yi Liu
@ 2025-03-18 19:22 ` Nicolin Chen
0 siblings, 0 replies; 48+ messages in thread
From: Nicolin Chen @ 2025-03-18 19:22 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, jgg, joro, baolu.lu, iommu
On Thu, Mar 13, 2025 at 05:35:16AM -0700, Yi Liu wrote:
> As the pasid is passed through the attach/replace/detach helpers, it is
> necessary to ensure only the non-pasid path adds reserved_iova.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable
2025-03-13 12:35 ` [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable Yi Liu
2025-03-18 12:25 ` Jason Gunthorpe
@ 2025-03-18 19:24 ` Nicolin Chen
1 sibling, 0 replies; 48+ messages in thread
From: Nicolin Chen @ 2025-03-18 19:24 UTC (permalink / raw)
To: Yi Liu; +Cc: kevin.tian, jgg, joro, baolu.lu, iommu
On Thu, Mar 13, 2025 at 05:35:17AM -0700, Yi Liu wrote:
> With more use of the fields of igroup, use a local vairable instead of
> using the idev->igroup heavily.
>
> No functional change expected.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2025-03-18 19:24 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13 12:35 [PATCH v9 00/21] iommufd support pasid attach/replace Yi Liu
2025-03-13 12:35 ` [PATCH v9 01/21] iommu: Clear handle->domain in detach Yi Liu
2025-03-18 11:46 ` Jason Gunthorpe
2025-03-18 13:30 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 02/21] iommu: Wrap pasid_array entry creation and setting Yi Liu
2025-03-18 12:04 ` Jason Gunthorpe
2025-03-18 14:40 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 03/21] iommu: Introduce a replace API for device pasid Yi Liu
2025-03-18 12:19 ` Jason Gunthorpe
2025-03-18 13:50 ` Baolu Lu
2025-03-18 13:57 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 04/21] iommufd: Pass @pasid through the device attach/replace path Yi Liu
2025-03-13 12:35 ` [PATCH v9 05/21] iommufd/device: Only add reserved_iova in non-pasid path Yi Liu
2025-03-18 19:22 ` Nicolin Chen
2025-03-13 12:35 ` [PATCH v9 06/21] iommufd/device: Replace idev->igroup with local variable Yi Liu
2025-03-18 12:25 ` Jason Gunthorpe
2025-03-18 19:24 ` Nicolin Chen
2025-03-13 12:35 ` [PATCH v9 07/21] iommufd/device: Check !igroup->hwpt in iommufd_device_attach_reserved_iova() Yi Liu
2025-03-18 12:27 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 08/21] iommufd/device: Lift iommufd_attach_handle handling to upper level helpers Yi Liu
2025-03-18 12:30 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 09/21] iommufd/device: Use iommufd_attach_handle track attachment Yi Liu
2025-03-13 12:35 ` [PATCH v9 10/21] iommufd/device: Replace device_list with device_array Yi Liu
2025-03-13 12:35 ` [PATCH v9 11/21] iommufd/device: Move attached device tracking to handle Yi Liu
2025-03-18 12:34 ` Jason Gunthorpe
2025-03-18 13:25 ` Yi Liu
2025-03-18 13:26 ` Jason Gunthorpe
2025-03-18 13:50 ` Yi Liu
2025-03-18 13:56 ` Jason Gunthorpe
2025-03-18 14:13 ` Yi Liu
2025-03-18 14:32 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 12/21] iommufd/device: Add pasid_attach array to track per-PASID attach Yi Liu
2025-03-17 7:07 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 13/21] iommufd: Enforce PASID-compatible domain in PASID path Yi Liu
2025-03-13 12:35 ` [PATCH v9 14/21] iommufd: Support pasid attach/replace Yi Liu
2025-03-18 12:35 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 15/21] iommufd: Enforce PASID-compatible domain for RID Yi Liu
2025-03-18 12:38 ` Jason Gunthorpe
2025-03-18 14:09 ` Yi Liu
2025-03-13 12:35 ` [PATCH v9 16/21] iommu/vt-d: Add IOMMU_HWPT_ALLOC_PASID support Yi Liu
2025-03-13 12:35 ` [PATCH v9 17/21] iommufd: Allow allocating PASID-compatible domain Yi Liu
2025-03-18 12:39 ` Jason Gunthorpe
2025-03-13 12:35 ` [PATCH v9 18/21] iommufd/selftest: Add set_dev_pasid in mock iommu Yi Liu
2025-03-13 12:35 ` [PATCH v9 19/21] iommufd/selftest: Add a helper to get test device Yi Liu
2025-03-13 12:35 ` [PATCH v9 20/21] iommufd/selftest: Add test ops to test pasid attach/detach Yi Liu
2025-03-13 12:35 ` [PATCH v9 21/21] iommufd/selftest: Add coverage for iommufd " Yi Liu
2025-03-18 12:41 ` [PATCH v9 00/21] iommufd support pasid attach/replace Jason Gunthorpe
2025-03-18 14:37 ` Yi Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.