Re: [PATCH v2] vfio: Remove vfio_group dev_counter

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Sep 06, 2022 at 09:21:35AM +0000, Tian, Kevin wrote:

> Although mlx5 internal tracking is not generic, the uAPI proposed
> in Yishai's series is pretty generic. I wonder whether it can be extended
> as a formal interface in iommufd with iommu dirty tracking also being
> a dirty tracker implementation.

It probably could, but I don't think it is helpful

> Currently the concept between Yishai's and Joao's series is actually
> similar, both having a capability interface and a command to read/clear
> the bitmap of a tracked range except that Yishai's series allows
> userspace to do range-based tracking while Joao's series currently
> have the tracking enabled per the entire page table.

This similarity is deliberate
 
> But in concept iommu dirty tracking can support range-based interfaces
> too. For global dirty bit control (e.g. Intel and AMD) the iommu driver
> can just turn it on globally upon any enabled range. For per-PTE
> dirty bit control (e.g. ARM) it's actually a nice fit for range-based
> tracking.

We don't actually have a use case for range-based tracking. The reason
it exists for mlx5 is only because the device just cannot do global
tracking.

I was hoping not to bring that complexity into the system iommu side.

> This pays the complexity of introducing a new object (dirty tracker)
> in iommufd object hierarchy, with the gain of providing a single
> dirty tracking interface to userspace.

Well, it isn't single, it still two interfaces that work in two quite
different ways. Userspace has to know exactly which of the two ways it
is working in and do everything accordingly.

We get to share some structs and ioctl #s, but I don't see a
real simplification here.

If anything the false sharing creates a new kind of complexity:

> Kind of like an page table can have a list of dirty trackers and each
> tracker is used by a list of devices.
> 
> If iommu dirty tracker is available it is preferred to and used by all
> devices (except mdev) attached to the page table.
> 
> Otherwise mlx5 etc. picks a vendor specific dirty tracker if available.
> 
> mdev's just share a generic software dirty tracker, not affected by 
> the presence of iommu dirty tracker.
> 
> Multiple trackers might be used for a page table at the same time.
> 
> Is above all worth it?

Because now we are trying to create one master interface but now we
need new APIs to control what trackers exists, what trackers are
turned on, and so forth. Now we've added complexity.

You end up with iommufd proxying a VFIO object that is completely
unrelated to drivers/iommu - I don't like it just on that basis.

The tracker is a vfio object, created by a vfio driver, with an
interface deliberately similar to iommufd's interface to ease the
implementation in userspace. It is not realed to drivers/iommu and it
doesn't benefit from anything in drivers/iommu beyond the shared code
to manage the shared uapi.

Jason