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

Yi Liu
2023-11-17T06:29:19Z

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;

--
Regards,
Yi Liu