mbox series

[v2,00/25] iommu: Make default_domain's mandatory

Message ID 0-v2-8d1dc464eac9+10f-iommu_all_defdom_jgg@nvidia.com (mailing list archive)
Headers show
Series iommu: Make default_domain's mandatory | expand

Message

Jason Gunthorpe May 16, 2023, midnight UTC
[ There was alot of unexpected complication after rc1 with this series,
several new patches were needed ]

It has been a long time coming, this series completes the default_domain
transition and makes it so that the core IOMMU code will always have a
non-NULL default_domain for every driver on every
platform. set_platform_dma_ops() turned out to be a bad idea, and so
completely remove it.

This is achieved by changing each driver to either:

1 - Convert the existing (or deleted) ops->detach_dev() into an
    op->attach_dev() of an IDENTITY domain.

    This is based on the theory that the ARM32 HW is able to function when
    the iommu is turned off as so the turned off state is an IDENTITY
    translation.

2 - Use a new PLATFORM domain type. This is a hack to accommodate drivers
    that we don't really know WTF they do. S390 is legitimately using this
    to switch to it's platform dma_ops implementation, which is where the
    name comes from.

3 - Do #1 and force the default domain to be IDENTITY, this corrects
    the tegra-smmu case where even an ARM64 system would have a NULL
    default_domain.

Using this we can apply the rules:

a) ARM_DMA_USE_IOMMU mode always uses either the driver's
   ops->default_domain, ops->def_domain_type(), or an IDENTITY domain.
   All ARM32 drivers provide one of these three options.

b) dma-iommu.c mode uses either the driver's ops->default_domain,
   ops->def_domain_type or the usual DMA API policy logic based on the
   command line/etc to pick IDENTITY/DMA domain types

c) All other arch's (PPC/S390) use ops->default_domain always.

See the patch "Require a default_domain for all iommu drivers" for a
per-driver breakdown.

The conversion broadly teaches a bunch of ARM32 drivers that they can do
IDENTITY domains. There is some educated guessing involved that these are
actual IDENTITY domains. If this turns out to be wrong the driver can be
trivially changed to use a BLOCKING domain type instead. Further, the
domain type only matters for drivers using ARM64's dma-iommu.c mode as it
will select IDENTITY based on the command line and expect IDENTITY to
work. For ARM32 and other arch cases it is purely documentation.

Finally, based on all the analysis in this series, we can purge
IOMMU_DOMAIN_UNMANAGED/DMA constants from most of the drivers. This
greatly simplifies understanding the driver contract to the core
code. IOMMU drivers should not be involved in policy for how the DMA API
works, that should be a core core decision.

The main gain from this work is to remove alot of ARM_DMA_USE_IOMMU
specific code and behaviors from drivers. All that remains in iommu
drivers after this series is the calls to arm_iommu_create_mapping().

This is a step toward removing ARM_DMA_USE_IOMMU.

The IDENTITY domains added to the ARM64 supporting drivers can be tested
by booting in ARM64 mode and enabling CONFIG_IOMMU_DEFAULT_PASSTHROUGH. If
the system still boots then most likely the implementation is an IDENTITY
domain. If not we can trivially change it to BLOCKING or at worst PLATFORM
if there is no detail what is going on in the HW.

I think this is pretty safe for the ARM32 drivers as they don't really
change, the code that was in detach_dev continues to be called in the same
places it was called before.

This follows the prior series:

https://lore.kernel.org/r/0-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com

This is on github: https://github.com/jgunthorpe/linux/commits/iommu_all_defdom

v2:
 - FSL is an IDENTITY domain
 - Delete terga-gart instead of trying to carry it
 - Use the policy determination from iommu_get_default_domain_type() to
   drive the arm_iommu mode
 - Reorganize and introduce new patches to do the above:
    * Split the ops->identity_domain to an independent earlier patch
    * Remove the UNMANAGED return from def_domain_type in mtk_v1 earlier
      so the new iommu_get_default_domain_type() can work
    * Make the driver's def_domain_type have higher policy priority than
      untrusted
    * Merge the set_platfom_dma_ops hunk from mtk_v1 along with rockchip
      into the patch that forced IDENTITY on ARM32
 - Revise sun50i to be cleaner and have a non-NULL internal domain
 - Reword logging in exynos
 - Remove the gdev from the group alloc path, instead add a new
   function __iommu_group_domain_alloc() that takes in the group
   and uses the first device. Split this to its own patch
 - New patch to make iommufd's mock selftest into a real driver
 - New patch to fix power's partial iommu driver
v1: https://lore.kernel.org/r/0-v1-21cc72fcfb22+a7a-iommu_all_defdom_jgg@nvidia.com

Jason Gunthorpe (25):
  iommu: Add iommu_ops->identity_domain
  iommu: Add IOMMU_DOMAIN_PLATFORM
  powerpc/iommu: Setup a default domain and remove set_platform_dma_ops
  iommu: Add IOMMU_DOMAIN_PLATFORM for S390
  iommu/tegra-gart: Remove tegra-gart
  iommu/mtk_iommu_v1: Implement an IDENTITY domain
  iommu: Reorganize iommu_get_default_domain_type() to respect
    def_domain_type()
  iommu: Allow an IDENTITY domain as the default_domain in ARM32
  iommu/fsl_pamu: Implement an IDENTITY domain
  iommu/exynos: Implement an IDENTITY domain
  iommu/tegra-smmu: Implement an IDENTITY domain
  iommu/tegra-smmu: Support DMA domains in tegra
  iommu/omap: Implement an IDENTITY domain
  iommu/msm: Implement an IDENTITY domain
  iommufd/selftest: Make the mock iommu driver into a real driver
  iommu: Remove ops->set_platform_dma_ops()
  iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
  iommu/ipmmu: Add an IOMMU_IDENTITIY_DOMAIN
  iommu/mtk_iommu: Add an IOMMU_IDENTITIY_DOMAIN
  iommu/sun50i: Add an IOMMU_IDENTITIY_DOMAIN
  iommu: Require a default_domain for all iommu drivers
  iommu: Add __iommu_group_domain_alloc()
  iommu: Add ops->domain_alloc_paging()
  iommu: Convert simple drivers with DOMAIN_DMA to domain_alloc_paging()
  iommu: Convert remaining simple drivers to domain_alloc_paging()

 arch/arm/configs/multi_v7_defconfig     |   1 -
 arch/arm/configs/tegra_defconfig        |   1 -
 arch/powerpc/kernel/iommu.c             |  38 ++-
 drivers/iommu/Kconfig                   |  11 -
 drivers/iommu/Makefile                  |   1 -
 drivers/iommu/arm/arm-smmu/qcom_iommu.c |  45 ++-
 drivers/iommu/exynos-iommu.c            |  73 +++--
 drivers/iommu/fsl_pamu_domain.c         |  39 ++-
 drivers/iommu/iommu-priv.h              |  16 +
 drivers/iommu/iommu.c                   | 263 +++++++++++------
 drivers/iommu/iommufd/iommufd_private.h |   5 +-
 drivers/iommu/iommufd/main.c            |   8 +-
 drivers/iommu/iommufd/selftest.c        | 141 ++++-----
 drivers/iommu/ipmmu-vmsa.c              |  50 +++-
 drivers/iommu/msm_iommu.c               |  30 +-
 drivers/iommu/mtk_iommu.c               |  30 +-
 drivers/iommu/mtk_iommu_v1.c            |  28 +-
 drivers/iommu/omap-iommu.c              |  28 +-
 drivers/iommu/rockchip-iommu.c          |  26 +-
 drivers/iommu/s390-iommu.c              |  28 +-
 drivers/iommu/sprd-iommu.c              |   7 +-
 drivers/iommu/sun50i-iommu.c            |  35 ++-
 drivers/iommu/tegra-gart.c              | 371 ------------------------
 drivers/iommu/tegra-smmu.c              |  50 +++-
 drivers/memory/tegra/mc.c               |  34 ---
 drivers/memory/tegra/tegra20.c          |  28 --
 include/linux/iommu.h                   |  16 +-
 include/soc/tegra/mc.h                  |  26 --
 28 files changed, 622 insertions(+), 807 deletions(-)
 create mode 100644 drivers/iommu/iommu-priv.h
 delete mode 100644 drivers/iommu/tegra-gart.c


base-commit: 0b355ade2baf262f213da274411c0e05d8232a21

Comments

Marek Szyprowski May 17, 2023, 10:42 p.m. UTC | #1
On 16.05.2023 02:00, Jason Gunthorpe wrote:
> [ There was alot of unexpected complication after rc1 with this series,
> several new patches were needed ]
>
> It has been a long time coming, this series completes the default_domain
> transition and makes it so that the core IOMMU code will always have a
> non-NULL default_domain for every driver on every
> platform. set_platform_dma_ops() turned out to be a bad idea, and so
> completely remove it.
>
> This is achieved by changing each driver to either:
>
> 1 - Convert the existing (or deleted) ops->detach_dev() into an
>      op->attach_dev() of an IDENTITY domain.
>
>      This is based on the theory that the ARM32 HW is able to function when
>      the iommu is turned off as so the turned off state is an IDENTITY
>      translation.
>
> 2 - Use a new PLATFORM domain type. This is a hack to accommodate drivers
>      that we don't really know WTF they do. S390 is legitimately using this
>      to switch to it's platform dma_ops implementation, which is where the
>      name comes from.
>
> 3 - Do #1 and force the default domain to be IDENTITY, this corrects
>      the tegra-smmu case where even an ARM64 system would have a NULL
>      default_domain.
>
> Using this we can apply the rules:
>
> a) ARM_DMA_USE_IOMMU mode always uses either the driver's
>     ops->default_domain, ops->def_domain_type(), or an IDENTITY domain.
>     All ARM32 drivers provide one of these three options.
>
> b) dma-iommu.c mode uses either the driver's ops->default_domain,
>     ops->def_domain_type or the usual DMA API policy logic based on the
>     command line/etc to pick IDENTITY/DMA domain types
>
> c) All other arch's (PPC/S390) use ops->default_domain always.
>
> See the patch "Require a default_domain for all iommu drivers" for a
> per-driver breakdown.
>
> The conversion broadly teaches a bunch of ARM32 drivers that they can do
> IDENTITY domains. There is some educated guessing involved that these are
> actual IDENTITY domains. If this turns out to be wrong the driver can be
> trivially changed to use a BLOCKING domain type instead. Further, the
> domain type only matters for drivers using ARM64's dma-iommu.c mode as it
> will select IDENTITY based on the command line and expect IDENTITY to
> work. For ARM32 and other arch cases it is purely documentation.
>
> Finally, based on all the analysis in this series, we can purge
> IOMMU_DOMAIN_UNMANAGED/DMA constants from most of the drivers. This
> greatly simplifies understanding the driver contract to the core
> code. IOMMU drivers should not be involved in policy for how the DMA API
> works, that should be a core core decision.
>
> The main gain from this work is to remove alot of ARM_DMA_USE_IOMMU
> specific code and behaviors from drivers. All that remains in iommu
> drivers after this series is the calls to arm_iommu_create_mapping().
>
> This is a step toward removing ARM_DMA_USE_IOMMU.
>
> The IDENTITY domains added to the ARM64 supporting drivers can be tested
> by booting in ARM64 mode and enabling CONFIG_IOMMU_DEFAULT_PASSTHROUGH. If
> the system still boots then most likely the implementation is an IDENTITY
> domain. If not we can trivially change it to BLOCKING or at worst PLATFORM
> if there is no detail what is going on in the HW.
>
> I think this is pretty safe for the ARM32 drivers as they don't really
> change, the code that was in detach_dev continues to be called in the same
> places it was called before.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Works fine on ARM 32bit Exynos based boards.

> This follows the prior series:
>
> https://lore.kernel.org/r/0-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com
>
> This is on github: https://protect2.fireeye.com/v1/url?k=773809ed-1645e36e-773982a2-74fe48600158-bd3a7b89de1f2061&q=1&e=7f176af7-9cf3-429b-a0ce-8812c59dfb5c&u=https%3A%2F%2Fgithub.com%2Fjgunthorpe%2Flinux%2Fcommits%2Fiommu_all_defdom
>
> v2:
>   - FSL is an IDENTITY domain
>   - Delete terga-gart instead of trying to carry it
>   - Use the policy determination from iommu_get_default_domain_type() to
>     drive the arm_iommu mode
>   - Reorganize and introduce new patches to do the above:
>      * Split the ops->identity_domain to an independent earlier patch
>      * Remove the UNMANAGED return from def_domain_type in mtk_v1 earlier
>        so the new iommu_get_default_domain_type() can work
>      * Make the driver's def_domain_type have higher policy priority than
>        untrusted
>      * Merge the set_platfom_dma_ops hunk from mtk_v1 along with rockchip
>        into the patch that forced IDENTITY on ARM32
>   - Revise sun50i to be cleaner and have a non-NULL internal domain
>   - Reword logging in exynos
>   - Remove the gdev from the group alloc path, instead add a new
>     function __iommu_group_domain_alloc() that takes in the group
>     and uses the first device. Split this to its own patch
>   - New patch to make iommufd's mock selftest into a real driver
>   - New patch to fix power's partial iommu driver
> v1: https://lore.kernel.org/r/0-v1-21cc72fcfb22+a7a-iommu_all_defdom_jgg@nvidia.com
>
> Jason Gunthorpe (25):
>    iommu: Add iommu_ops->identity_domain
>    iommu: Add IOMMU_DOMAIN_PLATFORM
>    powerpc/iommu: Setup a default domain and remove set_platform_dma_ops
>    iommu: Add IOMMU_DOMAIN_PLATFORM for S390
>    iommu/tegra-gart: Remove tegra-gart
>    iommu/mtk_iommu_v1: Implement an IDENTITY domain
>    iommu: Reorganize iommu_get_default_domain_type() to respect
>      def_domain_type()
>    iommu: Allow an IDENTITY domain as the default_domain in ARM32
>    iommu/fsl_pamu: Implement an IDENTITY domain
>    iommu/exynos: Implement an IDENTITY domain
>    iommu/tegra-smmu: Implement an IDENTITY domain
>    iommu/tegra-smmu: Support DMA domains in tegra
>    iommu/omap: Implement an IDENTITY domain
>    iommu/msm: Implement an IDENTITY domain
>    iommufd/selftest: Make the mock iommu driver into a real driver
>    iommu: Remove ops->set_platform_dma_ops()
>    iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
>    iommu/ipmmu: Add an IOMMU_IDENTITIY_DOMAIN
>    iommu/mtk_iommu: Add an IOMMU_IDENTITIY_DOMAIN
>    iommu/sun50i: Add an IOMMU_IDENTITIY_DOMAIN
>    iommu: Require a default_domain for all iommu drivers
>    iommu: Add __iommu_group_domain_alloc()
>    iommu: Add ops->domain_alloc_paging()
>    iommu: Convert simple drivers with DOMAIN_DMA to domain_alloc_paging()
>    iommu: Convert remaining simple drivers to domain_alloc_paging()
>
>   arch/arm/configs/multi_v7_defconfig     |   1 -
>   arch/arm/configs/tegra_defconfig        |   1 -
>   arch/powerpc/kernel/iommu.c             |  38 ++-
>   drivers/iommu/Kconfig                   |  11 -
>   drivers/iommu/Makefile                  |   1 -
>   drivers/iommu/arm/arm-smmu/qcom_iommu.c |  45 ++-
>   drivers/iommu/exynos-iommu.c            |  73 +++--
>   drivers/iommu/fsl_pamu_domain.c         |  39 ++-
>   drivers/iommu/iommu-priv.h              |  16 +
>   drivers/iommu/iommu.c                   | 263 +++++++++++------
>   drivers/iommu/iommufd/iommufd_private.h |   5 +-
>   drivers/iommu/iommufd/main.c            |   8 +-
>   drivers/iommu/iommufd/selftest.c        | 141 ++++-----
>   drivers/iommu/ipmmu-vmsa.c              |  50 +++-
>   drivers/iommu/msm_iommu.c               |  30 +-
>   drivers/iommu/mtk_iommu.c               |  30 +-
>   drivers/iommu/mtk_iommu_v1.c            |  28 +-
>   drivers/iommu/omap-iommu.c              |  28 +-
>   drivers/iommu/rockchip-iommu.c          |  26 +-
>   drivers/iommu/s390-iommu.c              |  28 +-
>   drivers/iommu/sprd-iommu.c              |   7 +-
>   drivers/iommu/sun50i-iommu.c            |  35 ++-
>   drivers/iommu/tegra-gart.c              | 371 ------------------------
>   drivers/iommu/tegra-smmu.c              |  50 +++-
>   drivers/memory/tegra/mc.c               |  34 ---
>   drivers/memory/tegra/tegra20.c          |  28 --
>   include/linux/iommu.h                   |  16 +-
>   include/soc/tegra/mc.h                  |  26 --
>   28 files changed, 622 insertions(+), 807 deletions(-)
>   create mode 100644 drivers/iommu/iommu-priv.h
>   delete mode 100644 drivers/iommu/tegra-gart.c
>
>
> base-commit: 0b355ade2baf262f213da274411c0e05d8232a21

Best regards
Nicolin Chen May 17, 2023, 11:32 p.m. UTC | #2
On Mon, May 15, 2023 at 09:00:33PM -0300, Jason Gunthorpe wrote:
 
> This is on github: https://github.com/jgunthorpe/linux/commits/iommu_all_defdom

Ran some VFIO-passthrough sanity on x86 and ARM64, using this
branch. It should cover partially this series. So, if I may:

Tested-by: Nicolin Chen <nicolinc@nvidia.com>

Thanks
Nic

> v2:
>  - FSL is an IDENTITY domain
>  - Delete terga-gart instead of trying to carry it
>  - Use the policy determination from iommu_get_default_domain_type() to
>    drive the arm_iommu mode
>  - Reorganize and introduce new patches to do the above:
>     * Split the ops->identity_domain to an independent earlier patch
>     * Remove the UNMANAGED return from def_domain_type in mtk_v1 earlier
>       so the new iommu_get_default_domain_type() can work
>     * Make the driver's def_domain_type have higher policy priority than
>       untrusted
>     * Merge the set_platfom_dma_ops hunk from mtk_v1 along with rockchip
>       into the patch that forced IDENTITY on ARM32
>  - Revise sun50i to be cleaner and have a non-NULL internal domain
>  - Reword logging in exynos
>  - Remove the gdev from the group alloc path, instead add a new
>    function __iommu_group_domain_alloc() that takes in the group
>    and uses the first device. Split this to its own patch
>  - New patch to make iommufd's mock selftest into a real driver
>  - New patch to fix power's partial iommu driver
Steven Price May 18, 2023, 10:56 a.m. UTC | #3
On 16/05/2023 01:00, Jason Gunthorpe wrote:
> This is on github: https://github.com/jgunthorpe/linux/commits/iommu_all_defdom

Tested-by: Steven Price <steven.price@arm.com>

Works fine on my Firefly-RK3288.

Thanks,

Steve