Re: [PATCH v5 04/12] iommu/sva: Basic data structures for SVA

Baolu Lu
2022-05-07T12:40:20Z

On 2022/5/7 16:32, Baolu Lu wrote:
> Hi Jean,
>
> On 2022/5/5 14:42, Baolu Lu wrote:
>> On 2022/5/4 02:09, Jean-Philippe Brucker wrote:
>>> On Mon, May 02, 2022 at 09:48:34AM +0800, Lu Baolu wrote:
>>>> Use below data structures for SVA implementation in the IOMMU core:
>>>>
>>>> - struct iommu_sva_ioas
>>>>    Represent the I/O address space shared with an application CPU
>>>> address
>>>>    space. This structure has a 1:1 relationship with an mm_struct. It
>>>>    grabs a "mm->mm_count" refcount during creation and drop it on
>>>> release.
>>>
>>> Do we actually need this structure?  At the moment it only keeps
>>> track of
>>> bonds, which we can move to struct dev_iommu. Replacing it by a mm
>>> pointer
>>> in struct iommu_domain simplifies the driver and seems to work
>>
>> Fair enough.
>>
>> +struct iommu_sva_ioas {
>> +    struct mm_struct *mm;
>> +    ioasid_t pasid;
>> +
>> +    /* Counter of domains attached to this ioas. */
>> +    refcount_t users;
>> +
>> +    /* All bindings are linked here. */
>> +    struct list_head bonds;
>> +};
>>
>> By moving @mm to struct iommu_domain and @bonds to struct dev_iommu, the
>> code looks simpler. The mm, sva domain and per-device dev_iommu are
>> guaranteed to be valid during bind() and unbind().
>>
>> Will head this direction in the next version.
>
> I'm trying to implement this idea in real code. It seems that we need
> additional fields in struct iommu_domain to track which devices the mm
> was bound to. It doesn't simplify the code much. Any thoughts?

Sorry, Jean. This has been discussed. We don't need to share sva domain
among devices at this stage. It's not a big issue to sva domain as it's
a dumb domain which has no support for map()/unmap() and the cache
manipulation.

I will still head this direction. Sorry for the noise.

Best regards,
baolu