From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [Qemu-devel] [RFC v4 00/58] Memory API Date: Tue, 19 Jul 2011 19:05:11 +0300 Message-ID: <4E25AB37.2030808@redhat.com> References: <1310901265-32051-1-git-send-email-avi@redhat.com> <4E2581F4.5090004@codemonkey.ws> <4E25864A.10905@redhat.com> <4E2599BC.9070407@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org To: Anthony Liguori Return-path: Received: from mx1.redhat.com ([209.132.183.28]:8051 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751871Ab1GSQFT (ORCPT ); Tue, 19 Jul 2011 12:05:19 -0400 In-Reply-To: <4E2599BC.9070407@codemonkey.ws> Sender: kvm-owner@vger.kernel.org List-ID: 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