| Message ID | 59ad8c075bfa9de7098fd79a30b51d1f53d61a94.1723061378.git.nicolinc@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | iommufd: Add VIOMMU infrastructure (Part-1) | expand |
On Thu, Aug 15, 2024 at 08:24:05PM -0300, Jason Gunthorpe wrote: > On Wed, Aug 07, 2024 at 01:10:49PM -0700, Nicolin Chen wrote: > > @@ -946,4 +947,40 @@ struct iommu_viommu_unset_vdev_id { > > __aligned_u64 vdev_id; > > }; > > #define IOMMU_VIOMMU_UNSET_VDEV_ID _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID) > > + > > +/** > > + * enum iommu_viommu_invalidate_data_type - VIOMMU Cache Invalidate Data Type > > + * @IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3: Invalidation data for ARM SMMUv3 > > + */ > > +enum iommu_viommu_invalidate_data_type { > > + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3, > > +}; > > =1 here I think. Lets try to avoid 0 for the types.. > > And this shouldn't be in this patch > > But also we can probably just use reuse enum iommu_hwpt_invalidate_data_type > here? Would that force IOMMU drivers to implement both hwpt and viommu invalidations? SMMUv3 driver would implement both anyway though.. > > +struct iommu_viommu_invalidate { > > + __u32 size; > > + __u32 viommu_id; > > + __aligned_u64 data_uptr; > > + __u32 data_type; > > + __u32 entry_len; > > + __u32 entry_num; > > + __u32 __reserved; > > +}; > > +#define IOMMU_VIOMMU_INVALIDATE _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_INVALIDATE) > > I wonder if we should use IOMMU_HWPT_INVALIDATE? the hwpt_id can tell > which mode it is in. The ioctl becomes badly named but these have > identical arguments. Mostly they would be identical. So, I think that's doable. If someday we need something hwpt-specific or viommu-specific, we can always duplicate a different structure. Thanks Nicolin
On Thu, Aug 15, 2024 at 04:51:39PM -0700, Nicolin Chen wrote: > On Thu, Aug 15, 2024 at 08:24:05PM -0300, Jason Gunthorpe wrote: > > On Wed, Aug 07, 2024 at 01:10:49PM -0700, Nicolin Chen wrote: > > > @@ -946,4 +947,40 @@ struct iommu_viommu_unset_vdev_id { > > > __aligned_u64 vdev_id; > > > }; > > > #define IOMMU_VIOMMU_UNSET_VDEV_ID _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID) > > > + > > > +/** > > > + * enum iommu_viommu_invalidate_data_type - VIOMMU Cache Invalidate Data Type > > > + * @IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3: Invalidation data for ARM SMMUv3 > > > + */ > > > +enum iommu_viommu_invalidate_data_type { > > > + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3, > > > +}; > > > > =1 here I think. Lets try to avoid 0 for the types.. > > > > And this shouldn't be in this patch > > > > But also we can probably just use reuse enum iommu_hwpt_invalidate_data_type > > here? > > Would that force IOMMU drivers to implement both hwpt and viommu > invalidations? SMMUv3 driver would implement both anyway though.. I wouldn't say force, just that they have to use a consistent numbering if they do choose to do both. Jason
On Mon, Aug 19, 2024 at 02:30:56PM -0300, Jason Gunthorpe wrote: > On Thu, Aug 15, 2024 at 04:51:39PM -0700, Nicolin Chen wrote: > > On Thu, Aug 15, 2024 at 08:24:05PM -0300, Jason Gunthorpe wrote: > > > On Wed, Aug 07, 2024 at 01:10:49PM -0700, Nicolin Chen wrote: > > > > @@ -946,4 +947,40 @@ struct iommu_viommu_unset_vdev_id { > > > > __aligned_u64 vdev_id; > > > > }; > > > > #define IOMMU_VIOMMU_UNSET_VDEV_ID _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID) > > > > + > > > > +/** > > > > + * enum iommu_viommu_invalidate_data_type - VIOMMU Cache Invalidate Data Type > > > > + * @IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3: Invalidation data for ARM SMMUv3 > > > > + */ > > > > +enum iommu_viommu_invalidate_data_type { > > > > + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3, > > > > +}; > > > > > > =1 here I think. Lets try to avoid 0 for the types.. > > > > > > And this shouldn't be in this patch > > > > > > But also we can probably just use reuse enum iommu_hwpt_invalidate_data_type > > > here? > > > > Would that force IOMMU drivers to implement both hwpt and viommu > > invalidations? SMMUv3 driver would implement both anyway though.. > > I wouldn't say force, just that they have to use a consistent > numbering if they do choose to do both. But if we duplicate a driver type for two IOCTLs, that assumes our ABI supports both IOCTLs? No? Thanks Nicolin
On Mon, Aug 19, 2024 at 10:49:56AM -0700, Nicolin Chen wrote: > On Mon, Aug 19, 2024 at 02:30:56PM -0300, Jason Gunthorpe wrote: > > On Thu, Aug 15, 2024 at 04:51:39PM -0700, Nicolin Chen wrote: > > > On Thu, Aug 15, 2024 at 08:24:05PM -0300, Jason Gunthorpe wrote: > > > > On Wed, Aug 07, 2024 at 01:10:49PM -0700, Nicolin Chen wrote: > > > > > @@ -946,4 +947,40 @@ struct iommu_viommu_unset_vdev_id { > > > > > __aligned_u64 vdev_id; > > > > > }; > > > > > #define IOMMU_VIOMMU_UNSET_VDEV_ID _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID) > > > > > + > > > > > +/** > > > > > + * enum iommu_viommu_invalidate_data_type - VIOMMU Cache Invalidate Data Type > > > > > + * @IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3: Invalidation data for ARM SMMUv3 > > > > > + */ > > > > > +enum iommu_viommu_invalidate_data_type { > > > > > + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3, > > > > > +}; > > > > > > > > =1 here I think. Lets try to avoid 0 for the types.. > > > > > > > > And this shouldn't be in this patch > > > > > > > > But also we can probably just use reuse enum iommu_hwpt_invalidate_data_type > > > > here? > > > > > > Would that force IOMMU drivers to implement both hwpt and viommu > > > invalidations? SMMUv3 driver would implement both anyway though.. > > > > I wouldn't say force, just that they have to use a consistent > > numbering if they do choose to do both. > > But if we duplicate a driver type for two IOCTLs, that assumes > our ABI supports both IOCTLs? No? No, it is just a numbering system to label the struct layout. Jason
On Mon, Aug 19, 2024 at 03:20:35PM -0300, Jason Gunthorpe wrote: > On Mon, Aug 19, 2024 at 10:49:56AM -0700, Nicolin Chen wrote: > > On Mon, Aug 19, 2024 at 02:30:56PM -0300, Jason Gunthorpe wrote: > > > On Thu, Aug 15, 2024 at 04:51:39PM -0700, Nicolin Chen wrote: > > > > On Thu, Aug 15, 2024 at 08:24:05PM -0300, Jason Gunthorpe wrote: > > > > > On Wed, Aug 07, 2024 at 01:10:49PM -0700, Nicolin Chen wrote: > > > > > > @@ -946,4 +947,40 @@ struct iommu_viommu_unset_vdev_id { > > > > > > __aligned_u64 vdev_id; > > > > > > }; > > > > > > #define IOMMU_VIOMMU_UNSET_VDEV_ID _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID) > > > > > > + > > > > > > +/** > > > > > > + * enum iommu_viommu_invalidate_data_type - VIOMMU Cache Invalidate Data Type > > > > > > + * @IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3: Invalidation data for ARM SMMUv3 > > > > > > + */ > > > > > > +enum iommu_viommu_invalidate_data_type { > > > > > > + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3, > > > > > > +}; > > > > > > > > > > =1 here I think. Lets try to avoid 0 for the types.. > > > > > > > > > > And this shouldn't be in this patch > > > > > > > > > > But also we can probably just use reuse enum iommu_hwpt_invalidate_data_type > > > > > here? > > > > > > > > Would that force IOMMU drivers to implement both hwpt and viommu > > > > invalidations? SMMUv3 driver would implement both anyway though.. > > > > > > I wouldn't say force, just that they have to use a consistent > > > numbering if they do choose to do both. > > > > But if we duplicate a driver type for two IOCTLs, that assumes > > our ABI supports both IOCTLs? No? > > No, it is just a numbering system to label the struct layout. OK. Let's merge the then. Nicolin
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 65023211db47..57c4045f3788 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -561,6 +561,7 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd); void iommufd_viommu_destroy(struct iommufd_object *obj); int iommufd_viommu_set_vdev_id(struct iommufd_ucmd *ucmd); int iommufd_viommu_unset_vdev_id(struct iommufd_ucmd *ucmd); +int iommufd_viommu_invalidate(struct iommufd_ucmd *ucmd); #ifdef CONFIG_IOMMUFD_TEST int iommufd_test(struct iommufd_ucmd *ucmd); diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c index 199ad90fa36b..85d4d01f9ef6 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -334,6 +334,7 @@ union ucmd_buffer { struct iommu_option option; struct iommu_vfio_ioas vfio_ioas; struct iommu_viommu_alloc viommu; + struct iommu_viommu_invalidate viommu_cache; struct iommu_viommu_set_vdev_id set_vdev_id; struct iommu_viommu_unset_vdev_id unset_vdev_id; #ifdef CONFIG_IOMMUFD_TEST @@ -389,6 +390,8 @@ static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = { __reserved), IOCTL_OP(IOMMU_VIOMMU_ALLOC, iommufd_viommu_alloc_ioctl, struct iommu_viommu_alloc, out_viommu_id), + IOCTL_OP(IOMMU_VIOMMU_INVALIDATE, iommufd_viommu_invalidate, + struct iommu_viommu_invalidate, entry_num), IOCTL_OP(IOMMU_VIOMMU_SET_VDEV_ID, iommufd_viommu_set_vdev_id, struct iommu_viommu_set_vdev_id, vdev_id), IOCTL_OP(IOMMU_VIOMMU_UNSET_VDEV_ID, iommufd_viommu_unset_vdev_id, diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c index 209d419fe5cd..ada9f968b9f6 100644 --- a/drivers/iommu/iommufd/viommu.c +++ b/drivers/iommu/iommufd/viommu.c @@ -194,3 +194,48 @@ int iommufd_viommu_unset_vdev_id(struct iommufd_ucmd *ucmd) iommufd_put_object(ucmd->ictx, &idev->obj); return rc; } + +int iommufd_viommu_invalidate(struct iommufd_ucmd *ucmd) +{ + struct iommu_viommu_invalidate *cmd = ucmd->cmd; + struct iommu_user_data_array data_array = { + .type = cmd->data_type, + .uptr = u64_to_user_ptr(cmd->data_uptr), + .entry_len = cmd->entry_len, + .entry_num = cmd->entry_num, + }; + struct iommufd_viommu *viommu; + u32 done_num = 0; + int rc; + + if (cmd->__reserved) { + rc = -EOPNOTSUPP; + goto out; + } + + if (cmd->entry_num && (!cmd->data_uptr || !cmd->entry_len)) { + rc = -EINVAL; + goto out; + } + + viommu = iommufd_get_viommu(ucmd, cmd->viommu_id); + if (IS_ERR(viommu)) + return PTR_ERR(viommu); + + if (!viommu->ops || !viommu->ops->cache_invalidate) { + rc = -EOPNOTSUPP; + goto out_put_viommu; + } + + rc = viommu->ops->cache_invalidate(viommu, &data_array); + + done_num = data_array.entry_num; + +out_put_viommu: + iommufd_put_object(ucmd->ictx, &viommu->obj); +out: + cmd->entry_num = done_num; + if (iommufd_ucmd_respond(ucmd, sizeof(*cmd))) + return -EFAULT; + return rc; +} diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index d5e72682ba57..998b3f2cd2b5 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -54,6 +54,7 @@ enum { IOMMUFD_CMD_VIOMMU_ALLOC = 0x8f, IOMMUFD_CMD_VIOMMU_SET_VDEV_ID = 0x90, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID = 0x91, + IOMMUFD_CMD_VIOMMU_INVALIDATE = 0x92, }; /** @@ -946,4 +947,40 @@ struct iommu_viommu_unset_vdev_id { __aligned_u64 vdev_id; }; #define IOMMU_VIOMMU_UNSET_VDEV_ID _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_UNSET_VDEV_ID) + +/** + * enum iommu_viommu_invalidate_data_type - VIOMMU Cache Invalidate Data Type + * @IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3: Invalidation data for ARM SMMUv3 + */ +enum iommu_viommu_invalidate_data_type { + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3, +}; + +/** + * struct iommu_viommu_invalidate - ioctl(IOMMU_VIOMMU_INVALIDATE) + * @size: sizeof(struct iommu_viommu_invalidate) + * @viommu_id: viommu ID for cache invalidation + * @data_uptr: User pointer to an array of driver-specific viommu cache + * invalidation data + * @data_type: One of enum iommu_viommu_invalidate_data_type, defining the data + * type of all the entries in the invalidation request array. + * @entry_len: Length (in bytes) of a request entry in the request array + * @entry_num: Input the number of cache invalidation requests in the array. + * Output the number of requests successfully handled by kernel. + * @__reserved: Must be 0 + * + * Invalidate an iommu HW cache used by a viommu in the user space. + * Each ioctl can support one or more cache invalidation requests in the array + * that has a total size of @entry_len * @entry_num. + */ +struct iommu_viommu_invalidate { + __u32 size; + __u32 viommu_id; + __aligned_u64 data_uptr; + __u32 data_type; + __u32 entry_len; + __u32 entry_num; + __u32 __reserved; +}; +#define IOMMU_VIOMMU_INVALIDATE _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_INVALIDATE) #endif
Add a new IOMMU_VIOMMU_INVALIDATE ioctl, similar to IOMMU_HWPT_INVALIDATE. This is used by the user space to flush any IOMMU specific cache that can be directed using a viommu object. Add IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3 as the initial data type whose support is in the one of the following patches. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/iommufd/iommufd_private.h | 1 + drivers/iommu/iommufd/main.c | 3 ++ drivers/iommu/iommufd/viommu.c | 45 +++++++++++++++++++++++++ include/uapi/linux/iommufd.h | 37 ++++++++++++++++++++ 4 files changed, 86 insertions(+)