Re: [PATCH 3/5] iommu/omap: Flush the TLB only after updating translation table entries

[Suman Anna <s-anna-l0cyMroinI0@public.gmane.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.

regards
Suman

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> ---
>   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);
>