From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60149) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO7bm-0005P1-FF for qemu-devel@nongnu.org; Sun, 22 May 2011 08:18:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QO7bl-0005Qc-Hz for qemu-devel@nongnu.org; Sun, 22 May 2011 08:18:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39247) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO7bl-0005QP-9S for qemu-devel@nongnu.org; Sun, 22 May 2011 08:18:09 -0400 Message-ID: <4DD8FEFA.1020604@redhat.com> Date: Sun, 22 May 2011 15:18:02 +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> <4DD8F52B.1070905@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 03:06 PM, Blue Swirl wrote: > On Sun, May 22, 2011 at 2:36 PM, Avi Kivity 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.