Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type
From: Lu Baolu
Date: Wed May 08 2019 - 22:37:54 EST
Hi Robin,
On 5/7/19 6:28 PM, Robin Murphy wrote:
On 06/05/2019 16:32, Tom Murphy via iommu wrote:
The AMD driver already solves this problem and uses the generic
iommu_request_dm_for_dev function. It seems like both drivers have the
same problem and could use the same solution. Is there any reason we
can't have use the same solution for the intel and amd driver?
Could we just copy the implementation of the AMD driver? It would be
nice to have the same behavior across both drivers especially as we
move to make both drivers use more generic code.
TBH I don't think the API really needs to be involved at all here.
Drivers can already not provide the requested default domain type if
they don't support it, so as long as the driver can ensure that the
device ends up with IOMMU or direct DMA ops as appropriate, I don't see
any great problem with drivers just returning a passthrough domain when
a DMA domain was requested, or vice versa (and logging a message that
the requested type was overridden). The only type that we really do have
to honour strictly is non-default (i.e. unmanaged) domains.
I agree with you that we only have to honor strictly the non-default
domains. But domain type saved in iommu_domain is consumed in iommu.c
and exposed to user through sysfs. It's not clean if the iommu driver
silently replace the default domain.
Best regards,
Lu Baolu
Robin.
On Mon, Apr 29, 2019 at 3:16 AM Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
wrote:
This adds an optional ops entry to query the default domain
types supported by the iommu driver for a specific device.
This is necessary in cases where the iommu driver can only
support a specific type of default domain for a device. In
normal cases, this ops will return IOMMU_DOMAIN_ANY which
indicates that the iommu driver supports both IOMMU_DOMAIN_DMA
and IOMMU_DOMAIN_IDENTITY, hence the static default domain
type will be used.
Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
---
 drivers/iommu/iommu.c | 13 ++++++++++---
 include/linux/iommu.h | 11 +++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index acd6830e6e9b..1ad9a1f2e078 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1097,15 +1097,22 @@ struct iommu_group
*iommu_group_get_for_dev(struct device *dev)
ÂÂÂÂÂÂÂÂÂ * IOMMU driver.
ÂÂÂÂÂÂÂÂÂ */
ÂÂÂÂÂÂÂÂ if (!group->default_domain) {
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ unsigned int domain_type = IOMMU_DOMAIN_ANY;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct iommu_domain *dom;
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dom = __iommu_domain_alloc(dev->bus,
iommu_def_domain_type);
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (ops->def_domain_type)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ domain_type = ops->def_domain_type(dev);
+
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (domain_type == IOMMU_DOMAIN_ANY)
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ domain_type = iommu_def_domain_type;
+
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dom = __iommu_domain_alloc(dev->bus, domain_type);
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (!dom && domain_type != IOMMU_DOMAIN_DMA) {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dom = __iommu_domain_alloc(dev->bus,
IOMMU_DOMAIN_DMA);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (dom) {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dev_warn(dev,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "failed to allocate default
IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ iommu_def_domain_type);
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ domain_type);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 8239ece9fdfc..ba9a5b996a63 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -79,12 +79,16 @@ struct iommu_domain_geometry {
ÂÂ *ÂÂÂÂ IOMMU_DOMAIN_DMAÂÂÂÂÂÂÂ - Internally used for DMA-API
implementations.
ÂÂ *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ This flag allows IOMMU drivers to
implement
ÂÂ *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ certain optimizations for these
domains
+ *ÂÂÂÂ IOMMU_DOMAIN_ANYÂÂÂÂÂÂÂ - All domain types defined here
ÂÂ */
 #define IOMMU_DOMAIN_BLOCKED (0U)
 #define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT)
 #define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
 #define IOMMU_DOMAIN_DMA (__IOMMU_DOMAIN_PAGING | \
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ __IOMMU_DOMAIN_DMA_API)
+#define IOMMU_DOMAIN_ANYÂÂÂÂÂÂ (IOMMU_DOMAIN_IDENTITY |ÂÂÂÂÂÂÂ \
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ IOMMU_DOMAIN_UNMANAGED |ÂÂÂÂÂÂ \
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ IOMMU_DOMAIN_DMA)
 struct iommu_domain {
ÂÂÂÂÂÂÂÂ unsigned type;
@@ -196,6 +200,11 @@ enum iommu_dev_features {
ÂÂ * @dev_feat_enabled: check enabled feature
ÂÂ * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
ÂÂ * @aux_get_pasid: get the pasid given an aux-domain
+ * @def_domain_type: get per-device default domain type that the IOMMU
+ *ÂÂÂÂÂÂÂÂÂÂÂÂ driver is able to support. Valid returns values:
+ *ÂÂÂÂÂÂÂÂÂÂÂÂ - IOMMU_DOMAIN_DMA: only suports non-identity domain
+ *ÂÂÂÂÂÂÂÂÂÂÂÂ - IOMMU_DOMAIN_IDENTITY: only supports identity domain
+ *ÂÂÂÂÂÂÂÂÂÂÂÂ - IOMMU_DOMAIN_ANY: supports all
ÂÂ * @pgsize_bitmap: bitmap of all possible supported page sizes
ÂÂ */
 struct iommu_ops {
@@ -251,6 +260,8 @@ struct iommu_ops {
ÂÂÂÂÂÂÂÂ void (*aux_detach_dev)(struct iommu_domain *domain, struct
device *dev);
ÂÂÂÂÂÂÂÂ int (*aux_get_pasid)(struct iommu_domain *domain, struct
device *dev);
+ÂÂÂÂÂÂ int (*def_domain_type)(struct device *dev);
+
ÂÂÂÂÂÂÂÂ unsigned long pgsize_bitmap;
 };
--
2.17.1
_______________________________________________
iommu mailing list
iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/iommu