Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <86fd8977-1134-02d2-d9e3-19ce58cb9de4@linux.intel.com>
Date:   Sat, 7 May 2022 20:39:49 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH v5 04/12] iommu/sva: Basic data structures for SVA
Content-Language: en-US
From:   Baolu Lu <baolu.lu@linux.intel.com>
To:     Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc:     Vinod Koul <vkoul@kernel.org>, Kevin Tian <kevin.tian@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>,
        Jean-Philippe Brucker <jean-philippe@linaro.com>,
        linux-kernel@vger.kernel.org,
        Christoph Hellwig <hch@infradead.org>,
        iommu@lists.linux-foundation.org,
        Jacob jun Pan <jacob.jun.pan@intel.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Robin Murphy <robin.murphy@arm.com>
References: <20220502014842.991097-1-baolu.lu@linux.intel.com>
 <20220502014842.991097-5-baolu.lu@linux.intel.com> <YnFv0ps0Ad8v+7uH@myrica>
 <d490e542-140c-58c3-bb11-9990795272b1@linux.intel.com>
 <669fb111-9821-aadc-acbf-de42bc551fc4@linux.intel.com>
In-Reply-To: <669fb111-9821-aadc-acbf-de42bc551fc4@linux.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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