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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Sep 10, 2013 at 02:12:56PM +0100, Peter Maydell wrote:
> On 10 September 2013 14:02, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Sep 10, 2013 at 01:50:47PM +0100, Peter Maydell wrote:
> >> On 10 September 2013 13:39, Michael S. Tsirkin <mst@redhat.com> 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.
>  * 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.

This confused me even more.
dma root covers whole 64 bit doesn't it?

The doc says:
"This is done with memory_region_add_subregion_overlap(), which
allows the region to overlap any other region in the same container, and
specifies a priority that allows the core to decide which of two regions
at
the same address are visible (highest wins)."

So if I create an alias that also covers whole 64 bit
and background in the same
container, background with a negative priority,
won't alias always win?

> So when I said "that" and "it" I meant dma_as->root.
> 
> Hope that is a little less opaque.
> 
> -- PMM