| Message ID | 20210901102705.556093-3-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | iommu/ipmmu-vmsa: Add support for r8a779a0 | expand |
Hi Shimoda-san, On Wed, Sep 1, 2021 at 12:27 PM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add support for r8a779a0 (R-Car V3U). The IPMMU hardware design > of this SoC differs than others. So, add a new ipmmu_features for it. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- a/drivers/iommu/ipmmu-vmsa.c > +++ b/drivers/iommu/ipmmu-vmsa.c > @@ -922,6 +922,20 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = { > .utlb_offset_base = 0, > }; > > +static const struct ipmmu_features ipmmu_features_r8a779a0 = { > + .use_ns_alias_offset = false, > + .has_cache_leaf_nodes = true, > + .number_of_contexts = 8, Shouldn't this be 16? Or do you plan to add support for more than 8 contexts later, as that would require increasing IPMMU_CTX_MAX, and updating ipmmu_ctx_reg() to handle the second bank of 8 contexts? Regardless, I assume this will still work when when limiting to 8 contexts, so Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, September 7, 2021 12:34 AM > > Hi Shimoda-san, > > On Wed, Sep 1, 2021 at 12:27 PM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add support for r8a779a0 (R-Car V3U). The IPMMU hardware design > > of this SoC differs than others. So, add a new ipmmu_features for it. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- a/drivers/iommu/ipmmu-vmsa.c > > +++ b/drivers/iommu/ipmmu-vmsa.c > > > @@ -922,6 +922,20 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = { > > .utlb_offset_base = 0, > > }; > > > > +static const struct ipmmu_features ipmmu_features_r8a779a0 = { > > + .use_ns_alias_offset = false, > > + .has_cache_leaf_nodes = true, > > + .number_of_contexts = 8, > > Shouldn't this be 16? > Or do you plan to add support for more than 8 contexts later, as that > would require increasing IPMMU_CTX_MAX, and updating ipmmu_ctx_reg() > to handle the second bank of 8 contexts? I would like to add support for more than 8 contexts later because I realized that ctx_offset_{base,stride} are not suitable for the second bank of 8 contexts... > Regardless, I assume this will still work when when limiting to 8 > contexts, so > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thank you for your review! Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Tue, Sep 7, 2021 at 2:02 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Tuesday, September 7, 2021 12:34 AM > > On Wed, Sep 1, 2021 at 12:27 PM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Add support for r8a779a0 (R-Car V3U). The IPMMU hardware design > > > of this SoC differs than others. So, add a new ipmmu_features for it. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > --- a/drivers/iommu/ipmmu-vmsa.c > > > +++ b/drivers/iommu/ipmmu-vmsa.c > > > > > @@ -922,6 +922,20 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = { > > > .utlb_offset_base = 0, > > > }; > > > > > > +static const struct ipmmu_features ipmmu_features_r8a779a0 = { > > > + .use_ns_alias_offset = false, > > > + .has_cache_leaf_nodes = true, > > > + .number_of_contexts = 8, > > > > Shouldn't this be 16? > > Or do you plan to add support for more than 8 contexts later, as that > > would require increasing IPMMU_CTX_MAX, and updating ipmmu_ctx_reg() > > to handle the second bank of 8 contexts? > > I would like to add support for more than 8 contexts later because > I realized that ctx_offset_{base,stride} are not suitable for the second bank > of 8 contexts... Wouldn't something like below be sufficient? static unsigned int 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; + unsigned int base = mmu->features->ctx_offset_base; + + if (context_id > 7) + base += 0x800 - 8 * 0x1040; + + return base + context_id * mmu->features->ctx_offset_stride + reg; } Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, September 7, 2021 3:34 PM > > Hi Shimoda-san, > > On Tue, Sep 7, 2021 at 2:02 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Geert Uytterhoeven, Sent: Tuesday, September 7, 2021 12:34 AM > > > On Wed, Sep 1, 2021 at 12:27 PM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > Add support for r8a779a0 (R-Car V3U). The IPMMU hardware design > > > > of this SoC differs than others. So, add a new ipmmu_features for it. > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > > --- a/drivers/iommu/ipmmu-vmsa.c > > > > +++ b/drivers/iommu/ipmmu-vmsa.c > > > > > > > @@ -922,6 +922,20 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = { > > > > .utlb_offset_base = 0, > > > > }; > > > > > > > > +static const struct ipmmu_features ipmmu_features_r8a779a0 = { > > > > + .use_ns_alias_offset = false, > > > > + .has_cache_leaf_nodes = true, > > > > + .number_of_contexts = 8, > > > > > > Shouldn't this be 16? > > > Or do you plan to add support for more than 8 contexts later, as that > > > would require increasing IPMMU_CTX_MAX, and updating ipmmu_ctx_reg() > > > to handle the second bank of 8 contexts? > > > > I would like to add support for more than 8 contexts later because > > I realized that ctx_offset_{base,stride} are not suitable for the second bank > > of 8 contexts... > > Wouldn't something like below be sufficient? Thank you for your suggestion! > static unsigned int 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; > + unsigned int base = mmu->features->ctx_offset_base; > + > + if (context_id > 7) > + base += 0x800 - 8 * 0x1040; This should be "base += 0x800 - 8 * 0x40;" because the 8th context address is 0x18800, not 0x10800. I'll send v2 patch to support 16 contexts. (I'll change IPMMU_CTX_MAX to 16 too.) Best regards, Yoshihiro Shimoda > + > + return base + context_id * mmu->features->ctx_offset_stride + reg; > } > > 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
Hi Shimoda-san, On Tue, Sep 7, 2021 at 9:29 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Tuesday, September 7, 2021 3:34 PM > > On Tue, Sep 7, 2021 at 2:02 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > From: Geert Uytterhoeven, Sent: Tuesday, September 7, 2021 12:34 AM > > > > On Wed, Sep 1, 2021 at 12:27 PM Yoshihiro Shimoda > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > Add support for r8a779a0 (R-Car V3U). The IPMMU hardware design > > > > > of this SoC differs than others. So, add a new ipmmu_features for it. > > > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > > > > --- a/drivers/iommu/ipmmu-vmsa.c > > > > > +++ b/drivers/iommu/ipmmu-vmsa.c > > > > > > > > > @@ -922,6 +922,20 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = { > > > > > .utlb_offset_base = 0, > > > > > }; > > > > > > > > > > +static const struct ipmmu_features ipmmu_features_r8a779a0 = { > > > > > + .use_ns_alias_offset = false, > > > > > + .has_cache_leaf_nodes = true, > > > > > + .number_of_contexts = 8, > > > > > > > > Shouldn't this be 16? > > > > Or do you plan to add support for more than 8 contexts later, as that > > > > would require increasing IPMMU_CTX_MAX, and updating ipmmu_ctx_reg() > > > > to handle the second bank of 8 contexts? > > > > > > I would like to add support for more than 8 contexts later because > > > I realized that ctx_offset_{base,stride} are not suitable for the second bank > > > of 8 contexts... > > > > Wouldn't something like below be sufficient? > > Thank you for your suggestion! > > > static unsigned int 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; > > + unsigned int base = mmu->features->ctx_offset_base; > > + > > + if (context_id > 7) > > + base += 0x800 - 8 * 0x1040; > > This should be "base += 0x800 - 8 * 0x40;" because the 8th context address is > 0x18800, not 0x10800. Doh, I should have written my first thought ("base += FIXME" ;-) > I'll send v2 patch to support 16 contexts. > (I'll change IPMMU_CTX_MAX to 16 too.) Thanks! Gr{oetje,eeting}s, Geert
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c index d38ff29a76e8..c46951367f93 100644 --- a/drivers/iommu/ipmmu-vmsa.c +++ b/drivers/iommu/ipmmu-vmsa.c @@ -36,7 +36,7 @@ #define IPMMU_CTX_MAX 8U #define IPMMU_CTX_INVALID -1 -#define IPMMU_UTLB_MAX 48U +#define IPMMU_UTLB_MAX 64U struct ipmmu_features { bool use_ns_alias_offset; @@ -922,6 +922,20 @@ static const struct ipmmu_features ipmmu_features_rcar_gen3 = { .utlb_offset_base = 0, }; +static const struct ipmmu_features ipmmu_features_r8a779a0 = { + .use_ns_alias_offset = false, + .has_cache_leaf_nodes = true, + .number_of_contexts = 8, + .num_utlbs = 64, + .setup_imbuscr = false, + .twobit_imttbcr_sl0 = true, + .reserved_context = true, + .cache_snoop = false, + .ctx_offset_base = 0x10000, + .ctx_offset_stride = 0x1040, + .utlb_offset_base = 0x3000, +}; + static const struct of_device_id ipmmu_of_ids[] = { { .compatible = "renesas,ipmmu-vmsa", @@ -959,6 +973,9 @@ static const struct of_device_id ipmmu_of_ids[] = { }, { .compatible = "renesas,ipmmu-r8a77995", .data = &ipmmu_features_rcar_gen3, + }, { + .compatible = "renesas,ipmmu-r8a779a0", + .data = &ipmmu_features_r8a779a0, }, { /* Terminator */ },
Add support for r8a779a0 (R-Car V3U). The IPMMU hardware design of this SoC differs than others. So, add a new ipmmu_features for it. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/iommu/ipmmu-vmsa.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)