On Mon, Apr 20, 2015 at 7:43 PM, Tomasz Figa <tfiga at chromium.org> wrote: > To flush created mappings, current mapping code relies on the fact that > during unmap the driver zaps every IOVA being unmapped and that it is > enough to zap a single IOVA of page table to remove the entire page > table from IOMMU cache. Based on these assumptions the driver was made to > simply zap the first IOVA of the mapping being created. This is enough > to invalidate first page table, which could be shared with another > mapping (and thus could be already present in IOMMU cache), but > unfortunately it does not do anything about the last page table that > could be shared with other mappings as well. > > Moreover, the flushing is performed before page table contents are > actually modified, so there is a race between the CPU updating the page > tables and hardware that could be possibly running at the same time and > triggering IOMMU look-ups, which could bring back the page tables back > to the cache. > > To fix both issues, this patch makes the mapping code zap first and last > (if they are different) IOVAs of new mapping after the page table is > updated. > > Signed-off-by: Tomasz Figa <tfiga at chromium.org> > Reviewed-by: Daniel Kurtz <djkurtz at chromium.org> > Tested-by: Heiko Stuebner <heiko at sntech.de> You probably want to remove the "CHROMIUM: " label in the subject. Other than that, this is still: Reviewed-by: Daniel Kurtz <djkurtz at chromium.org> > --- > drivers/iommu/rockchip-iommu.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c > index 4015560..31004c0 100644 > --- a/drivers/iommu/rockchip-iommu.c > +++ b/drivers/iommu/rockchip-iommu.c > @@ -551,6 +551,15 @@ static void rk_iommu_zap_iova(struct rk_iommu_domain *rk_domain, > spin_unlock_irqrestore(&rk_domain->iommus_lock, flags); > } > > +static void rk_iommu_zap_iova_first_last(struct rk_iommu_domain *rk_domain, > + dma_addr_t iova, size_t size) > +{ > + rk_iommu_zap_iova(rk_domain, iova, SPAGE_SIZE); > + if (size > SPAGE_SIZE) > + rk_iommu_zap_iova(rk_domain, iova + size - SPAGE_SIZE, > + SPAGE_SIZE); > +} > + > static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, > dma_addr_t iova) > { > @@ -575,12 +584,6 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain, > rk_table_flush(page_table, NUM_PT_ENTRIES); > rk_table_flush(dte_addr, 1); > > - /* > - * Zap the first iova of newly allocated page table so iommu evicts > - * old cached value of new dte from the iotlb. > - */ > - rk_iommu_zap_iova(rk_domain, iova, SPAGE_SIZE); > - > done: > pt_phys = rk_dte_pt_address(dte); > return (u32 *)phys_to_virt(pt_phys); > @@ -630,6 +633,14 @@ static int rk_iommu_map_iova(struct rk_iommu_domain *rk_domain, u32 *pte_addr, > > rk_table_flush(pte_addr, pte_count); > > + /* > + * Zap the first and last iova to evict from iotlb any previously > + * mapped cachelines holding stale values for its dte and pte. > + * We only zap the first and last iova, since only they could have > + * dte or pte shared with an existing mapping. > + */ > + rk_iommu_zap_iova_first_last(rk_domain, iova, size); > + > return 0; > unwind: > /* Unmap the range of iovas that we just mapped */ > -- > 2.2.0.rc0.207.ga3a616c >