Re: [PATCH RFCv2] iommu: Add domain type and flag to domain_alloc_paging()

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, Aug 02, 2024 at 11:23:52AM +0530, Vasant Hegde wrote:

> >> +    struct iommu_domain *(*domain_alloc_paging)(struct device *dev,
> >> +                            u32 iommu_domain_type, u32 flags);
> > 
> > I still can't see a value to pass the domain type in this callback.
> > Different domain could have different domain allocation callback, hence
> > the domain type has already been implied.

+1

> > For the paging domain, there should be no difference between DMA and
> > UNMNANAGED from iommu driver's point of view.
> 
> That's true. Its all paging domain. But we need a way to indicate the desired
> capability like PASID.
 
> I thoughts we can use `type` for allocating domain and then `flag` to pass the
> quirks. Otherwise we have to club everything in `flags` itself.
> 
> Something like below works ?
> 
>   - DMA-API domain : flag - DOMAIN_ALLOC_FLAG_PASID
>     If both device and IOMMU supports PASID it will allocate PASID capable
>     domain (Like AMD case domain with V2 page table). Else it will alloate
>     non-pasid capable domain (In AMD case it will be domain with v1 page table).

This is a much larger problem. The DMA API domain is created way early
before any drivers are bound. If it doesn't support PASID then no
drivers will get to use PASID at all.

You'd need to figure out some way to switch the DMA API domain on the
fly around when a PASID wanting driver binds. This might be reasonable
since PASID devices tend to need single device groups to work at all
and we could conceivably switch the group under driver control during
early binding.

For now we expect that the DMA API domain will support PASID if the
underyling device supports PASID, that is the only way this can work
today. Meaning domain_alloc_paging() must always return something that
can enable PASID.

>   - UNMANAGED domain : Do not pass *_PASID support flag
>     Since PASID flag is *not* passed, driver will decide best suitable page
>     table (in AMD case, we will allocate V1 page table)

And here this is only VFIO. When you figure out with Alex and Yi how
VFIO will decide to do PASID or not then pass that indication through
the existing flag argument on domain_alloc_user(). At least this is
pretty simple.

But as I said before, the important thing from your perspective is
that VFIO does not default-on PASID support!

Jason