WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: mthca use of dma_sync_single is bogus

To: Roland Dreier <rdreier@xxxxxxxxx>
Subject: [Xen-devel] Re: mthca use of dma_sync_single is bogus
From: "Michael S. Tsirkin" <mst@xxxxxxxxxxxxxxxxxx>
Date: Tue, 10 Jul 2007 00:39:13 +0300
Cc: Lukas Hejtmanek <xhejtman@xxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, Keir Fraser <keir@xxxxxxxxxxxxx>, general@xxxxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 10 Jul 2007 10:13:16 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <adalkdpxopo.fsf@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <adalkdpxopo.fsf@xxxxxxxxx>
Reply-to: "Michael S. Tsirkin" <mst@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.11
> Quoting Roland Dreier <rdreier@xxxxxxxxx>:
> Subject: mthca use of dma_sync_single is bogus
> 
> It seems the problems running mthca in a Xen domU have uncovered a bug
> in mthca: mthca uses dma_sync_single in mthca_arbel_write_mtt_seg()
> and mthca_arbel_map_phys_fmr() to sync the MTTs that get written.
> However, Documentation/DMA-API.txt says:
> 
>     void
>     dma_sync_single(struct device *dev, dma_addr_t dma_handle, size_t size,
>               enum dma_data_direction direction)
> 
>     synchronise a single contiguous or scatter/gather mapping.  All the
>     parameters must be the same as those passed into the single mapping
>     API.
> 
> and mthca is *not* following this clear rule: it is trying to sync
> only a subrange of the mapping.

Yes, this looks like a bug.

> Later on in the document, there is:
> 
>     void
>     dma_sync_single_range(struct device *dev, dma_addr_t dma_handle,
>                     unsigned long offset, size_t size,
>                     enum dma_data_direction direction)
>     
>     does a partial sync.  starting at offset and continuing for size.  You
>     must be careful to observe the cache alignment and width when doing
>     anything like this.  You must also be extra careful about accessing
>     memory you intend to sync partially.
> 
> but that is in a section dealing with non-consistent memory so it's
> not entirely clear to me whether it's kosher to use this as mthca
> wants.

This is under Part II - Advanced dma_ usage - I don't think it's dealing with
non-consistent memory only (e.g. dma_declare_coherent_memory is there), and this
looks like a good fit.  Most functions here work for both consistent and
non-consistent memory...  What makes you suspicious?

> The other alternative is to put the MTT table in coherent memory just
> like the MPT table.  That might be the best solution I suppose...
> 
> Michael, anyone else, thoughts on this?

Certainly easy ...

I'm concerned that MTTs need a fair amount of memory,
while the amount of coherent memory might be limited.
Not that non-coherent memory systems are widespread ...

-- 
MST

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel