Re: [PATCH RFCv2 04/24] iommu: Add iommu_domain ops for dirty tracking

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Aug 10, 2023 at 09:36:37PM +0100, Joao Martins wrote:

> > Yes, the "domain_default_ops" is basically a transitional hack to
> > help migrate to narrowly defined per-usage domain ops.
> > 
> > eg things like blocking and identity should not have mapping ops.
> > 
> My earlier point was more about not what 'domain_default_ops' represents
> but that it's a pointer. Shared by everyone (devices and domains alike). But you
> sort of made it clear that it's OK to duplicate it to not have dirty tracking.
> The duplication is what I felt a little odd.

Well, it is one path, we could also add a dirty_ops to the
domain. Hard to say which is better.

> (...) I wasn't quite bodging, just trying to parallelize what was bus cleanup
> could be tackling domain/device-independent ops without them being global. Maybe
> I read too much into it hence my previous question.

domain_alloc_user bypasses the bus cleanup

> > Return the IOMMU_CAP_DIRTY as generic data in the new GET_INFO
> 
> I have this one here:
> 
> https://lore.kernel.org/linux-iommu/20230518204650.14541-14-joao.m.martins@oracle.com/
> 
> I can rework to GET_HW_INFO but it really needs to be generic bits of data and
> not iommu hw specific e.g. that translates into device_iommu_capable() cap
> checking. 

Yes, HW_INFO seems the right way. Just add a

   __aligned_u64 out_capabilities;

To that struct iommu_hw_info and fill it with generic code.

> > Accept some generic flag in the alloc_hwpt requesting dirty
> > Pass generic flags down to the driver.
> > Reject set flags and drivers that don't implement alloc_domain_user.
> > Driver refuses to attach the dirty enabled domain to places that do
> > dirty tracking.
> 
> This is already done, and so far I have an unsigned long flags to
> domain_alloc_user() and probably be kept defining it as
> iommu-domain-feature bit

Yes a flag in some way is the best choice

> (unless it's better to follow similar direction as hwpt_type like in
> domain_alloc_user). And gets stored as iommu_domain::flags, like this series
> had. Though if majority of driver rejection flows via alloc_domain_user only
> (which has a struct device), perhaps it's not even needed to store as a new
> iommu_domain::flags

Right, we don't need to reflect it back if the dirty ops are NULL.

Jason