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 for Android: free password hash cracker in your pocket

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

Date: Wed, 21 Feb 2024 07:41:49 +0000
From: "Zhang, Tina" <tina.zhang@...el.com>
To: Zhangfei Gao <zhangfei.gao@...aro.org>, Baolu Lu
	<baolu.lu@...ux.intel.com>
CC: "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

Hi,

> -----Original Message-----
> From: Zhangfei Gao <zhangfei.gao@...aro.org>
> Sent: Wednesday, February 21, 2024 10:45 AM
> To: Baolu Lu <baolu.lu@...ux.intel.com>
> Cc: Zhang, Tina <tina.zhang@...el.com>; iommu@...ts.linux.dev; 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 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.
The domain->user is a refcount of the devices (or iommu group) attached to the domain.  IOMMU core needs to keep this refcount to ensure that a sva domain will be released when no device uses it.

Regards,
-Tina

> 
> Thanks