[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <55a4b5e5-e0ad-4aa4-a29b-5fff4ef69063@linux.intel.com>
Date: Wed, 21 Feb 2024 11:52:48 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Zhangfei Gao <zhangfei.gao@...aro.org>
Cc: baolu.lu@...ux.intel.com, "Zhang, Tina" <tina.zhang@...el.com>,
"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
David Woodhouse <dwmw2@...radead.org>, Joerg Roedel <joro@...tes.org>,
Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
Jason Gunthorpe <jgg@...pe.ca>, "Tian, Kevin" <kevin.tian@...el.com>,
Nicolin Chen <nicolinc@...dia.com>, Michael Shavit <mshavit@...gle.com>,
Vasant Hegde <vasant.hegde@....com>, Jason Gunthorpe <jgg@...dia.com>,
Jean-Philippe Brucker <jean-philippe@...aro.org>,
Hao Fang <fanghao11@...wei.com>
Subject: Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva domains
On 2024/2/21 10:45, Zhangfei Gao wrote:
> On Wed, 21 Feb 2024 at 10:06, Baolu Lu<baolu.lu@...ux.intel.com>
> wrote:
>> On 2024/2/21 9:28, Zhangfei Gao wrote:
>>> On Wed, 21 Feb 2024 at 07:58, Zhang, Tina<tina.zhang@...el.com>
>>> wrote:
>>>
>>>>>> struct iommu_sva *iommu_sva_bind_device(struct device
>>>>>> *dev, struct mm_struct *mm) { + struct
>>>>>> iommu_mm_data *iommu_mm; struct iommu_domain *domain;
>>>>>> struct iommu_sva *handle; int ret;
>>>>>>
>>>>>> + mutex_lock(&iommu_sva_lock); + /* Allocate
>>>>>> mm->pasid if necessary. */ - ret =
>>>>>> iommu_sva_alloc_pasid(mm, dev); - if (ret) - return
>>>>>> ERR_PTR(ret); + iommu_mm = iommu_alloc_mm_data(mm,
>>>>>> dev); + if (IS_ERR(iommu_mm)) { + ret =
>>>>>> PTR_ERR(iommu_mm); + goto out_unlock; + }
>>>>>>
>>>>>> handle = kzalloc(sizeof(*handle), GFP_KERNEL); - if
>>>>>> (!handle) - return ERR_PTR(-ENOMEM); - -
>>>>>> mutex_lock(&iommu_sva_lock); - /* Search for an
>>>>>> existing domain. */ - domain =
>>>>>> iommu_get_domain_for_dev_pasid(dev, mm->pasid, -
>>>>>> IOMMU_DOMAIN_SVA); - if (IS_ERR(domain)) { - ret =
>>>>>> PTR_ERR(domain); + if (!handle) { + ret = -ENOMEM;
>>>>>> goto out_unlock; }
>>>>>>
>>>>>> - if (domain) { - domain->users++; -
>>>>>> goto out;
>>>>> Our multi bind test case broke since 6.8-rc1. The test case
>>>>> can use same domain & pasid, return different handle, 6.7
>>>>> simply domain->users ++ and return.
>>>>>
>>>>>> + /* Search for an existing domain. */ +
>>>>>> list_for_each_entry(domain, &mm->iommu_mm->sva_domains,
>>>>>> next)
>>>>> {
>>>>>> + ret = iommu_attach_device_pasid(domain,
>>>>>> dev, + iommu_mm->pasid);
>>>>> Now iommu_attach_device_pasid return BUSY since the same
>>>>> pasid. And then iommu_sva_bind_device attach ret=-16
>>>> Sounds like the test case tries to bind a device to a same mm
>>>> multiple times without unbinding the device and the
>>>> expectation is that it can always return a valid handle to pass
>>>> the test. Right?
>>> Yes
>>>
>>> The device can bind to the same mm multi-times and return
>>> different handle, Since the refcount, no need to unbind and bind
>>> sequently, The unbind can happen later with the handle.
>> Is there any real use case to bind an mm to the pasid of a device
>> multiple times? If there are cases, is it better to handle this in
>> the uacce driver?
> Yes, it is required for multi-thread, the device can provide
> multi-queue to speed up.
>
>> From iommu core's perspective, it doesn't make sense to attach the
>> same domain to the same device (or pasid) multiple times.
> But is it the refcount domain->user++ used for? Is there any reason
> not doing this.
I was just thinking about whether to do this in the iommu core, or in
the upper layers, like uacce or iommufd. It seems that there is no need
to attach a domain to a device or pasid again if it has already been
attached.
Best regards,
baolu
Powered by blists - more mailing lists