| Message ID | 12-v1-7612f88c19f5+2f21-iommufd_alloc_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add iommufd physical device operations for replace and alloc hwpt | expand |
On Fri, Feb 24, 2023 at 08:27:57PM -0400, Jason Gunthorpe wrote: > +int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) > +{ > + struct iommu_hwpt_alloc *cmd = ucmd->cmd; > + struct iommufd_hw_pagetable *hwpt; > + struct iommufd_device *idev; > + struct iommufd_ioas *ioas; > + int rc; > + > + if (cmd->flags) > + return -EOPNOTSUPP; > + > + idev = iommufd_get_device(ucmd, cmd->dev_id); > + if (IS_ERR(idev)) > + return PTR_ERR(idev); > + > + ioas = iommufd_get_ioas(ucmd, cmd->pt_id); > + if (IS_ERR(ioas)) { > + rc = PTR_ERR(idev); PTR_ERR(ioas) > + goto out_put_idev; > + } > + > + mutex_lock(&ioas->mutex); > + hwpt = iommufd_hw_pagetable_alloc(ucmd->ictx, ioas, idev, false); > + mutex_unlock(&ioas->mutex); > + if (IS_ERR(hwpt)) { > + rc = PTR_ERR(idev); PTR_ERR(hwpt) Thanks Nic
On Sun, Mar 05, 2023 at 05:42:56PM -0800, Nicolin Chen wrote: > On Fri, Feb 24, 2023 at 08:27:57PM -0400, Jason Gunthorpe wrote: > > > +int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) > > +{ > > + struct iommu_hwpt_alloc *cmd = ucmd->cmd; > > + struct iommufd_hw_pagetable *hwpt; > > + struct iommufd_device *idev; > > + struct iommufd_ioas *ioas; > > + int rc; > > + > > + if (cmd->flags) > > + return -EOPNOTSUPP; > > + > > + idev = iommufd_get_device(ucmd, cmd->dev_id); > > + if (IS_ERR(idev)) > > + return PTR_ERR(idev); > > + > > + ioas = iommufd_get_ioas(ucmd, cmd->pt_id); > > + if (IS_ERR(ioas)) { > > + rc = PTR_ERR(idev); > > PTR_ERR(ioas) > > > + goto out_put_idev; > > + } > > + > > + mutex_lock(&ioas->mutex); > > + hwpt = iommufd_hw_pagetable_alloc(ucmd->ictx, ioas, idev, false); > > + mutex_unlock(&ioas->mutex); > > + if (IS_ERR(hwpt)) { > > + rc = PTR_ERR(idev); > > PTR_ERR(hwpt) Oops, yep Thanks, Jason
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Saturday, February 25, 2023 8:28 AM > + > +int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) > +{ > + struct iommu_hwpt_alloc *cmd = ucmd->cmd; > + struct iommufd_hw_pagetable *hwpt; > + struct iommufd_device *idev; > + struct iommufd_ioas *ioas; > + int rc; > + > + if (cmd->flags) > + return -EOPNOTSUPP; miss a check on the __reserved field. > +/** > + * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC) > + * @size: sizeof(struct iommu_hwpt_alloc) > + * @flags: Must be 0 > + * @dev_id: The device to allocate this HWPT for > + * @pt_id: The IOAS to connect this HWPT to > + * @out_hwpt_id: The ID of the new HWPT > + * @__reserved: Must be 0 > + * > + * Explicitly allocate a hardware page table object. This is the same object > + * type that is returned by iommufd_device_attach() and represents the > + * underlying iommu driver's iommu_domain kernel object. > + * > + * A normal HWPT will be created with the mappings from the given IOAS. > + */ 'normal' is a confusing word in this context.
On Fri, Mar 17, 2023 at 03:02:26AM +0000, Tian, Kevin wrote: > > +/** > > + * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC) > > + * @size: sizeof(struct iommu_hwpt_alloc) > > + * @flags: Must be 0 > > + * @dev_id: The device to allocate this HWPT for > > + * @pt_id: The IOAS to connect this HWPT to > > + * @out_hwpt_id: The ID of the new HWPT > > + * @__reserved: Must be 0 > > + * > > + * Explicitly allocate a hardware page table object. This is the same object > > + * type that is returned by iommufd_device_attach() and represents the > > + * underlying iommu driver's iommu_domain kernel object. > > + * > > + * A normal HWPT will be created with the mappings from the given IOAS. > > + */ > > 'normal' is a confusing word in this context. Yea, Eric was asking about a related question in another thread, because he couldn't get this part. I think we could replace this "normal" with just "kernel-managed", so eventually it would look like the following narrative after adding user_data and nesting: * A kernel-managed HWPT will be created with the mappings from the given IOAS. * The @data_type for its allocation can be set to IOMMU_HWPT_TYPE_DEFAULT, or * another type (being listed below) to customize the allocation. * * A user-managed HWPT will be created from a given parent HWPT via @pt_id, in * which the parent HWPT must be allocated previously via the same ioctl from a * given IOAS. The @data_type must not be set to IOMMU_HWPT_TYPE_DEFAULT but a * pre-defined type corresponding to the underlying IOMMU hardware. Thanks Nic
> From: Nicolin Chen <nicolinc@nvidia.com> > Sent: Friday, March 17, 2023 12:02 PM > > On Fri, Mar 17, 2023 at 03:02:26AM +0000, Tian, Kevin wrote: > > > > +/** > > > + * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC) > > > + * @size: sizeof(struct iommu_hwpt_alloc) > > > + * @flags: Must be 0 > > > + * @dev_id: The device to allocate this HWPT for > > > + * @pt_id: The IOAS to connect this HWPT to > > > + * @out_hwpt_id: The ID of the new HWPT > > > + * @__reserved: Must be 0 > > > + * > > > + * Explicitly allocate a hardware page table object. This is the same > object > > > + * type that is returned by iommufd_device_attach() and represents the > > > + * underlying iommu driver's iommu_domain kernel object. > > > + * > > > + * A normal HWPT will be created with the mappings from the given > IOAS. > > > + */ > > > > 'normal' is a confusing word in this context. > > Yea, Eric was asking about a related question in another thread, > because he couldn't get this part. I think we could replace this > "normal" with just "kernel-managed", so eventually it would look > like the following narrative after adding user_data and nesting: > > * A kernel-managed HWPT will be created with the mappings from the given > IOAS. > * The @data_type for its allocation can be set to > IOMMU_HWPT_TYPE_DEFAULT, or > * another type (being listed below) to customize the allocation. > * > * A user-managed HWPT will be created from a given parent HWPT via > @pt_id, in > * which the parent HWPT must be allocated previously via the same ioctl > from a > * given IOAS. The @data_type must not be set to > IOMMU_HWPT_TYPE_DEFAULT but a > * pre-defined type corresponding to the underlying IOMMU hardware. that is fine. But at this point it's clearer to simply remove 'normal'.
On Fri, Mar 17, 2023 at 03:02:26AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@nvidia.com> > > Sent: Saturday, February 25, 2023 8:28 AM > > + > > +int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) > > +{ > > + struct iommu_hwpt_alloc *cmd = ucmd->cmd; > > + struct iommufd_hw_pagetable *hwpt; > > + struct iommufd_device *idev; > > + struct iommufd_ioas *ioas; > > + int rc; > > + > > + if (cmd->flags) > > + return -EOPNOTSUPP; > > miss a check on the __reserved field. > > > +/** > > + * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC) > > + * @size: sizeof(struct iommu_hwpt_alloc) > > + * @flags: Must be 0 > > + * @dev_id: The device to allocate this HWPT for > > + * @pt_id: The IOAS to connect this HWPT to > > + * @out_hwpt_id: The ID of the new HWPT > > + * @__reserved: Must be 0 > > + * > > + * Explicitly allocate a hardware page table object. This is the same object > > + * type that is returned by iommufd_device_attach() and represents the > > + * underlying iommu driver's iommu_domain kernel object. > > + * > > + * A normal HWPT will be created with the mappings from the given IOAS. > > + */ > > 'normal' is a confusing word in this context. Got it, thanks Jason
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c index 2584f9038b29a2..a1f87193057385 100644 --- a/drivers/iommu/iommufd/hw_pagetable.c +++ b/drivers/iommu/iommufd/hw_pagetable.c @@ -3,6 +3,7 @@ * Copyright (c) 2021-2022, NVIDIA CORPORATION & AFFILIATES */ #include <linux/iommu.h> +#include <uapi/linux/iommufd.h> #include "iommufd_private.h" @@ -121,3 +122,48 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, iommufd_object_abort_and_destroy(ictx, &hwpt->obj); return ERR_PTR(rc); } + +int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd) +{ + struct iommu_hwpt_alloc *cmd = ucmd->cmd; + struct iommufd_hw_pagetable *hwpt; + struct iommufd_device *idev; + struct iommufd_ioas *ioas; + int rc; + + if (cmd->flags) + return -EOPNOTSUPP; + + idev = iommufd_get_device(ucmd, cmd->dev_id); + if (IS_ERR(idev)) + return PTR_ERR(idev); + + ioas = iommufd_get_ioas(ucmd, cmd->pt_id); + if (IS_ERR(ioas)) { + rc = PTR_ERR(idev); + goto out_put_idev; + } + + mutex_lock(&ioas->mutex); + hwpt = iommufd_hw_pagetable_alloc(ucmd->ictx, ioas, idev, false); + mutex_unlock(&ioas->mutex); + if (IS_ERR(hwpt)) { + rc = PTR_ERR(idev); + goto out_put_ioas; + } + + cmd->out_hwpt_id = hwpt->obj.id; + rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd)); + if (rc) + goto out_hwpt; + iommufd_object_finalize(ucmd->ictx, &hwpt->obj); + goto out_put_ioas; + +out_hwpt: + iommufd_object_abort_and_destroy(ucmd->ictx, &hwpt->obj); +out_put_ioas: + iommufd_put_object(&ioas->obj); +out_put_idev: + iommufd_put_object(&idev->obj); + return rc; +} diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index cfcda73942b533..c9acc70d84f794 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -261,6 +261,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt, struct iommufd_hw_pagetable * iommufd_hw_pagetable_detach(struct iommufd_device *idev); void iommufd_hw_pagetable_destroy(struct iommufd_object *obj); +int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd); static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx, struct iommufd_hw_pagetable *hwpt) @@ -296,6 +297,14 @@ struct iommufd_device { bool enforce_cache_coherency; }; +static inline struct iommufd_device * +iommufd_get_device(struct iommufd_ucmd *ucmd, u32 id) +{ + return container_of(iommufd_get_object(ucmd->ictx, id, + IOMMUFD_OBJ_DEVICE), + struct iommufd_device, obj); +} + void iommufd_device_destroy(struct iommufd_object *obj); struct iommufd_access { diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c index 9cba592d0482e7..694da191e4b155 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -261,6 +261,7 @@ static int iommufd_option(struct iommufd_ucmd *ucmd) union ucmd_buffer { struct iommu_destroy destroy; + struct iommu_hwpt_alloc hwpt; struct iommu_ioas_alloc alloc; struct iommu_ioas_allow_iovas allow_iovas; struct iommu_ioas_copy ioas_copy; @@ -292,6 +293,8 @@ struct iommufd_ioctl_op { } static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = { IOCTL_OP(IOMMU_DESTROY, iommufd_destroy, struct iommu_destroy, id), + IOCTL_OP(IOMMU_HWPT_ALLOC, iommufd_hwpt_alloc, struct iommu_hwpt_alloc, + __reserved), IOCTL_OP(IOMMU_IOAS_ALLOC, iommufd_ioas_alloc_ioctl, struct iommu_ioas_alloc, out_ioas_id), IOCTL_OP(IOMMU_IOAS_ALLOW_IOVAS, iommufd_ioas_allow_iovas, diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index 98ebba80cfa1fc..ccd36acad36a3f 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -45,6 +45,7 @@ enum { IOMMUFD_CMD_IOAS_UNMAP, IOMMUFD_CMD_OPTION, IOMMUFD_CMD_VFIO_IOAS, + IOMMUFD_CMD_HWPT_ALLOC, }; /** @@ -344,4 +345,29 @@ struct iommu_vfio_ioas { __u16 __reserved; }; #define IOMMU_VFIO_IOAS _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VFIO_IOAS) + +/** + * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC) + * @size: sizeof(struct iommu_hwpt_alloc) + * @flags: Must be 0 + * @dev_id: The device to allocate this HWPT for + * @pt_id: The IOAS to connect this HWPT to + * @out_hwpt_id: The ID of the new HWPT + * @__reserved: Must be 0 + * + * Explicitly allocate a hardware page table object. This is the same object + * type that is returned by iommufd_device_attach() and represents the + * underlying iommu driver's iommu_domain kernel object. + * + * A normal HWPT will be created with the mappings from the given IOAS. + */ +struct iommu_hwpt_alloc { + __u32 size; + __u32 flags; + __u32 dev_id; + __u32 pt_id; + __u32 out_hwpt_id; + __u32 __reserved; +}; +#define IOMMU_HWPT_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_HWPT_ALLOC) #endif
This allows userspace to manually create HWPTs on IOAS's and then use those HWPTs as inputs to iommufd_device_attach/replace(). Following series will extend this to allow creating iommu_domains with driver specific parameters. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommufd/hw_pagetable.c | 46 +++++++++++++++++++++++++ drivers/iommu/iommufd/iommufd_private.h | 9 +++++ drivers/iommu/iommufd/main.c | 3 ++ include/uapi/linux/iommufd.h | 26 ++++++++++++++ 4 files changed, 84 insertions(+)