Messages in this thread | | | Date | Tue, 22 Aug 2023 09:32:24 -0700 | From | Nicolin Chen <> | Subject | Re: [PATCH 3/3] iommu/arm-smmu-v3: Add a max_tlbi_ops for __arm_smmu_tlb_inv_range() |
| |
On Tue, Aug 22, 2023 at 10:30:35AM +0100, Robin Murphy wrote:
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > index d6c647e1eb01..3f0db30932bd 100644 > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > @@ -1897,7 +1897,14 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd, > > if (!size) > > return; > > > > - if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) { > > + if (!(smmu->features & ARM_SMMU_FEAT_RANGE_INV)) { > > + /* > > + * When the size reaches a threshold, replace per-granule TLBI > > + * commands with one single per-asid or per-vmid TLBI command. > > + */ > > + if (size >= granule * smmu_domain->max_tlbi_ops) > > + return arm_smmu_tlb_inv_domain(smmu_domain); > > This looks like it's at the wrong level - we should have figured this > out before we got as far as low-level command-building. I'd have thought > it would be a case of short-circuiting directly from > arm_smmu_tlb_inv_range_domain() to arm_smmu_tlb_inv_context().
OK, I could do that. We would have copies of this same routine though. Also, the shortcut applies to !ARM_SMMU_FEAT_RANGE_INV cases only, so this function feels convenient to me.
> > + } else { > > /* Get the leaf page size */ > > tg = __ffs(smmu_domain->domain.pgsize_bitmap); > > > > @@ -2258,6 +2265,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain, > > } > > > > smmu_domain->pgtbl_ops = pgtbl_ops; > > + smmu_domain->max_tlbi_ops = pgtbl_cfg.nents_per_pgtable; > > And now we're carrying *three* copies of the same information around > everywhere? Honestly, just pull cfg->bits_per_level out of the > io_pgtable_ops at the point where you need it, like the pagetable code > itself manages to do perfectly happily. Wrap it in an io-pgtable helper > if you think that's cleaner.
OK. I overlooked io_pgtable_ops_to_pgtable. Will do that.
Thanks Nic
| |