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

[Jason Gunthorpe]

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.

> > > 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..

Jason