diff mbox series

[RFC,7/7] vfio: Add vfio_register_pasid_iommu_dev()

Message ID 20231009085123.463179-8-yi.l.liu@intel.com (mailing list archive)
State New
Headers show
Series Add SIOV virtual device support | expand

Commit Message

Yi Liu Oct. 9, 2023, 8:51 a.m. UTC 
From: Kevin Tian <kevin.tian@intel.com>

This adds vfio_register_pasid_iommu_dev() for device driver to register
virtual devices which are isolated per PASID in physical IOMMU. The major
usage is for the SIOV devices which allows device driver to tag the DMAs
out of virtual devices within it with different PASIDs.

For a given vfio device, VFIO core creates both group user interface and
device user interface (device cdev) if configured. However, for the virtual
devices backed by PASID of the device, VFIO core shall only create device
user interface as there is no plan to support such devices in the legacy
vfio_iommu drivers which is a must if creating group user interface for
such virtual devices. This introduces a VFIO_PASID_IOMMU group type for
the device driver to register PASID virtual devices, and provides a wrapper
API for it. In particular no iommu group (neither fake group or real group)
exists per PASID, hence no group interface for this type.

Signed-off-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/vfio/group.c     | 18 ++++++++++++++++++
 drivers/vfio/vfio.h      |  8 ++++++++
 drivers/vfio/vfio_main.c | 10 ++++++++++
 include/linux/vfio.h     |  1 +
 4 files changed, 37 insertions(+)

Comments

Tian, Kevin Oct. 10, 2023, 8:33 a.m. UTC | #1 
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Monday, October 9, 2023 4:51 PM
> 
> From: Kevin Tian <kevin.tian@intel.com>
> 
> This adds vfio_register_pasid_iommu_dev() for device driver to register
> virtual devices which are isolated per PASID in physical IOMMU. The major
> usage is for the SIOV devices which allows device driver to tag the DMAs
> out of virtual devices within it with different PASIDs.
> 
> For a given vfio device, VFIO core creates both group user interface and
> device user interface (device cdev) if configured. However, for the virtual
> devices backed by PASID of the device, VFIO core shall only create device
> user interface as there is no plan to support such devices in the legacy
> vfio_iommu drivers which is a must if creating group user interface for
> such virtual devices. This introduces a VFIO_PASID_IOMMU group type for
> the device driver to register PASID virtual devices, and provides a wrapper
> API for it. In particular no iommu group (neither fake group or real group)
> exists per PASID, hence no group interface for this type.
> 

this commit msg needs some revision. The key is that there is no group
per pasid *in concept* so it doesn't make sense to fake a group...
Yi Liu Nov. 9, 2023, 8:20 a.m. UTC | #2 
On 2023/10/10 16:33, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Monday, October 9, 2023 4:51 PM
>>
>> From: Kevin Tian <kevin.tian@intel.com>
>>
>> This adds vfio_register_pasid_iommu_dev() for device driver to register
>> virtual devices which are isolated per PASID in physical IOMMU. The major
>> usage is for the SIOV devices which allows device driver to tag the DMAs
>> out of virtual devices within it with different PASIDs.
>>
>> For a given vfio device, VFIO core creates both group user interface and
>> device user interface (device cdev) if configured. However, for the virtual
>> devices backed by PASID of the device, VFIO core shall only create device
>> user interface as there is no plan to support such devices in the legacy
>> vfio_iommu drivers which is a must if creating group user interface for
>> such virtual devices. This introduces a VFIO_PASID_IOMMU group type for
>> the device driver to register PASID virtual devices, and provides a wrapper
>> API for it. In particular no iommu group (neither fake group or real group)
>> exists per PASID, hence no group interface for this type.
>>
> 
> this commit msg needs some revision. The key is that there is no group
> per pasid *in concept* so it doesn't make sense to fake a group...

how about below?

This adds vfio_register_pasid_iommu_dev() for device driver to register
virtual devices which are isolated per PASID in physical IOMMU. The major
usage is for the SIOV devices which allows the device driver to tag the
DMAs out of virtual devices within it with different PASIDs.

For the PASID virtual devices, there is no iommu group in concept. Hence
the VFIO core only creates device user interface for such devices. This
introduces a VFIO_PASID_IOMMU group type to differentiate from the other
devices that have iommu group in concept.
Cao, Yahui Nov. 16, 2023, 5:35 a.m. UTC | #3 
On 10/9/2023 4:51 PM, Yi Liu wrote:
> From: Kevin Tian <kevin.tian@intel.com>
>
> This adds vfio_register_pasid_iommu_dev() for device driver to register
> virtual devices which are isolated per PASID in physical IOMMU. The major
> usage is for the SIOV devices which allows device driver to tag the DMAs
> out of virtual devices within it with different PASIDs.
>
> For a given vfio device, VFIO core creates both group user interface and
> device user interface (device cdev) if configured. However, for the virtual
> devices backed by PASID of the device, VFIO core shall only create device
> user interface as there is no plan to support such devices in the legacy
> vfio_iommu drivers which is a must if creating group user interface for
> such virtual devices. This introduces a VFIO_PASID_IOMMU group type for
> the device driver to register PASID virtual devices, and provides a wrapper
> API for it. In particular no iommu group (neither fake group or real group)
> exists per PASID, hence no group interface for this type.
>
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>
>   
> +/*
> + * Register a virtual device with IOMMU pasid protection. The user of
> + * this device can trigger DMA as long as all of its outgoing DMAs are
> + * always tagged with a pasid.
> + */
> +int vfio_register_pasid_iommu_dev(struct vfio_device *device)
> +{
> +	return __vfio_register_dev(device, VFIO_PASID_IOMMU);
> +}
> +

If CONFIG_VFIO_GROUP kconfig is selected, then there will be access to 
vdev->group shown as below
->__vfio_register_dev()
        ->vfio_device_add()
             ->vfio_device_is_noiommu() { return 
IS_ENABLED(CONFIG_VFIO_NOIOMMU) && vdev->group->type == VFIO_NO_IOMMU}

For SIOV virtual devices, vfio group is not created and vfio cdev is 
used. Thus vdev->group is NULL and there is NULL pointer access here.

Thanks.
Yahui.
Yi Liu Nov. 17, 2023, 6:30 a.m. UTC | #4 
On 2023/11/16 13:12, Cao, Yahui wrote:
> 
> On 10/9/2023 4:51 PM, Yi Liu wrote:
>> From: Kevin Tian<kevin.tian@intel.com>
>>
>> This adds vfio_register_pasid_iommu_dev() for device driver to register
>> virtual devices which are isolated per PASID in physical IOMMU. The major
>> usage is for the SIOV devices which allows device driver to tag the DMAs
>> out of virtual devices within it with different PASIDs.
>>
>> For a given vfio device, VFIO core creates both group user interface and
>> device user interface (device cdev) if configured. However, for the virtual
>> devices backed by PASID of the device, VFIO core shall only create device
>> user interface as there is no plan to support such devices in the legacy
>> vfio_iommu drivers which is a must if creating group user interface for
>> such virtual devices. This introduces a VFIO_PASID_IOMMU group type for
>> the device driver to register PASID virtual devices, and provides a wrapper
>> API for it. In particular no iommu group (neither fake group or real group)
>> exists per PASID, hence no group interface for this type.
>>
>> Signed-off-by: Kevin Tian<kevin.tian@intel.com>
>> Signed-off-by: Yi Liu<yi.l.liu@intel.com>
>> ---
>>   drivers/vfio/group.c     | 18 ++++++++++++++++++
>>   drivers/vfio/vfio.h      |  8 ++++++++
>>   drivers/vfio/vfio_main.c | 10 ++++++++++
>>   include/linux/vfio.h     |  1 +
>>   4 files changed, 37 insertions(+)
>>
>> ...
>> ...
>> +/*
>> + * Register a virtual device with IOMMU pasid protection. The user of
>> + * this device can trigger DMA as long as all of its outgoing DMAs are
>> + * always tagged with a pasid.
>> + */
>> +int vfio_register_pasid_iommu_dev(struct vfio_device *device)
>> +{
>> +    return __vfio_register_dev(device, VFIO_PASID_IOMMU);
>> +}
>> +
> 
> 
> Missing symbol export here.
> 
fixed.
Yi Liu Nov. 17, 2023, 6:31 a.m. UTC | #5 
On 2023/11/16 13:35, Cao, Yahui wrote:
> 
> On 10/9/2023 4:51 PM, Yi Liu wrote:
>> From: Kevin Tian <kevin.tian@intel.com>
>>
>> This adds vfio_register_pasid_iommu_dev() for device driver to register
>> virtual devices which are isolated per PASID in physical IOMMU. The major
>> usage is for the SIOV devices which allows device driver to tag the DMAs
>> out of virtual devices within it with different PASIDs.
>>
>> For a given vfio device, VFIO core creates both group user interface and
>> device user interface (device cdev) if configured. However, for the virtual
>> devices backed by PASID of the device, VFIO core shall only create device
>> user interface as there is no plan to support such devices in the legacy
>> vfio_iommu drivers which is a must if creating group user interface for
>> such virtual devices. This introduces a VFIO_PASID_IOMMU group type for
>> the device driver to register PASID virtual devices, and provides a wrapper
>> API for it. In particular no iommu group (neither fake group or real group)
>> exists per PASID, hence no group interface for this type.
>>
>> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>
>> +/*
>> + * Register a virtual device with IOMMU pasid protection. The user of
>> + * this device can trigger DMA as long as all of its outgoing DMAs are
>> + * always tagged with a pasid.
>> + */
>> +int vfio_register_pasid_iommu_dev(struct vfio_device *device)
>> +{
>> +    return __vfio_register_dev(device, VFIO_PASID_IOMMU);
>> +}
>> +
> 
> If CONFIG_VFIO_GROUP kconfig is selected, then there will be access to 
> vdev->group shown as below
> ->__vfio_register_dev()
>         ->vfio_device_add()
>              ->vfio_device_is_noiommu() { return 
> IS_ENABLED(CONFIG_VFIO_NOIOMMU) && vdev->group->type == VFIO_NO_IOMMU}
> 
> For SIOV virtual devices, vfio group is not created and vfio cdev is used. 
> Thus vdev->group is NULL and there is NULL pointer access here.
> 

yes. needs to be like below:

return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && vdev->group && vdev->group->type 
== VFIO_NO_IOMMU;
diff mbox series

Patch

diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index 610a429c6191..20771d0feb37 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -407,6 +407,9 @@  int vfio_device_block_group(struct vfio_device *device)
 	struct vfio_group *group = device->group;
 	int ret = 0;
 
+	if (!group)
+		return 0;
+
 	mutex_lock(&group->group_lock);
 	if (group->opened_file) {
 		ret = -EBUSY;
@@ -424,6 +427,8 @@  void vfio_device_unblock_group(struct vfio_device *device)
 {
 	struct vfio_group *group = device->group;
 
+	if (!group)
+		return;
 	mutex_lock(&group->group_lock);
 	group->cdev_device_open_cnt--;
 	mutex_unlock(&group->group_lock);
@@ -704,6 +709,10 @@  int vfio_device_set_group(struct vfio_device *device,
 {
 	struct vfio_group *group;
 
+	/* No group associate with a device with pasid */
+	if (type == VFIO_PASID_IOMMU)
+		return 0;
+
 	if (type == VFIO_IOMMU)
 		group = vfio_group_find_or_alloc(device->dev);
 	else
@@ -722,6 +731,9 @@  void vfio_device_remove_group(struct vfio_device *device)
 	struct vfio_group *group = device->group;
 	struct iommu_group *iommu_group;
 
+	if (!group)
+		return;
+
 	if (group->type == VFIO_NO_IOMMU || group->type == VFIO_EMULATED_IOMMU)
 		iommu_group_remove_device(device->dev);
 
@@ -766,6 +778,9 @@  void vfio_device_remove_group(struct vfio_device *device)
 
 void vfio_device_group_register(struct vfio_device *device)
 {
+	if (!device->group)
+		return;
+
 	mutex_lock(&device->group->device_lock);
 	list_add(&device->group_next, &device->group->device_list);
 	mutex_unlock(&device->group->device_lock);
@@ -773,6 +788,9 @@  void vfio_device_group_register(struct vfio_device *device)
 
 void vfio_device_group_unregister(struct vfio_device *device)
 {
+	if (!device->group)
+		return;
+
 	mutex_lock(&device->group->device_lock);
 	list_del(&device->group_next);
 	mutex_unlock(&device->group->device_lock);
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index d228cdb6b345..1ccc9aba6dc7 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -48,6 +48,14 @@  enum vfio_group_type {
 	 */
 	VFIO_IOMMU,
 
+	/*
+	 * Virtual device with IOMMU backing. The user of these devices can
+	 * trigger DMAs which are all tagged with a pasid. Pasid itself is
+	 * a device resource so there is no group associated. The VFIO core
+	 * doesn't create a vfio_group for such devices.
+	 */
+	VFIO_PASID_IOMMU,
+
 	/*
 	 * Virtual device without IOMMU backing. The VFIO core fakes up an
 	 * iommu_group as the iommu_group sysfs interface is part of the
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 850bbaebdd29..362de0ad36ce 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -334,6 +334,16 @@  int vfio_register_emulated_iommu_dev(struct vfio_device *device)
 }
 EXPORT_SYMBOL_GPL(vfio_register_emulated_iommu_dev);
 
+/*
+ * Register a virtual device with IOMMU pasid protection. The user of
+ * this device can trigger DMA as long as all of its outgoing DMAs are
+ * always tagged with a pasid.
+ */
+int vfio_register_pasid_iommu_dev(struct vfio_device *device)
+{
+	return __vfio_register_dev(device, VFIO_PASID_IOMMU);
+}
+
 /*
  * Decrement the device reference count and wait for the device to be
  * removed.  Open file descriptors for the device... */
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 7b06d1bc7cb3..2662f2ece924 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -281,6 +281,7 @@  static inline void vfio_put_device(struct vfio_device *device)
 
 int vfio_register_group_dev(struct vfio_device *device);
 int vfio_register_emulated_iommu_dev(struct vfio_device *device);
+int vfio_register_pasid_iommu_dev(struct vfio_device *device);
 void vfio_unregister_group_dev(struct vfio_device *device);
 
 int vfio_assign_device_set(struct vfio_device *device, void *set_id);