Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses

[Michael S. Tsirkin]

On Mon, Sep 09, 2013 at 02:16:41PM +0100, Peter Maydell wrote:
> On 9 September 2013 14:07, Marcel Apfelbaum <address@hidden> 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