Re: [RFC SWIOTLB-0.2]

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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