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
