Re: [RFC 11/12] vfio: Add ioctls for device cdev iommufd

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/1/9 15:47, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Monday, December 19, 2022 4:47 PM

@@ -415,7 +416,7 @@ static int vfio_device_first_open(struct
vfio_device_file *df,
  	if (!try_module_get(device->dev->driver->owner))
  		return -ENODEV;

-	if (iommufd)
+	if (iommufd && !IS_ERR(iommufd))
  		ret = vfio_iommufd_bind(device, iommufd, dev_id, pt_id);
  	else
  		ret = vfio_device_group_use_iommu(device);

can you elaborate how noiommu actually works in the cdev path?

I'm a bit lost here.

@@ -592,6 +600,8 @@ static int vfio_device_fops_release(struct inode
*inode, struct file *filep)
  	 */
  	if (!df->single_open)
  		vfio_device_group_close(df);
+	else
+		vfio_device_close(df);
  	kfree(df);
  	vfio_device_put_registration(device);

belong to last patch?

not really. In last patch, it only adds the cdev, but no ioctls.
Only when the BIND_IOMMUFD is added, should the df->single_open
possible be true. So I added it in this patch instead of last one.


+	mutex_lock(&device->dev_set->lock);
+	/* Paired with smp_store_release() in vfio_device_open/close() */
+	access = smp_load_acquire(&df->access_granted);
+	if (access) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}

Not sure it's required. The lock is already held then just checking
df->iommufd should be sufficient.

you are right. check df->iommufd is enough.

+	mutex_lock(&device->dev_set->lock);
+	pt_id = attach.pt_id;
+	ret = vfio_iommufd_attach(device,
+				  pt_id != IOMMUFD_INVALID_ID ? &pt_id :
NULL);
+	if (ret)
+		goto out_unlock;
+
+	if (pt_id != IOMMUFD_INVALID_ID) {

it's clearer to use an 'attach' local variable

not quit get. We already have 'attach' in above lines.:-)

diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 98ebba80cfa1..87680274c01b 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -9,6 +9,8 @@

  #define IOMMUFD_TYPE (';')

+#define IOMMUFD_INVALID_ID 0  /* valid ID starts from 1 */

Can you point out where valid IDs are guaranteed to start
from 1?

According to _iommufd_object_alloc() it uses xa_limit_32b as
limit which includes '0' as the lowest ID

xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT);

yes, but the xarray init uses XA_FLAGS_ALLOC1, and it means to allocate
ID from 1.

/* ALLOC is for a normal 0-based alloc.  ALLOC1 is for an 1-based alloc */

+/*
+ * VFIO_DEVICE_BIND_IOMMUFD - _IOR(VFIO_TYPE, VFIO_BASE + 19,
+ *				   struct vfio_device_bind_iommufd)
+ *
+ * Bind a vfio_device to the specified iommufd and an ioas or a hardware
+ * page table.

this is stale. BIND now is only about bind. No ioas.

Oops, yes.

+ * VFIO_DEVICE_ATTACH_IOMMUFD_PT - _IOW(VFIO_TYPE, VFIO_BASE +
20,
+ *					struct
vfio_device_attach_iommufd_pt)
+ *
+ * Attach a vfio device to an iommufd address space specified by IOAS
+ * id or hardware page table id.
+ *
+ * Available only after a device has been bound to iommufd via
+ * VFIO_DEVICE_BIND_IOMMUFD
+ *
+ * Undo by passing pt_id == IOMMUFD_INVALID_ID
+ *
+ * @argsz:	user filled size of this data.
+ * @flags:	must be 0.
+ * @pt_id:	Input the target id, can be an ioas or a hwpt allocated
+ *		via iommufd subsystem, and output the attached pt_id. It
+ *		be the ioas, hwpt itself or an hwpt created by kernel
+ *		during the attachment.

Input the target id which can represent an ioas or a hwpt allocated
via iommufd subsystem. Output the attached hwpt id which could
be the specified hwpt itself or a hwpt automatically created for the
specified ioas by kernel during the attachment.

got it.

--
Regards,
Yi Liu



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux