Re: [PATCH v6 08/12] iommu/sva: Use attach/detach_pasid_dev in SVA interfaces

[Baolu Lu <baolu.lu@linux.intel.com>]

On 2022/5/12 20:03, Jason Gunthorpe wrote:
> On Thu, May 12, 2022 at 07:59:41PM +0800, Baolu Lu wrote:
>> On 2022/5/12 19:48, Jason Gunthorpe wrote:
>>> On Thu, May 12, 2022 at 01:17:08PM +0800, Baolu Lu wrote:
>>>> On 2022/5/12 13:01, Tian, Kevin wrote:
>>>>>> From: Baolu Lu <baolu.lu@linux.intel.com>
>>>>>> Sent: Thursday, May 12, 2022 11:03 AM
>>>>>>
>>>>>> On 2022/5/11 22:53, Jason Gunthorpe wrote:
>>>>>>>>> Also, given the current arrangement it might make sense to have a
>>>>>>>>> struct iommu_domain_sva given that no driver is wrappering this in
>>>>>>>>> something else.
>>>>>>>> Fair enough. How about below wrapper?
>>>>>>>>
>>>>>>>> +struct iommu_sva_domain {
>>>>>>>> +       /*
>>>>>>>> +        * Common iommu domain header,*must*  be put at the top
>>>>>>>> +        * of the structure.
>>>>>>>> +        */
>>>>>>>> +       struct iommu_domain domain;
>>>>>>>> +       struct mm_struct *mm;
>>>>>>>> +       struct iommu_sva bond;
>>>>>>>> +}
>>>>>>>>
>>>>>>>> The refcount is wrapped in bond.
>>>>>>> I'm still not sure that bond is necessary
>>>>>>
>>>>>> "bond" is the sva handle that the device drivers get through calling
>>>>>> iommu_sva_bind().
>>>>>>
>>>>>
>>>>> 'bond' was required before because we didn't have a domain to wrap
>>>>> the page table at that time.
>>>>>
>>>>> Now we have a domain and it is 1:1 associated to bond. Probably
>>>>> make sense now by just returning the domain as the sva handle
>>>>> instead?
>>>>
>>>> It also includes the device information that the domain has been
>>>> attached. So the sva_unbind() looks like this:
>>>>
>>>> /**
>>>>    * iommu_sva_unbind_device() - Remove a bond created with
>>>> iommu_sva_bind_device
>>>>    * @handle: the handle returned by iommu_sva_bind_device()
>>>>    *
>>>>    * Put reference to a bond between device and address space. The device
>>>> should
>>>>    * not be issuing any more transaction for this PASID. All outstanding page
>>>>    * requests for this PASID must have been flushed to the IOMMU.
>>>>    */
>>>> void iommu_sva_unbind_device(struct iommu_sva *handle)
>>>>
>>>> It's fine to replace the iommu_sva with iommu_sva_domain for sva handle,
>>>> if we can include the device in the unbind() interface.
>>>
>>> Why would we have a special unbind for SVA?
>>
>> It's about SVA kAPI for device drivers. The existing kAPIs include:
>>
>> struct iommu_sva *iommu_sva_bind_device(struct device *dev,
>>                                          struct mm_struct *mm,
>>                                          void *drvdata);
>> void iommu_sva_unbind_device(struct iommu_sva *handle);
>> u32 iommu_sva_get_pasid(struct iommu_sva *handle);
> 
> This is not what we agreed the API should be. We agreed:
> 
>   iommu_sva_domain_alloc()
>   iommu_attach_device_pasid()
>   iommu_detach_device_pasid()
> 
> Again, SVA should not be different from normal domain stuff.

Yes, agreed.

I am trying to achieve this in two steps. This first step focuses on
internal iommu implementation and keep the driver kAPI untouched. Then,
the second step focus on the driver APIs.

Best regards,
baolu