linux-kernel - Re: [PATCH v3 01/12] iommu/arm-smmu-v3: Put iopf enablement in the domain attach path

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: <20250311161337.GD5138@willie-the-truck>
Date: Tue, 11 Mar 2025 16:13:38 +0000
From: Will Deacon <will@...nel.org>
To: Lu Baolu <baolu.lu@...ux.intel.com>
Cc: Joerg Roedel <joro@...tes.org>, Robin Murphy <robin.murphy@....com>,
	Jason Gunthorpe <jgg@...pe.ca>, Kevin Tian <kevin.tian@...el.com>,
	Dave Jiang <dave.jiang@...el.com>, Vinod Koul <vkoul@...nel.org>,
	Fenghua Yu <fenghuay@...dia.com>,
	Zhangfei Gao <zhangfei.gao@...aro.org>,
	Zhou Wang <wangzhou1@...ilicon.com>, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, Jason Gunthorpe <jgg@...dia.com>
Subject: Re: [PATCH v3 01/12] iommu/arm-smmu-v3: Put iopf enablement in the
 domain attach path

On Fri, Feb 28, 2025 at 05:26:20PM +0800, Lu Baolu wrote:
> From: Jason Gunthorpe <jgg@...dia.com>
> 
> SMMUv3 co-mingles FEAT_IOPF and FEAT_SVA behaviors so that fault reporting
> doesn't work unless both are enabled. This is not correct and causes
> problems for iommufd which does not enable FEAT_SVA for it's fault capable
> domains.
> 
> These APIs are both obsolete, update SMMUv3 to use the new method like AMD
> implements.
> 
> A driver should enable iopf support when a domain with an iopf_handler is
> attached, and disable iopf support when the domain is removed.
> 
> Move the fault support logic to sva domain allocation and to domain
> attach, refusing to create or attach fault capable domains if the HW
> doesn't support it.
> 
> Move all the logic for controlling the iopf queue under
> arm_smmu_attach_prepare(). Keep track of the number of domains on the
> master (over all the SSIDs) that require iopf. When the first domain
> requiring iopf is attached create the iopf queue, when the last domain is
> detached destroy it.
> 
> Turn FEAT_IOPF and FEAT_SVA into no ops.
> 
> Remove the sva_lock, this is all protected by the group mutex.
> 
> Signed-off-by: Jason Gunthorpe <jgg@...dia.com>
> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
> Tested-by: Zhangfei Gao <zhangfei.gao@...aro.org>
> ---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  86 +-------------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 105 +++++++++++++-----
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  39 ++-----
>  3 files changed, 91 insertions(+), 139 deletions(-)

[...]

> @@ -2748,6 +2750,54 @@ to_smmu_domain_devices(struct iommu_domain *domain)
>  	return NULL;
>  }
>  
> +static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
> +				struct arm_smmu_master_domain *master_domain)
> +{
> +	int ret;
> +
> +	iommu_group_mutex_assert(master->dev);
> +
> +	if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
> +		return -EOPNOTSUPP;
> +
> +	/*
> +	 * Drivers for devices supporting PRI or stall require iopf others have
> +	 * device-specific fault handlers and don't need IOPF, so this is not a
> +	 * failure.
> +	 */
> +	if (!master->stall_enabled)
> +		return 0;
> +
> +	/* We're not keeping track of SIDs in fault events */
> +	if (master->num_streams != 1)
> +		return -EOPNOTSUPP;
> +
> +	if (master->iopf_refcount) {
> +		master->iopf_refcount++;
> +		master_domain->using_iopf = true;
> +		return 0;
> +	}
> +
> +	ret = iopf_queue_add_device(master->smmu->evtq.iopf, master->dev);
> +	if (ret)
> +		return ret;
> +	master->iopf_refcount = 1;
> +	master_domain->using_iopf = true;
> +	return 0;
> +}
> +
> +static void arm_smmu_disable_iopf(struct arm_smmu_master *master)
> +{
> +	iommu_group_mutex_assert(master->dev);
> +
> +	if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
> +		return;

I think it would be a little cleaner to push the '->using_iopf' check
in here rather than have the callers check it. Then the SVA check above
makes more sense and I think the enable/disable paths are a bit more
symmetric.

With that:

Acked-by: Will Deacon <will@...nel.org>

Will