Re: [PATCH] iommu/io-pgtable-arm: Improve attribute handling

[Will Deacon <will@kernel.org>]

On Fri, Jan 10, 2020 at 03:21:51PM +0000, Robin Murphy wrote:
> By VMSA rules, using Normal Non-Cacheable type with a shareability
> attribute of anything other than Outer Shareable is liable to lead into
> unpredictable territory. Although the SMMU architectures seem to give
> some slightly stronger guarantees of Non-Cacheable output types becoming
> implicitly Outer Shareable in most cases, we may as well be explicit and
> not take any chances. It's also weird that LPAE attribute handling is
> currently split between prot_to_pte() and init_pte() given that it can
> all be statically determined up-front. Thus, collect *all* the LPAE
> attributes into prot_to_pte() in order to logically pick the
> shareability based on the incoming IOMMU API prot value, and tweak the
> short-descriptor code to stop setting TTBR0.NOS for Non-Cacheable walks.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/iommu/io-pgtable-arm-v7s.c |  7 +++----
>  drivers/iommu/io-pgtable-arm.c     | 17 +++++++++++------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index 7c3bd2c3cdca..7d6a8622f2e6 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -823,10 +823,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
>  	wmb();
>  
>  	/* TTBRs */
> -	cfg->arm_v7s_cfg.ttbr[0] = virt_to_phys(data->pgd) |
> -				   ARM_V7S_TTBR_S | ARM_V7S_TTBR_NOS |
> -				   (cfg->coherent_walk ?
> -				   (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |
> +	cfg->arm_v7s_cfg.ttbr[0] = virt_to_phys(data->pgd) | ARM_V7S_TTBR_S |
> +				   (cfg->coherent_walk ? (ARM_V7S_TTBR_NOS |
> +				    ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |

Ha, I just sent a broken version of this out myself, but with you as the
author ;)

I'll merge this hunk into that patch, if you don't mind?

> +	if (prot & IOMMU_CACHE)
> +		pte |= ARM_LPAE_PTE_SH_IS;
> +	else
> +		pte |= ARM_LPAE_PTE_SH_OS;
> +
>  	if (prot & IOMMU_NOEXEC)
>  		pte |= ARM_LPAE_PTE_XN;
>  
> +	if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
> +		pte |= ARM_LPAE_PTE_NS;
> +
> +	if (data->iop.fmt != ARM_MALI_LPAE)
> +		pte |= ARM_LPAE_PTE_AF;
> +

I left these last two where they were, just because they're not driven
directly from the IOMMU prot encoding. However, I don't mind moving them,
but let's do that as a separate patch.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel