From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suman Anna Subject: Re: [PATCH 3/5] iommu/omap: Flush the TLB only after updating translation table entries Date: Fri, 14 Mar 2014 19:18:43 -0500 Message-ID: <53239C63.5020807@ti.com> References: <1394239574-2389-1-git-send-email-laurent.pinchart@ideasonboard.com> <1394239574-2389-4-git-send-email-laurent.pinchart@ideasonboard.com> <532230DA.30302@ti.com> <7972929.biQkBVMjd1@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <7972929.biQkBVMjd1@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Laurent Pinchart Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, sakari.ailus-X3B1VOXEql0@public.gmane.org, Florian Vaussard List-Id: iommu@lists.linux-foundation.org Hi Laurent, >> >> On 03/07/2014 06:46 PM, Laurent Pinchart wrote: >>> Flushing the TLB before updating translation entries creates a race >>> condition and can lead to stale TLB entries if a translation request >>> arrives between flushing the TLB and updating the translation entries. >>> As there's no requirement to flush the TLB before updating the entries, >>> flush it after only. >> >> I do not expect a device to be actively using a region if we are about >> to change its mapping, we expect the access from the device to be only >> between a map and an unmap. The race condition (if any in such >> scenarios) would exist even after this patch, like after you programmed >> the entry, and the translation request comes before you flush the page. >> Then it is still operating on an older address, while you have already >> programmed a new one. An unmap currently clears the entry and flushes >> any TLB as well, so I don't think this patch makes a big difference. > > I agree that the patch won't make a difference in practice. Should I drop it > from v2 ? Yes, that should be safe. regards Suman > >>> Signed-off-by: Laurent Pinchart >>> --- >>> >>> drivers/iommu/omap-iommu.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c >>> index cb1e1de..fedd303 100644 >>> --- a/drivers/iommu/omap-iommu.c >>> +++ b/drivers/iommu/omap-iommu.c >>> @@ -662,10 +662,11 @@ int omap_iopgtable_store_entry(struct omap_iommu >>> *obj, struct iotlb_entry *e)> >>> { >>> int err; >>> >>> - flush_iotlb_page(obj, e->da); >>> >>> err = iopgtable_store_entry_core(obj, e); >>> - if (!err) >>> + if (!err) { >>> + flush_iotlb_page(obj, e->da); >>> prefetch_iotlb_entry(obj, e); >>> + } >>> >>> return err; >>> } >>> EXPORT_SYMBOL_GPL(omap_iopgtable_store_entry); >