| Message ID | 20231009085123.463179-1-yi.l.liu@intel.com |
|---|---|
| Headers | show |
| Series | Add SIOV virtual device support | expand |
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Monday, October 9, 2023 9:21 PM > > On Mon, Oct 09, 2023 at 01:51:16AM -0700, Yi Liu wrote: > > Intel SIOV allows creating virtual devices of which the vRID is > > represented by a pasid of a physical device. It is called as SIOV > > virtual device in this series. Such devices can be bound to an iommufd > > as physical device does and then later be attached to an IOAS/hwpt > > using that pasid. Such PASIDs are called as default pasid. > > I would want to see the idxd implementation too.. > It still needs some time (and unfortunately the guy working on idxd is currently on a long vacation). Instead of waiting we want to seek early comments on the iommufd changes given that part is relatively self-contained. Same as what Reinette is doing for IMS. Certainly this is not for merging w/o having a driver user. 😊
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Monday, October 9, 2023 4:51 PM > > From: Kevin Tian <kevin.tian@intel.com> > > SIOV devices allows driver to tag different PASIDs for the virtual devices > within it. Such driver should call iommufd_device_bind_pasid() to connect > the pasid of the device to iommufd, and then driver is able to attach the > virtual device to IOAS/HWPT with the iommufd_device_attach() API. > > Unlike physical devices, for SIOV virtual devices, iommufd_device_attach() > eventually uses the idev->default_pasid when the virtual device is attached s/default_pasid/rid_pasid/? or just call it idev->pasid. 'default' adds slight confusion instead... > to an IOAS/HWPT. Also, there is no need to do immediate_attach per iommu > domain allocation in the attach/replace path if any iommu domain allocation > happens since the attach/replace is eventually pasid attach/replace. immediate_attach rationale belongs to earlier pasid attach series when iommufd_device_pasid_attach() was introduced. Not here.
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Monday, October 9, 2023 4:51 PM > > This extends IOMMU_TEST_OP_MOCK_DOMAIN to accept a pasid from caller. > Hence it is able to cover the iommufd_device_bind_pasid() for SIOV > virtual devices. pasid #0 is selected to mark the physical devices, > non-zero pasid values would be considered as SIOV virtual device bind. > Will add SIOV test cases later. > ... > @@ -62,6 +62,8 @@ struct iommu_test_cmd { > __aligned_u64 length; > } add_reserved; > struct { > + /* #0 is invalid, any non-zero is meaningful */ > + __u32 default_pasid; #0 represents the physical device instead of being invalid.
> 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...
On 2023/10/10 16:24, 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> >> >> SIOV devices allows driver to tag different PASIDs for the virtual devices >> within it. Such driver should call iommufd_device_bind_pasid() to connect >> the pasid of the device to iommufd, and then driver is able to attach the >> virtual device to IOAS/HWPT with the iommufd_device_attach() API. >> >> Unlike physical devices, for SIOV virtual devices, iommufd_device_attach() >> eventually uses the idev->default_pasid when the virtual device is attached > > s/default_pasid/rid_pasid/? or just call it idev->pasid. 'default' adds slight > confusion instead... then let's use rid_pasid as it is used as vRID for virtual device. >> to an IOAS/HWPT. Also, there is no need to do immediate_attach per iommu >> domain allocation in the attach/replace path if any iommu domain allocation >> happens since the attach/replace is eventually pasid attach/replace. > > immediate_attach rationale belongs to earlier pasid attach series when > iommufd_device_pasid_attach() was introduced. Not here. > sure, will drop it.
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;
On 10/9/2023 9:21 PM, Jason Gunthorpe wrote: > On Mon, Oct 09, 2023 at 01:51:16AM -0700, Yi Liu wrote: >> Intel SIOV allows creating virtual devices of which the vRID is >> represented by a pasid of a physical device. It is called as SIOV >> virtual device in this series. Such devices can be bound to an iommufd >> as physical device does and then later be attached to an IOAS/hwpt >> using that pasid. Such PASIDs are called as default pasid. > > I would want to see the idxd implementation too.. > > Jason Hey Jason, ice(E810 NIC) driver implementation for SIOV virtual device is also working in progress. We are working closely with Kevin and Yi on the patchset. We'll send out the ice patch for SIOV as lan driver user example once it is available. (There is some format issue in the last email, re-send again and sorry for any inconvenience) Thanks. Yahui.
Intel SIOV allows creating virtual devices of which the vRID is represented by a pasid of a physical device. It is called as SIOV virtual device in this series. Such devices can be bound to an iommufd as physical device does and then later be attached to an IOAS/hwpt using that pasid. Such PASIDs are called as default pasid. iommufd has already supported pasid attach[1]. So a simple way to support SIOV virtual device attachment is to let device driver call the iommufd_device_pasid_attach() and pass in the default pasid for the virtual device. This should work for now, but it may have problem if iommufd core wants to differentiate the default pasids with other kind of pasids (e.g. pasid given by userspace). In the later forwarding page request to userspace, the default pasids are not supposed to send to userspace as default pasids are mainly used by the SIOV device driver. With above reason, this series chooses to have a new API to bind the default pasid to iommufd, and extends the iommufd_device_attach() to convert the attachment to be pasid attach with the default pasid. Device drivers (e.g. VFIO) that support SIOV shall call the below APIs to interact with iommufd: - iommufd_device_bind_pasid(): Bind virtual device (a pasid of a device) to iommufd; - iommufd_device_attach(): Attach a SIOV virtual device to IOAS/HWPT; - iommufd_device_replace(): Replace IOAS/HWPT of a SIOV virtual device; - iommufd_device_detach(): Detach IOAS/HWPT of a SIOV virtual device; - iommufd_device_unbind(): Unbind virtual device from iommufd; For vfio devices, the device drivers that support SIOV should: - use below API to register vdev for SIOV virtual device vfio_register_pasid_iommu_dev() - use below API to bind vdev to iommufd in .bind_iommufd() callback iommufd_device_bind_pasid() - allocate pasid by itself before calling iommufd_device_bind_pasid() Complete code can be found at[2] [1] https://lore.kernel.org/linux-iommu/20230926092651.17041-1-yi.l.liu@intel.com/ [2] https://github.com/yiliu1765/iommufd/tree/iommufd_pasid_siov Regards, Yi Liu Kevin Tian (5): iommufd: Handle unsafe interrupts in a separate function iommufd: Introduce iommufd_alloc_device() iommufd: Add iommufd_device_bind_pasid() iommufd: Support attach/replace for SIOV virtual device {dev, pasid} vfio: Add vfio_register_pasid_iommu_dev() Yi Liu (2): iommufd/selftest: Extend IOMMU_TEST_OP_MOCK_DOMAIN to pass in pasid iommufd/selftest: Add test coverage for SIOV virtual device drivers/iommu/iommufd/device.c | 163 ++++++++++++++---- drivers/iommu/iommufd/iommufd_private.h | 7 + drivers/iommu/iommufd/iommufd_test.h | 2 + drivers/iommu/iommufd/selftest.c | 10 +- drivers/vfio/group.c | 18 ++ drivers/vfio/vfio.h | 8 + drivers/vfio/vfio_main.c | 10 ++ include/linux/iommufd.h | 3 + include/linux/vfio.h | 1 + tools/testing/selftests/iommu/iommufd.c | 75 ++++++-- .../selftests/iommu/iommufd_fail_nth.c | 42 ++++- tools/testing/selftests/iommu/iommufd_utils.h | 21 ++- 12 files changed, 296 insertions(+), 64 deletions(-)