From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:49245) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoMAg-0002V7-Rw for qemu-devel@nongnu.org; Tue, 02 Aug 2011 17:06:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QoMAf-0008HG-GY for qemu-devel@nongnu.org; Tue, 02 Aug 2011 17:06:38 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:51303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoMAf-0008Ga-Al for qemu-devel@nongnu.org; Tue, 02 Aug 2011 17:06:37 -0400 Received: by yxt3 with SMTP id 3so133919yxt.4 for ; Tue, 02 Aug 2011 14:06:36 -0700 (PDT) Message-ID: <4E3866DA.2050608@codemonkey.ws> Date: Tue, 02 Aug 2011 16:06:34 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <4E381EA7.2070809@redhat.com> <4E383CE5.4010405@siemens.com> In-Reply-To: <4E383CE5.4010405@siemens.com> Content-Type: text/plain; charset=UTF-8; format=flowed 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: Jan Kiszka Cc: Peter Maydell , Avi Kivity , QEMU Developers On 08/02/2011 01:07 PM, Jan Kiszka wrote: > 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. The qdev level should be the common base that makes sense for *all* qdev devices. IRQ management does not belong in DeviceState because what you do for a simple LCD is not what you do for an MSI-X capable PCI device. This is what QOM properties tries to address. It should be possible to create a simple device, and register plugs/sockets for GPIO pins without pushing GPIO knowledge into the base class. In a QDev world, the right approach is to have a GpioDevice base class that implements this sort of logic for devices where it makes sense. That's what SysBusDevice sort of wants to be but it somehow ended up as yet another base class for everything. Regards, Anthony Liguori > Jan >