| 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
| ||
|
Message-ID: <20250324162558.GA198799@ax162>
Date: Mon, 24 Mar 2025 09:25:58 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: jgg@...dia.com, kevin.tian@...el.com, robin.murphy@....com,
joro@...tes.org, will@...nel.org, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/3] iommu: Drop sw_msi from iommu_domain
Hi Nicolin,
On Thu, Mar 06, 2025 at 01:00:49PM -0800, Nicolin Chen wrote:
> There are only two sw_msi implementations in the entire system, thus it's
> not very necessary to have an sw_msi pointer.
>
> Instead, check domain->cookie_type to call the two sw_msi implementations
> directly from the core code.
>
> Suggested-by: Robin Murphy <robin.murphy@....com>
> Reviewed-by: Robin Murphy <robin.murphy@....com>
> Reviewed-by: Kevin Tian <kevin.tian@...el.com>
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
> drivers/iommu/dma-iommu.h | 9 +++++++++
> include/linux/iommu.h | 15 ---------------
> drivers/iommu/dma-iommu.c | 14 ++------------
> drivers/iommu/iommu.c | 17 +++++++++++++++--
> drivers/iommu/iommufd/hw_pagetable.c | 3 ---
> 5 files changed, 26 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.h b/drivers/iommu/dma-iommu.h
> index 9cca11806e5d..eca201c1f963 100644
> --- a/drivers/iommu/dma-iommu.h
> +++ b/drivers/iommu/dma-iommu.h
> @@ -19,6 +19,9 @@ int iommu_dma_init_fq(struct iommu_domain *domain);
>
> void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
>
> +int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> + phys_addr_t msi_addr);
> +
> extern bool iommu_dma_forcedac;
>
> #else /* CONFIG_IOMMU_DMA */
> @@ -49,5 +52,11 @@ static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
> {
> }
>
> +static inline int iommu_dma_sw_msi(struct iommu_domain *domain,
> + struct msi_desc *desc, phys_addr_t msi_addr)
> +{
> + return -ENODEV;
> +}
> +
> #endif /* CONFIG_IOMMU_DMA */
> #endif /* __DMA_IOMMU_H */
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 06cc14e9993d..e01c855ae8a7 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -229,11 +229,6 @@ struct iommu_domain {
> struct iommu_domain_geometry geometry;
> int (*iopf_handler)(struct iopf_group *group);
>
> -#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
> - int (*sw_msi)(struct iommu_domain *domain, struct msi_desc *desc,
> - phys_addr_t msi_addr);
> -#endif
> -
> union { /* cookie */
> struct iommu_dma_cookie *iova_cookie;
> struct iommu_dma_msi_cookie *msi_cookie;
> @@ -254,16 +249,6 @@ struct iommu_domain {
> };
> };
>
> -static inline void iommu_domain_set_sw_msi(
> - struct iommu_domain *domain,
> - int (*sw_msi)(struct iommu_domain *domain, struct msi_desc *desc,
> - phys_addr_t msi_addr))
> -{
> -#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
> - domain->sw_msi = sw_msi;
> -#endif
> -}
> -
> static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
> {
> return domain->type & __IOMMU_DOMAIN_DMA_API;
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 31a7b4b81656..2bd9f80a83fe 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -94,9 +94,6 @@ static int __init iommu_dma_forcedac_setup(char *str)
> }
> early_param("iommu.forcedac", iommu_dma_forcedac_setup);
>
> -static int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> - phys_addr_t msi_addr);
> -
> /* Number of entries per flush queue */
> #define IOVA_DEFAULT_FQ_SIZE 256
> #define IOVA_SINGLE_FQ_SIZE 32768
> @@ -377,7 +374,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>
> mutex_init(&cookie->mutex);
> INIT_LIST_HEAD(&cookie->msi_page_list);
> - iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
> domain->cookie_type = IOMMU_COOKIE_DMA_IOVA;
> domain->iova_cookie = cookie;
> return 0;
> @@ -411,7 +407,6 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
>
> cookie->msi_iova = base;
> INIT_LIST_HEAD(&cookie->msi_page_list);
> - iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
> domain->cookie_type = IOMMU_COOKIE_DMA_MSI;
> domain->msi_cookie = cookie;
> return 0;
> @@ -427,11 +422,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
> struct iommu_dma_cookie *cookie = domain->iova_cookie;
> struct iommu_dma_msi_page *msi, *tmp;
>
> -#if IS_ENABLED(CONFIG_IRQ_MSI_IOMMU)
> - if (domain->sw_msi != iommu_dma_sw_msi)
> - return;
> -#endif
> -
> if (cookie->iovad.granule) {
> iommu_dma_free_fq(cookie);
> put_iova_domain(&cookie->iovad);
> @@ -1826,8 +1816,8 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
> return NULL;
> }
>
> -static int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> - phys_addr_t msi_addr)
> +int iommu_dma_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
> + phys_addr_t msi_addr)
> {
> struct device *dev = msi_desc_to_dev(desc);
> const struct iommu_dma_msi_page *msi_page;
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index c92e47f333cb..0f4cc15ded1c 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -18,6 +18,7 @@
> #include <linux/errno.h>
> #include <linux/host1x_context_bus.h>
> #include <linux/iommu.h>
> +#include <linux/iommufd.h>
> #include <linux/idr.h>
> #include <linux/err.h>
> #include <linux/pci.h>
> @@ -3650,8 +3651,20 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)
> return 0;
>
> mutex_lock(&group->mutex);
> - if (group->domain && group->domain->sw_msi)
> - ret = group->domain->sw_msi(group->domain, desc, msi_addr);
> + if (group->domain) {
> + switch (group->domain->cookie_type) {
> + case IOMMU_COOKIE_DMA_MSI:
> + case IOMMU_COOKIE_DMA_IOVA:
> + ret = iommu_dma_sw_msi(group->domain, desc, msi_addr);
> + break;
> + case IOMMU_COOKIE_IOMMUFD:
> + ret = iommufd_sw_msi(group->domain, desc, msi_addr);
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + break;
> + }
> + }
> mutex_unlock(&group->mutex);
> return ret;
> }
> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
> index cefe71a4e9e8..f97c4e49d486 100644
> --- a/drivers/iommu/iommufd/hw_pagetable.c
> +++ b/drivers/iommu/iommufd/hw_pagetable.c
> @@ -161,7 +161,6 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
> }
> hwpt->domain->iommufd_hwpt = hwpt;
> hwpt->domain->cookie_type = IOMMU_COOKIE_IOMMUFD;
> - iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
>
> /*
> * Set the coherency mode before we do iopt_table_add_domain() as some
> @@ -259,7 +258,6 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
> hwpt->domain->owner = ops;
> hwpt->domain->iommufd_hwpt = hwpt;
> hwpt->domain->cookie_type = IOMMU_COOKIE_IOMMUFD;
> - iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
>
> if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
> rc = -EINVAL;
> @@ -318,7 +316,6 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
> hwpt->domain->iommufd_hwpt = hwpt;
> hwpt->domain->owner = viommu->iommu_dev->ops;
> hwpt->domain->cookie_type = IOMMU_COOKIE_IOMMUFD;
> - iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
>
> if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
> rc = -EINVAL;
> --
> 2.43.0
>
I bisected a loss of networking on one of my machines to this change as
commit e009e088d88e ("iommu: Drop sw_msi from iommu_domain") in -next.
With the parent change, I see:
[ +0.000000] Linux version 6.14.0-rc2-00032-g916a207692ce (nathan@...62) (aarch64-linux-gcc (GCC) 14.2.0, GNU ld (GNU Binutils) 2.42) #1 SMP PREEMPT_DYNAMIC Mon Mar 24 09:13:34 MST 2025
...
[ +0.002375] fsl_mc_bus NXP0008:00: Adding to iommu group 0
[ +0.000532] fsl_mc_bus NXP0008:00: MC firmware version: 10.28.1
[ +0.002566] fsl_mc_dprc dprc.1: DMA mask not set
[ +0.019213] fsl_mc_dprc dprc.1: Adding to iommu group 1
[ +0.050801] fsl_mc_allocator dpbp.0: Adding to iommu group 1
[ +0.006767] fsl_mc_allocator dpmcp.35: Adding to iommu group 1
...
At this change, I see:
[ +0.000000] Linux version 6.14.0-rc2-00033-ge009e088d88e (nathan@...62) (aarch64-linux-gcc (GCC) 14.2.0, GNU ld (GNU Binutils) 2.42) #1 SMP PREEMPT_DYNAMIC Mon Mar 24 08:57:49 MST 2025
...
[ +0.002355] fsl_mc_bus NXP0008:00: Adding to iommu group 0
[ +0.000533] fsl_mc_bus NXP0008:00: MC firmware version: 10.28.1
[ +0.002565] fsl_mc_dprc dprc.1: DMA mask not set
[ +0.019255] fsl_mc_dprc dprc.1: Adding to iommu group 1
[ +0.046820] fsl_mc_dprc dprc.1: Failed to allocate IRQs
[ +0.005798] fsl_mc_dprc dprc.1: fsl_mc_driver_probe failed: -28
[ +0.005931] fsl_mc_dprc dprc.1: probe with driver fsl_mc_dprc failed with error -28
...
I know this is not much to go on initially but I am more than happy to
provide any additional information and test patches as necessary.
Cheers,
Nathan
# bad: [9388ec571cb1adba59d1cded2300eeb11827679c] Add linux-next specific files for 20250321
# good: [b5329d5a35582abbef57562f9fb6cb26a643f252] Merge tag 'vfs-6.14-final.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
git bisect start '9388ec571cb1adba59d1cded2300eeb11827679c' 'b5329d5a35582abbef57562f9fb6cb26a643f252'
# good: [81a405abf1b06a6088197fe1d039ec5dbfd17989] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git
git bisect good 81a405abf1b06a6088197fe1d039ec5dbfd17989
# good: [1afe6ad5ffa395d0809210a3b609751109de3f44] Merge branch 'apparmor-next' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
git bisect good 1afe6ad5ffa395d0809210a3b609751109de3f44
# good: [b97539c1de91aa73b589e145e3c804b9ba5178f2] Merge branch 'driver-core-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
git bisect good b97539c1de91aa73b589e145e3c804b9ba5178f2
# good: [db08813487b74820cb40f507fc6ea38994a44776] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
git bisect good db08813487b74820cb40f507fc6ea38994a44776
# good: [8dc029f217de83114fe7a59803ea0ac398db414c] Merge branch 'hyperv-next' of git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
git bisect good 8dc029f217de83114fe7a59803ea0ac398db414c
# good: [33239be7f682bf0ef9293c0b8ad53e691275e3a2] Merge branch 'rust-next' of https://github.com/Rust-for-Linux/linux.git
git bisect good 33239be7f682bf0ef9293c0b8ad53e691275e3a2
# good: [f57f2954197b265b6d793ac89d774aae1473854d] Merge branch 'for-next/kspp' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
git bisect good f57f2954197b265b6d793ac89d774aae1473854d
# good: [acf9f8da5e19fc1cbf26f2ecb749369e13e7cd85] x86/crc: drop the avx10_256 functions and rename avx10_512 to avx512
git bisect good acf9f8da5e19fc1cbf26f2ecb749369e13e7cd85
# good: [da0c56520e880441d0503d0cf0d6853dcfb5f1a4] iommu/arm-smmu-v3: Set MEV bit in nested STE for DoS mitigations
git bisect good da0c56520e880441d0503d0cf0d6853dcfb5f1a4
# bad: [fe0cdd47410f7d5fc26e81c958cde3fe1d57d0a0] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git
git bisect bad fe0cdd47410f7d5fc26e81c958cde3fe1d57d0a0
# good: [447c98c1ca4a4b0d43be99f76c558c09956484f3] tools/power turbostat: Add idle governor statistics reporting
git bisect good 447c98c1ca4a4b0d43be99f76c558c09956484f3
# good: [916a207692ce0a6f82ced6b37ca895d5219efb17] iommufd: Move iommufd_sw_msi and related functions to driver.c
git bisect good 916a207692ce0a6f82ced6b37ca895d5219efb17
# bad: [71c4c1a2a057bd800e08b752bac660f5426bf6b5] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git
git bisect bad 71c4c1a2a057bd800e08b752bac660f5426bf6b5
# bad: [e009e088d88e8402539f9595b10c0014125a70c1] iommu: Drop sw_msi from iommu_domain
git bisect bad e009e088d88e8402539f9595b10c0014125a70c1
# first bad commit: [e009e088d88e8402539f9595b10c0014125a70c1] iommu: Drop sw_msi from iommu_domain
Powered by blists - more mailing lists