linux-kernel - Re: [PATCH] vfio: mlx5, pds: add IOMMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <5d7cb04d-9e79-43b9-9dd2-7d7803c93f4f@app.fastmail.com>
Date:   Mon, 23 Oct 2023 16:02:11 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Jason Gunthorpe" <jgg@...dia.com>
Cc:     "Joao Martins" <joao.m.martins@...cle.com>,
        "Arnd Bergmann" <arnd@...nel.org>,
        "Kevin Tian" <kevin.tian@...el.com>,
        "Yishai Hadas" <yishaih@...dia.com>,
        "Shameerali Kolothum Thodi" <shameerali.kolothum.thodi@...wei.com>,
        "Alex Williamson" <alex.williamson@...hat.com>,
        "Brett Creeley" <brett.creeley@....com>,
        oushixiong <oushixiong@...inos.cn>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfio: mlx5, pds: add IOMMU_SUPPORT dependency

On Mon, Oct 23, 2023, at 15:23, Jason Gunthorpe wrote:
> On Mon, Oct 23, 2023 at 02:55:13PM +0200, Arnd Bergmann wrote:
>> On Mon, Oct 23, 2023, at 14:37, Joao Martins wrote:
>> 
>> Are there any useful configurations with IOMMU_API but
>> not IOMMU_SUPPORT though? My first approach was actually
>
> IOMMU_SUPPORT is just the menu option in kconfig, it doesn't actually
> do anything functional as far as I can tell
>
> But you can have IOMMU_API turned on without IOMMU_SUPPORT still on
> power
>
> I think the right thing is to combine IOMMU_SUPPORT and IOMMU_API into
> the same thing.

I've had a closer look now and I think the way it is currently
designed to be used makes some sense: IOMMU implementations almost
universally depend on both a CPU architecture and CONFIG_IOMMU_SUPPORT,
but select IOMMU_API. So if you enable IOMMU_SUPPORT on an
architecture that has no IOMMU implementations, none of the drivers
are visible and nothing happens. Similarly, almost all drivers
using the IOMMU interface depend on IOMMU_API, so they can only
be built if at least one IOMMU driver is configured.

I made a patch to fix the outliers, which fixes the problem
by ensuring that nothing selects IOMMU_API without also depending
on IOMMU_SUPPORT. Unfortunately that introduces circular dependencies
with CONFIG_VIRTIO, which needs a similar patch to ensure that
only VIRTIO transport providers select it, not virtio drivers
(console, gpu, i2c, caif and fs get this one wrong).

> Since VFIO already must depend on IOMMU_API it would be sufficient for
> this problem too.

Right, having all IOMMU users depend on IOMMU_API certainly makes
sense, though at the moment it uses 'select', which is part of
the problem.

    Arnd


diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index ad70b611b44f0..57ebd7f1a3b29 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -5,7 +5,7 @@ config DRM_MSM
 	depends on DRM
 	depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
 	depends on COMMON_CLK
-	depends on IOMMU_SUPPORT
+	depends on IOMMU_API
 	depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
 	depends on QCOM_OCMEM || QCOM_OCMEM=n
 	depends on QCOM_LLCC || QCOM_LLCC=n
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 4a79704b164f7..2902b89a48f17 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -4,7 +4,7 @@ config DRM_NOUVEAU
 	depends on DRM && PCI && MMU
 	depends on (ACPI_VIDEO && ACPI_WMI && MXM_WMI) || !(ACPI && X86)
 	depends on BACKLIGHT_CLASS_DEVICE
-	select IOMMU_API
+	depends on IOMMU_API
 	select FW_LOADER
 	select DRM_DISPLAY_DP_HELPER
 	select DRM_DISPLAY_HDMI_HELPER
diff --git a/drivers/gpu/drm/panfrost/Kconfig b/drivers/gpu/drm/panfrost/Kconfig
index e6403a9d66ade..acdcad76d92a8 100644
--- a/drivers/gpu/drm/panfrost/Kconfig
+++ b/drivers/gpu/drm/panfrost/Kconfig
@@ -5,9 +5,9 @@ config DRM_PANFROST
 	depends on DRM
 	depends on ARM || ARM64 || COMPILE_TEST
 	depends on !GENERIC_ATOMIC64    # for IOMMU_IO_PGTABLE_LPAE
+	depends on IOMMU_API
 	depends on MMU
 	select DRM_SCHED
-	select IOMMU_SUPPORT
 	select IOMMU_IO_PGTABLE_LPAE
 	select DRM_GEM_SHMEM_HELPER
 	select PM_DEVFREQ
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 3199fd54b462c..be3a8bf42f6ca 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -3,10 +3,6 @@
 config IOMMU_IOVA
 	tristate
 
-# IOMMU_API always gets selected by whoever wants it.
-config IOMMU_API
-	bool
-
 menuconfig IOMMU_SUPPORT
 	bool "IOMMU Hardware Support"
 	depends on MMU
@@ -483,4 +479,8 @@ config SPRD_IOMMU
 
 	  Say Y here if you want to use the multimedia devices listed above.
 
+# IOMMU_API must be selected by any IOMMU provider
+config IOMMU_API
+	bool
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 6bda6dbb48784..8c56189c95b38 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig VFIO
 	tristate "VFIO Non-Privileged userspace driver framework"
-	select IOMMU_API
 	depends on IOMMUFD || !IOMMUFD
+	depends on IOMMU_API
 	select INTERVAL_TREE
 	select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n
 	select VFIO_DEVICE_CDEV if !VFIO_GROUP
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index d5989871dd5de..9b0eeffc9a3b3 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -353,6 +353,7 @@ config XEN_GRANT_DMA_OPS
 config XEN_VIRTIO
 	bool "Xen virtio support"
 	depends on VIRTIO
+	depends on IOMMU_SUPPORT
 	select XEN_GRANT_DMA_OPS
 	select XEN_GRANT_DMA_IOMMU if OF
 	help