Re: [PATCH V2 2/3] powerpc: Add support for swiotlb on 32-bit

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Wed, 2009-05-27 at 23:42 -0500, Kumar Gala wrote:
> Ben,
> 
> Any comments on this.. need a decision so we can have patches ready  
> for .31.

> > Clamping the DMA mask is even worse than the additional indirection  
> > for us.  We have valid scenarios in which we'd have 512M of outbound  
> > PCI address space and 4G of mem and thus 3.5G of inbound PCI address  
> > space.  With the DMA mask we'd be limited to 2G and bouncing from  
> > 2..3.5G when we don't need to.

Ok and agreed.

> > I think our options are to change archdata as follows:
> >
> > Option 1 - just add a new data member to dev_archdata
> >
> > struct dev_archdata {
> >        /* Optional pointer to an OF device node */
> >        struct device_node      *of_node;
> >
> >        /* DMA operations on that device */
> >        struct dma_mapping_ops  *dma_ops;
> >        void 		        *dma_data;
> > 	dma_addr_t		direct_dma_addr;
> > };

That sounds like the "simple" option, might want for now to make
it conditional on SWIOTLB but the bloat is reasonably small I would
expect.

> > Option 2 - introduce a proper container for how we use dma_data.   
> > This may just be moving the indirection from an indirection function  
> > call to an indirection data reference:

Right, it somewhat defeats the purpose though an indirect data reference
tends to hit the pipeline less hard than an indirect function call...
 
> > Option 3 - use dma_data to keep the addr at which we need to bounce  
> > vs not for SWIOTLB - this has potential issues w/conflicting with  
> > dma_data being used as the dma_offset. (need to think on that a bit  
> > more).  Additionally this has the benefit in that we need dma_data  
> > to be a 64-bit quantity on ppc32 w/>32-bit phys addr.

Well, that means that swiotlb can not be used with a !0 offset. That
-might- be an issue if the PCI is setup "backward", ie with 0..N being
the outbound MMIO and N..4G the DMA region, remapped to 0. There are
reasons to do it this way, it's not invalid, for example it allow easy
access to ISA/VGA holes.

At this stage I have no firm opinion. I'm thinking that either we try
to limit the overhead and option 1 is probably the simplest, at the
expense of a little bit of memory, or we think the overhead is going
to be minimum and we may as well stick to 2 functions since that's
going to be more flexible.

Cheers,
Ben.