On Mon, Jun 19, 2023 at 02:33:18PM +0100, Robin Murphy wrote:
@@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
{
int ret;
+ /*
+ * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
+ * the blocking domain to be attached as it does not contain the
+ * required 1:1 mapping. This test effectively exclusive the device from
+ * being used with iommu_group_claim_dma_owner() which will block vfio
+ * and iommufd as well.
+ */
+ if (dev->iommu->requires_direct &&
+ (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
Given the notion elsewhere that we want to use the blocking domain as a last
resort to handle an attach failure,
We shouldn't do that for cases where requires_direct is true, the last
resort will have to be the static identity domain.
at face value it looks suspect that failing to attach to a blocking
domain could also be a thing. I guess technically this is failing at
a slightly different level so maybe it does work out OK, but it's
still smelly.
It basically says that this driver doesn't support blocking domains on
this device. What we don't want is for the driver to fail blocking or
identity attaches.
The main thing, though, is that not everything implements the
IOMMU_DOMAIN_BLOCKED optimisation, so a nominal blocking domain could be
IOMMU_DOMAIN_UNMANAGED as well.
Yes, it should check new_domain == group->blocking_domain as well.
FWIW I'd prefer to make the RESV_DIRECT check explicit in
__iommu_take_dma_ownership() rather than hide it in an
implementation detail; that's going to be a lot clearer to reason
about as time goes on.
We want to completely forbid blocking domains at all on these devices
because they are not supported (by FW request). I don't really like
the idea that we go and assume the only users of blocking domains are
also using take_dma_ownership() - that feels like a future bug waiting
to happen.