On Tue, Jul 28, 2015 at 04:48:09PM +0100, Robin Murphy wrote: > On 28/07/15 11:18, Will Deacon wrote: > > On Mon, Jul 27, 2015 at 07:18:10PM +0100, Robin Murphy wrote: > >> With the correct DMA API calls now integrated into the io-pgtable code, > >> let that handle the flushing of non-coherent page table updates. > >> > >> Signed-off-by: Robin Murphy <[email protected]> > >> --- > >> drivers/iommu/arm-smmu.c | 23 +++++++---------------- > >> 1 file changed, 7 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > >> index 9a59bef..9501668 100644 > >> --- a/drivers/iommu/arm-smmu.c > >> +++ b/drivers/iommu/arm-smmu.c > >> @@ -610,24 +610,13 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned > >> long iova, size_t size, > >> static void arm_smmu_flush_pgtable(void *addr, size_t size, void *cookie) > >> { > >> struct arm_smmu_domain *smmu_domain = cookie; > >> - struct arm_smmu_device *smmu = smmu_domain->smmu; > >> - unsigned long offset = (unsigned long)addr & ~PAGE_MASK; > >> > >> - > >> - /* Ensure new page tables are visible to the hardware walker */ > >> - if (smmu->features & ARM_SMMU_FEAT_COHERENT_WALK) { > >> + /* > >> + * Ensure new page tables are visible to a coherent hardware walker. > >> + * The page table code deals with flushing for the non-coherent case. > >> + */ > >> + if (smmu_domain->smmu->features & ARM_SMMU_FEAT_COHERENT_WALK) > >> dsb(ishst); > >> - } else { > >> - /* > >> - * If the SMMU can't walk tables in the CPU caches, treat them > >> - * like non-coherent DMA since we need to flush the new entries > >> - * all the way out to memory. There's no possibility of > >> - * recursion here as the SMMU table walker will not be wired > >> - * through another SMMU. > >> - */ > >> - dma_map_page(smmu->dev, virt_to_page(addr), offset, size, > >> - DMA_TO_DEVICE); > >> - } > >> } > >> > >> static struct iommu_gather_ops arm_smmu_gather_ops = { > >> @@ -899,6 +888,8 @@ static int arm_smmu_init_domain_context(struct > >> iommu_domain *domain, > >> .oas = oas, > >> .tlb = &arm_smmu_gather_ops, > >> }; > >> + if (!(smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)) > >> + pgtbl_cfg.iommu_dev = smmu->dev; > > > > Ideally, I'd like to set the dev unconditionally and kill the flush_pgtable > > callback. See my comments on the other patch. > > I'm not sure setting dev unconditionally would be right - we'd end up > doing a load of busy work in the DMA API to decide to do nothing and > never get a barrier out of it. We could have a "bool coherent" in the > cfg to make the wmb() happen, but the mere absence of dev would do that > job just as well (wmb() isn't going to need it to report anything, > unlike the DMA API path); either way flush_pgtable can go.
I think we should move to using the DMA API as step (1), then discuss its inefficiencies in step (2) :) No point working around perceived performance issues before we know they're actually hurting us. Will _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
