linux-kernel - Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva domains

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] [thread-next>] [day] [month] [year] [list]

Date: Wed, 21 Feb 2024 10:06:15 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Zhangfei Gao <zhangfei.gao@...aro.org>, "Zhang, Tina"
 <tina.zhang@...el.com>
Cc: baolu.lu@...ux.intel.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>
Subject: Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva domains

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?

 From iommu core's perspective, it doesn't make sense to attach the same
domain to the same device (or pasid) multiple times.

Best regards,
baolu