From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48907) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO6xG-0006HB-Gq for qemu-devel@nongnu.org; Sun, 22 May 2011 07:36:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QO6xF-0007T6-Ji for qemu-devel@nongnu.org; Sun, 22 May 2011 07:36:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17407) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO6xF-0007T2-8B for qemu-devel@nongnu.org; Sun, 22 May 2011 07:36:17 -0400 Message-ID: <4DD8F52B.1070905@redhat.com> Date: Sun, 22 May 2011 14:36:11 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1305814352-15044-1-git-send-email-avi@redhat.com> <1305814352-15044-2-git-send-email-avi@redhat.com> <4DD8B103.6080409@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org 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. (motivation - hw/e1000.c). > >> > + * conflicts are resolved by having a higher @priority hide a lower > >> > @priority. > >> > + * Subregions without priority are taken as @priority 0. > >> > + */ > >> > +void memory_region_add_subregion_overlap(MemoryRegion *mr, > >> > + target_phys_addr_t offset, > >> > + MemoryRegion *subregion, > >> > + unsigned priority); > >> > +/* Remove a subregion. */ > >> > +void memory_region_del_subregion(MemoryRegion *mr, > >> > + MemoryRegion *subregion); > >> > >> What would the subregions be used for? > > > > Subregions describe the flow of data through the memory bus. We'd have a > > subregion for the PCI bus, with its own subregions for various BARs, with > > some having subregions for dispatching different MMIO types within the BAR. > > > > This allows, for example, the PCI layer to move a BAR without the PCI device > > knowing anything about it. > > But why can't a first class region be used for that? Subregions are first-class regions. In fact all regions are subregions except the root. 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 have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.