diff mbox series

[v1,08/16] iommufd/viommu: Add IOMMU_VIOMMU_INVALIDATE ioctl

Message ID 59ad8c075bfa9de7098fd79a30b51d1f53d61a94.1723061378.git.nicolinc@nvidia.com (mailing list archive)
State New, archived
Headers show
Series iommufd: Add VIOMMU infrastructure (Part-1) | expand

Commit Message

Nicolin Chen Aug. 7, 2024, 8:10 p.m. UTC 
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(+)

Comments

Jason Gunthorpe Aug. 15, 2024, 11:24 p.m. UTC | #1 
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?

> +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.

Jason
Nicolin Chen Aug. 15, 2024, 11:51 p.m. UTC | #2 
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
Jason Gunthorpe Aug. 19, 2024, 5:30 p.m. UTC | #3 
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
Nicolin Chen Aug. 19, 2024, 5:49 p.m. UTC | #4 
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
Jason Gunthorpe Aug. 19, 2024, 6:20 p.m. UTC | #5 
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
Nicolin Chen Aug. 19, 2024, 6:22 p.m. UTC | #6 
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 mbox series

Patch

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