2023-05-04 21:26:57

by Robin Murphy

[permalink] [raw]
Subject: [PATCH 0/2] iommu: Make flush queues a proper capability

Hi all,

Since it came up in discussion on the default domain series, it seemed
pertinent to dig this idea up, whcih I started a while ago, and finish
it off.

Cheers,
Robin.


Robin Murphy (2):
iommu: Add a capability for flush queue support
iommu: Use flush queue capability

drivers/iommu/amd/iommu.c | 2 ++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 4 ++--
drivers/iommu/dma-iommu.c | 3 ++-
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/iommu.c | 3 ++-
include/linux/iommu.h | 6 ++++++
7 files changed, 16 insertions(+), 6 deletions(-)

--
2.39.2.101.g768bb238c484.dirty


2023-05-04 21:28:20

by Robin Murphy

[permalink] [raw]
Subject: [PATCH 1/2] iommu: Add a capability for flush queue support

Passing a special type to domain_alloc to indirectly query whether flush
queues are a worthwhile optimisation with the given driver is a bit
clunky, and looking increasingly anachronistic. Let's put that into an
explicit capability instead.

Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/amd/iommu.c | 2 ++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
drivers/iommu/arm/arm-smmu/arm-smmu.c | 1 +
drivers/iommu/intel/iommu.c | 1 +
include/linux/iommu.h | 5 +++++
5 files changed, 10 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 4a314647d1f7..9b7bd6bed664 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2293,6 +2293,8 @@ static bool amd_iommu_capable(struct device *dev, enum iommu_cap cap)
return amdr_ivrs_remap_support;
case IOMMU_CAP_ENFORCE_CACHE_COHERENCY:
return true;
+ case IOMMU_CAP_DEFERRED_FLUSH:
+ return true;
default:
break;
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 3fd83fb75722..6d65a7e81df4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2008,6 +2008,7 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
/* Assume that a coherent TCU implies coherent TBUs */
return master->smmu->features & ARM_SMMU_FEAT_COHERENCY;
case IOMMU_CAP_NOEXEC:
+ case IOMMU_CAP_DEFERRED_FLUSH:
return true;
default:
return false;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 6e0813b26fb6..7f4ee365912c 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1325,6 +1325,7 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap)
return cfg->smmu->features & ARM_SMMU_FEAT_COHERENT_WALK ||
device_get_dma_attr(dev) == DEV_DMA_COHERENT;
case IOMMU_CAP_NOEXEC:
+ case IOMMU_CAP_DEFERRED_FLUSH:
return true;
default:
return false;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index b871a6afd803..ff923298f8ed 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4369,6 +4369,7 @@ static bool intel_iommu_capable(struct device *dev, enum iommu_cap cap)

switch (cap) {
case IOMMU_CAP_CACHE_COHERENCY:
+ case IOMMU_CAP_DEFERRED_FLUSH:
return true;
case IOMMU_CAP_PRE_BOOT_PROTECTION:
return dmar_platform_optin();
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e8c9a7da1060..1b7180d6edae 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -127,6 +127,11 @@ enum iommu_cap {
* this device.
*/
IOMMU_CAP_ENFORCE_CACHE_COHERENCY,
+ /*
+ * IOMMU driver does not issue TLB maintenance during .unmap, so can
+ * usefully support the non-strict DMA flush queue.
+ */
+ IOMMU_CAP_DEFERRED_FLUSH,
};

/* These are the possible reserved region types */
--
2.39.2.101.g768bb238c484.dirty

2023-05-04 21:57:40

by Robin Murphy

[permalink] [raw]
Subject: [PATCH 2/2] iommu: Use flush queue capability

It remains really handy to have distinct DMA domain types within core
code for the sake of default domain policy selection, but we can now
hide that detail from drivers by using the new capability instead.

Signed-off-by: Robin Murphy <[email protected]>

---

Note that IOMMU_DOMAIN_ALLOC_FLAGS would go away again with the
proposed domain_alloc_paging() interface design.
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 -
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +--
drivers/iommu/dma-iommu.c | 3 ++-
drivers/iommu/intel/iommu.c | 1 -
drivers/iommu/iommu.c | 3 ++-
include/linux/iommu.h | 1 +
6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 6d65a7e81df4..1ed9c4ed5db9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2024,7 +2024,6 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)

if (type != IOMMU_DOMAIN_UNMANAGED &&
type != IOMMU_DOMAIN_DMA &&
- type != IOMMU_DOMAIN_DMA_FQ &&
type != IOMMU_DOMAIN_IDENTITY)
return NULL;

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 7f4ee365912c..a86acd76c1df 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -856,8 +856,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
struct arm_smmu_domain *smmu_domain;

if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY) {
- if (using_legacy_binding ||
- (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_DMA_FQ))
+ if (using_legacy_binding || type != IOMMU_DOMAIN_DMA)
return NULL;
}
/*
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7a9f0b0bddbd..c4bdd2587daf 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -586,7 +586,8 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
goto done_unlock;

/* If the FQ fails we can simply fall back to strict mode */
- if (domain->type == IOMMU_DOMAIN_DMA_FQ && iommu_dma_init_fq(domain))
+ if (domain->type == IOMMU_DOMAIN_DMA_FQ &&
+ (!device_iommu_capable(dev, IOMMU_CAP_DEFERRED_FLUSH) || iommu_dma_init_fq(domain)))
domain->type = IOMMU_DOMAIN_DMA;

ret = iova_reserve_iommu_regions(dev, domain);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ff923298f8ed..8096273b034c 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4064,7 +4064,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
case IOMMU_DOMAIN_BLOCKED:
return &blocking_domain;
case IOMMU_DOMAIN_DMA:
- case IOMMU_DOMAIN_DMA_FQ:
case IOMMU_DOMAIN_UNMANAGED:
dmar_domain = alloc_domain(type);
if (!dmar_domain) {
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f1dcfa3f1a1b..7078bf4a8ec8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1980,11 +1980,12 @@ static struct iommu_domain *__iommu_domain_alloc(const struct bus_type *bus,
unsigned type)
{
struct iommu_domain *domain;
+ unsigned int alloc_type = type & IOMMU_DOMAIN_ALLOC_FLAGS;

if (bus == NULL || bus->iommu_ops == NULL)
return NULL;

- domain = bus->iommu_ops->domain_alloc(type);
+ domain = bus->iommu_ops->domain_alloc(alloc_type);
if (!domain)
return NULL;

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 1b7180d6edae..d31642596675 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -65,6 +65,7 @@ struct iommu_domain_geometry {

#define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */

+#define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ
/*
* This are the possible domain-types
*
--
2.39.2.101.g768bb238c484.dirty

2023-05-04 22:56:52

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH 0/2] iommu: Make flush queues a proper capability

On Thu, May 04, 2023 at 10:10:54PM +0100, Robin Murphy wrote:
> Hi all,
>
> Since it came up in discussion on the default domain series, it seemed
> pertinent to dig this idea up, whcih I started a while ago, and finish
> it off.

I was using a flag in the ops since no driver need to make a dynamic
decision in code, but this works well enough too.

It is a good step forward, so

Reviewed-by: Jason Gunthorpe <[email protected]>

Jason

2023-05-05 02:21:50

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH 1/2] iommu: Add a capability for flush queue support

On 5/5/23 5:10 AM, Robin Murphy wrote:
> Passing a special type to domain_alloc to indirectly query whether flush
> queues are a worthwhile optimisation with the given driver is a bit
> clunky, and looking increasingly anachronistic. Let's put that into an
> explicit capability instead.
>
> Signed-off-by: Robin Murphy<[email protected]>

Reviewed-by: Lu Baolu <[email protected]>

Best regards,
baolu

2023-05-05 02:59:31

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH 0/2] iommu: Make flush queues a proper capability

On Thu, May 04, 2023 at 10:10:54PM +0100, Robin Murphy wrote:
> Hi all,
>
> Since it came up in discussion on the default domain series, it seemed
> pertinent to dig this idea up, whcih I started a while ago, and finish
> it off.
>
> Cheers,
> Robin.
>
>
> Robin Murphy (2):
> iommu: Add a capability for flush queue support
> iommu: Use flush queue capability
>
> drivers/iommu/amd/iommu.c | 2 ++
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 4 ++--
> drivers/iommu/dma-iommu.c | 3 ++-
> drivers/iommu/intel/iommu.c | 2 +-
> drivers/iommu/iommu.c | 3 ++-
> include/linux/iommu.h | 6 ++++++
> 7 files changed, 16 insertions(+), 6 deletions(-)
>
> --
> 2.39.2.101.g768bb238c484.dirty
>

Reviewed-by: Jerry Snitselaar <[email protected]>

2023-05-05 04:53:15

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH 0/2] iommu: Make flush queues a proper capability

On Thu, May 04, 2023 at 07:54:44PM -0700, Jerry Snitselaar wrote:
> On Thu, May 04, 2023 at 10:10:54PM +0100, Robin Murphy wrote:
> > Hi all,
> >
> > Since it came up in discussion on the default domain series, it seemed
> > pertinent to dig this idea up, whcih I started a while ago, and finish
> > it off.
> >
> > Cheers,
> > Robin.
> >
> >
> > Robin Murphy (2):
> > iommu: Add a capability for flush queue support
> > iommu: Use flush queue capability
> >
> > drivers/iommu/amd/iommu.c | 2 ++
> > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
> > drivers/iommu/arm/arm-smmu/arm-smmu.c | 4 ++--
> > drivers/iommu/dma-iommu.c | 3 ++-
> > drivers/iommu/intel/iommu.c | 2 +-
> > drivers/iommu/iommu.c | 3 ++-
> > include/linux/iommu.h | 6 ++++++
> > 7 files changed, 16 insertions(+), 6 deletions(-)
> >
> > --
> > 2.39.2.101.g768bb238c484.dirty
> >
>

Tested-by: Jerry Snitselaar <[email protected]> # amd, intel, smmu-v3

> Reviewed-by: Jerry Snitselaar <[email protected]>
>

2023-05-22 15:53:20

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 0/2] iommu: Make flush queues a proper capability

On Thu, May 04, 2023 at 10:10:54PM +0100, Robin Murphy wrote:
> Robin Murphy (2):
> iommu: Add a capability for flush queue support
> iommu: Use flush queue capability

Applied, thanks Robin.