* [PATCH 0/3] iommu/ipmmu-vmsa: minor updates
@ 2019-10-09 8:26 Yoshihiro Shimoda
2019-10-09 8:26 ` [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations Yoshihiro Shimoda
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Yoshihiro Shimoda @ 2019-10-09 8:26 UTC (permalink / raw)
To: joro; +Cc: iommu, linux-renesas-soc, 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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations 2019-10-09 8:26 [PATCH 0/3] iommu/ipmmu-vmsa: minor updates Yoshihiro Shimoda @ 2019-10-09 8:26 ` Yoshihiro Shimoda 2019-10-09 21:03 ` Niklas Söderlund 2019-10-11 12:10 ` Geert Uytterhoeven 2019-10-09 8:26 ` [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base Yoshihiro Shimoda 2 siblings, 2 replies; 13+ messages in thread From: Yoshihiro Shimoda @ 2019-10-09 8:26 UTC (permalink / raw) To: joro; +Cc: iommu, linux-renesas-soc, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations 2019-10-09 8:26 ` [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations Yoshihiro Shimoda @ 2019-10-09 21:03 ` Niklas Söderlund 2019-10-11 12:10 ` Geert Uytterhoeven 1 sibling, 0 replies; 13+ messages in thread From: Niklas Söderlund @ 2019-10-09 21:03 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: joro, iommu, linux-renesas-soc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations 2019-10-09 8:26 ` [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations Yoshihiro Shimoda 2019-10-09 21:03 ` Niklas Söderlund @ 2019-10-11 12:10 ` Geert Uytterhoeven 2019-10-15 5:17 ` Yoshihiro Shimoda 1 sibling, 1 reply; 13+ messages in thread From: Geert Uytterhoeven @ 2019-10-11 12:10 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: Joerg Roedel, Linux IOMMU, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations 2019-10-11 12:10 ` Geert Uytterhoeven @ 2019-10-15 5:17 ` Yoshihiro Shimoda 0 siblings, 0 replies; 13+ messages in thread From: Yoshihiro Shimoda @ 2019-10-15 5:17 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Joerg Roedel, Linux IOMMU, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro 2019-10-09 8:26 [PATCH 0/3] iommu/ipmmu-vmsa: minor updates Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations Yoshihiro Shimoda @ 2019-10-09 8:26 ` Yoshihiro Shimoda 2019-10-09 21:13 ` Niklas Söderlund 2019-10-11 12:29 ` Geert Uytterhoeven 2019-10-09 8:26 ` [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base Yoshihiro Shimoda 2 siblings, 2 replies; 13+ messages in thread From: Yoshihiro Shimoda @ 2019-10-09 8:26 UTC (permalink / raw) To: joro; +Cc: iommu, linux-renesas-soc, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro 2019-10-09 8:26 ` [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro Yoshihiro Shimoda @ 2019-10-09 21:13 ` Niklas Söderlund 2019-10-11 12:29 ` Geert Uytterhoeven 1 sibling, 0 replies; 13+ messages in thread From: Niklas Söderlund @ 2019-10-09 21:13 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: joro, iommu, linux-renesas-soc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro 2019-10-09 8:26 ` [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro Yoshihiro Shimoda 2019-10-09 21:13 ` Niklas Söderlund @ 2019-10-11 12:29 ` Geert Uytterhoeven 2019-10-15 5:28 ` Yoshihiro Shimoda 1 sibling, 1 reply; 13+ messages in thread From: Geert Uytterhoeven @ 2019-10-11 12:29 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: Joerg Roedel, Linux IOMMU, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro 2019-10-11 12:29 ` Geert Uytterhoeven @ 2019-10-15 5:28 ` Yoshihiro Shimoda 0 siblings, 0 replies; 13+ messages in thread From: Yoshihiro Shimoda @ 2019-10-15 5:28 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Joerg Roedel, Linux IOMMU, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base 2019-10-09 8:26 [PATCH 0/3] iommu/ipmmu-vmsa: minor updates Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro Yoshihiro Shimoda @ 2019-10-09 8:26 ` Yoshihiro Shimoda 2019-10-09 22:28 ` Niklas Söderlund 2019-10-11 12:32 ` Geert Uytterhoeven 2 siblings, 2 replies; 13+ messages in thread From: Yoshihiro Shimoda @ 2019-10-09 8:26 UTC (permalink / raw) To: joro; +Cc: iommu, linux-renesas-soc, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base 2019-10-09 8:26 ` [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base Yoshihiro Shimoda @ 2019-10-09 22:28 ` Niklas Söderlund 2019-10-11 12:32 ` Geert Uytterhoeven 1 sibling, 0 replies; 13+ messages in thread From: Niklas Söderlund @ 2019-10-09 22:28 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: joro, iommu, linux-renesas-soc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base 2019-10-09 8:26 ` [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base Yoshihiro Shimoda 2019-10-09 22:28 ` Niklas Söderlund @ 2019-10-11 12:32 ` Geert Uytterhoeven 2019-10-15 5:28 ` Yoshihiro Shimoda 1 sibling, 1 reply; 13+ messages in thread From: Geert Uytterhoeven @ 2019-10-11 12:32 UTC (permalink / raw) To: Yoshihiro Shimoda; +Cc: Joerg Roedel, Linux IOMMU, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base 2019-10-11 12:32 ` Geert Uytterhoeven @ 2019-10-15 5:28 ` Yoshihiro Shimoda 0 siblings, 0 replies; 13+ messages in thread From: Yoshihiro Shimoda @ 2019-10-15 5:28 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Joerg Roedel, Linux IOMMU, Linux-Renesas 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-15 5:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-09 8:26 [PATCH 0/3] iommu/ipmmu-vmsa: minor updates Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 1/3] iommu/ipmmu-vmsa: Remove some unused register declarations Yoshihiro Shimoda 2019-10-09 21:03 ` Niklas Söderlund 2019-10-11 12:10 ` Geert Uytterhoeven 2019-10-15 5:17 ` Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 2/3] iommu/ipmmu-vmsa: Calculate context registers' offset instead of a macro Yoshihiro Shimoda 2019-10-09 21:13 ` Niklas Söderlund 2019-10-11 12:29 ` Geert Uytterhoeven 2019-10-15 5:28 ` Yoshihiro Shimoda 2019-10-09 8:26 ` [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base Yoshihiro Shimoda 2019-10-09 22:28 ` Niklas Söderlund 2019-10-11 12:32 ` Geert Uytterhoeven 2019-10-15 5:28 ` Yoshihiro Shimoda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).