Re: [PATCH 3/3] iommu: Allow drivers to say if they use report_iommu_fault()

[Robin Murphy <robin.murphy@arm.com>]

On 2025-10-22 6:12 pm, Jason Gunthorpe wrote:
> report_iommu_fault() is an older API that has been superseded by
> iommu_report_device_fault() which is capable to support PRI.
> 
> Only two external drivers consume this, drivers/remoteproc and
> drivers/gpu/drm/msm. Ideally they would move over to the new APIs, but for
> now protect against accidentally mix and matching the wrong components.
> 
> The iommu drivers support either the old iommu_set_fault_handler() via the
> driver calling report_iommu_fault(), or they are newer server focused
> drivers that call iommu_report_device_fault().
> 
> Include a flag in the domain_ops if it calls report_iommu_fault() and
> block iommu_set_fault_handler() on iommu's that can't support it.

This isn't a domain operation though; depending on how you look at it, 
supporting a legacy fault_handler is either a capability of the IOMMU 
driver (that would be reachable via domain->owner->capable) or a 
property of the iommu_domain itself that the drivers can set at 
allocation time (basically this same patch just with the lines in 
slightly different places).

Thanks,
Robin.
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   drivers/iommu/arm/arm-smmu/arm-smmu.c   | 1 +
>   drivers/iommu/arm/arm-smmu/qcom_iommu.c | 1 +
>   drivers/iommu/iommu.c                   | 6 +++++-
>   drivers/iommu/ipmmu-vmsa.c              | 1 +
>   drivers/iommu/mtk_iommu.c               | 1 +
>   drivers/iommu/mtk_iommu_v1.c            | 1 +
>   drivers/iommu/omap-iommu.c              | 1 +
>   drivers/iommu/rockchip-iommu.c          | 1 +
>   drivers/iommu/sun50i-iommu.c            | 1 +
>   include/linux/iommu.h                   | 3 +++
>   10 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 4ced4b5bee4df3..5ce8f82ddb534b 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -1644,6 +1644,7 @@ static const struct iommu_ops arm_smmu_ops = {
>   	.def_domain_type	= arm_smmu_def_domain_type,
>   	.owner			= THIS_MODULE,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev		= arm_smmu_attach_dev,
>   		.map_pages		= arm_smmu_map_pages,
>   		.unmap_pages		= arm_smmu_unmap_pages,
> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index c5be95e560317e..3163a23fcbaa4f 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> @@ -598,6 +598,7 @@ static const struct iommu_ops qcom_iommu_ops = {
>   	.device_group	= generic_device_group,
>   	.of_xlate	= qcom_iommu_of_xlate,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= qcom_iommu_attach_dev,
>   		.map_pages	= qcom_iommu_map,
>   		.unmap_pages	= qcom_iommu_unmap,
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 59244c744eabd2..34546a70fb5279 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2005,6 +2005,9 @@ EXPORT_SYMBOL_GPL(iommu_group_has_isolated_msi);
>    * This function should be used by IOMMU users which want to be notified
>    * whenever an IOMMU fault happens.
>    *
> + * This is a legacy API not supported by all drivers. New users should look
> + * to using domain->iopf_handler for the modern API.
> + *
>    * The fault handler itself should return 0 on success, and an appropriate
>    * error code otherwise.
>    */
> @@ -2012,7 +2015,8 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
>   					iommu_fault_handler_t handler,
>   					void *token)
>   {
> -	if (WARN_ON(!domain || domain->cookie_type != IOMMU_COOKIE_NONE))
> +	if (WARN_ON(!domain || domain->cookie_type != IOMMU_COOKIE_NONE ||
> +		    !domain->ops->report_iommu_fault_supported))
>   		return;
>   
>   	domain->cookie_type = IOMMU_COOKIE_FAULT_HANDLER;
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index ffa892f6571406..770fa248e30477 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -885,6 +885,7 @@ static const struct iommu_ops ipmmu_ops = {
>   			? generic_device_group : generic_single_device_group,
>   	.of_xlate = ipmmu_of_xlate,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= ipmmu_attach_device,
>   		.map_pages	= ipmmu_map,
>   		.unmap_pages	= ipmmu_unmap,
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 0e0285348d2b8e..0f44993eaadce3 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -1019,6 +1019,7 @@ static const struct iommu_ops mtk_iommu_ops = {
>   	.get_resv_regions = mtk_iommu_get_resv_regions,
>   	.owner		= THIS_MODULE,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= mtk_iommu_attach_device,
>   		.map_pages	= mtk_iommu_map,
>   		.unmap_pages	= mtk_iommu_unmap,
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index 10cc0b1197e801..279e7acfd5c6d3 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -582,6 +582,7 @@ static const struct iommu_ops mtk_iommu_v1_ops = {
>   	.device_group	= generic_device_group,
>   	.owner          = THIS_MODULE,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= mtk_iommu_v1_attach_device,
>   		.map_pages	= mtk_iommu_v1_map,
>   		.unmap_pages	= mtk_iommu_v1_unmap,
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 5c6f5943f44b1f..3f3193de3ecd86 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1724,6 +1724,7 @@ static const struct iommu_ops omap_iommu_ops = {
>   	.device_group	= generic_single_device_group,
>   	.of_xlate	= omap_iommu_of_xlate,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= omap_iommu_attach_dev,
>   		.map_pages	= omap_iommu_map,
>   		.unmap_pages	= omap_iommu_unmap,
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0861dd469bd866..0053f5aa2cb781 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1174,6 +1174,7 @@ static const struct iommu_ops rk_iommu_ops = {
>   	.device_group = generic_single_device_group,
>   	.of_xlate = rk_iommu_of_xlate,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= rk_iommu_attach_device,
>   		.map_pages	= rk_iommu_map,
>   		.unmap_pages	= rk_iommu_unmap,
> diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
> index de10b569d9a940..29b230050906a2 100644
> --- a/drivers/iommu/sun50i-iommu.c
> +++ b/drivers/iommu/sun50i-iommu.c
> @@ -849,6 +849,7 @@ static const struct iommu_ops sun50i_iommu_ops = {
>   	.of_xlate	= sun50i_iommu_of_xlate,
>   	.probe_device	= sun50i_iommu_probe_device,
>   	.default_domain_ops = &(const struct iommu_domain_ops) {
> +		.report_iommu_fault_supported = true,
>   		.attach_dev	= sun50i_iommu_attach_device,
>   		.flush_iotlb_all = sun50i_iommu_flush_iotlb_all,
>   		.iotlb_sync_map = sun50i_iommu_iotlb_sync_map,
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index c30d12e16473df..e2bf7885287fac 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -714,6 +714,8 @@ struct iommu_ops {
>   
>   /**
>    * struct iommu_domain_ops - domain specific operations
> + * @report_iommu_fault_supported: True if the domain supports
> + *                                iommu_set_fault_handler()
>    * @attach_dev: attach an iommu domain to a device
>    *  Return:
>    * * 0		- success
> @@ -751,6 +753,7 @@ struct iommu_ops {
>    * @free: Release the domain after use.
>    */
>   struct iommu_domain_ops {
> +	bool report_iommu_fault_supported : 1;
>   	int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
>   	int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
>   			     ioasid_t pasid, struct iommu_domain *old);