[PATCH 0/4] iommu: Reject drivers with broken_unmanaged_domain

[PATCH 0/4] iommu: Reject drivers with broken_unmanaged_domain

[Nicolin Chen]

Hi all,

Both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA require the support
of __IOMMU_DOMAIN_PAGING capability, i.e. iommu_map/unmap. However,
some older iommu drivers do not fully support that, and these drivers
also do not advertise support for dma-iommu.c via IOMMU_DOMAIN_DMA,
or use arm_iommu_create_mapping(), so largely their implementations
of IOMMU_DOMAIN_UNMANAGED are untested. This means that a user like
vfio/iommufd does not likely work with them.

Thus, mark all these drivers as having "broken" UNAMANGED domains and
add a new device_iommu_unmanaged_supported() API for vfio/iommufd and
dma-iommu to refuse to work with these drivers.

These are preparatory changes for a new IOMMU API that will be used by
iommufd. Having this flag allows us to prevent any use of the new API
in iommufd, to keep things clean.

You can also find this series on Github:
https://github.com/nicolinc/iommufd/commits/broken_unmanaged_domain

Thank you
Nicolin Chen

Nicolin Chen (4):
  iommu: Add a broken_unmanaged_domain flag in iommu_ops
  iommu/dma: Do not init domain if broken_unmanaged_domain
  iommufd: Do not allocate device object if broken_unmanaged_domain
  vfio: Do not allocate domain if broken_unmanaged_domain

 drivers/iommu/dma-iommu.c       |  3 +++
 drivers/iommu/fsl_pamu_domain.c |  1 +
 drivers/iommu/iommu.c           | 24 ++++++++++++++++++++++++
 drivers/iommu/iommufd/device.c  |  3 +++
 drivers/iommu/msm_iommu.c       |  1 +
 drivers/iommu/mtk_iommu_v1.c    |  1 +
 drivers/iommu/omap-iommu.c      |  1 +
 drivers/iommu/tegra-gart.c      |  1 +
 drivers/iommu/tegra-smmu.c      |  1 +
 drivers/vfio/vfio_iommu_type1.c |  5 ++++-
 include/linux/iommu.h           | 11 +++++++++++
 11 files changed, 51 insertions(+), 1 deletion(-)

-- 
2.39.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Nicolin Chen]

Both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA require the support
of __IOMMU_DOMAIN_PAGING capability, i.e. iommu_map/unmap. However,
some older iommu drivers do not fully support that, and these drivers
also do not advertise support for dma-iommu.c via IOMMU_DOMAIN_DMA,
or use arm_iommu_create_mapping(), so largely their implementations
of IOMMU_DOMAIN_UNMANAGED are untested. This means that a user like
vfio/iommufd does not likely work with them.

Several of them have obvious problems:
  * fsl_pamu_domain.c
    Without map/unmap ops in the default_domain_ops, it isn't an
    unmanaged domain at all.
  * mtk_iommu_v1.c
    With a fixed 4M "pagetable", it can only map exactly 4G of
    memory, but doesn't set the aperture.
  * tegra-gart.c
    Its notion of attach/detach and groups has to be a complete lie to
    get around all the other API expectations.

Some others might work but have never been tested with vfio/iommufd:
  * msm_iommu.c
  * omap-iommu.c
  * tegra-smmu.c

Thus, mark all these drivers as having "broken" UNAMANGED domains and
add a new device_iommu_unmanaged_supported() API for vfio/iommufd and
dma-iommu to refuse to work with these drivers.

Co-developed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/fsl_pamu_domain.c |  1 +
 drivers/iommu/iommu.c           | 24 ++++++++++++++++++++++++
 drivers/iommu/msm_iommu.c       |  1 +
 drivers/iommu/mtk_iommu_v1.c    |  1 +
 drivers/iommu/omap-iommu.c      |  1 +
 drivers/iommu/tegra-gart.c      |  1 +
 drivers/iommu/tegra-smmu.c      |  1 +
 include/linux/iommu.h           | 11 +++++++++++
 8 files changed, 41 insertions(+)

diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 4408ac3c49b6..1f3a4c8c78ad 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -448,6 +448,7 @@ static struct iommu_device *fsl_pamu_probe_device(struct device *dev)
 }
 
 static const struct iommu_ops fsl_pamu_ops = {
+	.broken_unmanaged_domain = true,
 	.capable	= fsl_pamu_capable,
 	.domain_alloc	= fsl_pamu_domain_alloc,
 	.probe_device	= fsl_pamu_probe_device,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5f6a85aea501..648fc04143b8 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1897,6 +1897,30 @@ bool device_iommu_capable(struct device *dev, enum iommu_cap cap)
 }
 EXPORT_SYMBOL_GPL(device_iommu_capable);
 
+/**
+ * device_iommu_unmanaged_supported() - full support of IOMMU_DOMAIN_UNMANAGED
+ * @dev: device that is behind the iommu
+ *
+ * In general, all IOMMU drivers can allocate IOMMU_DOMAIN_UNMANAGED domains.
+ * However, some of them set the broken_unmanaged_domain, indicating something
+ * is wrong about its support of IOMMU_DOMAIN_UNMANAGED/__IOMMU_DOMAIN_PAGING.
+ * This can happen, when a driver lies about the support to get around with the
+ * IOMMU API, merely for other drivers to use, or when a driver has never been
+ * tested with vfio/iommufd that need a full support of IOMMU_DOMAIN_UNMANAGED.
+ *
+ * Return: true if an IOMMU is present and broken_unmanaged_domain is not set,
+ *         otherwise, false.
+ */
+bool device_iommu_unmanaged_supported(struct device *dev)
+{
+	if (dev->iommu && dev->iommu->iommu_dev &&
+	    !dev_iommu_ops(dev)->broken_unmanaged_domain)
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(device_iommu_unmanaged_supported);
+
 /**
  * iommu_set_fault_handler() - set a fault handler for an iommu domain
  * @domain: iommu domain
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index c60624910872..9c0bd5aee10b 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -675,6 +675,7 @@ irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id)
 }
 
 static struct iommu_ops msm_iommu_ops = {
+	.broken_unmanaged_domain = true,
 	.domain_alloc = msm_iommu_domain_alloc,
 	.probe_device = msm_iommu_probe_device,
 	.device_group = generic_device_group,
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index ca581ff1c769..c2ecbff6592c 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -578,6 +578,7 @@ static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
 }
 
 static const struct iommu_ops mtk_iommu_v1_ops = {
+	.broken_unmanaged_domain = true,
 	.domain_alloc	= mtk_iommu_v1_domain_alloc,
 	.probe_device	= mtk_iommu_v1_probe_device,
 	.probe_finalize = mtk_iommu_v1_probe_finalize,
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 2fd7702c6709..17ed392b9f63 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1733,6 +1733,7 @@ static struct iommu_group *omap_iommu_device_group(struct device *dev)
 }
 
 static const struct iommu_ops omap_iommu_ops = {
+	.broken_unmanaged_domain = true,
 	.domain_alloc	= omap_iommu_domain_alloc,
 	.probe_device	= omap_iommu_probe_device,
 	.release_device	= omap_iommu_release_device,
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index ed53279d1106..9af56d2ec6c1 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -267,6 +267,7 @@ static void gart_iommu_sync(struct iommu_domain *domain,
 }
 
 static const struct iommu_ops gart_iommu_ops = {
+	.broken_unmanaged_domain = true,
 	.domain_alloc	= gart_iommu_domain_alloc,
 	.probe_device	= gart_iommu_probe_device,
 	.device_group	= generic_device_group,
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 5b1af40221ec..d1e4c4825d74 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -962,6 +962,7 @@ static int tegra_smmu_of_xlate(struct device *dev,
 }
 
 static const struct iommu_ops tegra_smmu_ops = {
+	.broken_unmanaged_domain = true,
 	.domain_alloc = tegra_smmu_domain_alloc,
 	.probe_device = tegra_smmu_probe_device,
 	.device_group = tegra_smmu_device_group,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 46e1347bfa22..919a5dbad75b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -245,6 +245,10 @@ struct iommu_iotlb_gather {
  *                    pasid, so that any DMA transactions with this pasid
  *                    will be blocked by the hardware.
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @broken_unmanaged_domain: IOMMU_DOMAIN_UNMANAGED is not fully functional; the
+ *                           driver does not really support iommu_map/unmap, but
+ *                           uses UNMANAGED domains for the IOMMU API, called by
+ *                           other SOC drivers.
  * @owner: Driver module providing these ops
  */
 struct iommu_ops {
@@ -277,6 +281,7 @@ struct iommu_ops {
 
 	const struct iommu_domain_ops *default_domain_ops;
 	unsigned long pgsize_bitmap;
+	bool broken_unmanaged_domain;
 	struct module *owner;
 };
 
@@ -455,6 +460,7 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
 extern int bus_iommu_probe(struct bus_type *bus);
 extern bool iommu_present(struct bus_type *bus);
 extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap);
+extern bool device_iommu_unmanaged_supported(struct device *dev);
 extern struct iommu_domain *iommu_domain_alloc(struct bus_type *bus);
 extern struct iommu_group *iommu_group_get_by_id(int id);
 extern void iommu_domain_free(struct iommu_domain *domain);
@@ -742,6 +748,11 @@ static inline bool device_iommu_capable(struct device *dev, enum iommu_cap cap)
 	return false;
 }
 
+static inline bool device_iommu_unmanaged_supported(struct device *dev)
+{
+	return false;
+}
+
 static inline struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
 {
 	return NULL;
-- 
2.39.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Robin Murphy]

On 2023-01-27 20:04, Nicolin Chen wrote:
> Both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA require the support
> of __IOMMU_DOMAIN_PAGING capability, i.e. iommu_map/unmap. However,
> some older iommu drivers do not fully support that, and these drivers
> also do not advertise support for dma-iommu.c via IOMMU_DOMAIN_DMA,
> or use arm_iommu_create_mapping(), so largely their implementations
> of IOMMU_DOMAIN_UNMANAGED are untested. This means that a user like
> vfio/iommufd does not likely work with them.
> 
> Several of them have obvious problems:
>    * fsl_pamu_domain.c
>      Without map/unmap ops in the default_domain_ops, it isn't an
>      unmanaged domain at all.
>    * mtk_iommu_v1.c
>      With a fixed 4M "pagetable", it can only map exactly 4G of
>      memory, but doesn't set the aperture.

The aperture is easily fixed (one could argue that what's broken there 
are the ARM DMA ops for assuming every IOMMU has a 32-bit IOVA space and 
not checking).

>    * tegra-gart.c
>      Its notion of attach/detach and groups has to be a complete lie to
>      get around all the other API expectations.

That's true, and the domain is tiny and not isolated from the rest of 
the address space outside the aperture, but the one thing it does do is 
support iommu_map/unmap just fine, which is what this flag is documented 
as saying it doesn't.

> Some others might work but have never been tested with vfio/iommufd:
>    * msm_iommu.c
>    * omap-iommu.c
>    * tegra-smmu.c

And yet they all have other in-tree users (GPUs on MSM and Tegra, 
remoteproc on OMAP) that allocate unmanaged domains and use 
iommu_map/unmap just fine, so they're clearly not broken either.

On the flipside, you're also missing cases like apple-dart, which can 
have broken unmanaged domains by any definition, but only under certain 
conditions (at least it "fails safe" and they will refuse attempts to 
attach anything). I'd also question sprd-iommu, which hardly has a 
generally-useful domain size, and has only just recently gained the 
ability to unmap anything successfully. TBH none of the SoC IOMMUs are 
likely to ever be of interest to VFIO or IOMMUFD, since the only things 
they could assign to userspace are the individual devices - usually 
graphics and media engines - that they're coupled to, whose useful 
functionality tends to depend on clocks, phys, and random other 
low-level stuff that would be somewhere between impractical and 
downright unsafe to attempt to somehow expose as well.

> Thus, mark all these drivers as having "broken" UNAMANGED domains and
> add a new device_iommu_unmanaged_supported() API for vfio/iommufd and
> dma-iommu to refuse to work with these drivers.
> 
> Co-developed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>

[...]

> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 46e1347bfa22..919a5dbad75b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -245,6 +245,10 @@ struct iommu_iotlb_gather {
>    *                    pasid, so that any DMA transactions with this pasid
>    *                    will be blocked by the hardware.
>    * @pgsize_bitmap: bitmap of all possible supported page sizes
> + * @broken_unmanaged_domain: IOMMU_DOMAIN_UNMANAGED is not fully functional; the
> + *                           driver does not really support iommu_map/unmap, but
> + *                           uses UNMANAGED domains for the IOMMU API, called by
> + *                           other SOC drivers.

"uses UNMANAGED domains for the IOMMU API" is literally the definition 
of unmanaged domains :/

Some "other SOC drivers" use more of the IOMMU API than VFIO does :/

Please just add IOMMU_CAP_IOMMUFD to represent whatever the nebulous 
requirements of IOMMUFD actually are (frankly it's no less informative 
than calling domains "broken"), handle that in the drivers you care 
about and have tested, and use device_iommu_capable(). What you're 
describing in this series is a capability, and we have a perfectly good 
API for drivers to express those already. Plus, as demonstrated above, a 
positive capability based on empirical testing will be infinitely more 
robust than a negative one based on guessing.

Thanks,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Nicolin Chen]

Hi Robin.

On Fri, Jan 27, 2023 at 09:58:46PM +0000, Robin Murphy wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 2023-01-27 20:04, Nicolin Chen wrote:
> > Both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA require the support
> > of __IOMMU_DOMAIN_PAGING capability, i.e. iommu_map/unmap. However,
> > some older iommu drivers do not fully support that, and these drivers
> > also do not advertise support for dma-iommu.c via IOMMU_DOMAIN_DMA,
> > or use arm_iommu_create_mapping(), so largely their implementations
> > of IOMMU_DOMAIN_UNMANAGED are untested. This means that a user like
> > vfio/iommufd does not likely work with them.
> > 
> > Several of them have obvious problems:
> >    * fsl_pamu_domain.c
> >      Without map/unmap ops in the default_domain_ops, it isn't an
> >      unmanaged domain at all.
> >    * mtk_iommu_v1.c
> >      With a fixed 4M "pagetable", it can only map exactly 4G of
> >      memory, but doesn't set the aperture.
> 
> The aperture is easily fixed (one could argue that what's broken there
> are the ARM DMA ops for assuming every IOMMU has a 32-bit IOVA space and
> not checking).
>
> >    * tegra-gart.c
> >      Its notion of attach/detach and groups has to be a complete lie to
> >      get around all the other API expectations.
> 
> That's true, and the domain is tiny and not isolated from the rest of
> the address space outside the aperture, but the one thing it does do is
> support iommu_map/unmap just fine, which is what this flag is documented
> as saying it doesn't.
> 
> > Some others might work but have never been tested with vfio/iommufd:
> >    * msm_iommu.c
> >    * omap-iommu.c
> >    * tegra-smmu.c
> 
> And yet they all have other in-tree users (GPUs on MSM and Tegra,
> remoteproc on OMAP) that allocate unmanaged domains and use
> iommu_map/unmap just fine, so they're clearly not broken either.
> 
> On the flipside, you're also missing cases like apple-dart, which can
> have broken unmanaged domains by any definition, but only under certain
> conditions (at least it "fails safe" and they will refuse attempts to
> attach anything). I'd also question sprd-iommu, which hardly has a
> generally-useful domain size, and has only just recently gained the
> ability to unmap anything successfully. TBH none of the SoC IOMMUs are
> likely to ever be of interest to VFIO or IOMMUFD, since the only things
> they could assign to userspace are the individual devices - usually
> graphics and media engines - that they're coupled to, whose useful
> functionality tends to depend on clocks, phys, and random other
> low-level stuff that would be somewhere between impractical and
> downright unsafe to attempt to somehow expose as well.

Thanks for all the inputs.

> > Thus, mark all these drivers as having "broken" UNAMANGED domains and
> > add a new device_iommu_unmanaged_supported() API for vfio/iommufd and
> > dma-iommu to refuse to work with these drivers.
> > 
> > Co-developed-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> 
> [...]
> 
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index 46e1347bfa22..919a5dbad75b 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -245,6 +245,10 @@ struct iommu_iotlb_gather {
> >    *                    pasid, so that any DMA transactions with this pasid
> >    *                    will be blocked by the hardware.
> >    * @pgsize_bitmap: bitmap of all possible supported page sizes
> > + * @broken_unmanaged_domain: IOMMU_DOMAIN_UNMANAGED is not fully functional; the
> > + *                           driver does not really support iommu_map/unmap, but
> > + *                           uses UNMANAGED domains for the IOMMU API, called by
> > + *                           other SOC drivers.
> 
> "uses UNMANAGED domains for the IOMMU API" is literally the definition
> of unmanaged domains :/
> 
> Some "other SOC drivers" use more of the IOMMU API than VFIO does :/
> 
> Please just add IOMMU_CAP_IOMMUFD to represent whatever the nebulous
> requirements of IOMMUFD actually are (frankly it's no less informative
> than calling domains "broken"), handle that in the drivers you care
> about and have tested, and use device_iommu_capable(). What you're
> describing in this series is a capability, and we have a perfectly good
> API for drivers to express those already. Plus, as demonstrated above, a
> positive capability based on empirical testing will be infinitely more
> robust than a negative one based on guessing.

OK. I can change to IOMMU_CAP_IOMMUFD, and add to the drivers that
are tested. And an IOMMU driver that wants to use IOMMUFD can add
such a CAP later whenever it's ready.

Yet, "IOMMU_CAP_IOMMUFD" would make the VFIO change suspicious, so
perhaps the next version is just one CAP patch + one IOMMUFD patch.
@Jason, any concern?

Thank you
Nicolin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Jason Gunthorpe]

On Fri, Jan 27, 2023 at 09:58:46PM +0000, Robin Murphy wrote:

> Please just add IOMMU_CAP_IOMMUFD to represent whatever the nebulous
> requirements of IOMMUFD actually are (frankly it's no less informative than
> calling domains "broken"), handle that in the drivers you care about
> and

I don't want to tie this to iommufd, that isn't the point.

We clearly have drivers that don't implement the iommu kernel API
properly, because their only use is between the iommu driver and some
other same-SOC driver.

As a user of the iommu API iommufd and VFIO want to avoid these
drivers.

We have that acknowledgment already via the IOMMU_DOMAIN_DMA stuff
protecting the dma_iommu.c from those same drivers.

So, how about this below instead. Imagine it is followed by something along
the lines of my other sketch:

https://lore.kernel.org/linux-iommu/Y4%2FLsZKmR3iWFphU@nvidia.com/

And we completely delete IOMMU_DOMAIN_DMA/_FQ and get dma-iommu.c
mostly out of driver code.

iommufd/vfio would refuse to work with drivers that don't indicate
they support dma_iommu.c, that is good enough.

We'd eventually rename IOMMU_DOMAIN_UNMANAGED to IOMMU_DOMAIN_MAPPING.

Subject: [PATCH] iommu: Remove IOMMU_DOMAIN_DMA from drivers

The IOMMU_DOMAIN_DMA is exactly the same driver functionality as
IOMMU_DOMAIN_UNMANAGED, except it also implies that dma_iommu.c should
be able to use this driver.

As a first step to removing IOMMU_DOMAIN_DMA remove all references to
it from the drivers. Two simple ops flags are enough to indicate if
the driver is compatible with dma_iommu.c and if the driver will allow
the FQ mode to be selected. When dma-iommu.c needs an a domain it will
request an IOMMU_DOMAIN_UNMANAGED.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/apple-dart.c                  |  3 ++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  4 +--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 11 ++++----
 drivers/iommu/arm/arm-smmu/qcom_iommu.c     |  3 ++-
 drivers/iommu/exynos-iommu.c                |  3 ++-
 drivers/iommu/intel/iommu.c                 |  3 +--
 drivers/iommu/iommu.c                       | 29 ++++++++++++++++-----
 drivers/iommu/ipmmu-vmsa.c                  |  3 ++-
 drivers/iommu/mtk_iommu.c                   |  3 ++-
 drivers/iommu/rockchip-iommu.c              |  3 ++-
 drivers/iommu/sprd-iommu.c                  |  3 ++-
 drivers/iommu/sun50i-iommu.c                |  4 +--
 drivers/iommu/virtio-iommu.c                |  2 +-
 include/linux/iommu.h                       |  2 ++
 14 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index 4f4a323be0d0ff..0eb3eb857d7e9e 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -580,7 +580,7 @@ static struct iommu_domain *apple_dart_domain_alloc(unsigned int type)
 {
 	struct apple_dart_domain *dart_domain;
 
-	if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED &&
+	if (type != IOMMU_DOMAIN_UNMANAGED &&
 	    type != IOMMU_DOMAIN_IDENTITY && type != IOMMU_DOMAIN_BLOCKED)
 		return NULL;
 
@@ -769,6 +769,7 @@ static void apple_dart_get_resv_regions(struct device *dev,
 }
 
 static const struct iommu_ops apple_dart_iommu_ops = {
+	.use_dma_iommu = true,
 	.domain_alloc = apple_dart_domain_alloc,
 	.probe_device = apple_dart_probe_device,
 	.release_device = apple_dart_release_device,
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 ab160198edd6b1..2253c5598092d0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2013,8 +2013,6 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return arm_smmu_sva_domain_alloc();
 
 	if (type != IOMMU_DOMAIN_UNMANAGED &&
-	    type != IOMMU_DOMAIN_DMA &&
-	    type != IOMMU_DOMAIN_DMA_FQ &&
 	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 
@@ -2844,6 +2842,8 @@ static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 }
 
 static struct iommu_ops arm_smmu_ops = {
+	.use_dma_iommu		= true,
+	.allow_dma_fq		= true,
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
 	.probe_device		= arm_smmu_probe_device,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 719fbca1fe52a0..7bb160bdc4b594 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -855,11 +855,9 @@ 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))
-			return NULL;
-	}
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY)
+		return NULL;
+
 	/*
 	 * Allocate the domain and initialise some of its data structures.
 	 * We can't really do anything meaningful until we've added a
@@ -1555,6 +1553,8 @@ static int arm_smmu_def_domain_type(struct device *dev)
 }
 
 static struct iommu_ops arm_smmu_ops = {
+	.use_dma_iommu		= true,
+	.allow_dma_fq		= true,
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
 	.probe_device		= arm_smmu_probe_device,
@@ -1982,6 +1982,7 @@ static int arm_smmu_device_dt_probe(struct arm_smmu_device *smmu,
 				  IS_ENABLED(CONFIG_ARM_SMMU_LEGACY_DT_BINDINGS) ? "DMA API" : "SMMU");
 		}
 		using_legacy_binding = true;
+		arm_smmu_ops.use_dma_iommu = false;
 	} else if (!legacy_binding && !using_legacy_binding) {
 		using_generic_binding = true;
 	} else {
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 270c3d9128bab8..babd20108f6a17 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -323,7 +323,7 @@ static struct iommu_domain *qcom_iommu_domain_alloc(unsigned type)
 {
 	struct qcom_iommu_domain *qcom_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 	/*
 	 * Allocate the domain and initialise some of its data structures.
@@ -575,6 +575,7 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
 }
 
 static const struct iommu_ops qcom_iommu_ops = {
+	.use_dma_iommu  = true,
 	.capable	= qcom_iommu_capable,
 	.domain_alloc	= qcom_iommu_domain_alloc,
 	.probe_device	= qcom_iommu_probe_device,
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index b0cde22119875e..824350551e5933 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -825,7 +825,7 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type)
 	/* Check if correct PTE offsets are initialized */
 	BUG_ON(PG_ENT_SHIFT < 0 || !dma_dev);
 
-	if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 
 	domain = kzalloc(sizeof(*domain), GFP_KERNEL);
@@ -1396,6 +1396,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
 }
 
 static const struct iommu_ops exynos_iommu_ops = {
+	.use_dma_iommu = true,
 	.domain_alloc = exynos_iommu_domain_alloc,
 	.device_group = generic_device_group,
 	.probe_device = exynos_iommu_probe_device,
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 59df7e42fd533c..bb34d3f641f17b 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4165,8 +4165,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 	switch (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) {
@@ -4761,6 +4759,7 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 }
 
 const struct iommu_ops intel_iommu_ops = {
+	.use_dma_iommu		= true,
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
 	.probe_device		= intel_iommu_probe_device,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index de91dd88705bd3..877ef0a58b07f4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1618,17 +1618,32 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus,
 {
 	struct iommu_domain *dom;
 
-	dom = __iommu_domain_alloc(bus, type);
-	if (!dom && type != IOMMU_DOMAIN_DMA) {
-		dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_DMA);
-		if (dom)
+	/*
+	 * Keep the DMA domain type out of the drivers, eventually it will go
+	 * away completely.
+	 */
+	if (type == IOMMU_DOMAIN_IDENTITY) {
+		dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_IDENTITY);
+		if (!dom)
+			return -ENOMEM;
+	} else if (type == IOMMU_DOMAIN_DMA_FQ || type == IOMMU_DOMAIN_DMA) {
+		if (!bus->iommu_ops->use_dma_iommu)
+			return -EINVAL;
+
+		dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED);
+		if (!dom)
+			return -ENOMEM;
+		if (type == IOMMU_DOMAIN_DMA_FQ &&
+		    !bus->iommu_ops->allow_dma_fq) {
 			pr_warn("Failed to allocate default IOMMU domain of type %u for group %s - Falling back to IOMMU_DOMAIN_DMA",
 				type, group->name);
+			type = IOMMU_DOMAIN_DMA;
+		}
+		dom->type = type;
+	} else {
+		return -EINVAL;
 	}
 
-	if (!dom)
-		return -ENOMEM;
-
 	group->default_domain = dom;
 	if (!group->domain)
 		group->domain = dom;
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index a003bd5fc65c13..a22df69f7f4775 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -571,7 +571,7 @@ static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
 {
 	struct ipmmu_vmsa_domain *domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 
 	domain = kzalloc(sizeof(*domain), GFP_KERNEL);
@@ -866,6 +866,7 @@ static struct iommu_group *ipmmu_find_group(struct device *dev)
 }
 
 static const struct iommu_ops ipmmu_ops = {
+	.use_dma_iommu = true,
 	.domain_alloc = ipmmu_domain_alloc,
 	.probe_device = ipmmu_probe_device,
 	.release_device = ipmmu_release_device,
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2badd6acfb23d6..e27cb428bd9583 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -636,7 +636,7 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
 {
 	struct mtk_iommu_domain *dom;
 
-	if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 
 	dom = kzalloc(sizeof(*dom), GFP_KERNEL);
@@ -936,6 +936,7 @@ static void mtk_iommu_get_resv_regions(struct device *dev,
 }
 
 static const struct iommu_ops mtk_iommu_ops = {
+	.use_dma_iommu  = true,
 	.domain_alloc	= mtk_iommu_domain_alloc,
 	.probe_device	= mtk_iommu_probe_device,
 	.release_device	= mtk_iommu_release_device,
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index a68eadd64f38db..fa3ec38e5244db 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1061,7 +1061,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
 {
 	struct rk_iommu_domain *rk_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 
 	if (!dma_dev)
@@ -1184,6 +1184,7 @@ static int rk_iommu_of_xlate(struct device *dev,
 }
 
 static const struct iommu_ops rk_iommu_ops = {
+	.use_dma_iommu = true,
 	.domain_alloc = rk_iommu_domain_alloc,
 	.probe_device = rk_iommu_probe_device,
 	.release_device = rk_iommu_release_device,
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index 219bfa11f7f48f..fbff1831c16e78 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -136,7 +136,7 @@ static struct iommu_domain *sprd_iommu_domain_alloc(unsigned int domain_type)
 {
 	struct sprd_iommu_domain *dom;
 
-	if (domain_type != IOMMU_DOMAIN_DMA && domain_type != IOMMU_DOMAIN_UNMANAGED)
+	if (domain_type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 
 	dom = kzalloc(sizeof(*dom), GFP_KERNEL);
@@ -406,6 +406,7 @@ static int sprd_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
 
 
 static const struct iommu_ops sprd_iommu_ops = {
+	.use_dma_iommu  = true,
 	.domain_alloc	= sprd_iommu_domain_alloc,
 	.probe_device	= sprd_iommu_probe_device,
 	.device_group	= sprd_iommu_device_group,
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 5b585eace3d46f..de2a033f65197a 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -671,8 +671,7 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
 {
 	struct sun50i_iommu_domain *sun50i_domain;
 
-	if (type != IOMMU_DOMAIN_DMA &&
-	    type != IOMMU_DOMAIN_UNMANAGED)
+	if (type != IOMMU_DOMAIN_UNMANAGED)
 		return NULL;
 
 	sun50i_domain = kzalloc(sizeof(*sun50i_domain), GFP_KERNEL);
@@ -827,6 +826,7 @@ static int sun50i_iommu_of_xlate(struct device *dev,
 }
 
 static const struct iommu_ops sun50i_iommu_ops = {
+	.use_dma_iommu  = true,
 	.pgsize_bitmap	= SZ_4K,
 	.device_group	= sun50i_iommu_device_group,
 	.domain_alloc	= sun50i_iommu_domain_alloc,
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 5b8fe9bfa9a5b9..8afa21a9c0b9d6 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -642,7 +642,6 @@ static struct iommu_domain *viommu_domain_alloc(unsigned type)
 	struct viommu_domain *vdomain;
 
 	if (type != IOMMU_DOMAIN_UNMANAGED &&
-	    type != IOMMU_DOMAIN_DMA &&
 	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 
@@ -1018,6 +1017,7 @@ static bool viommu_capable(struct device *dev, enum iommu_cap cap)
 }
 
 static struct iommu_ops viommu_ops = {
+	.use_dma_iommu		= true,
 	.capable		= viommu_capable,
 	.domain_alloc		= viommu_domain_alloc,
 	.probe_device		= viommu_probe_device,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 46e1347bfa2286..954db8e77491c7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -277,6 +277,8 @@ struct iommu_ops {
 
 	const struct iommu_domain_ops *default_domain_ops;
 	unsigned long pgsize_bitmap;
+	u8 use_dma_iommu : 1;
+	u8 allow_dma_fq : 1;
 	struct module *owner;
 };
 
-- 
2.39.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Saturday, January 28, 2023 7:54 AM
> 
> On Fri, Jan 27, 2023 at 09:58:46PM +0000, Robin Murphy wrote:
> 
> > Please just add IOMMU_CAP_IOMMUFD to represent whatever the
> nebulous
> > requirements of IOMMUFD actually are (frankly it's no less informative
> than
> > calling domains "broken"), handle that in the drivers you care about
> > and
> 
> I don't want to tie this to iommufd, that isn't the point.
> 
> We clearly have drivers that don't implement the iommu kernel API
> properly, because their only use is between the iommu driver and some
> other same-SOC driver.
> 
> As a user of the iommu API iommufd and VFIO want to avoid these
> drivers.
> 
> We have that acknowledgment already via the IOMMU_DOMAIN_DMA stuff
> protecting the dma_iommu.c from those same drivers.
> 
> So, how about this below instead. Imagine it is followed by something along
> the lines of my other sketch:
> 
> https://lore.kernel.org/linux-iommu/Y4%2FLsZKmR3iWFphU@nvidia.com/
> 
> And we completely delete IOMMU_DOMAIN_DMA/_FQ and get dma-
> iommu.c
> mostly out of driver code.
> 
> iommufd/vfio would refuse to work with drivers that don't indicate
> they support dma_iommu.c, that is good enough.

this is a good idea. Just one doubt. Robin mentioned that sprd-iommu
might have a broken unmanaged domains:

" I'd also question sprd-iommu, which hardly has a generally-useful
domain size, and has only just recently gained the ability to unmap
anything successfully."

Want to understand why that restriction is not a problem for DMA API.

what Jason proposed here assumes that existing driver support for
DMA API can be safely applied to vfio/iommufd. But above looks
warning certain exception?

> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 59df7e42fd533c..bb34d3f641f17b 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4165,8 +4165,6 @@ static struct iommu_domain
> *intel_iommu_domain_alloc(unsigned type)
>  	switch (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) {
> @@ -4761,6 +4759,7 @@ static void intel_iommu_remove_dev_pasid(struct
> device *dev, ioasid_t pasid)
>  }
> 
>  const struct iommu_ops intel_iommu_ops = {
> +	.use_dma_iommu		= true,

missed:
+	.allow_dma_fq			= true,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Jason Gunthorpe]

On Sun, Jan 29, 2023 at 08:11:48AM +0000, Tian, Kevin wrote:

> " I'd also question sprd-iommu, which hardly has a generally-useful
> domain size, and has only just recently gained the ability to unmap
> anything successfully."

So long as it has a correct kernel API and exposes the right (small)
aperture then it is OK.

The device will not be useful for qemu, but it would run some dpdk
configurations just fine.

All the VFIO world is not VMs.

> > @@ -4761,6 +4759,7 @@ static void intel_iommu_remove_dev_pasid(struct
> > device *dev, ioasid_t pasid)
> >  }
> > 
> >  const struct iommu_ops intel_iommu_ops = {
> > +	.use_dma_iommu		= true,
> 
> missed:
> +	.allow_dma_fq			= true,

Yep

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Monday, January 30, 2023 9:33 PM
> 
> On Sun, Jan 29, 2023 at 08:11:48AM +0000, Tian, Kevin wrote:
> 
> > " I'd also question sprd-iommu, which hardly has a generally-useful
> > domain size, and has only just recently gained the ability to unmap
> > anything successfully."
> 
> So long as it has a correct kernel API and exposes the right (small)
> aperture then it is OK.
> 
> The device will not be useful for qemu, but it would run some dpdk
> configurations just fine.

I still didn't get the restriction here. Can you elaborate why it works
with dpdk but not qemu?

Can qemu verify this restriction via existing path or need new uAPI
flag to communicate?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Jason Gunthorpe]

On Wed, Feb 01, 2023 at 03:14:03AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Monday, January 30, 2023 9:33 PM
> > 
> > On Sun, Jan 29, 2023 at 08:11:48AM +0000, Tian, Kevin wrote:
> > 
> > > " I'd also question sprd-iommu, which hardly has a generally-useful
> > > domain size, and has only just recently gained the ability to unmap
> > > anything successfully."
> > 
> > So long as it has a correct kernel API and exposes the right (small)
> > aperture then it is OK.
> > 
> > The device will not be useful for qemu, but it would run some dpdk
> > configurations just fine.
> 
> I still didn't get the restriction here. Can you elaborate why it works
> with dpdk but not qemu?

dpdk needs like, say, 64M of aperture and doesn't care what the IOVAs
are

qemu needs the entire guest memory of aperture and must have IOVAs
that are 1:1 with the GPA.

So aperture size and location can exclude qemu

> Can qemu verify this restriction via existing path or need new uAPI
> flag to communicate?

It already happens, the aperture/etc is convayed to qemu through
IOMMUFD_CMD_IOAS_IOVA_RANGES and if qemu cannot get the IOVA's it
needs to create the guest it should fail.

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, February 1, 2023 10:56 PM
> 
> On Wed, Feb 01, 2023 at 03:14:03AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Monday, January 30, 2023 9:33 PM
> > >
> > > On Sun, Jan 29, 2023 at 08:11:48AM +0000, Tian, Kevin wrote:
> > >
> > > > " I'd also question sprd-iommu, which hardly has a generally-useful
> > > > domain size, and has only just recently gained the ability to unmap
> > > > anything successfully."
> > >
> > > So long as it has a correct kernel API and exposes the right (small)
> > > aperture then it is OK.
> > >
> > > The device will not be useful for qemu, but it would run some dpdk
> > > configurations just fine.
> >
> > I still didn't get the restriction here. Can you elaborate why it works
> > with dpdk but not qemu?
> 
> dpdk needs like, say, 64M of aperture and doesn't care what the IOVAs
> are
> 
> qemu needs the entire guest memory of aperture and must have IOVAs
> that are 1:1 with the GPA.
> 
> So aperture size and location can exclude qemu
> 
> > Can qemu verify this restriction via existing path or need new uAPI
> > flag to communicate?
> 
> It already happens, the aperture/etc is convayed to qemu through
> IOMMUFD_CMD_IOAS_IOVA_RANGES and if qemu cannot get the IOVA's it
> needs to create the guest it should fail.
> 

Make sense.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/4] iommu: Add a broken_unmanaged_domain flag in iommu_ops

[Tian, Kevin]

> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Saturday, January 28, 2023 4:04 AM
> 
> Both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA require
> the support
> of __IOMMU_DOMAIN_PAGING capability, i.e. iommu_map/unmap.
> However,
> some older iommu drivers do not fully support that, and these drivers
> also do not advertise support for dma-iommu.c via IOMMU_DOMAIN_DMA,
> or use arm_iommu_create_mapping(), so largely their implementations
> of IOMMU_DOMAIN_UNMANAGED are untested. This means that a user like
> vfio/iommufd does not likely work with them.
> 
> Several of them have obvious problems:
>   * fsl_pamu_domain.c
>     Without map/unmap ops in the default_domain_ops, it isn't an
>     unmanaged domain at all.
>   * mtk_iommu_v1.c
>     With a fixed 4M "pagetable", it can only map exactly 4G of
>     memory, but doesn't set the aperture.
>   * tegra-gart.c
>     Its notion of attach/detach and groups has to be a complete lie to
>     get around all the other API expectations.
> 
> Some others might work but have never been tested with vfio/iommufd:
>   * msm_iommu.c
>   * omap-iommu.c
>   * tegra-smmu.c
> 

Do we have a link where all drivers tested with vfio/iommufd have been
listed?

In a quick glance at least exynos-iommu.c and apple-dart.c both support
UNMANAGED with map/unmap ops. They are not mentioned in above
list but I doubt they are tested for vfio/iommufd.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] iommu/dma: Do not init domain if broken_unmanaged_domain

[Nicolin Chen]

Add a sanity of the broken_unmanaged_domain flag to reject the use of
dma-iommu in the early stage, if the flag is set by the iommu driver.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/dma-iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f798c44e0903..f1e8d952b17d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -545,6 +545,9 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 	if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
 		return -EINVAL;
 
+	if (WARN_ON(!device_iommu_unmanaged_supported(dev)))
+		return -EINVAL;
+
 	iovad = &cookie->iovad;
 
 	/* Use the smallest supported page size for IOVA granularity */
-- 
2.39.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] iommu/dma: Do not init domain if broken_unmanaged_domain

[Robin Murphy]

On 2023-01-27 20:04, Nicolin Chen wrote:
> Add a sanity of the broken_unmanaged_domain flag to reject the use of
> dma-iommu in the early stage, if the flag is set by the iommu driver.

Realistically, iommu-dma will never be enabled on PPC32, let alone used. 
It will not be enabled on ARM until I've fixed any drivers which need 
fixing to work with it. We don't need to add dead code here.

Thanks,
Robin.

> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   drivers/iommu/dma-iommu.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index f798c44e0903..f1e8d952b17d 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -545,6 +545,9 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>   	if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
>   		return -EINVAL;
>   
> +	if (WARN_ON(!device_iommu_unmanaged_supported(dev)))
> +		return -EINVAL;
> +
>   	iovad = &cookie->iovad;
>   
>   	/* Use the smallest supported page size for IOVA granularity */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] iommu/dma: Do not init domain if broken_unmanaged_domain

[Nicolin Chen]

On Fri, Jan 27, 2023 at 09:59:50PM +0000, Robin Murphy wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 2023-01-27 20:04, Nicolin Chen wrote:
> > Add a sanity of the broken_unmanaged_domain flag to reject the use of
> > dma-iommu in the early stage, if the flag is set by the iommu driver.
> 
> Realistically, iommu-dma will never be enabled on PPC32, let alone used.
> It will not be enabled on ARM until I've fixed any drivers which need
> fixing to work with it. We don't need to add dead code here.

OK. Sounds like I can drop this patch.

Thanks
Nicolin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] iommufd: Do not allocate device object if broken_unmanaged_domain

[Nicolin Chen]

Add a sanity of the broken_unmanaged_domain flag to reject the use of
iommufd in the early stage, if the flag is set by the iommu driver.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommufd/device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index d81f93a321af..41b55615a693 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -73,6 +73,9 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
 	if (!device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY))
 		return ERR_PTR(-EINVAL);
 
+	if (!device_iommu_unmanaged_supported(dev))
+		return ERR_PTR(-EINVAL);
+
 	group = iommu_group_get(dev);
 	if (!group)
 		return ERR_PTR(-ENODEV);
-- 
2.39.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] vfio: Do not allocate domain if broken_unmanaged_domain

[Nicolin Chen]

Add a sanity of the broken_unmanaged_domain flag to reject the use of
vfio_iommu_type1 in the early stage, if the flag is set by the iommu
driver.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/vfio/vfio_iommu_type1.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 23c24fe98c00..6ec238aefe89 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -2170,7 +2170,10 @@ static int vfio_iommu_domain_alloc(struct device *dev, void *data)
 {
 	struct iommu_domain **domain = data;
 
-	*domain = iommu_domain_alloc(dev->bus);
+	if (device_iommu_unmanaged_supported(dev))
+		*domain = iommu_domain_alloc(dev->bus);
+	else
+		*domain = NULL;
 	return 1; /* Don't iterate */
 }
 
-- 
2.39.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 0/4] iommu: Reject drivers with broken_unmanaged_domain

[Tian, Kevin]

> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Saturday, January 28, 2023 4:04 AM
> 
> Hi all,
> 
> Both IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA require
> the support
> of __IOMMU_DOMAIN_PAGING capability, i.e. iommu_map/unmap.
> However,
> some older iommu drivers do not fully support that, and these drivers
> also do not advertise support for dma-iommu.c via IOMMU_DOMAIN_DMA,
> or use arm_iommu_create_mapping(), so largely their implementations
> of IOMMU_DOMAIN_UNMANAGED are untested. This means that a user like
> vfio/iommufd does not likely work with them.
> 
> Thus, mark all these drivers as having "broken" UNAMANGED domains and
> add a new device_iommu_unmanaged_supported() API for vfio/iommufd
> and
> dma-iommu to refuse to work with these drivers.
> 

Why not making an explicit way to mark which drivers are tested to
support vfio/iommufd? Not sure whether pure code review is reliable
to filter out 'broken' vs. 'non-broken'...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel