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

[Michael S. Tsirkin]

On Sun, Sep 15, 2013 at 11:56:40AM +0100, Peter Maydell wrote:
> On 15 September 2013 08:14, Michael S. Tsirkin <address@hidden> wrote:
> > On Tue, Sep 10, 2013 at 02:12:56PM +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.
> >>  * 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?
> 
> It has a size of 64 bits but it doesn't necessarily have
> actual memory subregions with I/O operations covering the
> whole contiguous range from 0 to 2^64-1
> (that would only
> happen if the things responding to DMA cover the whole
> 64 bit address, which corresponds to the situation in hardware
> where something will respond to the bus master transaction
> for any address. The hardware still has the "if nothing
> responds do this" capability, it just never gets used.)
> 
> The case where the DMA root is an IOMMU could be
> a problem though, because address_space_translate()
> hardcodes io_mem_unassigned as the fallback if the
> IOMMU says "no, can't write here".
> 
> > 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?
> 
> The alias will win for the addresses it handles. But if
> the alias is a container with "holes" then it doesn't handle
> the "holes" and the lower priority background region will
> get them.
> 
> -- PMM

Confused. How can there be a container with holes?
I thought the only way to create non-contigious
configurations is by creating multiple aliases?


Imagine this configuration:

region B - subregion of A, from 0x1000 to 0x3000
region C - subregion of A, from 0x2000 to 0x4000

region D - subregion of B from offset 0 to 0x1000

If B has higher priority that C, then part of C
from 0x2000 to 0x3000 is hidden, even though B
is a container and there's no subregion of B covering
that address range.

Right?