Re: [PATCH v12 10/10] iommu/arm-smmu-v3: Add stall support for platform devices

From: Zhou Wang <wangzhou1@hisilicon.com>
To: Jean-Philippe Brucker <jean-philippe@linaro.org>,
	<joro@8bytes.org>, <will@kernel.org>
Cc: devicetree@vger.kernel.org, kevin.tian@intel.com,
	linux-acpi@vger.kernel.org, sudeep.holla@arm.com,
	rjw@rjwysocki.net, vivek.gautam@arm.com,
	iommu@lists.linux-foundation.org, robh+dt@kernel.org,
	linux-accelerators@lists.ozlabs.org, guohanjun@huawei.com,
	zhangfei.gao@linaro.org, robin.murphy@arm.com,
	linux-arm-kernel@lists.infradead.org, lenb@kernel.org
Subject: Re: [PATCH v12 10/10] iommu/arm-smmu-v3: Add stall support for platform devices
Date: Mon, 1 Feb 2021 09:18:42 +0800	[thread overview]
Message-ID: <8adc79cc-7afb-dfe8-4f7b-07fa6dc5b905@hisilicon.com> (raw)
In-Reply-To: <20210127154322.3959196-11-jean-philippe@linaro.org>

On 2021/1/27 23:43, Jean-Philippe Brucker wrote:
> The SMMU provides a Stall model for handling page faults in platform
> devices. It is similar to PCIe PRI, but doesn't require devices to have
> their own translation cache. Instead, faulting transactions are parked
> and the OS is given a chance to fix the page tables and retry the
> transaction.
> 
> Enable stall for devices that support it (opt-in by firmware). When an
> event corresponds to a translation error, call the IOMMU fault handler.
> If the fault is recoverable, it will call us back to terminate or
> continue the stall.
> 
> To use stall device drivers need to enable IOMMU_DEV_FEAT_IOPF, which
> initializes the fault queue for the device.
> 
> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  43 ++++
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  59 +++++-
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 189 +++++++++++++++++-
>  3 files changed, 276 insertions(+), 15 deletions(-)
> 

[...]

> @@ -1033,8 +1076,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
>  			FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
>  			CTXDESC_CD_0_V;
>  
> -		/* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> -		if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> +		if (smmu_domain->stall_enabled)

Could we add ssid checking here? like: if (smmu_domain->stall_enabled && ssid).
The reason is if not CD.S will also be set when ssid is 0, which is not needed.

Best,
Zhou

>  			val |= CTXDESC_CD_0_S;
>  	}
>  
> @@ -1278,7 +1320,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
>  			 FIELD_PREP(STRTAB_STE_1_STRW, strw));
>  
>  		if (smmu->features & ARM_SMMU_FEAT_STALLS &&
> -		   !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
> +		    !master->stall_enabled)
>  			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>  
>  		val |= (s1_cfg->cdcfg.cdtab_dma & STRTAB_STE_0_S1CTXPTR_MASK) |

[...]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu