On Thu, Apr 20, 2023 at 03:49:33PM -0600, Alex Williamson wrote:
> > If we think this policy deserves to go beyond VFIO and userspace, and
> > it's reasonable that such devices should never be allowed to attach to
> > any other kind of kernel-owned unmanaged domain either, then we can
> > still trivially enforce that in core IOMMU code. I really see no need
> > for it to be in drivers at all.
>
> It seems like a reasonable choice to me that any mixing of unmanaged
> domains with IOMMU_RESV_DIRECT could be restricted globally. Do we
> even have infrastructure for a driver to honor the necessary mapping
> requirements?
What we discussed about the definition of IOMMU_RESV_DIRECT was that
an identity map needs to be present at all times. This is what is
documented at least:
/* Memory regions which must be mapped 1:1 at all times */
IOMMU_RESV_DIRECT,
Notably, this also means the device can never be attached to a
blocking domain. I would also think that drivers asking for this
should ideally implement the "atomic replace" we discussed already to
change between identity and unmanaged without disturbing the FW doing
DMA to these addresses..
I was looking at this when we talked about it earlier and we don't
follow that guideline today for vfio/iommufd.
Since taking ownership immediately switches to a blocking domain
restricting the use of blocking also restricts ownership thus vfio and
iommufd will be prevented.
Other places using unmanaged domains must follow the
iommu_get_resv_regions() and setup IOMMU_RESV_DIRECT - we should not
restrict them in the core code.
It also slightly changes my prior remarks to Robin about error domain
attach handling, since blocking domains are not available for these
devices the "error state" for such a device should be the identity
domain to preserve FW access.
Also, we have a functional gap, ARM would really like a
IOMMU_RESV_DIRECT_RELAXABLE_SAFE which would have iommufd/vfio install
the 1:1 map and allow the device to be used. This is necessary for the
GIC ITS page hack to support MSI since we should enable VFIO inside a
VM. It is always safe for hostile VFIO userspace to DMA to the ITS
page.
So, after my domain error handling series, the core fix is pretty
simple and universal. We should also remove all the redundant code in
drivers - drivers should report the regions each devices needs
properly and leave enforcement to the core code.. Lu/Kevin do you want
to take this?
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 19f8d28ff1323c..c15eb5e0ba761d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1059,6 +1059,9 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
entry->type != IOMMU_RESV_DIRECT_RELAXABLE)
continue;
+ if (entry->type == IOMMU_RESV_DIRECT)
+ dev->iommu->requires_direct = 1;
+
for (addr = start; addr <= end; addr += pg_size) {
phys_addr_t phys_addr;
@@ -2210,6 +2213,22 @@ 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 ||
+ new_domain == group->blocking_domain)) {
+ dev_warn(
+ dev,
+ "Firmware has requested this device have a 1:1 IOMMU mapping, rejecting configuring the device without a 1:1 mapping. Contact your platform vendor.");
+ return -EINVAL;
+ }
+
if (dev->iommu->attach_deferred) {
if (new_domain == group->default_domain)
return 0;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 3ad14437487638..7729a07923faa6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -407,6 +407,7 @@ struct iommu_fault_param {
* @priv: IOMMU Driver private data
* @max_pasids: number of PASIDs this device can consume
* @attach_deferred: the dma domain attachment is deferred
+ * @requires_direct: The driver requested IOMMU_RESV_DIRECT
*
* TODO: migrate other per device data pointers under iommu_dev_data, e.g.
* struct iommu_group *iommu_group;
@@ -420,6 +421,7 @@ struct dev_iommu {
void *priv;
u32 max_pasids;
u32 attach_deferred:1;
+ u32 requires_direct:1;
};
int iommu_device_register(struct iommu_device *iommu,
