Re: [Qemu-devel] [PATCH 04/13] usb-ohci: Use universal DMA helper functions

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jun 21, 2012 at 08:02:06AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2012-06-20 at 16:40 -0500, Anthony Liguori wrote:
> 
> > Well let's return void in the DMA methods and let the IOMMUs assert on error. 
> > At least that will avoid surprises until someone decides they care enough about 
> > errors to touch all callers.
> > 
> > I think silently failing a memcpy() can potentially lead to a vulnerability so 
> > I'd rather avoid that.
> 
> No I'd rather keep the error returns, really, even if that means fixing
> a few devices. I can look at making sure we don't pass random qemu data,
> on error that's reasonably easy.
> 
> assert on error means guest code can assert qemu ... not a great idea
> but maybe we can add a warning.

Why not?  Guest can always just halt if it wants to anyway.
On the other hand, warnings can fill up host logs so
represent a security problem.

> > >> Why leave pci accessors and not implement usb_memory_rw() wrappers?
> > >
> > > Well, "usb" is a bit too generic, ehci and ohci would each need to have
> > > their own sets of wrappers. But yes, that's possible... is it really
> > > worth it ? There's nothing fundamentally wrong with using the dma_*
> > > accessors.
> > 
> > So is using the pci accessors wrong?
> 
> Not really either, I don't think it matters :-)
> 
> > I'm not saying you should go and convert every caller of the pci_ functions, I 
> > just want a clear policy on what interface devices should use.
> 
> Ideally the bus interface for the bus they sit on so they don't have to
> bother digging the DMAContext and are immune to change we would do in
> that area.
> 
> Devices that mix multiple bus types however are a bit more tricky, but
> so far are few, and those can use dma_* and know where to get the
> DMAContext from.
> 
> If we ever replace DMAContext with something else we can probably just
> change the field to that "something else" with a very simple
> search/replace on those devices (at least that's the best case :-)
> 
> I think anything else is just no worth bothering.
> 
> Cheers,
> Ben.