Re: [RFC PATCH 0/3] allow fallback to swiotlb on hw iommu init failures

[Chris Wright <chrisw@sous-sol.org>]

* FUJITA Tomonori (fujita.tomonori@lab.ntt.co.jp) wrote:
> On Fri, 23 Oct 2009 09:39:23 -0700
> Chris Wright <chrisw@sous-sol.org> wrote:
> 
> > > The concept sounds fine but the third patch doesn't look correct.
> > > 
> > > Seems that the third patch doesn't take into account enabling both hw
> > > iommu and swiotlb (Calgary does and I guess VT-d and AMD need that
> > > too).
> > 
> > VT-d isn't using swiotlb.  Nor is AMD, although I think it will pick up
> > no_iommu on passthrough (seems like it would benefit from swiotlb in
> > that case).
> 
> I think that they need swiotlb for the same reason why Calgray needs
> it. IIRC, someone in VT-d camp said so.

Right, it was used as fallback for a bit when pass through mode was
first enabled to handle case where device has dma mask smaller than
physical memory.  That was removed, it was just recently w/ Alex's
comments that the idea of putting it back came up.

> > >  Also (iommu_detected && !dma_ops) trick doesn't work for
> > > Calgary, IIRC.
> > 
> > Yes, I think you are right.  I had stared at the calgary code and thought
> > it would DTRT due to calgary's use of no_iommu as fallback, but instead
> > it will never pick up swiotlb_dma_ops.
> 
> Note that Calgary comment 'falling back to no_iommu' is misleading. It
> actually falls back to swiotlb or nommu properly.
> 
> Calgary doesn't set to dma_ops to calgary_dma_ops so it doesn't need
> to pick up swiotlb_dma_ops.

It does need swiotlb_dma_ops even when calgary init succeeds for the devices
that aren't behind Calgary to deal w/ the case of those devices having
dma mask smaller than physical memory (i.e those that don't get device
specific dma_ops set to calgary_dma_ops).  So, the problem with this
patch is that it would fall back to nommu_dma_ops in cases where it was
expecting to fall back to swiotlb.

> > The calgary shouldn't even need to be manually setting up
> > nommu_dma_ops.
> 
> Yeah, but it needs because of how the dma startup code works.

Be much better to have the core handle all of this. Basically register
ops and then put the top one on the stack to actual use.  So the
fallback would be automatic, just pick the top of the stack and go.
Were you thinking of something along those lines?

thanks,
-chris