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

[Peter Maydell <peter.maydell@linaro.org>]

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.

So when I said "that" and "it" I meant dma_as->root.

Hope that is a little less opaque.

-- PMM