Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6cdbc7ec-0cf9-8854-d0e2-36aedda544e2@samsung.com>
Date:   Fri, 15 Jul 2022 09:28:21 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0)
        Gecko/20100101 Thunderbird/91.10.0
Subject: Re: [PATCH v3 5/6] iommu/exynos: Add SysMMU v7 register set
Content-Language: en-US
To:     Sam Protsenko <semen.protsenko@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Janghyuck Kim <janghyuck.kim@samsung.com>,
        Cho KyongHo <pullip.cho@samsung.com>,
        Daniel Mentz <danielmentz@google.com>,
        David Virag <virag.david003@gmail.com>,
        Sumit Semwal <sumit.semwal@linaro.org>, iommu@lists.linux.dev,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org
From:   Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <20220714165550.8884-6-semen.protsenko@linaro.org>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20220714165550.8884-1-semen.protsenko@linaro.org>
        <CGME20220714165604eucas1p28f7d938581361401c97135e2725f8c7e@eucas1p2.samsung.com>
        <20220714165550.8884-6-semen.protsenko@linaro.org>
Precedence: bulk

Hi Sam,

On 14.07.2022 18:55, Sam Protsenko wrote:
> SysMMU v7 might have different register layouts (VM capable or non-VM
> capable). Virtual Machine registers (if present) implement multiple
> translation domains. If VM registers are not present, the driver
> shouldn't try to access those.
>
> Check which layout is implemented in current SysMMU module (by reading
> the capability registers) and prepare the corresponding variant
> structure for further usage.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>

Looks fine.

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

What about the fault related code in the exynos_sysmmu_irq()? Afair v7 
has a bit different layout of the status bits.

> ---
> Changes in v3:
>    - Merged "Check if SysMMU v7 has VM registers" patch into this patch
>    - Reworked for using variant struct (instead of array)
>
> Changes in v2:
>    - (none) This patch is new and added in v2
>
>   drivers/iommu/exynos-iommu.c | 50 +++++++++++++++++++++++++++++++++---
>   1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 6a0299fe1722..fc9ef3ff0057 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>   #define CFG_SYSSEL	(1 << 22) /* System MMU 3.2 only */
>   #define CFG_FLPDCACHE	(1 << 20) /* System MMU 3.2+ only */
>   
> +#define CAPA0_CAPA1_EXIST		BIT(11)
> +#define CAPA1_VCR_ENABLED		BIT(14)
> +
>   /* common registers */
>   #define REG_MMU_CTRL		0x000
>   #define REG_MMU_CFG		0x004
> @@ -157,6 +160,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>   #define REG_V5_FAULT_AR_VA	0x070
>   #define REG_V5_FAULT_AW_VA	0x080
>   
> +/* v7.x registers */
> +#define REG_V7_CAPA0		0x870
> +#define REG_V7_CAPA1		0x874
> +
>   #define has_sysmmu(dev)		(dev_iommu_priv_get(dev) != NULL)
>   
>   static struct device *dma_dev;
> @@ -276,6 +283,9 @@ struct sysmmu_drvdata {
>   
>   	struct iommu_device iommu;	/* IOMMU core handle */
>   	const struct sysmmu_variant *variant; /* version specific data */
> +
> +	/* v7 fields */
> +	bool has_vcr;			/* virtual machine control register */
>   };
>   
>   #define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg)
> @@ -289,7 +299,7 @@ static const struct sysmmu_variant sysmmu_v1_variant = {
>   	.int_clear	= 0x1c,
>   };
>   
> -/* SysMMU v5 */
> +/* SysMMU v5 and v7 (non-VM capable) */
>   static const struct sysmmu_variant sysmmu_v5_variant = {
>   	.pt_base	= 0x0c,
>   	.flush_all	= 0x10,
> @@ -301,6 +311,18 @@ static const struct sysmmu_variant sysmmu_v5_variant = {
>   	.int_clear	= 0x64,
>   };
>   
> +/* SysMMU v7: VM capable register set */
> +static const struct sysmmu_variant sysmmu_v7_vm_variant = {
> +	.pt_base	= 0x800c,
> +	.flush_all	= 0x8010,
> +	.flush_entry	= 0x8014,
> +	.flush_range	= 0x8018,
> +	.flush_start	= 0x8020,
> +	.flush_end	= 0x8024,
> +	.int_status	= 0x60,
> +	.int_clear	= 0x64,
> +};
> +
>   static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
>   {
>   	return container_of(dom, struct exynos_iommu_domain, domain);
> @@ -380,6 +402,20 @@ static void __sysmmu_disable_clocks(struct sysmmu_drvdata *data)
>   	clk_disable_unprepare(data->clk_master);
>   }
>   
> +static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
> +{
> +	u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);
> +
> +	return capa0 & CAPA0_CAPA1_EXIST;
> +}
> +
> +static void __sysmmu_get_vcr(struct sysmmu_drvdata *data)
> +{
> +	u32 capa1 = readl(data->sfrbase + REG_V7_CAPA1);
> +
> +	data->has_vcr = capa1 & CAPA1_VCR_ENABLED;
> +}
> +
>   static void __sysmmu_get_version(struct sysmmu_drvdata *data)
>   {
>   	u32 ver;
> @@ -397,10 +433,18 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data)
>   	dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
>   		MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
>   
> -	if (MMU_MAJ_VER(data->version) < 5)
> +	if (MMU_MAJ_VER(data->version) < 5) {
>   		data->variant = &sysmmu_v1_variant;
> -	else
> +	} else if (MMU_MAJ_VER(data->version) < 7) {
>   		data->variant = &sysmmu_v5_variant;
> +	} else {
> +		if (__sysmmu_has_capa1(data))
> +			__sysmmu_get_vcr(data);
> +		if (data->has_vcr)
> +			data->variant = &sysmmu_v7_vm_variant;
> +		else
> +			data->variant = &sysmmu_v5_variant;
> +	}
>   
>   	__sysmmu_disable_clocks(data);
>   }

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland