From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [RFC SWIOTLB-0.2] Date: Tue, 19 Jan 2010 13:20:57 -0500 Message-ID: <20100119182057.GS11986@phenom.dumpdata.com> References: <1263510064-16788-1-git-send-email-konrad.wilk@oracle.com> <20100115022510.GI6021@sequoia.sous-sol.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20100115022510.GI6021@sequoia.sous-sol.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Chris Wright Cc: Ian.Campbell@eu.citrix.com, jeremy@goop.org, xen-devel@lists.xensource.com, joerg.roedel@amd.com, fujita.tomonori@lab.ntt.co.jp, iommu@lists.linux-foundation.org, dwmw2@infradead.org, alex.williamson@hp.com List-Id: xen-devel@lists.xenproject.org On Thu, Jan 14, 2010 at 06:25:10PM -0800, Chris Wright wrote: > * Konrad Rzeszutek Wilk (konrad.wilk@oracle.com) wrote: > > Another approach, which this set of patches explores, is to abstract the > > address translation and address determination functions away from the > > SWIOTLB book-keeping functions. This way the core SWIOTLB library functions > > are present in one place, while the address related functions are in > > a separate library for different run-time platforms. I would very much > > appreciate input on this idea and the set of patches. > > It seems like it still needs some refinement, since the Xen Oh yes. > implementation is hooking into two layers. Both: > > + swiotlb_register_engine(&xen_ops); > > and > > +static struct dma_map_ops xen_swiotlb_dma_ops = { > > Wouldn't the idea be to get to the point that you'd use common swiotlb > and keep the hooks to one layer? I would love to. Maybe I can extend those two functions (alloc_coherent and free_coherent) to make an extra call after they have allocated/de-allocated a page? The reason is that in virtualized environments I MUST guarantee that those buffers are linearly contiguous. Meaning I need to post-processing of this buffer: ret = (void *)__get_free_pages(flags, order) If that can't be done, then I need a mix of DMA ops where the majority is SWIOTLB with the exception of the alloc_coherent and free_coherent). Hmm, I should follow the lead of what x86_swiotlb_alloc_coherent does and just make an extra call to 'is_swiotlb_buffer' on the return address and if not found to be within that SWIOTLB, do the fixup to make sure the pages are linearly contiguous. > > Also, it's unclear when some of the prior global to swiotlb variables > would actually be useful to a private implementation. For example, overflow, > which is just 32 * 1024 in both cases. Are those really needed to be > private to a swiotlb engine? Unfortunately yes. The same reason as mentioned above: MUST guarantee that those buffers (start, overflow) are linearly contiguous. For that I was doing something like: void __init xen_swiotlb_init(int verbose) { int rc = 0; swiotlb_register_engine(&xen_ops); swiotlb_init_with_default_size(&xen_ops, 64 * (1<<20), 0); if ((rc = xen_swiotlb_fixup(xen_ops.start, xen_ops.nslabs << IO_TLB_SHIFT, xen_ops.nslabs))) goto error; if ((rc = xen_swiotlb_fixup(xen_ops.overflow_buffer, xen_ops.overflow, xen_ops.overflow >> IO_TLB_SHIFT))) goto error; so that I can "fix" the start and overflow_buffer pages. > > Do you think you can reduce the swiotlb_engine to just the relevant ops? Yes. Let me reduce them. > > thanks, > -chris