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

[Marcel Apfelbaum]

On Tue, 2013-09-10 at 14:12 +0100, Peter Maydell wrote:
> On 10 September 2013 14:02, Michael S. Tsirkin <address@hidden> wrote:
> > On Tue, Sep 10, 2013 at 01:50:47PM +0100, Peter Maydell wrote:
> >> On 10 September 2013 13:39, Michael S. Tsirkin <address@hidden> wrote:
> >> > On Mon, Sep 09, 2013 at 02:16:41PM +0100, Peter Maydell wrote:
> >> >>     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?
> >>
> >> That should be itself a container,
> >> so assuming it doesn't
> >> itself have any kind of background region the "holes"
> >> inside it will still be present when we put it in
> >> our new container. (Basically putting a container,
> >> or an alias to one, inside a region is just saying
> >> "put everything in that container inside this region
> >> at the appropriate place").
> >
> > Confused.  "That" and "it" here refers to what exactly?
> 
> Well, I was a bit confused by your talking about
> the properties of "bus_master_enable_region" when my
> suggestion is effectively that we change what that is.
> So let's start again:
>  * create a container region
> This is 64 bits wide, but totally empty
>  * create a 'background' region at negative priority
> 64 bits wide
>  * put the alias and the background region into the container
> The alias is 64 bits wide too, but it is an alias of
> dma_as->root, which is a container with no background
> region.
Sadly, the priority is a local feature (per container),
so it cannot be used "cross container".
We need to find another way to leverage the fact that each device
has its own address space as target to transactions.
In the mean time I will submit another version that handles
only downstream transactions.
When we'll find a way to do it I will submit another series
Thanks for the help,
Marcel

>  * use the container as the bus_master_enable region
>  -- all you see in this container is the background region
> and anyhing that was in dma_as->root.
> 
> So when I said "that" and "it" I meant dma_as->root.
> 
> Hope that is a little less opaque.
> 
> -- PMM