From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40074) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMiyn-00019J-MZ for qemu-devel@nongnu.org; Wed, 18 May 2011 11:48:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QMiym-0002Hu-2V for qemu-devel@nongnu.org; Wed, 18 May 2011 11:48:09 -0400 Received: from moutng.kundenserver.de ([212.227.126.187]:61319) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMiyl-0002HB-L1 for qemu-devel@nongnu.org; Wed, 18 May 2011 11:48:08 -0400 Message-ID: <4DD3EA2F.4080200@mail.berlios.de> Date: Wed, 18 May 2011 17:47:59 +0200 From: Stefan Weil MIME-Version: 1.0 References: <4DD3C5B9.1080908@redhat.com> <4DD3E0E5.2030700@codemonkey.ws> In-Reply-To: <4DD3E0E5.2030700@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: Avi Kivity , qemu-devel Am 18.05.2011 17:08, schrieb Anthony Liguori: > On 05/18/2011 08:12 AM, 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. >> - 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 >> >> To fix that, I propose an new API to replace the existing one: >> >> >> #ifndef MEMORY_H >> #define MEMORY_H >> >> typedef struct MemoryRegionOps MemoryRegionOps; >> typedef struct MemoryRegion MemoryRegion; >> >> typedef uint32_t (*MemoryReadFunc)(MemoryRegion *mr, target_phys_addr_t >> addr); >> typedef void (*MemoryWriteFunc)(MemoryRegion *mr, target_phys_addr_t >> addr, >> uint32_t data); > > > The API should be 64-bit and needs to have a size associated with it. > >> struct MemoryRegionOps { >> MemoryReadFunc readb, readw, readl; >> MemoryWriteFunc writeb, writew, writel; >> }; > > I'm not a fan of having per-access type function pointers. Do you think of something like these declaration: typedef uint64_t (*MemoryReadFunc)(MemoryRegion *mr, target_phys_addr_t addr, size_t size); typedef void (*MemoryWriteFunc)(MemoryRegion *mr, target_phys_addr_t addr, uint64_t data, size_tsize); For 32 bit host / target, this would mean some unnecessary overhead. What about passing values by address: typedef void (*MemoryReadFunc)(MemoryRegion *mr, target_phys_addr_t addr, void *data, size_t size); typedef void (*MemoryWriteFunc)(MemoryRegion *mr, target_phys_addr_t addr, const void *data, size_t size); If we keep per-access type function pointers, they should use individual prototypes for the different access types: typedef uint8_t (*MemoryReadbFunc)(MemoryRegion *mr, target_phys_addr_t addr); typedef uint16_t (*MemoryReadwFunc)(MemoryRegion *mr, target_phys_addr_t addr); typedef uint32_t (*MemoryReadlFunc)(MemoryRegion *mr, target_phys_addr_t addr); typedef uint64_t (*MemoryReadllFunc)(MemoryRegion *mr, target_phys_addr_t addr); ... Regards, Stefan W.