Re: [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Thu, Dec 05, 2024 at 01:48:09PM +0000, Robin Murphy wrote:
> Process pagetables may now be using new permission-indirection-based
> features which an SMMU may not understand when given such a table for
> SVA. Although SMMUv3.4 does add its own S1PIE feature, realistically
> we're still going to have to cope with feature mismatches between CPUs
> and SMMUs, so let's start simple and essentially just document the
> expectations for what falls out as-is. Although it seems unlikely for
> SVA applications to also depend on memory-hardening features, or
> vice-versa,

Hard to predict, but SVA could be indirectly enabled by some common
library: for example a libcrypto plugin that shares the address space with
an accelerator when one is available (see uadk_engine). I'm not familiar
with the POE use-cases, but I guess a common library could also enable
those memory-hardening features, such that unsuspecting applications have
everything enabled.

> the relative lifecycles make it tricky to enforce mutual
> exclusivity. Thankfully our PIE index allocation makes it relatively
> benign for an SMMU to keep interpreting them as direct permissions, the
> only real implication is that an SVA application cannot harden itself
> against its own devices with these features. Thus, inform the user about
> that just in case they have other expectations.
> 
> Also we don't (yet) support LPA2, so deny SVA entirely if we're going to
> misunderstand the pagetable format altogether.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 1d3e71569775..9ba596430e7c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -112,6 +112,15 @@ void arm_smmu_make_sva_cd(struct arm_smmu_cd *target,
>  	 * from the current CPU register
>  	 */
>  	target->data[3] = cpu_to_le64(read_sysreg(mair_el1));
> +
> +	/*
> +	 * Note that we don't bother with S1PIE on the SMMU, we just rely on
> +	 * our default encoding scheme matching direct permissions anyway.
> +	 * SMMU has no notion of S1POE nor GCS, so make sure that is clear if
> +	 * either is enabled for CPUs, just in case anyone imagines otherwise.
> +	 */
> +	if (system_supports_poe() || system_supports_gcs())
> +		dev_warn_once(master->smmu->dev, "SVA devices ignore permission overlays and GCS\n");

If POE tightens the page permissions, enabling SVA on top lets the
application easily bypass the POE protection. Could we actually return
false in arm_smmu_sva_supported() instead of displaying a warning that
likely no one will notice?

I guess enforcing !(SVA & POE) on an address space would be too much work?

Thanks,
Jean

>  }
>  EXPORT_SYMBOL_IF_KUNIT(arm_smmu_make_sva_cd);
>  
> @@ -206,8 +215,12 @@ bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
>  	unsigned long asid_bits;
>  	u32 feat_mask = ARM_SMMU_FEAT_COHERENCY;
>  
> -	if (vabits_actual == 52)
> +	if (vabits_actual == 52) {
> +		/* We don't support LPA2 */
> +		if (PAGE_SIZE != SZ_64K)
> +			return false;
>  		feat_mask |= ARM_SMMU_FEAT_VAX;
> +	}
>  
>  	if ((smmu->features & feat_mask) != feat_mask)
>  		return false;
> -- 
> 2.39.2.101.g768bb238c484.dirty
>