On Mon, Sep 09, 2013 at 02:16:41PM +0100, Peter Maydell wrote: > On 9 September 2013 14:07, Marcel Apfelbaum <marce...@redhat.com> wrote: > > This is exactly my point. ALL device on the bus can be masters > > of a DMA transaction. So adding an interface as suggested by > > Michael: pci_set_master_for_master_abort(PCIBus *, PCIDevice *) > > for the general case (a device doing DMA) it is too far from reality. > > Actually I don't think it would be too painful. > At the moment in do_pci_register_device() we do this to > create the memory region used by a device for its bus > master transactions: > > memory_region_init_alias(&pci_dev->bus_master_enable_region, > OBJECT(pci_dev), "bus master", > dma_as->root, 0, > memory_region_size(dma_as->root)); > > If instead of using this alias directly as the > bus_master_enable region you instead: > * create a container region > * create a 'background' region at negative priority > (ie one per device, and you can make the 'opaque' pointer > point to the device, not the bus) > * put the alias and the background region into the container > * use the container as the bus_master_enable region
Interesting. There's one thing I don't understand here: as far as I can see bus_master_enable_region covers the whole 64 bit memory address space. It looks like it will always override the background region in the same container. What did I miss? > then you will get in your callback a pointer to the > device which caused the abort. You can then have your > callback call a method defined on PCIDevice which we > implement: > * as do-nothing in the PCI device base class > * as set-the-master-abort bit in the PCI host bridge > class > (and anybody who wants to get fancy about handling aborts > can override it in their own device implementation) > > That seems achievable without really requiring new > infrastructure. Have I missed something that wouldn't > work if we did this? > > thanks > -- PMM Actually, I think a base class would have to set received master abort bit in the status register. And it's not even that simple: memory writes are completed by a P2P bridge so I think it has to set a bit in the primary status register for the bridge and not for the device (though I'm speaking from memory, need to check the spec). -- MST
