From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51044) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMieP-0003Ud-Rp for qemu-devel@nongnu.org; Wed, 18 May 2011 11:27:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QMieK-0007C9-O8 for qemu-devel@nongnu.org; Wed, 18 May 2011 11:27:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9103) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMieK-0007Br-Gm for qemu-devel@nongnu.org; Wed, 18 May 2011 11:27:00 -0400 Message-ID: <4DD3E53F.202@redhat.com> Date: Wed, 18 May 2011 18:26:55 +0300 From: Avi Kivity MIME-Version: 1.0 References: <4DD3C5B9.1080908@redhat.com> <4DD3D236.90708@siemens.com> <4DD3E271.2010903@codemonkey.ws> In-Reply-To: <4DD3E271.2010903@codemonkey.ws> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC] Memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Jan Kiszka , qemu-devel On 05/18/2011 06:14 PM, Anthony Liguori wrote: > On 05/18/2011 09:05 AM, Jan Kiszka wrote: >> On 2011-05-18 15:12, Avi Kivity wrote: >>> The current memory APIs (cpu_register_io_memory, >>> cpu_register_physical_memory) suffer from a number of drawbacks: >>> >>> - lack of centralized bookkeeping >>> - a cpu_register_physical_memory() that overlaps an existing region >>> will overwrite the preexisting region; but a following >>> cpu_unregister_physical_memory() will not restore things to status quo >>> ante. >> >> Restoring is not the problem. The problem is that the current API >> deletes or truncates regions implicitly by overwriting. That makes >> tracking the layout hard, and it is also error-prone as the device that >> accidentally overlaps with some other device won't receive a >> notification of this potential conflict. >> >> Such implicite truncation or deletion must be avoided in a new API, >> forcing the users to explicitly reference an existing region when >> dropping or modifying it. But your API goes in the right direction. >> >>> - coalescing and dirty logging are all cleared on unregistering, so >>> the client has to re-issue them when re-registering >>> - lots of opaques >>> - no nesting >>> - if a region has multiple subregions that need different handling >>> (different callbacks, RAM interspersed with MMIO) then client code has >>> to deal with that manually >>> - we can't interpose code between a device and global memory >>> handling >> >> I would add another drawback: >> >> - Inability to identify the origin of a region accesses and handle >> them >> differently based on the source. >> >> That is at least problematic for the x86 APIC which is CPU local. >> Our >> current way do deal with it is, well, very creative and falls to >> dust if a guest actually tries to remap the APIC. > > This is about registration. Right now you can only register IO > intercepts in the chipset, not on a per-CPU basis. We could just as > easily have: > > CPUState { > MemoryRegion apic_region; > }; > > per_cpu_register_memory(env, &env->apic_region); > Right. Or all memory per-cpu, with two sub-regions: - global memory - overlaid apic memory for this, we need to have well defined semantics for overlap (perhaps a priority argument to memory_region_add_subregion). > To make that work. > > We need to revamp registration. We should be able to register at the > following levels: > > 1) per-CPU (new API) > > 2) chipset (effectively the current cpu_register_physical_memory) > > 3) per-BUS (pci_register_bar()) The important thing is that all of these take a MemoryRegion argument, so we don't have to duplicate the coalescing, logging, and other APIs. -- error compiling committee.c: too many arguments to function