On Wed, 13 Sept 2023 at 16:45, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
>
> At this point every iommu driver will cause a default_domain to be
> selected, so we can finally remove this gap from the core code.
>
> The following table explains what each driver supports and what the
> resulting default_domain will be:
>
> ops->defaut_domain
> IDENTITY DMA PLATFORM v ARM32 dma-iommu ARCH
> amd/iommu.c Y Y N/A either
> apple-dart.c Y Y N/A either
> arm-smmu.c Y Y IDENTITY either
> qcom_iommu.c G Y IDENTITY either
> arm-smmu-v3.c Y Y N/A either
> exynos-iommu.c G Y IDENTITY either
> fsl_pamu_domain.c Y Y N/A N/A PLATFORM
> intel/iommu.c Y Y N/A either
> ipmmu-vmsa.c G Y IDENTITY either
> msm_iommu.c G IDENTITY N/A
Unfortunately this patch breaks msm_iommu platforms. This driver
doesn't select ARM_DMA_USE_IOMMU, so iommu_get_default_domain_type()
returns 0, bus_iommu_probe() fails with -ENODEV.
If I make MSM_IOMMU select ARM_DMA_USE_IOMMU, then GPU probing fails
with -EBUSY.
> mtk_iommu.c G Y IDENTITY either
> mtk_iommu_v1.c G IDENTITY N/A
> omap-iommu.c G IDENTITY N/A
> rockchip-iommu.c G Y IDENTITY either
> s390-iommu.c Y Y N/A N/A PLATFORM
> sprd-iommu.c Y N/A DMA
> sun50i-iommu.c G Y IDENTITY either
> tegra-smmu.c G Y IDENTITY IDENTITY
> virtio-iommu.c Y Y N/A either
> spapr Y Y N/A N/A PLATFORM
> * G means ops->identity_domain is used
> * N/A means the driver will not compile in this configuration
>
> ARM32 drivers select an IDENTITY default domain through either the
> ops->identity_domain or directly requesting an IDENTIY domain through
> alloc_domain().
>
> In ARM64 mode tegra-smmu will still block the use of dma-iommu.c and
> forces an IDENTITY domain.
>
> S390 uses a PLATFORM domain to represent when the dma_ops are set to the
> s390 iommu code.
>
> fsl_pamu uses an PLATFORM domain.
>
> POWER SPAPR uses PLATFORM and blocking to enable its weird VFIO mode.
>
> The x86 drivers continue unchanged.
>
> After this patch group->default_domain is only NULL for a short period
> during bus iommu probing while all the groups are constituted. Otherwise
> it is always !NULL.
>
> This completes changing the iommu subsystem driver contract to a system
> where the current iommu_domain always represents some form of translation
> and the driver is continuously asserting a definable translation mode.
>
> It resolves the confusion that the original ops->detach_dev() caused
> around what translation, exactly, is the IOMMU performing after
> detach. There were at least three different answers to that question in
> the tree, they are all now clearly named with domain types.
>
> Tested-by: Heiko Stuebner <heiko@xxxxxxxxx>
> Tested-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> Tested-by: Steven Price <steven.price@xxxxxxx>
> Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Tested-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> Reviewed-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> drivers/iommu/iommu.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 42a4585dd76da6..cfb597751f5bad 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1865,7 +1865,6 @@ static int iommu_get_def_domain_type(struct iommu_group *group,
> static int iommu_get_default_domain_type(struct iommu_group *group,
> int target_type)
> {
> - const struct iommu_ops *ops = group_iommu_ops(group);
> struct device *untrusted = NULL;
> struct group_device *gdev;
> int driver_type = 0;
> @@ -1876,11 +1875,13 @@ static int iommu_get_default_domain_type(struct iommu_group *group,
> * ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an
> * identity_domain and it will automatically become their default
> * domain. Later on ARM_DMA_USE_IOMMU will install its UNMANAGED domain.
> - * Override the selection to IDENTITY if we are sure the driver supports
> - * it.
> + * Override the selection to IDENTITY.
> */
> - if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) && ops->identity_domain)
> + if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> + static_assert(!(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) &&
> + IS_ENABLED(CONFIG_IOMMU_DMA)));
> driver_type = IOMMU_DOMAIN_IDENTITY;
> + }
>
> for_each_group_device(group, gdev) {
> driver_type = iommu_get_def_domain_type(group, gdev->dev,
> @@ -3016,18 +3017,9 @@ static int iommu_setup_default_domain(struct iommu_group *group,
> if (req_type < 0)
> return -EINVAL;
>
> - /*
> - * There are still some drivers which don't support default domains, so
> - * we ignore the failure and leave group->default_domain NULL.
> - */
> dom = iommu_group_alloc_default_domain(group, req_type);
> - if (!dom) {
> - /* Once in default_domain mode we never leave */
> - if (group->default_domain)
> - return -ENODEV;
> - group->default_domain = NULL;
> - return 0;
> - }
> + if (!dom)
> + return -ENODEV;
>
> if (group->default_domain == dom)
> return 0;
> --
> 2.42.0
>
--
With best wishes
Dmitry
