From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:44026) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoJNT-0001lB-Ai for qemu-devel@nongnu.org; Tue, 02 Aug 2011 14:07:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QoJNR-0000NU-Ef for qemu-devel@nongnu.org; Tue, 02 Aug 2011 14:07:39 -0400 Received: from thoth.sbs.de ([192.35.17.2]:17071) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoJNR-0000Mr-4b for qemu-devel@nongnu.org; Tue, 02 Aug 2011 14:07:37 -0400 Message-ID: <4E383CE5.4010405@siemens.com> Date: Tue, 02 Aug 2011 20:07:33 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <4E381EA7.2070809@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] modelling omap_gpmc with the hierarchical memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Avi Kivity , QEMU Developers On 2011-08-02 19:21, Peter Maydell wrote: > On 2 August 2011 16:58, Avi Kivity wrote: >> On 08/02/2011 06:47 PM, Peter Maydell wrote: >>> This kind of "I want to manage the memory layout of a pile of other >>> things" seems like what the hierarchical memory API should provide, >>> but there's a bit of a difficulty here in that sysbus MMIOs aren't >>> necessarily MemoryRegions (and sysbus doesn't provide a "MemoryRegion* >>> sysbus_get_memoryregion(SysBusDevice dev, int regionnumber)" >>> anyway). How are we planning to move forward here? Will all sysbus >>> MMIOs eventually be converted to MemoryRegions? >> >> Yes. > > Cool. I guess this means struct SysBusDevice's struct { ... } mmio[] > will turn into a MemoryRegion* mmio[] ? > >>> Secondly, I'm not sure how to implement the gpmc size registers with >>> the memory API: memory_region_add_subregion() lets you specify the >>> offset you want to put the subregion at, but doesn't provide any way >>> to say "limit the size of the mapping to this many bytes, regardless >>> of how big the underlying device thinks it is". >> >> You can interpose you own container region: >> >> >> system_memory >> | >> +--- cs_region-0 >> | >> +--- device-connected-to-that-region >> >> cs-region-0 will clip anything under it. > > OK, and when we change the size of CS0 we delete the container > region, create a new one of the new size and reconnect the > device-region to the new container? That's a bit clumsy but it > will work. That's why I was asking for a memory_region_update service + region description via some struct, not (only) via function arguments. > >>> So maybe I'm approaching the problem wrong -- how should I be doing >>> this? >> >> I don't think those devices should be connected to the sysbus (since they >> aren't on real hardware). Connect them to your gpmc instead. If the >> devices are already designed for sysbus, maybe we can dual-bus them, or make >> gpmc have eight sysbuses hanging off it. > > Actually I think in an ideal world omap_gpmc_attach() would > simply take a MemoryRegion* : > void omap_gpmc_attach(DeviceState *gpmc, int cs, > MemoryRegion *subdevice) > > (for "NOR-like" devices, with a second separate one for NAND-like > devices: > void omap_gpmc_attach_nand(DeviceState *gpmc, int cs, > DeviceState *nanddevice) > ...because for a NAND like device we need to do nand_setpins(), > nand_setio(), etc, but for a NOR-like device we just need to map > its memory.) > > So I think we just need a sysbus_mmio_get_memoryregion() > (and convert the devices I need to attach to use memory > regions, and live with not being able to attach unconverted > devices). > > Then in some future new-qemu-object-model world when devices > just directly expose their MemoryRegions it will be easy to > just pass mydevice->registers to omap_gpmc_attach(). > > [That is, the only reason I'm passing SysBus objects around > is that at the moment that is the only useful abstraction we > have for saying "I'm an arbitrary device object and I provide > some GPIO pins and some memory mappable regions". MemoryRegion* > allows me to pass around a memory mappable region in a more > direct way than having to pass a (SysBus*, mmio_index) tuple.] And that's why we need GPIO/IRQ services at qdev level, not exclusively for sysbus club members. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux