From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjCnR-0004uj-Kz for qemu-devel@nongnu.org; Tue, 19 Jul 2011 12:05:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QjCnP-0004ZK-MN for qemu-devel@nongnu.org; Tue, 19 Jul 2011 12:05:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53034) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QjCnO-0004Z9-SY for qemu-devel@nongnu.org; Tue, 19 Jul 2011 12:05:19 -0400 Message-ID: <4E25AB37.2030808@redhat.com> Date: Tue, 19 Jul 2011 19:05:11 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1310901265-32051-1-git-send-email-avi@redhat.com> <4E2581F4.5090004@codemonkey.ws> <4E25864A.10905@redhat.com> <4E2599BC.9070407@codemonkey.ws> In-Reply-To: <4E2599BC.9070407@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v4 00/58] Memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org On 07/19/2011 05:50 PM, Anthony Liguori wrote: > >>> >>> There's bits I don't like about the interface >> >> Which bits are these? > > Nothing I haven't already commented on. I think there's too much in > the generic level. I don't think coalesced I/O belongs here. It's a > concept that doesn't fit. I think a side-band API would be nicer. Well, it's impossible to do it in a side band. When a range that has coalesced mmio is exposed is completely orthogonal to programming the BAR register - it can happen, for example, due to another BAR being removed or the bridge window being programmed. You can also have a coalesced mmio region being partially clipped. > > Endianness also seems out of place. There are many layers that can > affect final endianness. It depends on how devices handle endianness > and also whether the bus modifies endianness. That is handled naturally by the API. Currently only leaves specify endianess, but in the futures containers (=buses) would as well. > > There are numerous devices that have a register that allows endianness > to be toggled for the device. That makes the actual endianness of the > device dynamic which doesn't fit the memory region API very well IMHO. static const MemoryRegionOps mydevice_ops = { ... .endianess_callback = mydevice_endianess, ... }; Or memory_region_set_endianess(...); > >> >>> but I think it's a huge improvement over what we have now so I'm >>> inclined to commit once it includes documentation. >>> >> >> My problem is that to start leveraging it, everything must flow through >> it. There are still several hundred call sites that are unconverted. > > Really several hundred? That surprises me. > $ git grep -w cpu_register_physical_memory | wc -l 222 $ git grep -w cpu_register_io_memory | wc -l 233 $ git grep -w qemu_ram_alloc | wc -l 113 $ git grep memory_region_init | wc -l 134 -- error compiling committee.c: too many arguments to function