On Thu, May 05, 2022 at 07:56:59PM +0100, Robin Murphy wrote:
> Ack to that, there are certainly further improvements to consider once we've
> got known-working code into a released kernel, but let's not get ahead of
> ourselves just now.
Yes please
> (I'm pretty sure we could get away with a single blocking domain per IOMMU
> instance, rather than per group, but I deliberately saved that one for later
> - right now one per group to match default domains is simpler to reason
> about and less churny in the context of the current ownership patches)
I noticed this too..
But I thought the driver can do a better job of this. There is no
reason it has to allocate memory to return a IOMMU_DOMAIN_BLOCKED
domain - this struct could be a globally allocated singleton for the
entire driver and that would be even better memory savings.
For instance, here is a sketch for the Intel driver based on Baolu's
remark that intel_iommu_detach_device() establishes a blocking
behavior already on detach_dev (Baolu if you like it can you make a
proper patch?):
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index df5c62ecf942b8..317945f6134d42 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4324,6 +4324,8 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
return domain;
case IOMMU_DOMAIN_IDENTITY:
return &si_domain->domain;
+ case IOMMU_DOMAIN_BLOCKED;
+ return &intel_blocking_domain;
default:
return NULL;
}
@@ -4404,12 +4406,25 @@ static int intel_iommu_attach_device(struct
iommu_domain *domain,
return domain_add_dev_info(to_dmar_domain(domain), dev);
}
-static void intel_iommu_detach_device(struct iommu_domain *domain,
- struct device *dev)
+static int intel_iommu_block_dev(struct iommu_domain *domain,
+ struct device *dev)
{
+ if (!device_to_iommu(dev, NULL, NULL))
+ return -EINVAL;
+
dmar_remove_one_dev_info(dev);
+ return 0;
}
+static struct iommu_domain intel_blocking_domain {
+ .type = IOMMU_DOMAIN_BLOCKED,
+ .ops = &(const struct iommu_domain_ops){
+ .attach_dev = intel_iommu_detach_device,
+ // Fix core code so this works
+ .free = NULL,
+ },
+};
+
static int intel_iommu_map(struct iommu_domain *domain,
unsigned long iova, phys_addr_t hpa,
size_t size, int iommu_prot, gfp_t gfp)
@@ -4890,7 +4905,6 @@ const struct iommu_ops intel_iommu_ops = {
#endif
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = intel_iommu_attach_device,
- .detach_dev = intel_iommu_detach_device,
.map_pages = intel_iommu_map_pages,
.unmap_pages = intel_iommu_unmap_pages,
.iotlb_sync_map = intel_iommu_iotlb_sync_map,
Thanks,
Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
