On Tue, Jun 12, 2012 at 12:31:58PM +0300, Avi Kivity wrote: > On 06/11/2012 09:35 PM, Anthony Liguori wrote: > >> > >> The other direction is currently cpu_physical_memory_rw(). > > > > Right, and with benh's proposal, it's dma_memory_rw(). It also adds a > > DMAContext parameter. > > > > I can't help think that the contents of DMAContext is awfully similar to > > MemoryRegionOps. > > > >> We do need > >> to support issuing transactions from arbitrary points in the memory > >> hierarchy, but I don't think a device's MemoryRegion is the right > >> interface. Being able to respond to memory transactions, and being able > >> to issue them are two different things. > > > > But an IOMMU has to be able to respond to a memory transaction. Many of > > the things it may do (like endianness conversion) also happen to already > > exist in the memory API. > > Right; it's both an initiator and a target. We'll need something for > initiators, we have MemoryRegion for targets, and iommus will use both. > > > > >> In fact we will probably have to add more details to the memory > >> hierarchy. Currently (for example) we model the memory hub passing > >> transactions destined for the various pci windows to the pci bus, but we > >> don't model the memory hub receiving a pci-initiated transaction and > >> passing it to the system bus. We simply pretend it originated on the > >> system bus in the first place. Perhaps we need parallel hierarchies: > >> > >> system_memory > >> alias -> pci > >> alias -> ram > >> pci > >> bar1 > >> bar2 > >> pcibm > >> alias -> pci (prio 1) > >> alias -> system_memory (prio 0) > >> > >> cpu_physical_memory_rw() would be implemented as > >> memory_region_rw(system_memory, ...) while pci_dma_rw() would be > >> implemented as memory_region_rw(pcibm, ...). This would allow different > >> address transformations for the two accesses. > > > > Yeah, this is what I'm basically thinking although I don't quite > > understand what 'pcibm' stands for. > > PCI bus master > > > > > My biggest worry is that we'll end up with parallel memory API > > implementations split between memory.c and dma.c. > > It definitely belongs together.
I agree. Would be good to keep DMA out of the naming. It's just about adding support for multiple initiators/masters (DMA is just one of the use-cases).. Cheers, Edgar