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.