Re: [PATCH v10 6/7] iommu/vt-d: Add set_dev_pasid callback for dma domain

[Baolu Lu]

On 2023/7/14 11:50, Tian, Kevin wrote:
> > From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> > Sent: Friday, July 14, 2023 11:34 AM
> >
> > On 2023/7/13 15:56, Tian, Kevin wrote:
> > > > From: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
> > > > Sent: Thursday, July 13, 2023 12:34 AM
> > > >
> > > >
> > > > -	/*
> > > > -	 * Should never reach here until we add support for attaching
> > > > -	 * non-SVA domain to a pasid.
> > > > -	 */
> > > > -	WARN_ON(1);
> > > > +	dmar_domain = to_dmar_domain(domain);
> > > > +	spin_lock_irqsave(&dmar_domain->lock, flags);
> > > > +	list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain)
> > > > {
> > > > +		if (curr->dev == dev && curr->pasid == pasid) {
> > > > +			list_del(&curr->link_domain);
> > > > +			dev_pasid = curr;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +	spin_unlock_irqrestore(&dmar_domain->lock, flags);
> > >
> > > what about no matching dev_pasid is find?
> >
> > kfree() can handle this gracefully.
>
> but what about domain_detach_iommu()? Is it correct to adjust
> the refcnting when there is no matching dev_pasid?

You are right.

Logically, if we get a valid domain for a pasid, we should have a
dev_pasid allocated for it. Perhaps, adding a check in the code will
make the code more readable?

Best regards,
baolu