On 9/6/24 12:21 PM, Yi Liu wrote:
On 2024/8/16 21:02, Jason Gunthorpe wrote:
On Fri, Aug 16, 2024 at 05:43:18PM +0800, Yi Liu wrote:
On 2024/7/18 16:27, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Friday, June 28, 2024 5:06 PM
@@ -3289,7 +3290,20 @@ static int __iommu_set_group_pasid(struct
iommu_domain *domain,
if (device == last_gdev)
break;
- ops->remove_dev_pasid(device->dev, pasid, domain);
+ /* If no old domain, undo the succeeded devices/pasid */
+ if (!old) {
+ ops->remove_dev_pasid(device->dev, pasid, domain);
+ continue;
+ }
+
+ /*
+ * Rollback the succeeded devices/pasid to the old domain.
+ * And it is a driver bug to fail attaching with a previously
+ * good domain.
+ */
+ if (WARN_ON(old->ops->set_dev_pasid(old, device->dev,
+ pasid, domain)))
+ ops->remove_dev_pasid(device->dev, pasid, domain);
I wonder whether @remove_dev_pasid() can be replaced by having
blocking_domain support @set_dev_pasid?
how about your thought, @Jason?
I think we talked about doing that once before, I forget why it was
not done. Maybe there was an issue?
But it seems worth trying.
Since remove_dev_pasid() does not return a result, so caller does not
need to check the result of it. If we want to replace it with the
blocked_domain->ops->set_dev_pasid(), shall we enforce that the
set_dev_pasid() op of blocked_domain to be always success. Is it?
Yes. The semantics of blocking domain is that the iommu driver must
ensure successful completion.
Thanks,
baolu
