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

[Vasant Hegde <vasant.hegde@amd.com>]

Jason,


On 8/6/2024 6:04 PM, Jason Gunthorpe wrote:
> 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.

This is the fair expectation for DMA API mode. That works for AMD driver as well.

I only have issue with allocating right page table for UNMANAGED domain. In this
case just 'dev' param is not sufficient. Ops should send some sort of indication
that its UNMANAGED domain (like its non DMA API domain). Otherwise AMD driver
cannot allocate right page table.

My understanding is we essentially have two modes for device passthrough.
  1 - Existing interface where vfio driver manages page table (call
iommu_paging_domain_alloc()). No PASID support in this mode.

  With this RFC I am trying to solve this scenario.

  2 - via iommufd - It will use domain_alloc_user() ops. This is the interface
where we want to support PASID w/ passthrough device.

   This is where you/Yi suggested to start discussing with Alex on Yi's PASID
support series. I am running behind. I will start looking into Yi's thread soon.

Is my understanding correct?


> 
>>   - 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.

Sure. I believe this  is for scenario 2. I will look into that soon.


-Vasant

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