Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers
On Thu, May 19, 2022 at 05:45:06PM +0800, Yi Liu wrote:
> Hi Jason,
>
> On 2022/3/19 01:27, Jason Gunthorpe wrote:
>
> > +/**
> > + * iommufd_device_attach - Connect a device to an iommu_domain
> > + * @idev: device to attach
> > + * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
> > + * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
> > + * @flags: Optional flags
> > + *
> > + * This connects the device to an iommu_domain, either automatically or
> > manually
> > + * selected. Once this completes the device could do DMA.
> > + *
> > + * 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,
> > + unsigned int flags)
> > +{
> Just double check here.
> This API doesn't prevent caller from calling this API multiple times with
> the same @idev and @pt_id. right? Note that idev has only one device_item
> list head. If caller does do multiple callings, then there should be
> problem. right? If so, this API assumes caller should take care of it and
> not do such bad function call. Is this the design here?
Yes, caller must ensure strict pairing, we don't have an assertion to
check it.
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers
Hi Jason,
On 2022/3/19 01:27, Jason Gunthorpe wrote:
+/**
+ * iommufd_device_attach - Connect a device to an iommu_domain
+ * @idev: device to attach
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ * @flags: Optional flags
+ *
+ * This connects the device to an iommu_domain, either automatically or
manually
+ * selected. Once this completes the device could do DMA.
+ *
+ * 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,
+ unsigned int flags)
+{
+ struct iommufd_hw_pagetable *hwpt;
+ int rc;
+
+ refcount_inc(>obj.users);
+
+ hwpt = iommufd_hw_pagetable_from_id(idev->ictx, *pt_id, idev->dev);
+ if (IS_ERR(hwpt)) {
+ rc = PTR_ERR(hwpt);
+ goto out_users;
+ }
+
+ mutex_lock(>devices_lock);
+ /* FIXME: Use a device-centric iommu api. For now check if the
+* hw_pagetable already has a device of the same group joined to tell if
+* we are the first and need to attach the group. */
+ if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+ phys_addr_t sw_msi_start = 0;
+
+ rc = iommu_attach_group(hwpt->domain, idev->group);
+ if (rc)
+ goto out_unlock;
+
+ /*
+* hwpt is now the exclusive owner of the group so this is the
+* first time enforce is called for this group.
+*/
+ rc = iopt_table_enforce_group_resv_regions(
+ >ioas->iopt, idev->group, _msi_start);
+ if (rc)
+ goto out_detach;
+ rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start, flags);
+ if (rc)
+ goto out_iova;
+ }
+
+ idev->hwpt = hwpt;
+ if (list_empty(>devices)) {
+ rc = iopt_table_add_domain(>ioas->iopt, hwpt->domain);
+ if (rc)
+ goto out_iova;
+ }
+ list_add(>devices_item, >devices);
Just double check here.
This API doesn't prevent caller from calling this API multiple times with
the same @idev and @pt_id. right? Note that idev has only one device_item
list head. If caller does do multiple callings, then there should be
problem. right? If so, this API assumes caller should take care of it and
not do such bad function call. Is this the design here?
+ mutex_unlock(>devices_lock);
+
+ *pt_id = idev->hwpt->obj.id;
+ return 0;
+
+out_iova:
+ iopt_remove_reserved_iova(>ioas->iopt, idev->group);
+out_detach:
+ iommu_detach_group(hwpt->domain, idev->group);
+out_unlock:
+ mutex_unlock(>devices_lock);
+ iommufd_hw_pagetable_put(idev->ictx, hwpt);
+out_users:
+ refcount_dec(>obj.users);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(iommufd_device_attach);
+
+void iommufd_device_detach(struct iommufd_device *idev)
+{
+ struct iommufd_hw_pagetable *hwpt = idev->hwpt;
+
+ mutex_lock(>devices_lock);
+ list_del(>devices_item);
+ if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+ iopt_remove_reserved_iova(>ioas->iopt, idev->group);
+ iommu_detach_group(hwpt->domain, idev->group);
+ }
+ if (list_empty(>devices))
+ iopt_table_remove_domain(>ioas->iopt, hwpt->domain);
+ mutex_unlock(>devices_lock);
+
+ iommufd_hw_pagetable_put(idev->ictx, hwpt);
+ idev->hwpt = NULL;
+
+ refcount_dec(>obj.users);
+}
+EXPORT_SYMBOL_GPL(iommufd_device_detach);
diff --git a/drivers/iommu/iommufd/iommufd_private.h
b/drivers/iommu/iommufd/iommufd_private.h
index c5c9650cc86818..e5c717231f851e 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -96,6 +96,7 @@ static inline int iommufd_ucmd_respond(struct iommufd_ucmd
*ucmd,
enum iommufd_object_type {
IOMMUFD_OBJ_NONE,
IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE,
+ IOMMUFD_OBJ_DEVICE,
IOMMUFD_OBJ_HW_PAGETABLE,
IOMMUFD_OBJ_IOAS,
IOMMUFD_OBJ_MAX,
@@ -196,6 +197,7 @@ struct iommufd_hw_pagetable {
struct iommufd_object obj;
struct iommufd_ioas *ioas;
struct iommu_domain *domain;
+ bool msi_cookie;
/* Head at iommufd_ioas::auto_domains */
struct list_head auto_domains_item;
struct mutex devices_lock;
@@ -209,4 +211,6 @@ void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
struct iommufd_hw_pagetable *hwpt);
void iommufd_hw_pagetable_destroy(struct iommufd_object *obj);
+void iommufd_device_destroy(struct iommufd_object *obj);
+
#endif
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index
Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers
On 2022/3/19 01:27, Jason Gunthorpe wrote:
+
+/**
+ * iommufd_device_attach - Connect a device to an iommu_domain
+ * @idev: device to attach
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ * @flags: Optional flags
+ *
+ * This connects the device to an iommu_domain, either automatically or
manually
+ * selected. Once this completes the device could do DMA.
+ *
+ * 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,
+ unsigned int flags)
+{
+ struct iommufd_hw_pagetable *hwpt;
+ int rc;
+
+ refcount_inc(>obj.users);
+
+ hwpt = iommufd_hw_pagetable_from_id(idev->ictx, *pt_id, idev->dev);
+ if (IS_ERR(hwpt)) {
+ rc = PTR_ERR(hwpt);
+ goto out_users;
+ }
+
+ mutex_lock(>devices_lock);
+ /* FIXME: Use a device-centric iommu api. For now check if the
+* hw_pagetable already has a device of the same group joined to tell if
+* we are the first and need to attach the group. */
+ if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+ phys_addr_t sw_msi_start = 0;
+
+ rc = iommu_attach_group(hwpt->domain, idev->group);
+ if (rc)
+ goto out_unlock;
+
+ /*
+* hwpt is now the exclusive owner of the group so this is the
+* first time enforce is called for this group.
+*/
+ rc = iopt_table_enforce_group_resv_regions(
+ >ioas->iopt, idev->group, _msi_start);
+ if (rc)
+ goto out_detach;
+ rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start, flags);
+ if (rc)
+ goto out_iova;
+ }
+
+ idev->hwpt = hwpt;
could the below list_empty check be moved to the above "if branch"? If
above "if branch" is false, that means there is already group attached with
the hwpt->domain. So the hwpt->devices should be non-empty. Only if the
above "if branch" is true, should the hwpt->devices possible to be empty.
So moving it into above "if branch" may be better?
+ if (list_empty(>devices)) {
+ rc = iopt_table_add_domain(>ioas->iopt, hwpt->domain);
+ if (rc)
+ goto out_iova;
+ }
+ list_add(>devices_item, >devices);
+ mutex_unlock(>devices_lock);
+
+ *pt_id = idev->hwpt->obj.id;
+ return 0;
+
+out_iova:
+ iopt_remove_reserved_iova(>ioas->iopt, idev->group);
+out_detach:
+ iommu_detach_group(hwpt->domain, idev->group);
+out_unlock:
+ mutex_unlock(>devices_lock);
+ iommufd_hw_pagetable_put(idev->ictx, hwpt);
+out_users:
+ refcount_dec(>obj.users);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(iommufd_device_attach);
--
Regards,
Yi Liu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers
On Wed, Mar 23, 2022 at 12:10:01PM -0600, Alex Williamson wrote: > > +EXPORT_SYMBOL_GPL(iommufd_bind_pci_device); > > I'm stumped why this needs to be PCI specific. Anything beyond the RID > comment? Please enlighten. Thanks, The way it turned out in the end it is not for a good reason any more. I'll change it Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers
On Fri, 18 Mar 2022 14:27:35 -0300
Jason Gunthorpe wrote:
> +/**
> + * iommufd_bind_pci_device - Bind a physical device to an iommu fd
> + * @fd: iommufd file descriptor.
> + * @pdev: Pointer to a physical PCI device struct
> + * @id: Output ID number to return to userspace for this device
> + *
> + * A successful bind establishes an ownership over the device and returns
> + * struct iommufd_device pointer, otherwise returns error pointer.
> + *
> + * A driver using this API must set driver_managed_dma and must not touch
> + * the device until this routine succeeds and establishes ownership.
> + *
> + * Binding a PCI device places the entire RID under iommufd control.
> + *
> + * The caller must undo this with iommufd_unbind_device()
> + */
> +struct iommufd_device *iommufd_bind_pci_device(int fd, struct pci_dev *pdev,
> +u32 *id)
> +{
> + struct iommufd_device *idev;
> + struct iommufd_ctx *ictx;
> + struct iommu_group *group;
> + int rc;
> +
> + ictx = iommufd_fget(fd);
> + if (!ictx)
> + return ERR_PTR(-EINVAL);
> +
> + group = iommu_group_get(>dev);
> + if (!group) {
> + rc = -ENODEV;
> + goto out_file_put;
> + }
> +
> + /*
> + * FIXME: Use a device-centric iommu api and this won't work with
> + * multi-device groups
> + */
> + rc = iommu_group_claim_dma_owner(group, ictx->filp);
> + if (rc)
> + goto out_group_put;
> +
> + idev = iommufd_object_alloc(ictx, idev, IOMMUFD_OBJ_DEVICE);
> + if (IS_ERR(idev)) {
> + rc = PTR_ERR(idev);
> + goto out_release_owner;
> + }
> + idev->ictx = ictx;
> + idev->dev = >dev;
> + /* The calling driver is a user until iommufd_unbind_device() */
> + refcount_inc(>obj.users);
> + /* group refcount moves into iommufd_device */
> + idev->group = group;
> +
> + /*
> + * If the caller fails after this success it must call
> + * iommufd_unbind_device() which is safe since we hold this refcount.
> + * This also means the device is a leaf in the graph and no other object
> + * can take a reference on it.
> + */
> + iommufd_object_finalize(ictx, >obj);
> + *id = idev->obj.id;
> + return idev;
> +
> +out_release_owner:
> + iommu_group_release_dma_owner(group);
> +out_group_put:
> + iommu_group_put(group);
> +out_file_put:
> + fput(ictx->filp);
> + return ERR_PTR(rc);
> +}
> +EXPORT_SYMBOL_GPL(iommufd_bind_pci_device);
I'm stumped why this needs to be PCI specific. Anything beyond the RID
comment? Please enlighten. Thanks,
Alex
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH RFC 10/12] iommufd: Add kAPI toward external drivers
Add the four functions external drivers need to connect physical DMA to
the IOMMUFD:
iommufd_bind_pci_device() / iommufd_unbind_device()
Register the device with iommufd and establish security isolation.
iommufd_device_attach() / iommufd_device_detach()
Connect a bound device to a page table
binding a device creates a device object ID in the uAPI, however the
generic API provides no IOCTLs to manipulate them.
An API to support the VFIO mdevs is a WIP at this point, but likely
involves requesting a struct iommufd_device without providing any struct
device, and then using the pin/unpin/rw operations on that iommufd_device.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/iommufd/Makefile | 1 +
drivers/iommu/iommufd/device.c | 274
drivers/iommu/iommufd/iommufd_private.h | 4 +
drivers/iommu/iommufd/main.c| 3 +
include/linux/iommufd.h | 50 +
5 files changed, 332 insertions(+)
create mode 100644 drivers/iommu/iommufd/device.c
create mode 100644 include/linux/iommufd.h
diff --git a/drivers/iommu/iommufd/Makefile b/drivers/iommu/iommufd/Makefile
index e13e971aa28c60..ca28a135b9675f 100644
--- a/drivers/iommu/iommufd/Makefile
+++ b/drivers/iommu/iommufd/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
iommufd-y := \
+ device.o \
hw_pagetable.o \
io_pagetable.o \
ioas.o \
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
new file mode 100644
index 00..c20bc9eab07e13
--- /dev/null
+++ b/drivers/iommu/iommufd/device.c
@@ -0,0 +1,274 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2021-2022, NVIDIA CORPORATION & AFFILIATES
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "iommufd_private.h"
+
+/*
+ * A iommufd_device object represents the binding relationship between a
+ * consuming driver and the iommufd. These objects are created/destroyed by
+ * external drivers, not by userspace.
+ */
+struct iommufd_device {
+ struct iommufd_object obj;
+ struct iommufd_ctx *ictx;
+ 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;
+};
+
+void iommufd_device_destroy(struct iommufd_object *obj)
+{
+ struct iommufd_device *idev =
+ container_of(obj, struct iommufd_device, obj);
+
+ iommu_group_release_dma_owner(idev->group);
+ iommu_group_put(idev->group);
+ fput(idev->ictx->filp);
+}
+
+/**
+ * iommufd_bind_pci_device - Bind a physical device to an iommu fd
+ * @fd: iommufd file descriptor.
+ * @pdev: Pointer to a physical PCI device struct
+ * @id: Output ID number to return to userspace for this device
+ *
+ * A successful bind establishes an ownership over the device and returns
+ * struct iommufd_device pointer, otherwise returns error pointer.
+ *
+ * A driver using this API must set driver_managed_dma and must not touch
+ * the device until this routine succeeds and establishes ownership.
+ *
+ * Binding a PCI device places the entire RID under iommufd control.
+ *
+ * The caller must undo this with iommufd_unbind_device()
+ */
+struct iommufd_device *iommufd_bind_pci_device(int fd, struct pci_dev *pdev,
+ u32 *id)
+{
+ struct iommufd_device *idev;
+ struct iommufd_ctx *ictx;
+ struct iommu_group *group;
+ int rc;
+
+ ictx = iommufd_fget(fd);
+ if (!ictx)
+ return ERR_PTR(-EINVAL);
+
+ group = iommu_group_get(>dev);
+ if (!group) {
+ rc = -ENODEV;
+ goto out_file_put;
+ }
+
+ /*
+* FIXME: Use a device-centric iommu api and this won't work with
+* multi-device groups
+*/
+ rc = iommu_group_claim_dma_owner(group, ictx->filp);
+ if (rc)
+ goto out_group_put;
+
+ idev = iommufd_object_alloc(ictx, idev, IOMMUFD_OBJ_DEVICE);
+ if (IS_ERR(idev)) {
+ rc = PTR_ERR(idev);
+ goto out_release_owner;
+ }
+ idev->ictx = ictx;
+ idev->dev = >dev;
+ /* The calling driver is a user until iommufd_unbind_device() */
+ refcount_inc(>obj.users);
+ /* group refcount moves into iommufd_device */
+ idev->group = group;
+
+ /*
+* If the caller fails after this success it must call
+* iommufd_unbind_device() which is safe since we hold this refcount.
+* This also means the device is a leaf in the graph and no other object
+* can take a reference on it.
+*/
+ iommufd_object_finalize(ictx, >obj);
+ *id = idev->obj.id;
+ return idev;
+
+out_release_owner:
+ iommu_group_release_dma_owner(group);
+out_group_put:
+
