On 02/07/2022 23:37, Sam Protsenko wrote:
> SysMMU v7 might have different register layouts (VM capable or non-VM
> capable). Check which layout is implemented in current SysMMU module and
> prepare the corresponding register table for futher usage. This way is
> faster and more elegant than checking corresponding condition (if it's
> VM or non-VM SysMMU) each time before accessing v7 registers. For now
> the register table contains only most basic registers needed to add the
> SysMMU v7 support.
> 
> This patch is based on downstream work of next authors:
>   - Janghyuck Kim <janghyuck....@samsung.com>
>   - Daniel Mentz <danielme...@google.com>
> 
> Signed-off-by: Sam Protsenko <semen.protse...@linaro.org>
> ---
>  drivers/iommu/exynos-iommu.c | 46 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index df6ddbebbe2b..47017e8945c5 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -180,6 +180,47 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>  
>  #define has_sysmmu(dev)              (dev_iommu_priv_get(dev) != NULL)
>  
> +#define MMU_REG(data, idx)           \
> +     ((data)->sfrbase + (data)->regs[idx].off)

I would expect to see users of this - convert all existing regs.

> +#define MMU_VM_REG(data, idx, vmid)  \
> +     (MMU_REG(data, idx) + (vmid) * (data)->regs[idx].mult)
> +
> +enum {
> +     REG_SET_NON_VM,
> +     REG_SET_VM,
> +     MAX_REG_SET
> +};
> +
> +enum {
> +     IDX_CTRL_VM,
> +     IDX_CFG_VM,
> +     IDX_FLPT_BASE,

Isn't this called REG_MMU_FLUSH? If yes, it's a bit confusing to see
different name used.

> +     IDX_ALL_INV,

Isn't this called REG_MMU_FLUSH_ENTRY?

> +     MAX_REG_IDX
> +};
> +
> +struct sysmmu_vm_reg {
> +     unsigned int off;       /* register offset */
> +     unsigned int mult;      /* VM index offset multiplier */
> +};
> +
> +static const struct sysmmu_vm_reg sysmmu_regs[MAX_REG_SET][MAX_REG_IDX] = {
> +     /* Default register set (non-VM) */
> +     {
> +             /*
> +              * SysMMUs without VM support do not have CTRL_VM and CFG_VM
> +              * registers. Setting the offsets to 1 will trigger an unaligned
> +              * access exception.

So why are you setting offset 1? To trigger unaligned access?

> +              */
> +             {0x1}, {0x1}, {0x000c}, {0x0010},
> +     },
> +     /* VM capable register set */
> +     {
> +             {0x8000, 0x1000}, {0x8004, 0x1000}, {0x800c, 0x1000},
> +             {0x8010, 0x1000},
> +     },
You add here quite a bit of dead code and some hard-coded numbers.

> +};
> +
>  static struct device *dma_dev;
>  static struct kmem_cache *lv2table_kmem_cache;
>  static sysmmu_pte_t *zero_lv2_table;
> @@ -284,6 +325,7 @@ struct sysmmu_drvdata {
>  
>       /* v7 fields */
>       bool has_vcr;                   /* virtual machine control register */
> +     const struct sysmmu_vm_reg *regs; /* register set */
>  };
>  
>  static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> @@ -407,6 +449,10 @@ static void sysmmu_get_hw_info(struct sysmmu_drvdata 
> *data)
>       __sysmmu_get_version(data);
>       if (MMU_MAJ_VER(data->version) >= 7 && __sysmmu_has_capa1(data))
>               __sysmmu_get_vcr(data);
> +     if (data->has_vcr)
> +             data->regs = sysmmu_regs[REG_SET_VM];
> +     else
> +             data->regs = sysmmu_regs[REG_SET_NON_VM];

This is set and not read.

>  
>       __sysmmu_disable_clocks(data);
>  }


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

Reply via email to