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@intel.com 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