From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:44113) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO51r-0002Ov-GZ for qemu-devel@nongnu.org; Sun, 22 May 2011 05:32:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QO51q-0007BQ-FC for qemu-devel@nongnu.org; Sun, 22 May 2011 05:32:55 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:50818) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QO51q-0007BK-CQ for qemu-devel@nongnu.org; Sun, 22 May 2011 05:32:54 -0400 Received: by qyk10 with SMTP id 10so2873079qyk.4 for ; Sun, 22 May 2011 02:32:53 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4DD8B103.6080409@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> From: Blue Swirl Date: Sun, 22 May 2011 12:32:33 +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 9:45 AM, Avi Kivity wrote: > On 05/20/2011 08:59 PM, Blue Swirl wrote: >> >> On Thu, May 19, 2011 at 5:12 PM, Avi Kivity =C2=A0wrote: >> > =C2=A0The memory API separates the attributes of a memory region (its = size, >> > how >> > =C2=A0reads or writes are handled, dirty logging, and coalescing) from= where >> > it >> > =C2=A0is mapped and whether it is enabled. =C2=A0This allows a device = to configure >> > =C2=A0a memory region once, then hand it off to its parent bus to map = it >> > according >> > =C2=A0to the bus configuration. >> > >> > =C2=A0Hierarchical registration also allows a device to compose a regi= on out >> > of >> > =C2=A0a number of sub-regions with different properties; for example s= ome may >> > be >> > =C2=A0RAM while others may be MMIO. > >> > =C2=A0+void memory_region_set_log(MemoryRegion *mr, bool log, unsigned >> > client); >> > =C2=A0+/* Enable memory coalescing for the region. =C2=A0MMIO ->write = callbacks >> > may be >> > =C2=A0+ * delayed until a non-coalesced MMIO is issued. >> > =C2=A0+ */ >> > =C2=A0+void memory_region_set_coalescing(MemoryRegion *mr); >> > =C2=A0+/* Enable memory coalescing for a sub-range of the region. =C2= =A0MMIO >> > ->write >> > =C2=A0+ * callbacks may be delayed until a non-coalesced MMIO is issue= d. >> > =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=A0target_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=A0target_phys_addr_t = size); >> > =C2=A0+/* Disable MMIO coalescing for the region. */ >> > =C2=A0+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. =C2=A0We = 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. >> > =C2=A0+ * conflicts are resolved by having a higher @priority hide a l= ower >> > @priority. >> > =C2=A0+ * Subregions without priority are taken as @priority 0. >> > =C2=A0+ */ >> > =C2=A0+void memory_region_add_subregion_overlap(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=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 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 unsigned priority); >> > =C2=A0+/* Remove a subregion. */ >> > =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 MemoryRegion *subregion); >> >> What would the subregions be used for? > > Subregions describe the flow of data through the memory bus. =C2=A0We'd h= ave a > subregion for the PCI bus, with its own subregions for various BARs, with > some having subregions for dispatching different MMIO types within the BA= R. > > This allows, for example, the PCI layer to move a BAR without the PCI dev= ice > knowing anything about it. But why can't a first class region be used for that?