On Tue, Sep 29, 2015 at 6:32 PM, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > On Tue, Sep 29, 2015 at 02:25:24PM +0900, Tomasz Figa wrote: >> This patch adds a new "flush" callback to iommu_ops, which is supposed >> to perform any necessary flushes within given IOMMU domain to make any >> changes to mappings of given area [iova; iova + size) be reflected to >> IOMMU clients. >> >> The purpose is to let IOMMU drivers skip page-by-page flushes and >> replace it with one flush of full address range on devices which support >> it. >> >> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> >> --- >> drivers/iommu/iommu.c | 33 ++++++++++++++++++++++++++++++--- >> include/linux/iommu.h | 2 ++ >> 2 files changed, 32 insertions(+), 3 deletions(-) > > I seem to remember that Rob Clark had proposed something like this back > before it was decided to introduce the ->map_sg() callback instead. I > can't find a reference to the discussion, so perhaps I'm misremembering > but adding Rob Clark in case he has any recollection of why the outcome > was what it was. Oops, I was supposed to add Rob, but forgot in the end. Thanks for remembering. >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > [...] >> @@ -1389,6 +1410,7 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) >> unmapped += unmapped_page; >> } >> >> + iommu_flush(domain, orig_iova, unmapped); >> trace_unmap(orig_iova, size, unmapped); >> return unmapped; >> } >> @@ -1419,19 +1441,24 @@ size_t default_iommu_map_sg(struct iommu_domain *domain, unsigned long iova, >> if (!IS_ALIGNED(s->offset, min_pagesz)) >> goto out_err; >> >> - ret = iommu_map(domain, iova + mapped, phys, s->length, prot); >> + ret = __iommu_map(domain, iova + mapped, phys, s->length, prot); >> if (ret) >> goto out_err; >> >> mapped += s->length; >> } >> >> + iommu_flush(domain, iova, mapped); >> + >> return mapped; >> >> out_err: >> /* undo mappings already done */ >> iommu_unmap(domain, iova, mapped); >> >> + /* flush in case part of our mapping already got cached */ >> + iommu_flush(domain, iova, mapped); >> + >> return 0; >> >> } > > iommu_unmap() already does an iommu_flush(), so why flush again after > iommu_unmap()? Right, my mistake. This should be removed. Thanks for catching. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html