Re: [PATCH 6/6] iommu/arm-smmu: Support buggy implemenations where all config accesses are secure

[Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 10/01/2013 07:39 AM, Andreas Herrmann wrote:
> In such a case we have to use secure aliases of some non-secure
> registers.
> 
> This behaviour is controlled via a flag in smmu->bugs. It is set
> based on DT information when probing an SMMU device.

Perhaps quirks would be a nicer word...

> 
> Signed-off-by: Andreas Herrmann <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu.c |   33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index b632bcd..46f2b78 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -60,6 +60,15 @@
>  #define ARM_SMMU_GR0(smmu)		((smmu)->base)
>  #define ARM_SMMU_GR1(smmu)		((smmu)->base + (smmu)->pagesize)
>  
> +/*
> + * SMMU global address space with conditional offset to access secure aliases of
> + * non-secure registers (e.g. nsCR0: 0x400, nsGFSR: 0x448, nsGFSYNR0: 0x450)
> + */
> +#define ARM_SMMU_GR0_NS(smmu)						\
> +	((smmu)->base +							\
> +		((smmu->bugs & ARM_SMMU_BUG_SECURE_CFG_ACCESS) ? 0x400 : 0))
> +
> +
>  /* Page table bits */
>  #define ARM_SMMU_PTE_PAGE		(((pteval_t)3) << 0)
>  #define ARM_SMMU_PTE_CONT		(((pteval_t)1) << 52)
> @@ -348,6 +357,8 @@ struct arm_smmu_device {
>  #define ARM_SMMU_FEAT_TRANS_S2		(1 << 3)
>  #define ARM_SMMU_FEAT_TRANS_NESTED	(1 << 4)
>  	u32				features;
> +#define ARM_SMMU_BUG_SECURE_CFG_ACCESS	(1 << 0)
> +	u32				bugs;
>  	int				version;
>  
>  	u32				num_context_banks;
> @@ -611,16 +622,16 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
>  {
>  	u32 gfsr, gfsynr0, gfsynr1, gfsynr2;
>  	struct arm_smmu_device *smmu = dev;
> -	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> +	void __iomem *gr0_base = ARM_SMMU_GR0_NS(smmu);
>  
>  	gfsr = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSR);
> -	if (!gfsr)
> -		return IRQ_NONE;
> -
>  	gfsynr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR0);
>  	gfsynr1 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR1);
>  	gfsynr2 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR2);
>  
> +	if (!gfsr)
> +		return IRQ_NONE;
> +
>  	dev_err_ratelimited(smmu->dev,
>  		"Unexpected global fault, this could be serious\n");
>  	dev_err_ratelimited(smmu->dev,
> @@ -1567,8 +1578,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
>  	u32 reg;
>  
>  	/* clear global FSR */
> -	reg = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSR);
> -	writel(reg, gr0_base + ARM_SMMU_GR0_sGFSR);
> +	reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR);
> +	writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sGFSR);
>  
>  	/* Mark all SMRn as invalid and all S2CRn as bypass */
>  	for (i = 0; i < smmu->num_mapping_groups; ++i) {
> @@ -1588,7 +1599,7 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
>  	writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH);
>  	writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH);
>  
> -	reg = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
> +	reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>  
>  	/* Enable fault reporting */
>  	reg |= (sCR0_GFRE | sCR0_GFIE | sCR0_GCFGFRE | sCR0_GCFGFIE);
> @@ -1607,7 +1618,7 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
>  
>  	/* Push the button */
>  	arm_smmu_tlb_sync(smmu);
> -	writel(reg, gr0_base + ARM_SMMU_GR0_sCR0);
> +	writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>  }
>  
>  static int arm_smmu_id_size_to_bits(int size)
> @@ -1881,6 +1892,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	if (of_property_read_bool(dev->of_node, "calxeda,smmu-secure-cfg-access"))

This needs to be documented.

Rob