[Qemu-devel] Re: [RFC PATCH 4/7] ide: IOMMU support

[Paul Brook]

> On Wed, Jul 14, 2010 at 02:53:03PM +0100, Paul Brook wrote:
> > > Memory accesses must go through the IOMMU layer.
> > 
> > No. Devices should not know or care whether an IOMMU is present.
> 
> They don't really care. iommu_get() et al. are convenience functions
> which can and do return NULL when there's no IOMMU and device code can
> pass that NULL around without checking. 

Devices should not need to know any of this. You're introducing a significant 
amount of duplication and complexity into every device.

The assumption that all accesses will go through the same IOMMU is also false. 
Accesses to devices on the same bus will not be translated by the IOMMU. 
Currently there are probably also other things that will break in this case, 
but your API seems fundamentally incapable of handling this.

> I could've probably made the r/w
> functions take only the DeviceState in addition to normal args, but
> wanted to avoid looking up the related structures on each I/O operation.

That's exactly what you should be doing.  If this is inefficient then there 
are much better ways of fixing this. e.g. by not having the device perform so 
many accesses, or by adding some sort of translation cache.

> > You should be adding a DeviceState argument to
> > cpu_physical_memory_{rw,map}. This should then handle IOMMU translation
> > transparently.
> > 
> > You also need to accomodate the the case where multiple IOMMU are
> > present.
> 
> We don't assume there's a single IOMMU in the generic layer. The
> callbacks within 'struct iommu' could very well dispatch the request to
> one of multiple, coexisting IOMMUs.

So you've now introduced yet another copy of the translation code. Not only 
does every device have to be IOMMU aware, but every IOMMU also has to be aware 
of nested IOMMUs.

Paul