[PATCH 0/3] iommu/ipmmu-vmsa: minor updates

[PATCH 0/3] iommu/ipmmu-vmsa: minor updates

[Yoshihiro Shimoda]

This patch series is based on the latest iommu.git / next branch
to modify the driver in the future's new hardware.

Yoshihiro Shimoda (3):
  iommu/ipmmu-vmsa: Remove some unused register declarations
  iommu/ipmmu-vmsa: Calculate context registers' offset instead of a
    macro
  iommu/ipmmu-vmsa: Add utlb_offset_base

 drivers/iommu/ipmmu-vmsa.c | 52 ++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

-- 
2.7.4

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

[PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations

[Yoshihiro Shimoda]

To support different registers memory mapping hardware easily
in the future, this patch removes some unused register
declarations.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/iommu/ipmmu-vmsa.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 9da8309..dd554c2 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -104,8 +104,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 #define IMCTR				0x0000
 #define IMCTR_TRE			(1 << 17)
 #define IMCTR_AFE			(1 << 16)
-#define IMCTR_RTSEL_MASK		(3 << 4)
-#define IMCTR_RTSEL_SHIFT		4
 #define IMCTR_TREN			(1 << 3)
 #define IMCTR_INTEN			(1 << 2)
 #define IMCTR_FLUSH			(1 << 1)
@@ -115,7 +113,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 
 #define IMTTBCR				0x0008
 #define IMTTBCR_EAE			(1 << 31)
-#define IMTTBCR_PMB			(1 << 30)
 #define IMTTBCR_SH1_NON_SHAREABLE	(0 << 28)	/* R-Car Gen2 only */
 #define IMTTBCR_SH1_OUTER_SHAREABLE	(2 << 28)	/* R-Car Gen2 only */
 #define IMTTBCR_SH1_INNER_SHAREABLE	(3 << 28)	/* R-Car Gen2 only */
@@ -193,12 +190,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 #define IMELAR				0x0030	/* IMEAR on R-Car Gen2 */
 #define IMEUAR				0x0034	/* R-Car Gen3 only */
 
-#define IMPCTR				0x0200
-#define IMPSTR				0x0208
-#define IMPEAR				0x020c
-#define IMPMBA(n)			(0x0280 + ((n) * 4))
-#define IMPMBD(n)			(0x02c0 + ((n) * 4))
-
 #define IMUCTR(n)			((n) < 32 ? IMUCTR0(n) : IMUCTR32(n))
 #define IMUCTR0(n)			(0x0300 + ((n) * 16))
 #define IMUCTR32(n)			(0x0600 + (((n) - 32) * 16))
@@ -206,8 +197,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 #define IMUCTR_FIXADD_MASK		(0xff << 16)
 #define IMUCTR_FIXADD_SHIFT		16
 #define IMUCTR_TTSEL_MMU(n)		((n) << 4)
-#define IMUCTR_TTSEL_PMB		(8 << 4)
-#define IMUCTR_TTSEL_MASK		(15 << 4)
 #define IMUCTR_FLUSH			(1 << 1)
 #define IMUCTR_MMUEN			(1 << 0)
 
-- 
2.7.4

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

[PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro

[Yoshihiro Shimoda]

Since we will have changed memory mapping of the IPMMU in the future,
this patch uses ipmmu_features values instead of a macro to
calculate context registers offset. No behavior change.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/iommu/ipmmu-vmsa.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index dd554c2..76fb250 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -50,6 +50,8 @@ struct ipmmu_features {
 	bool twobit_imttbcr_sl0;
 	bool reserved_context;
 	bool cache_snoop;
+	u32 ctx_offset_base;
+	u32 ctx_offset_stride;
 };
 
 struct ipmmu_vmsa_device {
@@ -99,8 +101,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
 
 #define IM_NS_ALIAS_OFFSET		0x800
 
-#define IM_CTX_SIZE			0x40
-
 #define IMCTR				0x0000
 #define IMCTR_TRE			(1 << 17)
 #define IMCTR_AFE			(1 << 16)
@@ -253,18 +253,25 @@ static void ipmmu_write(struct ipmmu_vmsa_device *mmu, unsigned int offset,
 	iowrite32(data, mmu->base + offset);
 }
 
+static u32 ipmmu_ctx_reg(struct ipmmu_vmsa_device *mmu, unsigned int context_id,
+			 unsigned int reg)
+{
+	return mmu->features->ctx_offset_base +
+	       context_id * mmu->features->ctx_offset_stride + reg;
+}
+
 static u32 ipmmu_ctx_read_root(struct ipmmu_vmsa_domain *domain,
 			       unsigned int reg)
 {
 	return ipmmu_read(domain->mmu->root,
-			  domain->context_id * IM_CTX_SIZE + reg);
+			  ipmmu_ctx_reg(domain->mmu, domain->context_id, reg));
 }
 
 static void ipmmu_ctx_write_root(struct ipmmu_vmsa_domain *domain,
 				 unsigned int reg, u32 data)
 {
 	ipmmu_write(domain->mmu->root,
-		    domain->context_id * IM_CTX_SIZE + reg, data);
+		    ipmmu_ctx_reg(domain->mmu, domain->context_id, reg), data);
 }
 
 static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
@@ -272,10 +279,10 @@ static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
 {
 	if (domain->mmu != domain->mmu->root)
 		ipmmu_write(domain->mmu,
-			    domain->context_id * IM_CTX_SIZE + reg, data);
+			    ipmmu_ctx_reg(domain->mmu, domain->context_id, reg),
+			    data);
 
-	ipmmu_write(domain->mmu->root,
-		    domain->context_id * IM_CTX_SIZE + reg, data);
+	ipmmu_ctx_write_root(domain, reg, data);
 }
 
 /* -----------------------------------------------------------------------------
@@ -974,7 +981,7 @@ static void ipmmu_device_reset(struct ipmmu_vmsa_device *mmu)
 
 	/* Disable all contexts. */
 	for (i = 0; i < mmu->num_ctx; ++i)
-		ipmmu_write(mmu, i * IM_CTX_SIZE + IMCTR, 0);
+		ipmmu_write(mmu, ipmmu_ctx_reg(mmu, i, IMCTR), 0);
 }
 
 static const struct ipmmu_features ipmmu_features_default = {
@@ -986,6 +993,8 @@ static const struct ipmmu_features ipmmu_features_default = {
 	.twobit_imttbcr_sl0 = false,
 	.reserved_context = false,
 	.cache_snoop = true,
+	.ctx_offset_base = 0,
+	.ctx_offset_stride = 0x40,
 };
 
 static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
@@ -997,6 +1006,8 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
 	.twobit_imttbcr_sl0 = true,
 	.reserved_context = true,
 	.cache_snoop = false,
+	.ctx_offset_base = 0,
+	.ctx_offset_stride = 0x40,
 };
 
 static const struct of_device_id ipmmu_of_ids[] = {
-- 
2.7.4

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

[PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base

[Yoshihiro Shimoda]

Since we will have changed memory mapping of the IPMMU in the future,
this patch adds a utlb_offset_base into struct ipmmu_features
for IMUCTR and IMUASID registers.
No behavior change.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/iommu/ipmmu-vmsa.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 76fb250..bc00e58 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -52,6 +52,7 @@ struct ipmmu_features {
 	bool cache_snoop;
 	u32 ctx_offset_base;
 	u32 ctx_offset_stride;
+	u32 utlb_offset_base;
 };
 
 struct ipmmu_vmsa_device {
@@ -285,6 +286,11 @@ static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
 	ipmmu_ctx_write_root(domain, reg, data);
 }
 
+static u32 ipmmu_utlb_reg(struct ipmmu_vmsa_device *mmu, unsigned int reg)
+{
+	return mmu->features->utlb_offset_base + reg;
+}
+
 /* -----------------------------------------------------------------------------
  * TLB and microTLB Management
  */
@@ -330,9 +336,9 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
 	 */
 
 	/* TODO: What should we set the ASID to ? */
-	ipmmu_write(mmu, IMUASID(utlb), 0);
+	ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUASID(utlb)), 0);
 	/* TODO: Do we need to flush the microTLB ? */
-	ipmmu_write(mmu, IMUCTR(utlb),
+	ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUCTR(utlb)),
 		    IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
 		    IMUCTR_MMUEN);
 	mmu->utlb_ctx[utlb] = domain->context_id;
@@ -346,7 +352,7 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
-	ipmmu_write(mmu, IMUCTR(utlb), 0);
+	ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUCTR(utlb)), 0);
 	mmu->utlb_ctx[utlb] = IPMMU_CTX_INVALID;
 }
 
@@ -995,6 +1001,7 @@ static const struct ipmmu_features ipmmu_features_default = {
 	.cache_snoop = true,
 	.ctx_offset_base = 0,
 	.ctx_offset_stride = 0x40,
+	.utlb_offset_base = 0,
 };
 
 static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
@@ -1008,6 +1015,7 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
 	.cache_snoop = false,
 	.ctx_offset_base = 0,
 	.ctx_offset_stride = 0x40,
+	.utlb_offset_base = 0,
 };
 
 static const struct of_device_id ipmmu_of_ids[] = {
-- 
2.7.4

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

Re: [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations

[Niklas Söderlund]

Hi Shimoda-san,

Thanks for your patch.

On 2019-10-09 17:26:47 +0900, Yoshihiro Shimoda wrote:
> To support different registers memory mapping hardware easily
> in the future, this patch removes some unused register
> declarations.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/iommu/ipmmu-vmsa.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 9da8309..dd554c2 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -104,8 +104,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>  #define IMCTR				0x0000
>  #define IMCTR_TRE			(1 << 17)
>  #define IMCTR_AFE			(1 << 16)
> -#define IMCTR_RTSEL_MASK		(3 << 4)
> -#define IMCTR_RTSEL_SHIFT		4
>  #define IMCTR_TREN			(1 << 3)
>  #define IMCTR_INTEN			(1 << 2)
>  #define IMCTR_FLUSH			(1 << 1)
> @@ -115,7 +113,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>  
>  #define IMTTBCR				0x0008
>  #define IMTTBCR_EAE			(1 << 31)
> -#define IMTTBCR_PMB			(1 << 30)
>  #define IMTTBCR_SH1_NON_SHAREABLE	(0 << 28)	/* R-Car Gen2 only */
>  #define IMTTBCR_SH1_OUTER_SHAREABLE	(2 << 28)	/* R-Car Gen2 only */
>  #define IMTTBCR_SH1_INNER_SHAREABLE	(3 << 28)	/* R-Car Gen2 only */
> @@ -193,12 +190,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>  #define IMELAR				0x0030	/* IMEAR on R-Car Gen2 */
>  #define IMEUAR				0x0034	/* R-Car Gen3 only */
>  
> -#define IMPCTR				0x0200
> -#define IMPSTR				0x0208
> -#define IMPEAR				0x020c
> -#define IMPMBA(n)			(0x0280 + ((n) * 4))
> -#define IMPMBD(n)			(0x02c0 + ((n) * 4))
> -
>  #define IMUCTR(n)			((n) < 32 ? IMUCTR0(n) : IMUCTR32(n))
>  #define IMUCTR0(n)			(0x0300 + ((n) * 16))
>  #define IMUCTR32(n)			(0x0600 + (((n) - 32) * 16))
> @@ -206,8 +197,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>  #define IMUCTR_FIXADD_MASK		(0xff << 16)
>  #define IMUCTR_FIXADD_SHIFT		16
>  #define IMUCTR_TTSEL_MMU(n)		((n) << 4)
> -#define IMUCTR_TTSEL_PMB		(8 << 4)
> -#define IMUCTR_TTSEL_MASK		(15 << 4)
>  #define IMUCTR_FLUSH			(1 << 1)
>  #define IMUCTR_MMUEN			(1 << 0)
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro

[Niklas Söderlund]

Hi Shimoda-san,

Thanks for your patch.

On 2019-10-09 17:26:48 +0900, Yoshihiro Shimoda wrote:
> Since we will have changed memory mapping of the IPMMU in the future,
> this patch uses ipmmu_features values instead of a macro to
> calculate context registers offset. No behavior change.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/iommu/ipmmu-vmsa.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index dd554c2..76fb250 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -50,6 +50,8 @@ struct ipmmu_features {
>  	bool twobit_imttbcr_sl0;
>  	bool reserved_context;
>  	bool cache_snoop;
> +	u32 ctx_offset_base;
> +	u32 ctx_offset_stride;
>  };
>  
>  struct ipmmu_vmsa_device {
> @@ -99,8 +101,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>  
>  #define IM_NS_ALIAS_OFFSET		0x800
>  
> -#define IM_CTX_SIZE			0x40
> -
>  #define IMCTR				0x0000
>  #define IMCTR_TRE			(1 << 17)
>  #define IMCTR_AFE			(1 << 16)
> @@ -253,18 +253,25 @@ static void ipmmu_write(struct ipmmu_vmsa_device *mmu, unsigned int offset,
>  	iowrite32(data, mmu->base + offset);
>  }
>  
> +static u32 ipmmu_ctx_reg(struct ipmmu_vmsa_device *mmu, unsigned int context_id,
> +			 unsigned int reg)
> +{
> +	return mmu->features->ctx_offset_base +
> +	       context_id * mmu->features->ctx_offset_stride + reg;
> +}
> +
>  static u32 ipmmu_ctx_read_root(struct ipmmu_vmsa_domain *domain,
>  			       unsigned int reg)
>  {
>  	return ipmmu_read(domain->mmu->root,
> -			  domain->context_id * IM_CTX_SIZE + reg);
> +			  ipmmu_ctx_reg(domain->mmu, domain->context_id, reg));
>  }
>  
>  static void ipmmu_ctx_write_root(struct ipmmu_vmsa_domain *domain,
>  				 unsigned int reg, u32 data)
>  {
>  	ipmmu_write(domain->mmu->root,
> -		    domain->context_id * IM_CTX_SIZE + reg, data);
> +		    ipmmu_ctx_reg(domain->mmu, domain->context_id, reg), data);
>  }
>  
>  static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
> @@ -272,10 +279,10 @@ static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
>  {
>  	if (domain->mmu != domain->mmu->root)
>  		ipmmu_write(domain->mmu,
> -			    domain->context_id * IM_CTX_SIZE + reg, data);
> +			    ipmmu_ctx_reg(domain->mmu, domain->context_id, reg),
> +			    data);
>  
> -	ipmmu_write(domain->mmu->root,
> -		    domain->context_id * IM_CTX_SIZE + reg, data);
> +	ipmmu_ctx_write_root(domain, reg, data);
>  }
>  
>  /* -----------------------------------------------------------------------------
> @@ -974,7 +981,7 @@ static void ipmmu_device_reset(struct ipmmu_vmsa_device *mmu)
>  
>  	/* Disable all contexts. */
>  	for (i = 0; i < mmu->num_ctx; ++i)
> -		ipmmu_write(mmu, i * IM_CTX_SIZE + IMCTR, 0);
> +		ipmmu_write(mmu, ipmmu_ctx_reg(mmu, i, IMCTR), 0);
>  }
>  
>  static const struct ipmmu_features ipmmu_features_default = {
> @@ -986,6 +993,8 @@ static const struct ipmmu_features ipmmu_features_default = {
>  	.twobit_imttbcr_sl0 = false,
>  	.reserved_context = false,
>  	.cache_snoop = true,
> +	.ctx_offset_base = 0,
> +	.ctx_offset_stride = 0x40,
>  };
>  
>  static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
> @@ -997,6 +1006,8 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
>  	.twobit_imttbcr_sl0 = true,
>  	.reserved_context = true,
>  	.cache_snoop = false,
> +	.ctx_offset_base = 0,
> +	.ctx_offset_stride = 0x40,
>  };
>  
>  static const struct of_device_id ipmmu_of_ids[] = {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base

[Niklas Söderlund]

Hi Shimoda-san,

Thanks for your patch.

On 2019-10-09 17:26:49 +0900, Yoshihiro Shimoda wrote:
> Since we will have changed memory mapping of the IPMMU in the future,
> this patch adds a utlb_offset_base into struct ipmmu_features
> for IMUCTR and IMUASID registers.
> No behavior change.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/iommu/ipmmu-vmsa.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 76fb250..bc00e58 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -52,6 +52,7 @@ struct ipmmu_features {
>  	bool cache_snoop;
>  	u32 ctx_offset_base;
>  	u32 ctx_offset_stride;
> +	u32 utlb_offset_base;
>  };
>  
>  struct ipmmu_vmsa_device {
> @@ -285,6 +286,11 @@ static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
>  	ipmmu_ctx_write_root(domain, reg, data);
>  }
>  
> +static u32 ipmmu_utlb_reg(struct ipmmu_vmsa_device *mmu, unsigned int reg)
> +{
> +	return mmu->features->utlb_offset_base + reg;
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * TLB and microTLB Management
>   */
> @@ -330,9 +336,9 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
>  	 */
>  
>  	/* TODO: What should we set the ASID to ? */
> -	ipmmu_write(mmu, IMUASID(utlb), 0);
> +	ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUASID(utlb)), 0);
>  	/* TODO: Do we need to flush the microTLB ? */
> -	ipmmu_write(mmu, IMUCTR(utlb),
> +	ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUCTR(utlb)),
>  		    IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
>  		    IMUCTR_MMUEN);
>  	mmu->utlb_ctx[utlb] = domain->context_id;
> @@ -346,7 +352,7 @@ static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
>  {
>  	struct ipmmu_vmsa_device *mmu = domain->mmu;
>  
> -	ipmmu_write(mmu, IMUCTR(utlb), 0);
> +	ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUCTR(utlb)), 0);
>  	mmu->utlb_ctx[utlb] = IPMMU_CTX_INVALID;
>  }
>  
> @@ -995,6 +1001,7 @@ static const struct ipmmu_features ipmmu_features_default = {
>  	.cache_snoop = true,
>  	.ctx_offset_base = 0,
>  	.ctx_offset_stride = 0x40,
> +	.utlb_offset_base = 0,
>  };
>  
>  static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
> @@ -1008,6 +1015,7 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = {
>  	.cache_snoop = false,
>  	.ctx_offset_base = 0,
>  	.ctx_offset_stride = 0x40,
> +	.utlb_offset_base = 0,
>  };
>  
>  static const struct of_device_id ipmmu_of_ids[] = {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations

[Geert Uytterhoeven]

Hi Shimoda-san,

Thanks for your patch!

On Wed, Oct 9, 2019 at 10:27 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> To support different registers memory mapping hardware easily
> in the future, this patch removes some unused register
> declarations.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

While I can confirm the removed definitions are unused, they were
still valid (but see comments below).
Perhaps it would be better to add comments, to state clearly to which
SoCs or SoC families they apply?  Or do you think this would be futile,
and would add too much clutter to the source file in the near future?

> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -104,8 +104,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>  #define IMCTR                          0x0000
>  #define IMCTR_TRE                      (1 << 17)
>  #define IMCTR_AFE                      (1 << 16)
> -#define IMCTR_RTSEL_MASK               (3 << 4)

FWIW, this is valid for R-Car Gen2 only.  On R-Car Gen3, the field
contains 3 bits.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Oct 9, 2019 at 10:27 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since we will have changed memory mapping of the IPMMU in the future,
> this patch uses ipmmu_features values instead of a macro to
> calculate context registers offset. No behavior change.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -50,6 +50,8 @@ struct ipmmu_features {
>         bool twobit_imttbcr_sl0;
>         bool reserved_context;
>         bool cache_snoop;
> +       u32 ctx_offset_base;
> +       u32 ctx_offset_stride;
>  };
>
>  struct ipmmu_vmsa_device {
> @@ -99,8 +101,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
>
>  #define IM_NS_ALIAS_OFFSET             0x800
>
> -#define IM_CTX_SIZE                    0x40
> -
>  #define IMCTR                          0x0000
>  #define IMCTR_TRE                      (1 << 17)
>  #define IMCTR_AFE                      (1 << 16)
> @@ -253,18 +253,25 @@ static void ipmmu_write(struct ipmmu_vmsa_device *mmu, unsigned int offset,
>         iowrite32(data, mmu->base + offset);
>  }
>
> +static u32 ipmmu_ctx_reg(struct ipmmu_vmsa_device *mmu, unsigned int context_id,
> +                        unsigned int reg)
> +{
> +       return mmu->features->ctx_offset_base +
> +              context_id * mmu->features->ctx_offset_stride + reg;
> +}
> +
>  static u32 ipmmu_ctx_read_root(struct ipmmu_vmsa_domain *domain,
>                                unsigned int reg)
>  {
>         return ipmmu_read(domain->mmu->root,
> -                         domain->context_id * IM_CTX_SIZE + reg);
> +                         ipmmu_ctx_reg(domain->mmu, domain->context_id, reg));

For consistency:

    ipmmu_ctx_reg(domain->mmu->root, ...)

but in practice the features for domain->mmu and domain->mmu->root are
identical anyway.

>  }
>
>  static void ipmmu_ctx_write_root(struct ipmmu_vmsa_domain *domain,
>                                  unsigned int reg, u32 data)
>  {
>         ipmmu_write(domain->mmu->root,
> -                   domain->context_id * IM_CTX_SIZE + reg, data);
> +                   ipmmu_ctx_reg(domain->mmu, domain->context_id, reg), data);

Likewise:

    ipmmu_ctx_reg(domain->mmu->root, ...)?

I find these ipmmu_{read,write}() a bit hard too read, with passing the
mmu to both ipmmu_{read,write}() and ipmmu_ctx_reg().

What do you think about providing two helpers ipmmu_ctx_{read,write}(),
so all users can just use e.g.

    ipmmu_ctx_write(mmu, context_id, reg, data);

instead of

    ipmmu_write(mmu, ipmmu_ctx_reg(mmu, context_id, reg), data);

?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Oct 9, 2019 at 10:27 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since we will have changed memory mapping of the IPMMU in the future,
> this patch adds a utlb_offset_base into struct ipmmu_features
> for IMUCTR and IMUASID registers.
> No behavior change.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -52,6 +52,7 @@ struct ipmmu_features {
>         bool cache_snoop;
>         u32 ctx_offset_base;
>         u32 ctx_offset_stride;
> +       u32 utlb_offset_base;
>  };
>
>  struct ipmmu_vmsa_device {
> @@ -285,6 +286,11 @@ static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
>         ipmmu_ctx_write_root(domain, reg, data);
>  }
>
> +static u32 ipmmu_utlb_reg(struct ipmmu_vmsa_device *mmu, unsigned int reg)
> +{
> +       return mmu->features->utlb_offset_base + reg;
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * TLB and microTLB Management
>   */
> @@ -330,9 +336,9 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
>          */
>
>         /* TODO: What should we set the ASID to ? */
> -       ipmmu_write(mmu, IMUASID(utlb), 0);
> +       ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUASID(utlb)), 0);
>         /* TODO: Do we need to flush the microTLB ? */
> -       ipmmu_write(mmu, IMUCTR(utlb),
> +       ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUCTR(utlb)),
>                     IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
>                     IMUCTR_MMUEN);

Like in [PATCH 2/3], I think providing two helpers would make this more
readable:

    ipmmu_imuasid_write(mmu, utlb, 0);
    ipmmu_imuctr_write(mmu, utlb, data);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, October 11, 2019 9:11 PM
> 
> Hi Shimoda-san,
> 
> Thanks for your patch!
> 
> On Wed, Oct 9, 2019 at 10:27 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > To support different registers memory mapping hardware easily
> > in the future, this patch removes some unused register
> > declarations.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thank you for your review!

> While I can confirm the removed definitions are unused, they were
> still valid (but see comments below).
> Perhaps it would be better to add comments, to state clearly to which
> SoCs or SoC families they apply?  Or do you think this would be futile,
> and would add too much clutter to the source file in the near future?

I think adding comments to the declarations are better to avoid
incorrect implementation in the future. So, I'll make such an incremental patch.

> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -104,8 +104,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
> >  #define IMCTR                          0x0000
> >  #define IMCTR_TRE                      (1 << 17)
> >  #define IMCTR_AFE                      (1 << 16)
> > -#define IMCTR_RTSEL_MASK               (3 << 4)
> 
> FWIW, this is valid for R-Car Gen2 only.  On R-Car Gen3, the field
> contains 3 bits.

That's correct.

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, October 11, 2019 9:29 PM
> 
> Hi Shimoda-san,
> 
> On Wed, Oct 9, 2019 at 10:27 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Since we will have changed memory mapping of the IPMMU in the future,
> > this patch uses ipmmu_features values instead of a macro to
> > calculate context registers offset. No behavior change.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -50,6 +50,8 @@ struct ipmmu_features {
> >         bool twobit_imttbcr_sl0;
> >         bool reserved_context;
> >         bool cache_snoop;
> > +       u32 ctx_offset_base;
> > +       u32 ctx_offset_stride;
> >  };
> >
> >  struct ipmmu_vmsa_device {
> > @@ -99,8 +101,6 @@ static struct ipmmu_vmsa_device *to_ipmmu(struct device *dev)
> >
> >  #define IM_NS_ALIAS_OFFSET             0x800
> >
> > -#define IM_CTX_SIZE                    0x40
> > -
> >  #define IMCTR                          0x0000
> >  #define IMCTR_TRE                      (1 << 17)
> >  #define IMCTR_AFE                      (1 << 16)
> > @@ -253,18 +253,25 @@ static void ipmmu_write(struct ipmmu_vmsa_device *mmu, unsigned int offset,
> >         iowrite32(data, mmu->base + offset);
> >  }
> >
> > +static u32 ipmmu_ctx_reg(struct ipmmu_vmsa_device *mmu, unsigned int context_id,
> > +                        unsigned int reg)
> > +{
> > +       return mmu->features->ctx_offset_base +
> > +              context_id * mmu->features->ctx_offset_stride + reg;
> > +}
> > +
> >  static u32 ipmmu_ctx_read_root(struct ipmmu_vmsa_domain *domain,
> >                                unsigned int reg)
> >  {
> >         return ipmmu_read(domain->mmu->root,
> > -                         domain->context_id * IM_CTX_SIZE + reg);
> > +                         ipmmu_ctx_reg(domain->mmu, domain->context_id, reg));
> 
> For consistency:
> 
>     ipmmu_ctx_reg(domain->mmu->root, ...)
> 
> but in practice the features for domain->mmu and domain->mmu->root are
> identical anyway.
> 
> >  }
> >
> >  static void ipmmu_ctx_write_root(struct ipmmu_vmsa_domain *domain,
> >                                  unsigned int reg, u32 data)
> >  {
> >         ipmmu_write(domain->mmu->root,
> > -                   domain->context_id * IM_CTX_SIZE + reg, data);
> > +                   ipmmu_ctx_reg(domain->mmu, domain->context_id, reg), data);
> 
> Likewise:
> 
>     ipmmu_ctx_reg(domain->mmu->root, ...)?

Thank you for the comments! Yes, we can use domain->mmu->root to ipmmu_ctx_reg()
because ipmmu_ctx_reg() only use mmu->features.

> I find these ipmmu_{read,write}() a bit hard too read, with passing the
> mmu to both ipmmu_{read,write}() and ipmmu_ctx_reg().

I completely agree.

> What do you think about providing two helpers ipmmu_ctx_{read,write}(),
> so all users can just use e.g.
> 
>     ipmmu_ctx_write(mmu, context_id, reg, data);
> 
> instead of
> 
>     ipmmu_write(mmu, ipmmu_ctx_reg(mmu, context_id, reg), data);
> 
> ?

I think so. I'll fix it. Perhaps, I'll make a patch which changes
the function name at first.

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, October 11, 2019 9:32 PM
> 
> Hi Shimoda-san,
> 
> On Wed, Oct 9, 2019 at 10:27 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Since we will have changed memory mapping of the IPMMU in the future,
> > this patch adds a utlb_offset_base into struct ipmmu_features
> > for IMUCTR and IMUASID registers.
> > No behavior change.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -52,6 +52,7 @@ struct ipmmu_features {
> >         bool cache_snoop;
> >         u32 ctx_offset_base;
> >         u32 ctx_offset_stride;
> > +       u32 utlb_offset_base;
> >  };
> >
> >  struct ipmmu_vmsa_device {
> > @@ -285,6 +286,11 @@ static void ipmmu_ctx_write_all(struct ipmmu_vmsa_domain *domain,
> >         ipmmu_ctx_write_root(domain, reg, data);
> >  }
> >
> > +static u32 ipmmu_utlb_reg(struct ipmmu_vmsa_device *mmu, unsigned int reg)
> > +{
> > +       return mmu->features->utlb_offset_base + reg;
> > +}
> > +
> >  /* -----------------------------------------------------------------------------
> >   * TLB and microTLB Management
> >   */
> > @@ -330,9 +336,9 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
> >          */
> >
> >         /* TODO: What should we set the ASID to ? */
> > -       ipmmu_write(mmu, IMUASID(utlb), 0);
> > +       ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUASID(utlb)), 0);
> >         /* TODO: Do we need to flush the microTLB ? */
> > -       ipmmu_write(mmu, IMUCTR(utlb),
> > +       ipmmu_write(mmu, ipmmu_utlb_reg(mmu, IMUCTR(utlb)),
> >                     IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
> >                     IMUCTR_MMUEN);
> 
> Like in [PATCH 2/3], I think providing two helpers would make this more
> readable:
> 
>     ipmmu_imuasid_write(mmu, utlb, 0);
>     ipmmu_imuctr_write(mmu, utlb, data);

I agree. I'll fix it.

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu