Re: [Xen-devel] [PATCH v2 1/2] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 18 Jan 2017, Andrii Anisov wrote:
> Dear Stefano,
> 
> 
> > Only one suggestion more. For this to work correctly, we are assuming
> > that no foreging pages are involved here, which is a very reasonable
> > assumption given that mmap should be called on memory returned by
> > dma_alloc_coherent.
> 
> I also kept in mind this problem, that's why the first version was RFC.
> 
> > Please add an in-code comment here so that we'll remember.
> 
> Do you think comment would be enough so far?

A comment is enough in the case of xen_swiotlb_dma_mmap, because we are
sure that the function can only be called with local pages. See the
comment above dma_mmap_attrs:

 * Map a coherent DMA buffer previously allocated by dma_alloc_attrs
 * into user space.  The coherent DMA buffer must not be freed by the
 * driver until the user space mapping has been released.

If the page must comes from dma_alloc_coherent, then we are safe.


I wasn't sure about dma_get_sgtable_attrs, because there is no in-tree
description, but looking at git log:

  commit d2b7428eb0caa7c66e34b6ac869a43915b294123
  Author: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
  Date:   Wed Jun 13 10:05:52 2012 +0200
  
      common: dma-mapping: introduce dma_get_sgtable() function
      
      This patch adds dma_get_sgtable() function which is required to let
      drivers to share the buffers allocated by DMA-mapping subsystem. Right

It looks like dma_get_sgtable is also supposed to be called on buffers
returned by dma_alloc_coherent. We should be safe in both cases.


> Maybe fallback to common ops would be better in order to keep current (even 
> broken) functionality for now? Or
> BUG_ON as you suggested for get_sgtable callback?

BUG_ON is good because it is an obvious failure for a case we don't know
how to handle. If it actually works as expected, we could add it to
both functions anyway, surrounded by #ifdef DEBUG_DRIVER not to slow
down the common case.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel