[openib-general] How to support IOMMUs for ipath driver

[Ralph Campbell]

On Tue, 2006-09-12 at 20:15 -0700, Roland Dreier wrote:
>  > My current proposal is to provide wrapper routines for the
>  > dma_*() routines which only the IB kernel code would use.
>  > These ib_dma_*() variants would allow a device driver to interpose
>  > on the call and do appropriate code to convert the kernel virtual
>  > or physical page addresses to something the device driver can handle.
>  > For ib_mthca and ib_ehca, these would result in the corresponding
>  > dma_*() routine being called. For ib_ipath, a different implementation
>  > would be needed.
> 
> Seems like the least-bad way forward.
> 
> A few comments on the proposed implementation:
> 
>  > @@ -984,6 +985,19 @@ struct ib_device {
>  >  						  struct ib_grh *in_grh,
>  >  						  struct ib_mad *in_mad,
>  >  						  struct ib_mad *out_mad);
>  > +	int                        (*mapping_error)(dma_addr_t dma_addr);
>  > +	dma_addr_t                 (*map_single)(struct device *hwdev,
>  > +						 void *ptr, size_t size,
>  > +						 int direction);
>  > +	void                       (*unmap_single)(struct device *dev,
>  > +						   dma_addr_t addr,
>  > +						   size_t size, int direction);
>  > +	int                        (*map_sg)(struct device *hwdev,
>  > +					     struct scatterlist *sg,
>  > +					     int nents, int direction);
>  > +	void                       (*unmap_sg)(struct device *hwdev,
>  > +					       struct scatterlist *sg,
>  > +					       int nents, int direction);
> 
> First of all I would put all this into a "struct ib_dma_ops" or
> something like that, so struct ib_device can have just a member like
> 
> 	struct ib_dma_ops	*dma_ops;
> 
> That keeps the definition of struct ib_device from getting too much
> more gigantic, and also makes it easy for the core to export a
> standard dma_ops pointer that devices that use the default
> implementation can use.
> 
> Why not make the DMA operations take a struct ib_device * instead of a
> struct device *?  I think that would actually clean up the consumer
> code, and it would make it easier for ipath -- otherwise you have to
> find your way back from the struct device *.
> 
> Also, I think you will need a few more methods.  <asm-x86_64/dma-mapping.h>
> has a definition of DMA operations that might be useful to refer too.
> But for example SRP uses at least dma_sync_single_for_cpu() and
> dma_sync_single_for_device().  Actually that might be the only extra
> method needed for now.
> 
>  - R.

These are all good suggestions and I will incorporate them.