Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [PATCH v1 2/8] iommu: Introduce a new iommu_group_replace_domain() API | Date | Fri, 3 Feb 2023 08:26:44 +0000 |
| |
> From: Nicolin Chen <nicolinc@nvidia.com> > Sent: Thursday, February 2, 2023 3:05 PM > > All drivers are already required to support changing between active > UNMANAGED domains when using their attach_dev ops.
All drivers which don't have *broken* UNMANAGED domain?
> > +/** > + * iommu_group_replace_domain - replace the domain that a group is > attached to > + * @new_domain: new IOMMU domain to replace with > + * @group: IOMMU group that will be attached to the new domain > + * > + * This API allows the group to switch domains without being forced to go to > + * the blocking domain in-between. > + * > + * If the attached domain is a core domain (e.g. a default_domain), it will act > + * just like the iommu_attach_group().
I think you meant "the currently-attached domain", which implies a 'detached' state as you replied to Baolu.
> + */ > +int iommu_group_replace_domain(struct iommu_group *group, > + struct iommu_domain *new_domain)
what actual value does 'replace' give us? It's just a wrapper of __iommu_group_set_domain() then calling it set_domain is probably clearer. You can clarify the 'replace' behavior in the comment.
> +{ > + int ret; > + > + if (!new_domain) > + return -EINVAL; > + > + mutex_lock(&group->mutex); > + ret = __iommu_group_set_domain(group, new_domain); > + if (ret) { > + if (__iommu_group_set_domain(group, group->domain)) > + __iommu_group_set_core_domain(group); > + }
Can you elaborate the error handling here? Ideally if __iommu_group_set_domain() fails then group->domain shouldn't be changed. Why do we need further housekeeping here?
| |