2023-09-26 12:39:58

by Yi Liu

[permalink] [raw]
Subject: [RFC 3/8] iommufd: Support attach/replace hwpt per pasid

From: Kevin Tian <[email protected]>

This introduces three APIs for device drivers to manage pasid attach/
replace/detach.

int iommufd_device_pasid_attach(struct iommufd_device *idev,
u32 pasid, u32 *pt_id);
int iommufd_device_pasid_replace(struct iommufd_device *idev,
u32 pasid, u32 *pt_id);
void iommufd_device_pasid_detach(struct iommufd_device *idev,
u32 pasid);

pasid operations have different implications when comparing to device
operations:

- No connection to iommufd_group since pasid is a device capability
and can be enabled only in singleton group;

- no reserved region per pasid otherwise SVA architecture is already
broken (CPU address space doesn't count device reserved regions);

- accordingly no sw_msi trick;

- immediated_attach is not supported, expecting that arm-smmu driver
will already remove that requirement before supporting this pasid
operation. This avoids unnecessary change in iommufd_hw_pagetable_alloc()
to carry the pasid from device.c.

With above differences, this puts all pasid related logics into a new
pasid.c file.

Cache coherency enforcement is still applied to pasid operations since
it is about memory accesses post page table walking (no matter the walk
is per RID or per PASID).

Since the attach is per PASID, this introduces a pasid_hwpts xarray to
track the per-pasid attach data.

Signed-off-by: Kevin Tian <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
drivers/iommu/iommufd/Makefile | 1 +
drivers/iommu/iommufd/device.c | 9 +-
drivers/iommu/iommufd/iommufd_private.h | 8 ++
drivers/iommu/iommufd/pasid.c | 152 ++++++++++++++++++++++++
include/linux/iommufd.h | 6 +
5 files changed, 175 insertions(+), 1 deletion(-)
create mode 100644 drivers/iommu/iommufd/pasid.c

diff --git a/drivers/iommu/iommufd/Makefile b/drivers/iommu/iommufd/Makefile
index 8aeba81800c5..be83044a5101 100644
--- a/drivers/iommu/iommufd/Makefile
+++ b/drivers/iommu/iommufd/Makefile
@@ -6,6 +6,7 @@ iommufd-y := \
ioas.o \
main.o \
pages.o \
+ pasid.o \
vfio_compat.o

iommufd-$(CONFIG_IOMMUFD_TEST) += selftest.o
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 4fa4153c5df7..f88f31368eec 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -136,6 +136,7 @@ void iommufd_device_destroy(struct iommufd_object *obj)
struct iommufd_device *idev =
container_of(obj, struct iommufd_device, obj);

+ WARN_ON(!xa_empty(&idev->pasid_hwpts));
if (idev->has_user_data)
dev_iommu_ops(idev->dev)->unset_dev_user_data(idev->dev);
iommu_device_release_dma_owner(idev->dev);
@@ -218,6 +219,8 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
/* igroup refcount moves into iommufd_device */
idev->igroup = igroup;

+ xa_init(&idev->pasid_hwpts);
+
/*
* If the caller fails after this success it must call
* iommufd_unbind_device() which is safe since we hold this refcount.
@@ -600,7 +603,9 @@ iommufd_device_do_replace(struct iommufd_device *idev,
static struct iommufd_hw_pagetable *do_attach(struct iommufd_device *idev,
struct iommufd_hw_pagetable *hwpt, struct attach_data *data)
{
- return data->attach_fn(idev, hwpt);
+ return data->pasid == IOMMU_PASID_INVALID ?
+ data->attach_fn(idev, hwpt) :
+ data->pasid_attach_fn(idev, data->pasid, hwpt);
}

/*
@@ -747,6 +752,7 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
int rc;
struct attach_data data = {
.attach_fn = &iommufd_device_do_attach,
+ .pasid = IOMMU_PASID_INVALID
};

rc = iommufd_device_change_pt(idev, pt_id, &data);
@@ -781,6 +787,7 @@ int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id)
{
struct attach_data data = {
.attach_fn = &iommufd_device_do_replace,
+ .pasid = IOMMU_PASID_INVALID
};

return iommufd_device_change_pt(idev, pt_id, &data);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index f1fe4120c3b1..97f0e8f28d69 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -336,6 +336,7 @@ struct iommufd_device {
struct list_head group_item;
/* always the physical device */
struct device *dev;
+ struct xarray pasid_hwpts;
bool enforce_cache_coherency;
bool has_user_data;
};
@@ -358,9 +359,16 @@ struct attach_data {
struct iommufd_hw_pagetable *(*attach_fn)(
struct iommufd_device *idev,
struct iommufd_hw_pagetable *hwpt);
+ struct iommufd_hw_pagetable *(*pasid_attach_fn)(
+ struct iommufd_device *idev, u32 pasid,
+ struct iommufd_hw_pagetable *hwpt);
};
+ u32 pasid;
};

+int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id,
+ struct attach_data *data);
+
struct iommufd_access {
struct iommufd_object obj;
struct iommufd_ctx *ictx;
diff --git a/drivers/iommu/iommufd/pasid.c b/drivers/iommu/iommufd/pasid.c
new file mode 100644
index 000000000000..bdfdf5b2a48d
--- /dev/null
+++ b/drivers/iommu/iommufd/pasid.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2023, Intel Corporation
+ */
+#include <linux/iommufd.h>
+#include <linux/iommu.h>
+#include "../iommu-priv.h"
+
+#include "iommufd_private.h"
+
+static int __iommufd_device_pasid_do_attach(struct iommufd_device *idev,
+ u32 pasid,
+ struct iommufd_hw_pagetable *hwpt,
+ bool replace)
+{
+ int rc;
+
+ /*
+ * Try to upgrade the domain we have. This is also required for
+ * pasid attach since pasid only matters for identifying a hwpt
+ * while cache coherency is about memory access semantics post
+ * walking hwpt.
+ *
+ * Only for kernel-managed hwpt.
+ */
+ if (idev->enforce_cache_coherency && !hwpt->user_managed) {
+ rc = iommufd_hw_pagetable_enforce_cc(hwpt);
+ if (rc)
+ return rc;
+ }
+
+ if (!replace)
+ rc = iommu_attach_device_pasid(hwpt->domain, idev->dev, pasid);
+ else
+ rc = iommu_replace_device_pasid(hwpt->domain, idev->dev, pasid);
+ if (rc)
+ return rc;
+
+ refcount_inc(&hwpt->obj.users);
+ xa_store(&idev->pasid_hwpts, pasid, hwpt, GFP_KERNEL);
+ return 0;
+}
+
+static struct iommufd_hw_pagetable *
+iommufd_device_pasid_do_attach(struct iommufd_device *idev, u32 pasid,
+ struct iommufd_hw_pagetable *hwpt)
+{
+ struct iommufd_hw_pagetable *old_hwpt;
+ int rc;
+
+ old_hwpt = xa_load(&idev->pasid_hwpts, pasid);
+ if (old_hwpt) {
+ /* Attach does not allow overwrite */
+ if (old_hwpt == hwpt)
+ return NULL;
+ else
+ return ERR_PTR(-EINVAL);
+ }
+
+ rc = __iommufd_device_pasid_do_attach(idev, pasid, hwpt, false);
+ return rc ? ERR_PTR(rc) : NULL;
+}
+
+static struct iommufd_hw_pagetable *
+iommufd_device_pasid_do_replace(struct iommufd_device *idev, u32 pasid,
+ struct iommufd_hw_pagetable *hwpt)
+{
+ struct iommufd_hw_pagetable *old_hwpt;
+ int rc;
+
+ old_hwpt = xa_load(&idev->pasid_hwpts, pasid);
+ if (!old_hwpt)
+ return ERR_PTR(-EINVAL);
+
+ if (hwpt == old_hwpt)
+ return NULL;
+
+ rc = __iommufd_device_pasid_do_attach(idev, pasid, hwpt, true);
+ /* Caller must destroy old_hwpt */
+ return rc ? ERR_PTR(rc) : old_hwpt;
+}
+
+/**
+ * iommufd_device_pasid_attach - Connect a {device, pasid} to an iommu_domain
+ * @idev: device to attach
+ * @pasid: pasid to attach
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ *
+ * This connects a pasid of the device to an iommu_domain. Once this
+ * completes the device could do DMA with the pasid.
+ *
+ * This function is undone by calling iommufd_device_detach_pasid().
+ */
+int iommufd_device_pasid_attach(struct iommufd_device *idev,
+ u32 pasid, u32 *pt_id)
+{
+ struct attach_data data = {
+ .pasid_attach_fn = &iommufd_device_pasid_do_attach,
+ .pasid = pasid
+ };
+
+ return iommufd_device_change_pt(idev, pt_id, &data);
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_device_pasid_attach, IOMMUFD);
+
+/**
+ * iommufd_device_pasid_replace- Change the {device, pasid}'s iommu_domain
+ * @idev: device to change
+ * @pasid: pasid to change
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ *
+ * This is the same as
+ * iommufd_device_pasid_detach();
+ * iommufd_device_pasid_attach();
+ *
+ * If it fails then no change is made to the attachment. The iommu driver may
+ * implement this so there is no disruption in translation. This can only be
+ * called if iommufd_device_pasid_attach() has already succeeded.
+ */
+int iommufd_device_pasid_replace(struct iommufd_device *idev,
+ u32 pasid, u32 *pt_id)
+{
+ struct attach_data data = {
+ .pasid_attach_fn = &iommufd_device_pasid_do_replace,
+ .pasid = pasid
+ };
+
+ return iommufd_device_change_pt(idev, pt_id, &data);
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_device_pasid_replace, IOMMUFD);
+
+/**
+ * iommufd_device_pasid_detach - Disconnect a {device, pasid} to an iommu_domain
+ * @idev: device to detach
+ * @pasid: pasid to detach
+ *
+ * Undo iommufd_device_pasid_attach(). This disconnects the idev/pasid from
+ * the previously attached pt_id.
+ */
+void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid)
+{
+ struct iommufd_hw_pagetable *hwpt;
+
+ hwpt = xa_load(&idev->pasid_hwpts, pasid);
+ if (!hwpt)
+ return;
+ xa_erase(&idev->pasid_hwpts, pasid);
+ iommu_detach_device_pasid(hwpt->domain, idev->dev, pasid);
+ iommufd_hw_pagetable_put(idev->ictx, hwpt);
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_device_pasid_detach, IOMMUFD);
diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index ffc3a949f837..0b007c376306 100644
--- a/include/linux/iommufd.h
+++ b/include/linux/iommufd.h
@@ -26,6 +26,12 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id);
int iommufd_device_replace(struct iommufd_device *idev, u32 *pt_id);
void iommufd_device_detach(struct iommufd_device *idev);

+int iommufd_device_pasid_attach(struct iommufd_device *idev,
+ u32 pasid, u32 *pt_id);
+int iommufd_device_pasid_replace(struct iommufd_device *idev,
+ u32 pasid, u32 *pt_id);
+void iommufd_device_pasid_detach(struct iommufd_device *idev, u32 pasid);
+
struct iommufd_ctx *iommufd_device_to_ictx(struct iommufd_device *idev);
u32 iommufd_device_to_id(struct iommufd_device *idev);

--
2.34.1


2023-09-27 04:49:07

by Tian, Kevin

[permalink] [raw]
Subject: RE: [RFC 3/8] iommufd: Support attach/replace hwpt per pasid

> From: Baolu Lu <[email protected]>
> Sent: Wednesday, September 27, 2023 10:49 AM
>
> On 9/26/23 5:26 PM, Yi Liu wrote:
> > From: Kevin Tian<[email protected]>
> >
> > This introduces three APIs for device drivers to manage pasid attach/
> > replace/detach.
> >
> > int iommufd_device_pasid_attach(struct iommufd_device *idev,
> > u32 pasid, u32 *pt_id);
> > int iommufd_device_pasid_replace(struct iommufd_device *idev,
> > u32 pasid, u32 *pt_id);
> > void iommufd_device_pasid_detach(struct iommufd_device *idev,
> > u32 pasid);
>
> I am a bit puzzled. Do we really need both attach and replace interfaces
> to install a hwpt onto a pasid on device? The IOMMUFD already tracks the
> connections between hwpt and {device, pasid}, so it could easily call
> the right iommu interfaces (attach vs. replace). Perhaps I overlooked
> previous discussion on this.
>

attach means a transition from non-present to present.

replace can support changing a present entry atomically if iommu driver
support it.

the necessity of supporting both applies to both RID and RID+PASID.

2023-09-27 05:38:55

by Lu Baolu

[permalink] [raw]
Subject: Re: [RFC 3/8] iommufd: Support attach/replace hwpt per pasid

On 9/26/23 5:26 PM, Yi Liu wrote:
> From: Kevin Tian<[email protected]>
>
> This introduces three APIs for device drivers to manage pasid attach/
> replace/detach.
>
> int iommufd_device_pasid_attach(struct iommufd_device *idev,
> u32 pasid, u32 *pt_id);
> int iommufd_device_pasid_replace(struct iommufd_device *idev,
> u32 pasid, u32 *pt_id);
> void iommufd_device_pasid_detach(struct iommufd_device *idev,
> u32 pasid);

I am a bit puzzled. Do we really need both attach and replace interfaces
to install a hwpt onto a pasid on device? The IOMMUFD already tracks the
connections between hwpt and {device, pasid}, so it could easily call
the right iommu interfaces (attach vs. replace). Perhaps I overlooked
previous discussion on this.

Best regards,
baolu

2023-09-27 17:36:51

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [RFC 3/8] iommufd: Support attach/replace hwpt per pasid

On Wed, Sep 27, 2023 at 10:49:29AM +0800, Baolu Lu wrote:
> On 9/26/23 5:26 PM, Yi Liu wrote:
> > From: Kevin Tian<[email protected]>
> >
> > This introduces three APIs for device drivers to manage pasid attach/
> > replace/detach.
> >
> > int iommufd_device_pasid_attach(struct iommufd_device *idev,
> > u32 pasid, u32 *pt_id);
> > int iommufd_device_pasid_replace(struct iommufd_device *idev,
> > u32 pasid, u32 *pt_id);
> > void iommufd_device_pasid_detach(struct iommufd_device *idev,
> > u32 pasid);
>
> I am a bit puzzled. Do we really need both attach and replace interfaces
> to install a hwpt onto a pasid on device? The IOMMUFD already tracks the
> connections between hwpt and {device, pasid}, so it could easily call
> the right iommu interfaces (attach vs. replace). Perhaps I overlooked
> previous discussion on this.

It was a decision that attach will fail if something is already
attached..

But for this API we could go the way of the iommu code and have only
'set' and 'unset' as the two operations.

Jason

2023-09-28 03:05:18

by Tian, Kevin

[permalink] [raw]
Subject: RE: [RFC 3/8] iommufd: Support attach/replace hwpt per pasid

> From: Jason Gunthorpe <[email protected]>
> Sent: Wednesday, September 27, 2023 11:44 PM
>
> On Wed, Sep 27, 2023 at 10:49:29AM +0800, Baolu Lu wrote:
> > On 9/26/23 5:26 PM, Yi Liu wrote:
> > > From: Kevin Tian<[email protected]>
> > >
> > > This introduces three APIs for device drivers to manage pasid attach/
> > > replace/detach.
> > >
> > > int iommufd_device_pasid_attach(struct iommufd_device *idev,
> > > u32 pasid, u32 *pt_id);
> > > int iommufd_device_pasid_replace(struct iommufd_device *idev,
> > > u32 pasid, u32 *pt_id);
> > > void iommufd_device_pasid_detach(struct iommufd_device *idev,
> > > u32 pasid);
> >
> > I am a bit puzzled. Do we really need both attach and replace interfaces
> > to install a hwpt onto a pasid on device? The IOMMUFD already tracks the
> > connections between hwpt and {device, pasid}, so it could easily call
> > the right iommu interfaces (attach vs. replace). Perhaps I overlooked
> > previous discussion on this.
>
> It was a decision that attach will fail if something is already
> attached..
>
> But for this API we could go the way of the iommu code and have only
> 'set' and 'unset' as the two operations.
>

I'm not sure the benefit of doing so. Instead it makes the caller side
vfio more confusing by using attach/replace/detach for device vs.
using set/unset for pasid?