From: Jason Gunthorpe <jgg@nvidia.com>
To: iommu@lists.linux.dev, Kevin Tian <kevin.tian@intel.com>,
linux-kselftest@vger.kernel.org
Cc: kvm@vger.kernel.org, Nicolin Chen <nicolinc@nvidia.com>,
Yi Liu <yi.l.liu@intel.com>
Subject: [PATCH 02/14] iommufd: Add iommufd_group
Date: Fri, 24 Feb 2023 20:27:47 -0400 [thread overview]
Message-ID: <2-v1-7612f88c19f5+2f21-iommufd_alloc_jgg@nvidia.com> (raw)
In-Reply-To: <0-v1-7612f88c19f5+2f21-iommufd_alloc_jgg@nvidia.com>
When the hwpt to device attachment is fairly static we could get
away with the simple approach of keeping track of the groups via a
device list. But with replace this is infeasible.
Add an automatically managed struct that is 1:1 with the iommu_group
per-ictx so we can store the necessary tracking information there.
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/iommu/iommufd/device.c | 128 +++++++++++++++++++++---
drivers/iommu/iommufd/iommufd_private.h | 9 +-
drivers/iommu/iommufd/main.c | 2 +
3 files changed, 123 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 63b65cdfe97f29..d1e227f310e823 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -15,13 +15,110 @@ MODULE_PARM_DESC(
"Allow IOMMUFD to bind to devices even if the platform cannot isolate "
"the MSI interrupt window. Enabling this is a security weakness.");
+static void iommufd_group_release(struct kref *kref)
+{
+ struct iommufd_group *igroup =
+ container_of(kref, struct iommufd_group, ref);
+
+ xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
+ NULL, GFP_KERNEL);
+ iommu_group_put(igroup->group);
+ kfree(igroup);
+}
+
+static void iommufd_put_group(struct iommufd_group *group)
+{
+ kref_put(&group->ref, iommufd_group_release);
+}
+
+/*
+ * iommufd needs to store some more data for each iommu_group, we keep a
+ * parallel xarray indexed by iommu_group id to hold this instead of putting it
+ * in the core structure. To keep things simple the iommufd_group memory is
+ * unique within the iommufd_ctx. This makes it easy to check there are no
+ * memory leaks.
+ */
+static struct iommufd_group *iommufd_get_group(struct iommufd_ctx *ictx,
+ struct device *dev)
+{
+ struct iommufd_group *new_igroup;
+ struct iommufd_group *cur_igroup;
+ struct iommufd_group *igroup;
+ struct iommu_group *group;
+ unsigned int id;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return ERR_PTR(-ENODEV);
+
+ id = iommu_group_id(group);
+
+ xa_lock(&ictx->groups);
+ igroup = xa_load(&ictx->groups, id);
+ if (igroup && igroup->group == group &&
+ kref_get_unless_zero(&igroup->ref)) {
+ xa_unlock(&ictx->groups);
+ iommu_group_put(group);
+ return igroup;
+ }
+ xa_unlock(&ictx->groups);
+
+ new_igroup = kzalloc(sizeof(*new_igroup), GFP_KERNEL);
+ if (!new_igroup) {
+ iommu_group_put(group);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ kref_init(&new_igroup->ref);
+ /* group reference moves into new_igroup */
+ new_igroup->group = group;
+
+ /*
+ * All objects using a group reference must put it before their destroy
+ * completes
+ */
+ new_igroup->ictx = ictx;
+
+ /*
+ * We dropped the lock so igroup is invalid. Assume that the
+ * xa had NULL in it, if this guess is wrong then we will obtain
+ * the actual value under lock and try again once.
+ */
+ cur_igroup = NULL;
+ xa_lock(&ictx->groups);
+ while (true) {
+ igroup = __xa_cmpxchg(&ictx->groups, id, cur_igroup, new_igroup,
+ GFP_KERNEL);
+ if (xa_is_err(igroup)) {
+ xa_unlock(&ictx->groups);
+ iommufd_put_group(new_igroup);
+ return ERR_PTR(xa_err(igroup));
+ }
+
+ /* new_group was successfully installed */
+ if (cur_igroup == igroup) {
+ xa_unlock(&ictx->groups);
+ return new_igroup;
+ }
+
+ /* Check again if the current group is any good */
+ if (igroup && igroup->group == group &&
+ kref_get_unless_zero(&igroup->ref)) {
+ xa_unlock(&ictx->groups);
+ iommufd_put_group(new_igroup);
+ return igroup;
+ }
+ cur_igroup = igroup;
+ }
+}
+
void iommufd_device_destroy(struct iommufd_object *obj)
{
struct iommufd_device *idev =
container_of(obj, struct iommufd_device, obj);
iommu_device_release_dma_owner(idev->dev);
- iommu_group_put(idev->group);
+ iommufd_put_group(idev->igroup);
if (!iommufd_selftest_is_mock_dev(idev->dev))
iommufd_ctx_put(idev->ictx);
}
@@ -46,7 +143,7 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
struct device *dev, u32 *id)
{
struct iommufd_device *idev;
- struct iommu_group *group;
+ struct iommufd_group *igroup;
int rc;
/*
@@ -56,9 +153,9 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
if (!device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY))
return ERR_PTR(-EINVAL);
- group = iommu_group_get(dev);
- if (!group)
- return ERR_PTR(-ENODEV);
+ igroup = iommufd_get_group(ictx, dev);
+ if (IS_ERR(igroup))
+ return ERR_CAST(igroup);
/*
* For historical compat with VFIO the insecure interrupt path is
@@ -67,7 +164,7 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
* interrupt outside this iommufd context.
*/
if (!iommufd_selftest_is_mock_dev(dev) &&
- !iommu_group_has_isolated_msi(group)) {
+ !iommu_group_has_isolated_msi(igroup->group)) {
if (!allow_unsafe_interrupts) {
rc = -EPERM;
goto out_group_put;
@@ -98,7 +195,7 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
/* The calling driver is a user until iommufd_device_unbind() */
refcount_inc(&idev->obj.users);
/* group refcount moves into iommufd_device */
- idev->group = group;
+ idev->igroup = igroup;
/*
* If the caller fails after this success it must call
@@ -113,7 +210,7 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
out_release_owner:
iommu_device_release_dma_owner(dev);
out_group_put:
- iommu_group_put(group);
+ iommufd_put_group(igroup);
return ERR_PTR(rc);
}
EXPORT_SYMBOL_NS_GPL(iommufd_device_bind, IOMMUFD);
@@ -170,14 +267,14 @@ static int iommufd_device_setup_msi(struct iommufd_device *idev,
}
static bool iommufd_hw_pagetable_has_group(struct iommufd_hw_pagetable *hwpt,
- struct iommu_group *group)
+ struct iommufd_group *igroup)
{
struct iommufd_device *cur_dev;
lockdep_assert_held(&hwpt->devices_lock);
list_for_each_entry(cur_dev, &hwpt->devices, devices_item)
- if (cur_dev->group == group)
+ if (cur_dev->igroup->group == igroup->group)
return true;
return false;
}
@@ -211,7 +308,8 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
}
rc = iopt_table_enforce_group_resv_regions(&hwpt->ioas->iopt, idev->dev,
- idev->group, &sw_msi_start);
+ idev->igroup->group,
+ &sw_msi_start);
if (rc)
return rc;
@@ -223,8 +321,8 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
* FIXME: Hack around missing a device-centric iommu api, only attach to
* the group once for the first device that is in the group.
*/
- if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
- rc = iommu_attach_group(hwpt->domain, idev->group);
+ if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup)) {
+ rc = iommu_attach_group(hwpt->domain, idev->igroup->group);
if (rc)
goto err_unresv;
}
@@ -237,8 +335,8 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
void iommufd_hw_pagetable_detach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev)
{
- if (!iommufd_hw_pagetable_has_group(hwpt, idev->group))
- iommu_detach_group(hwpt->domain, idev->group);
+ if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup))
+ iommu_detach_group(hwpt->domain, idev->igroup->group);
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
}
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index d523ef12890e1e..2544f10dae9aef 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -17,6 +17,7 @@ struct iommufd_device;
struct iommufd_ctx {
struct file *file;
struct xarray objects;
+ struct xarray groups;
u8 account_mode;
/* Compatibility with VFIO no iommu */
@@ -262,6 +263,12 @@ void iommufd_hw_pagetable_detach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev);
void iommufd_hw_pagetable_destroy(struct iommufd_object *obj);
+struct iommufd_group {
+ struct kref ref;
+ struct iommufd_ctx *ictx;
+ struct iommu_group *group;
+};
+
/*
* A iommufd_device object represents the binding relationship between a
* consuming driver and the iommufd. These objects are created/destroyed by
@@ -270,12 +277,12 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj);
struct iommufd_device {
struct iommufd_object obj;
struct iommufd_ctx *ictx;
+ struct iommufd_group *igroup;
struct iommufd_hw_pagetable *hwpt;
/* Head at iommufd_hw_pagetable::devices */
struct list_head devices_item;
/* always the physical device */
struct device *dev;
- struct iommu_group *group;
bool enforce_cache_coherency;
};
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index 3fbe636c3d8a69..e5ed5dfa91a0b5 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -183,6 +183,7 @@ static int iommufd_fops_open(struct inode *inode, struct file *filp)
}
xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT);
+ xa_init(&ictx->groups);
ictx->file = filp;
filp->private_data = ictx;
return 0;
@@ -218,6 +219,7 @@ static int iommufd_fops_release(struct inode *inode, struct file *filp)
if (WARN_ON(!destroyed))
break;
}
+ WARN_ON(!xa_empty(&ictx->groups));
kfree(ictx);
return 0;
}
--
2.39.1
next prev parent reply other threads:[~2023-02-25 0:28 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-25 0:27 [PATCH 00/14] Add iommufd physical device operations for replace and alloc hwpt Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 01/14] iommufd: Move isolated msi enforcement to iommufd_device_bind() Jason Gunthorpe
2023-03-02 7:45 ` Tian, Kevin
2023-02-25 0:27 ` Jason Gunthorpe [this message]
2023-03-02 7:55 ` [PATCH 02/14] iommufd: Add iommufd_group Tian, Kevin
2023-03-02 12:51 ` Jason Gunthorpe
2023-03-03 2:13 ` Tian, Kevin
2023-03-06 19:16 ` Jason Gunthorpe
2023-03-07 2:32 ` Tian, Kevin
2023-02-25 0:27 ` [PATCH 03/14] iommufd: Replace the hwpt->devices list with iommufd_group Jason Gunthorpe
2023-03-02 8:01 ` Tian, Kevin
2023-03-06 20:22 ` Jason Gunthorpe
2023-03-07 2:38 ` Tian, Kevin
2023-03-07 13:53 ` Jason Gunthorpe
2023-03-08 7:29 ` Tian, Kevin
2023-03-08 19:00 ` Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 04/14] iommufd: Use the iommufd_group to avoid duplicate reserved groups and msi setup Jason Gunthorpe
2023-03-02 8:06 ` Tian, Kevin
2023-03-02 12:55 ` Jason Gunthorpe
2023-03-03 2:16 ` Tian, Kevin
2023-02-25 0:27 ` [PATCH 05/14] iommufd: Make sw_msi_start a group global Jason Gunthorpe
2023-03-02 8:09 ` Tian, Kevin
2023-03-06 20:27 ` Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 06/14] iommufd: Move putting a hwpt to a helper function Jason Gunthorpe
2023-03-02 8:12 ` Tian, Kevin
2023-03-06 20:29 ` Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 07/14] iommufd: Add enforced_cache_coherency to iommufd_hw_pagetable_alloc() Jason Gunthorpe
2023-03-02 8:14 ` Tian, Kevin
2023-02-25 0:27 ` [PATCH 08/14] iommu: Introduce a new iommu_group_replace_domain() API Jason Gunthorpe
2023-03-02 8:16 ` Tian, Kevin
2023-02-25 0:27 ` [PATCH 09/14] iommufd: Add iommufd_device_replace() Jason Gunthorpe
2023-02-26 3:01 ` Baolu Lu
2023-02-27 13:58 ` Jason Gunthorpe
2023-02-28 1:50 ` Baolu Lu
2023-02-28 13:51 ` Jason Gunthorpe
2023-03-01 1:55 ` Baolu Lu
2023-02-26 3:13 ` Baolu Lu
2023-02-27 14:00 ` Jason Gunthorpe
2023-02-28 2:10 ` Baolu Lu
2023-02-28 13:52 ` Jason Gunthorpe
2023-03-01 2:23 ` Baolu Lu
2023-03-02 8:20 ` Tian, Kevin
2023-03-06 20:44 ` Jason Gunthorpe
2023-03-07 2:42 ` Tian, Kevin
2023-03-07 13:54 ` Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 10/14] iommufd: Make destroy_rwsem use a lock class per object type Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 11/14] iommufd/selftest: Test iommufd_device_replace() Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 12/14] iommufd: Add IOMMU_HWPT_ALLOC Jason Gunthorpe
2023-03-06 1:42 ` Nicolin Chen
2023-03-06 20:31 ` Jason Gunthorpe
2023-03-17 3:02 ` Tian, Kevin
2023-03-17 4:02 ` Nicolin Chen
2023-03-17 10:20 ` Tian, Kevin
2023-03-21 17:16 ` Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 13/14] iommufd/selftest: Return the real idev id from selftest mock_domain Jason Gunthorpe
2023-02-25 0:27 ` [PATCH 14/14] iommufd/selftest: Add a selftest for IOMMU_HWPT_ALLOC Jason Gunthorpe
2023-02-26 19:29 ` Nicolin Chen
2023-02-27 15:02 ` Jason Gunthorpe
2023-02-28 0:17 ` Nicolin Chen
2023-03-07 8:42 ` [PATCH 00/14] Add iommufd physical device operations for replace and alloc hwpt Tian, Kevin
2023-03-07 12:46 ` Jason Gunthorpe
2023-03-08 2:08 ` Baolu Lu
2023-03-08 7:38 ` Tian, Kevin
2023-03-08 18:59 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2-v1-7612f88c19f5+2f21-iommufd_alloc_jgg@nvidia.com \
--to=jgg@nvidia.com \
--cc=iommu@lists.linux.dev \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=nicolinc@nvidia.com \
--cc=yi.l.liu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).