All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
To: Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	"iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org"
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	Olav Haugan <ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Alexandre Courbot
	<gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Tomeu Vizoso
	<tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>,
	Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	Antonios Motakis
	<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>,
	Will Deacon <Will.Deacon-5wv7dgnIgG8@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Thierry Reding
	<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Nicolas Iooss <nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org>,
	Vince Hsu <vince.h-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Mikko Perttunen
	<mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Subject: Re: [RFC PATCH 0/3] iommu: Add range flush operation
Date: Tue, 29 Sep 2015 18:13:17 +0100	[thread overview]
Message-ID: <560AC6AD.3010307@arm.com> (raw)
In-Reply-To: <20150929164014.GL21513-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On 29/09/15 17:40, Russell King - ARM Linux wrote:
> On Tue, Sep 29, 2015 at 05:27:12PM +0100, Robin Murphy wrote:
>> Eh, swings and roundabouts. An argument denoting whether the flush is being
>> called on the map or unmap path would be fine,
>
> Sorry, that statement is wrong.  It's not about whether you flush before
> or after the DMA operation.  I'm afraid I'm probably going to tell you
> how to suck eggs here, because I don't think you quite "get it" with
> non-dma-coherent modern CPUs.
>
> Modern CPUs prefetch data into their caches, and they also randomly write
> back data from their caches to memory.  When performing a DMA operation
> from device to memory, you need to do two things with CPU caches which
> aren't coherent:
>
> 1. Before starting the DMA operation, you need to walk over the memory to
>     be mapped, ensuring that any dirty cache lines are written back.  This
>     is to prevent dirty cache lines overwriting data that has already been
>     DMA'd from the device.
>
> 2. After the DMA operation has completed, you need to walk over the
>     memory again, invalidating any cache lines which may have been
>     speculatively loaded from that memory while DMA was running.  These
>     cache lines may have been loaded prior to the DMA operation placing
>     the new data into memory.
>
> So, it's not a before-or-after, you have to always perform write-back
> cache maintanence prior to any DMA operation, and then invalidate cache
> maintanence after the DMA operation has completed for any mapping which
> the DMA may have written to (which means device-to-memory and
> bidirectional mappings.)

Yup, I'm well aware of all that; in fact you and I have already agreed 
elsewhere that we can only really get away with using the streaming DMA 
API to flush IOMMU page table updates _because_ they aren't written back 
to, thus data only ever goes from CPU->IOMMU and we can skip the problem 
of where to put an invalidation; you wrote the tegra-smmu code that does 
this. The coherency of whatever device which made a DMA API call for 
which the IOMMU API is creating/removing a mapping is irrelevant at this 
point - this is the DMA operation within the DMA operation.

None of which has anything to do with the point I raised, which is that 
if iommu_unmap() calls iommu_flush(), I want to issue TLB invalidations, 
but if iommu_map() calls iommu_flush(), I don't.

WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Tomasz Figa <tfiga@chromium.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	Olav Haugan <ohaugan@codeaurora.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Paul Walmsley <paul@pwsan.com>, Arnd Bergmann <arnd@arndb.de>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Antonios Motakis <a.motakis@virtualopensystems.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Nicolas Iooss <nicolas.iooss_linux@m4x.org>,
	Vince Hsu <vince.h@nvidia.com>,
	Mikko Perttunen <mperttunen@nvidia.com>
Subject: Re: [RFC PATCH 0/3] iommu: Add range flush operation
Date: Tue, 29 Sep 2015 18:13:17 +0100	[thread overview]
Message-ID: <560AC6AD.3010307@arm.com> (raw)
In-Reply-To: <20150929164014.GL21513@n2100.arm.linux.org.uk>

On 29/09/15 17:40, Russell King - ARM Linux wrote:
> On Tue, Sep 29, 2015 at 05:27:12PM +0100, Robin Murphy wrote:
>> Eh, swings and roundabouts. An argument denoting whether the flush is being
>> called on the map or unmap path would be fine,
>
> Sorry, that statement is wrong.  It's not about whether you flush before
> or after the DMA operation.  I'm afraid I'm probably going to tell you
> how to suck eggs here, because I don't think you quite "get it" with
> non-dma-coherent modern CPUs.
>
> Modern CPUs prefetch data into their caches, and they also randomly write
> back data from their caches to memory.  When performing a DMA operation
> from device to memory, you need to do two things with CPU caches which
> aren't coherent:
>
> 1. Before starting the DMA operation, you need to walk over the memory to
>     be mapped, ensuring that any dirty cache lines are written back.  This
>     is to prevent dirty cache lines overwriting data that has already been
>     DMA'd from the device.
>
> 2. After the DMA operation has completed, you need to walk over the
>     memory again, invalidating any cache lines which may have been
>     speculatively loaded from that memory while DMA was running.  These
>     cache lines may have been loaded prior to the DMA operation placing
>     the new data into memory.
>
> So, it's not a before-or-after, you have to always perform write-back
> cache maintanence prior to any DMA operation, and then invalidate cache
> maintanence after the DMA operation has completed for any mapping which
> the DMA may have written to (which means device-to-memory and
> bidirectional mappings.)

Yup, I'm well aware of all that; in fact you and I have already agreed 
elsewhere that we can only really get away with using the streaming DMA 
API to flush IOMMU page table updates _because_ they aren't written back 
to, thus data only ever goes from CPU->IOMMU and we can skip the problem 
of where to put an invalidation; you wrote the tegra-smmu code that does 
this. The coherency of whatever device which made a DMA API call for 
which the IOMMU API is creating/removing a mapping is irrelevant at this 
point - this is the DMA operation within the DMA operation.

None of which has anything to do with the point I raised, which is that 
if iommu_unmap() calls iommu_flush(), I want to issue TLB invalidations, 
but if iommu_map() calls iommu_flush(), I don't.


  parent reply	other threads:[~2015-09-29 17:13 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-29  5:25 [RFC PATCH 0/3] iommu: Add range flush operation Tomasz Figa
2015-09-29  5:25 ` Tomasz Figa
     [not found] ` <1443504379-31841-1-git-send-email-tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2015-09-29  5:25   ` [RFC PATCH 1/3] iommu: Add support for out of band flushing Tomasz Figa
2015-09-29  5:25     ` Tomasz Figa
     [not found]     ` <1443504379-31841-2-git-send-email-tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2015-09-29  9:32       ` Thierry Reding
2015-09-29  9:32         ` Thierry Reding
     [not found]         ` <20150929093257.GE9460-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2015-09-29 11:56           ` Tomasz Figa
2015-09-29 11:56             ` Tomasz Figa
2015-09-29  5:25   ` [RFC PATCH 2/3] memory: tegra: add TLB cache line size Tomasz Figa
2015-09-29  5:25     ` Tomasz Figa
     [not found]     ` <1443504379-31841-3-git-send-email-tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2015-09-29  9:43       ` Thierry Reding
2015-09-29  9:43         ` Thierry Reding
     [not found]         ` <20150929094348.GF9460-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2015-09-29 12:11           ` Tomasz Figa
2015-09-29 12:11             ` Tomasz Figa
2015-09-29  5:25   ` [RFC PATCH 3/3] iommu/tegra-smmu: Make the driver use out of band flushing Tomasz Figa
2015-09-29  5:25     ` Tomasz Figa
2015-09-29  9:27   ` [RFC PATCH 0/3] iommu: Add range flush operation Thierry Reding
2015-09-29  9:27     ` Thierry Reding
     [not found]     ` <20150929092714.GD9460-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2015-09-29 11:54       ` Tomasz Figa
2015-09-29 11:54         ` Tomasz Figa
2015-09-29 12:22       ` Joerg Roedel
2015-09-29 12:22         ` Joerg Roedel
2015-09-29 12:20   ` Joerg Roedel
2015-09-29 12:20     ` Joerg Roedel
2015-09-29 14:20   ` Robin Murphy
2015-09-29 14:20     ` Robin Murphy
     [not found]     ` <560A9E36.9030903-5wv7dgnIgG8@public.gmane.org>
2015-09-29 14:32       ` Russell King - ARM Linux
2015-09-29 14:32         ` Russell King - ARM Linux
     [not found]         ` <20150929143241.GI21513-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2015-09-29 16:27           ` Robin Murphy
2015-09-29 16:27             ` Robin Murphy
     [not found]             ` <560ABBE0.8020805-5wv7dgnIgG8@public.gmane.org>
2015-09-29 16:40               ` Russell King - ARM Linux
2015-09-29 16:40                 ` Russell King - ARM Linux
     [not found]                 ` <20150929164014.GL21513-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2015-09-29 17:13                   ` Robin Murphy [this message]
2015-09-29 17:13                     ` Robin Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=560AC6AD.3010307@arm.com \
    --to=robin.murphy-5wv7dgnigg8@public.gmane.org \
    --cc=Will.Deacon-5wv7dgnIgG8@public.gmane.org \
    --cc=a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=nicolas.iooss_linux-oWGTIYur0i8@public.gmane.org \
    --cc=ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org \
    --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \
    --cc=tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org \
    --cc=vince.h-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.