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

[Yi Liu]

On 2023/11/16 13:35, Cao, Yahui wrote:
> On 10/9/2023 4:51 PM, Yi Liu wrote:
> > From: Kevin Tian <kevin.tian@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> > ---
> >
> > +/*
> > + * 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