Re: [PATCH 1/8] iommu: Introduce a replace API for device pasid

[Yi Liu]

On 2024/3/21 20:20, Jason Gunthorpe wrote:
> On Thu, Mar 21, 2024 at 07:26:41PM +0800, Yi Liu wrote:
> > > yes, the correct way is to undo what have been done before the fail
> > > device. However, I somehow remember that pasid capability is only
> > > available when the group is singleton. So iterate all devices of the
> > > devices just means one device in fact. If this is true, then the
> > > current code is fine although a bit confusing.
>
> Platform devicse don't have that limitation.. It is PCI only.

ok.

> > > > And the whole thing is easier to reason about if an input argument
> > > > specifies the current attached domain instead of having the driver
> > > > read it from the xarray.
> > >
> > > yep, will correct it as a fix patch.
> >
> > Hi Jason,
> >
> > It appears there are two solutions here.
> >
> > First, only undo the devices that have set_dev_pasid successfully in
> > the __iommu_set_group_pasid(), so the problematic
> > __iommu_remove_group_pasid() call at line 3378 [1] would go away.
> > This also makes the helper more self-contained. Draft patch in [2]
> >
> > Second, pass in the domain to remove_dev_pasid(). Draft patch in [3]
> >
> > Either of the above two should be able to solve the mistake you mentioned.
> > BTW. They are orthogonal, so it's also possible to apply both of them.
> > Which one is your preference then?
>
> I would do both because I also think it is not nice that the drivers
> always have to have the boiler plate to read the xarray in their
> remove..

sure. I'll send the two patches as Fix series.

--
Regards,
Yi Liu