From: Nathan Chancellor <nathan@kernel.org>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: jgg@nvidia.com, kevin.tian@intel.com, robin.murphy@arm.com,
joro@8bytes.org, will@kernel.org, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 3/3] iommu: Drop sw_msi from iommu_domain
Date: Mon, 24 Mar 2025 09:25:58 -0700 [thread overview]
Message-ID: <20250324162558.GA198799@ax162> (raw)
In-Reply-To: <6d4b493d20926c8d14e29bc776639cdcf2095146.1741294235.git.nicolinc@nvidia.com>
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@arm.com>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.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@ax162) (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@ax162) (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
next prev parent reply other threads:[~2025-03-24 16:26 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-06 21:00 [PATCH v4 0/3] iommu: Clean up cookie and sw_msi in struct iommu_domain Nicolin Chen
2025-03-06 21:00 ` [PATCH v4 1/3] iommu: Sort out domain user data Nicolin Chen
2025-03-07 2:28 ` Baolu Lu
2025-03-07 5:57 ` Nicolin Chen
2025-03-07 7:03 ` Baolu Lu
2025-03-07 11:49 ` Robin Murphy
2025-03-07 15:32 ` Jason Gunthorpe
2025-03-17 19:37 ` Jason Gunthorpe
2025-03-06 21:00 ` [PATCH v4 2/3] iommufd: Move iommufd_sw_msi and related functions to driver.c Nicolin Chen
2025-03-12 7:37 ` Tian, Kevin
2025-03-17 20:20 ` Jason Gunthorpe
2025-03-06 21:00 ` [PATCH v4 3/3] iommu: Drop sw_msi from iommu_domain Nicolin Chen
2025-03-17 20:20 ` Jason Gunthorpe
2025-03-24 16:25 ` Nathan Chancellor [this message]
2025-03-24 16:40 ` Jason Gunthorpe
2025-03-24 16:55 ` Nicolin Chen
2025-03-24 17:05 ` Nicolin Chen
2025-03-24 17:07 ` Nathan Chancellor
2025-03-24 20:05 ` Jason Gunthorpe
2025-03-24 20:43 ` Nathan Chancellor
2025-03-24 21:38 ` Nicolin Chen
2025-03-24 22:29 ` Jason Gunthorpe
2025-03-24 22:45 ` Nicolin Chen
2025-03-17 20:21 ` [PATCH v4 0/3] iommu: Clean up cookie and sw_msi in struct iommu_domain Jason Gunthorpe
2025-03-20 23:16 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250324162558.GA198799@ax162 \
--to=nathan@kernel.org \
--cc=iommu@lists.linux.dev \
--cc=jgg@nvidia.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolinc@nvidia.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.