On Sun, May 22, 2011 at 3:18 PM, Avi Kivity <a...@redhat.com> wrote: > On 05/22/2011 03:06 PM, Blue Swirl wrote: >> >> On Sun, May 22, 2011 at 2:36 PM, Avi Kivity<a...@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.
Since we don't know about those yet, coalescing API could be like you propose. Later it can be changed to the property API, or leave for convenience. >> > >> > 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? How about memory_region_container_init() memory_region_add()
