Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity <avi@redhat.com>]

On 05/22/2011 03:06 PM, Blue Swirl wrote:
> On Sun, May 22, 2011 at 2:36 PM, Avi Kivity<avi@redhat.com>  wrote:
> >  On 05/22/2011 12:32 PM, Blue Swirl wrote:
> >>
> >>  >>    >      +void memory_region_add_coalescing(MemoryRegion *mr,
> >>  >>    >      +                                  target_phys_addr_t offset,
> >>  >>    >      +                                  target_phys_addr_t size);
> >>  >>    >      +/* Disable MMIO coalescing for the region. */
> >>  >>    >      +void memory_region_clear_coalescing(MemoryRegion *mr);
> >>  >>
> >>  >>    Perhaps the interface could be more generic, like
> >>  >>    +void memory_region_set_property(MemoryRegion *mr, unsigned flags);
> >>  >>    +void memory_region_clear_property(MemoryRegion *mr, unsigned flags);
> >>  >>
> >>  >
> >>  >    Coalescing is a complex property, not just a boolean attribute.  We
> >>  >  probably
> >>  >    will have a number of boolean attributes later, though.
> >>
> >>  But what is the difference between adding coalescing to an area and
> >>  setting the bit property 'coalescing' to an area? At least what you
> >>  propose now is not so complex that it couldn't be handled as a single
> >>  bit.
> >
> >  Look at the API - add_coalescing() sets the coalescing property on a
> >  subrange of the memory region, not the entire region.
>
> Right, but doesn't the same apply to any other properties, they may
> apply to a full range or just a subrange?

We'll know when we have more properties.  I expect most will be region-wide.


> >
> >  Subregions are first-class regions.  In fact all regions are subregions
> >  except the root.
>
> Oh, I see now. Maybe the comments should describe this. Or perhaps the
> terms should be something like 'bus/bridge/root' and 'region' instead
> of 'region' and 'subregion'?

Problem is, memory_region_add_subregion() adds both sub-bridges and leaf 
regions.

It's quite possible that BAR 0 will be a leaf region, and BAR 1 will be 
a sub-bridge.

Can you suggest an alternative naming for the API?


> >  It's a tree of regions, each level adding an offset, clipping, and perhaps
> >  other attributes, with the leaves providing actual memory (mmio or RAM).
>
> I thought that there are two classes of regions, like PCI device vs. a
> single BAR.

It's true in a way, except the mapping is not 1:1.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.