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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 7 May 2022 20:39:49 +0800
From:   Baolu Lu <baolu.lu@...ux.intel.com>
To:     Jean-Philippe Brucker <jean-philippe@...aro.org>
Cc:     Vinod Koul <vkoul@...nel.org>, Kevin Tian <kevin.tian@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        linux-kernel@...r.kernel.org,
        Christoph Hellwig <hch@...radead.org>,
        iommu@...ts.linux-foundation.org,
        Jacob jun Pan <jacob.jun.pan@...el.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Robin Murphy <robin.murphy@....com>
Subject: Re: [PATCH v5 04/12] iommu/sva: Basic data structures for SVA

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