public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: robin.murphy@arm•com (Robin Murphy)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC PATCH v2 2/3] arm64: add IOMMU dma_ops
Date: Mon, 09 Mar 2015 20:09:49 +0000	[thread overview]
Message-ID: <54FDFE0D.8030807@arm.com> (raw)
In-Reply-To: <20150309175904.GC8656@n2100.arm.linux.org.uk>

Hi Russell,

On 09/03/15 17:59, Russell King - ARM Linux wrote:
[...]
>>>> For a noncoherent device, dma_map_single() will end up calling
>>> __dma_map_area() with the page offset and size of the original request, so
>>> the updated part gets flushed by VA, and the rest of the page isn't touched
>>> if it doesn't need to be. On the other hand if the page tables were
>>> allocated with dma_alloc_coherent() in the first place, then just calling
>>> dma_sync_single_for_device() for the updated region should suffice.
>
> That's wrong.  dma_sync_single_*() is not permitted to be called on
> coherently allocated memory.  Where coherent memory needs to be remapped,
> dma_sync_single_*() will panic the kernel.
>
> If it's in coherent memory, all you should need is the appropriate
> memory barrier to ensure that the DMA agent can see the writes.

You're quite right, that's the whole point of *coherent* allocations 
after all. I got my syncs and barriers muddled there.

>>> Where exactly would you call the dma_unmap? It seems a bit strange to
>>> be repeatedly calling dma_map and never calling dma_unmap. I don't see it
>>> explicitly forbidden in the docs anywhere to do this but it seems like
>>> it would be violating the implicit handoff of dma_map/dma_unmap.
>>
>> I think ideally you'd call dma_map_page when you first create the page
>> table, dma_sync_single_for_device on any update, and dma_unmap_page when you
>> tear it down, and you'd also use the appropriate DMA addresses everywhere
>> instead of physical addresses.
>
> No.
>
> dma_map_page()			ownership changes CPU->DMA
> dma_sync_single_for_cpu()	ownership changes DMA->CPU
> dma_sync_single_for_device()	ownership changes CPU->DMA
> dma_unmap_page()		ownership changes DMA->CPU
>
> It's invalid to miss out the pairing that give those ownership changes.

Thanks for the clarification - the wording in DMA-API.txt rather implies 
that in the DMA_TO_DEVICE case you only have to sync the updated data 
/after/ writing it. For the sake of purely getting pages flushed, would 
it be more reasonable then to call dma_map_single() followed immediately 
by dma_unmap_single_attrs() with DMA_ATTR_SKIP_CPU_SYNC? Since we know 
the IOMMU can never write back to memory  (ones that can are a different 
issue) it would be nice to be able to skip the extra invalidations 
somehow, without too heinously abusing the API.

Robin.

  reply	other threads:[~2015-03-09 20:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-06 14:55 [RFC PATCH v2 0/3] arm64: IOMMU-backed DMA mapping Robin Murphy
2015-02-06 14:55 ` [RFC PATCH v2 1/3] iommu: implement common IOMMU ops for " Robin Murphy
2015-02-09  4:05   ` Will Deacon
2015-02-10 15:11     ` Robin Murphy
2015-03-12 12:45   ` Marek Szyprowski
2015-02-06 14:55 ` [RFC PATCH v2 2/3] arm64: add IOMMU dma_ops Robin Murphy
2015-02-09  6:02   ` Will Deacon
2015-02-10 15:40     ` Robin Murphy
2015-02-10  4:39   ` Yingjoe Chen
2015-02-10 12:07     ` Robin Murphy
2015-02-14  8:03       ` Yong Wu
2015-02-16 20:04         ` Robin Murphy
2015-03-03  3:38           ` Yong Wu
2015-03-03 12:15             ` Robin Murphy
2015-03-05  0:19               ` Laura Abbott
2015-03-05 11:16                 ` Robin Murphy
2015-03-09 17:59                   ` Russell King - ARM Linux
2015-03-09 20:09                     ` Robin Murphy [this message]
2015-03-10 10:16                       ` Robin Murphy
2015-03-12 12:50   ` Marek Szyprowski
2015-02-06 14:55 ` [RFC PATCH v2 3/3] arm64: hook up " Robin Murphy
2015-03-03 11:05   ` leizhen
2015-03-03 13:10     ` 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=54FDFE0D.8030807@arm.com \
    --to=robin.murphy@arm$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox