On 2022/8/16 20:29, Jason Gunthorpe wrote:
On Tue, Aug 16, 2022 at 09:21:05AM +0800, Yi Liu wrote:
Hi Jason,
On 2022/8/16 00:50, Jason Gunthorpe wrote:
This counts the number of devices attached to a vfio_group, ie the number
of items in the group->device_list.
yes. This dev_counter is added to ensure only singleton vfio group supports
pin page. Although I don't think it is a good approach as it only counts the
registered devices.
Ah, I missed explaining this, lets try again:
vfio: Remove vfio_group dev_counter
This counts the number of devices attached to a vfio_group, ie the number
of items in the group->device_list.
It has two purposes
- To ensure the vfio_device is opened
hmmm, surely vfio_pin_pages() needs to ensure vfio_device is opened.
But it's not the purpose of adding dev_counter. So I guess this line
can be removed. :)
- To assert the group is singleton because the dirty tracking code in the
type1 iommu has limitations
However, vfio_pin_pages() already calls vfio_assert_device_open() so the
first is taken care of, and all callers of vfio_pin_pages() use now use
vfio_register_emulated_iommu_dev() which guarentees single groups by
constrution.
So delete dev_counter and leave a note that vfio_pin_pages() can only be
used with vfio_register_emulated_iommu_dev().
oh, yes. The fake group is allocated in vfio_register_emulated_iommu_dev().
So the group is surely singleton. Maybe for simple just say
vfio_register_emulated_iommu_dev() guarantees the group is singleton, and
vfio_pin_pages() can only be used with vfio_register_emulated_iommu_dev().
So no need to use dev_counter in vfio_pin_pages(). Such commit message may
be more accurate and no unnecessary distraction. :-) btw. The change looks
good to me.
Reviewed-by: Yi Liu <yi.l.liu@xxxxxxxxx>
--
Regards,
Yi Liu
