On 2022-05-04 01:11, Jason Gunthorpe wrote:
Once the group enters 'owned' mode it can never be assigned back to the
default_domain or to a NULL domain. It must always be actively assigned to
a current domain. If the caller hasn't provided a domain then the core
must provide an explicit DMA blocking domain that has no DMA map.
Lazily create a group-global blocking DMA domain when
iommu_group_claim_dma_owner is first called and immediately assign the
group to it. This ensures that DMA is immediately fully isolated on all
IOMMU drivers.
If the user attaches/detaches while owned then detach will set the group
back to the blocking domain.
Slightly reorganize the call chains so that
__iommu_group_attach_core_domain() is the function that removes any caller
configured domain and sets the domains back a core owned domain with an
appropriate lifetime.
__iommu_group_attach_domain() is the worker function that can change the
domain assigned to a group to any target domain, including NULL.
Add comments clarifying how the NULL vs detach_dev vs default_domain works
based on Robin's remarks.
This fixes an oops with VFIO and SMMUv3 because VFIO will call
iommu_detach_group() and then immediately iommu_domain_free(), but
SMMUv3 has no way to know that the domain it is holding a pointer to
has been freed. Now the iommu_detach_group() will assign the blocking
domain and SMMUv3 will no longer hold a stale domain reference.
Thanks for taking this on! I do like the overall structure and naming
much more than my initial sketch :)
Fixes: 1ea2a07a532b ("iommu: Add DMA ownership management interfaces")
Reported-by: Qian Cai <quic_qian...@quicinc.com>
Co-developed-by: Robin Murphy <robin.mur...@arm.com>
(so the sign-off chain makes sense - you deserve overall authorship here)
Signed-off-by: Robin Murphy <robin.mur...@arm.com>
Signed-off-by: Jason Gunthorpe <j...@nvidia.com>
---
[...]
@@ -2072,38 +2072,66 @@ static int iommu_group_do_detach_device(struct device
*dev, void *data)
return 0;
}
-static void __iommu_detach_group(struct iommu_domain *domain,
- struct iommu_group *group)
+static int __iommu_group_attach_domain(struct iommu_group *group,
+ struct iommu_domain *new_domain)
{
int ret;
+ if (group->domain == new_domain)
+ return 0;
+
/*
- * If the group has been claimed already, do not re-attach the default
- * domain.
+ * A NULL domain means to call the detach_dev() op. New drivers should
+ * use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain
Nit: IOMMU_DOMAIN_DMA is the baseline of default domain support,
passthrough is more of an optional extra.
+ * and detatch_dev().
Nit: detach_dev
Otherwise, modulo the other things already pointed out, this looks OK to me.
Cheers,
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
