RE: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base
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 > 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 > > 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 -- > ge...@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
Hi Shimoda-san, On Wed, Oct 9, 2019 at 10:27 AM 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 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 -- ge...@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
Re: [PATCH 3/3] iommu/ipmmu-vmsa: Add utlb_offset_base
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 Reviewed-by: Niklas Söderlund > --- > 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