On 6/12/23 4:28 PM, Liu, Jingqi wrote:
On 6/7/2023 11:51 AM, Lu Baolu wrote:
The IOMMU_RESV_DIRECT flag indicates that a memory region must be mapped
1:1 at all times. This means that the region must always be accessible to
the device, even if the device is attached to a blocking domain. This is
equal to saying that IOMMU_RESV_DIRECT flag prevents devices from being
attached to blocking domains.
This also implies that devices that implement RESV_DIRECT regions will be
prevented from being assigned to user space since taking the DMA
ownership
immediately switches to a blocking domain.
The rule of preventing devices with the IOMMU_RESV_DIRECT regions from
being assigned to user space has existed in the Intel IOMMU driver for
a long time. Now, this rule is being lifted up to a general core rule,
as other architectures like AMD and ARM also have RMRR-like reserved
regions. This has been discussed in the community mailing list and refer
to below link for more details.
Other places using unmanaged domains for kernel DMA must follow the
iommu_get_resv_regions() and setup IOMMU_RESV_DIRECT - we do not restrict
them in the core code.
Cc: Robin Murphy <robin.murphy@xxxxxxx>
Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Link:
https://lore.kernel.org/linux-iommu/BN9PR11MB5276E84229B5BD952D78E9598C639@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
---
include/linux/iommu.h | 2 ++
drivers/iommu/iommu.c | 39 +++++++++++++++++++++++++++++----------
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d31642596675..fd18019ac951 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -409,6 +409,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;
@@ -422,6 +423,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,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9e0228ef612b..e59de7852067 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -959,12 +959,7 @@ static int
iommu_create_device_direct_mappings(struct iommu_domain *domain,
unsigned long pg_size;
int ret = 0;
- if (!iommu_is_dma_domain(domain))
- return 0;
-
- BUG_ON(!domain->pgsize_bitmap);
-
- pg_size = 1UL << __ffs(domain->pgsize_bitmap);
+ pg_size = domain->pgsize_bitmap ? 1UL <<
__ffs(domain->pgsize_bitmap) : 0;
Would it be better to add the following check here?
if (WARN_ON(!pg_size))
return -EINVAL;
Instead of checking latter in the loop as follows.
if (WARN_ON_ONCE(!pg_size)) {
ret = -EINVAL;
goto out;
}
I am afraid no. Only the paging domains need a valid pg_size. That's the
reason why I put it after the iommu_is_dma_domain() check. The previous
code has the same behavior too.
Best regards,
baolu
