From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34202) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO7QM-0002tx-4t for qemu-devel@nongnu.org; Sun, 22 May 2011 08:06:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QO7QK-0003a1-R0 for qemu-devel@nongnu.org; Sun, 22 May 2011 08:06:22 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:53145) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO7QK-0003Zx-K2 for qemu-devel@nongnu.org; Sun, 22 May 2011 08:06:20 -0400 Received: by qwj8 with SMTP id 8so2898150qwj.4 for ; Sun, 22 May 2011 05:06:20 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4DD8F52B.1070905@redhat.com> 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> From: Blue Swirl Date: Sun, 22 May 2011 15:06:00 +0300 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Avi Kivity Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org On Sun, May 22, 2011 at 2:36 PM, Avi Kivity wrote: > On 05/22/2011 12:32 PM, Blue Swirl wrote: >> >> >> =C2=A0> =C2=A0 =C2=A0+void memory_region_add_coalescing(MemoryRegion = *mr, >> >> =C2=A0> =C2=A0 =C2=A0+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ta= rget_phys_addr_t offset, >> >> =C2=A0> =C2=A0 =C2=A0+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ta= rget_phys_addr_t size); >> >> =C2=A0> =C2=A0 =C2=A0+/* Disable MMIO coalescing for the region. */ >> >> =C2=A0> =C2=A0 =C2=A0+void memory_region_clear_coalescing(MemoryRegio= n *mr); >> >> >> >> =C2=A0Perhaps the interface could be more generic, like >> >> =C2=A0+void memory_region_set_property(MemoryRegion *mr, unsigned fla= gs); >> >> =C2=A0+void memory_region_clear_property(MemoryRegion *mr, unsigned f= lags); >> >> >> > >> > =C2=A0Coalescing is a complex property, not just a boolean attribute. = =C2=A0We >> > probably >> > =C2=A0will 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? > (motivation - hw/e1000.c). > >> >> =C2=A0> =C2=A0 =C2=A0+ * conflicts are resolved by having a higher @p= riority hide a >> >> lower >> >> =C2=A0> =C2=A0@priority. >> >> =C2=A0> =C2=A0 =C2=A0+ * Subregions without priority are taken as @pr= iority 0. >> >> =C2=A0> =C2=A0 =C2=A0+ */ >> >> =C2=A0> =C2=A0 =C2=A0+void memory_region_add_subregion_overlap(Memory= Region *mr, >> >> =C2=A0> =C2=A0 =C2=A0+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 target_phys_addr_t >> >> offset, >> >> =C2=A0> =C2=A0 =C2=A0+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 MemoryRegion >> >> *subregion, >> >> =C2=A0> =C2=A0 =C2=A0+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 unsigned priority); >> >> =C2=A0> =C2=A0 =C2=A0+/* Remove a subregion. */ >> >> =C2=A0> =C2=A0 =C2=A0+void memory_region_del_subregion(MemoryRegion *= mr, >> >> =C2=A0> =C2=A0 =C2=A0+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 MemoryRe= gion *subregion); >> >> >> >> =C2=A0What would the subregions be used for? >> > >> > =C2=A0Subregions describe the flow of data through the memory bus. =C2= =A0We'd have >> > a >> > =C2=A0subregion for the PCI bus, with its own subregions for various B= ARs, >> > with >> > =C2=A0some having subregions for dispatching different MMIO types with= in the >> > BAR. >> > >> > =C2=A0This allows, for example, the PCI layer to move a BAR without th= e PCI >> > device >> > =C2=A0knowing anything about it. >> >> But why can't a first class region be used for that? > > Subregions are first-class regions. =C2=A0In fact all regions are subregi= ons > 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'? > It's a tree of regions, each level adding an offset, clipping, and perhap= s > 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.