From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51626) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoM5L-00008b-2W for qemu-devel@nongnu.org; Tue, 02 Aug 2011 17:01:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QoM5J-0006vl-Ox for qemu-devel@nongnu.org; Tue, 02 Aug 2011 17:01:07 -0400 Received: from mail-pz0-f43.google.com ([209.85.210.43]:64990) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoM5J-0006vQ-9U for qemu-devel@nongnu.org; Tue, 02 Aug 2011 17:01:05 -0400 Received: by pzk1 with SMTP id 1so291358pzk.30 for ; Tue, 02 Aug 2011 14:01:04 -0700 (PDT) Message-ID: <4E38658B.7080906@codemonkey.ws> Date: Tue, 02 Aug 2011 16:00:59 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <4E381EA7.2070809@redhat.com> <4E383C55.5050703@redhat.com> <4E384BF8.60204@redhat.com> In-Reply-To: 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: Peter Maydell Cc: Avi Kivity , QEMU Developers On 08/02/2011 02:38 PM, Peter Maydell wrote: > On 2 August 2011 20:11, Avi Kivity wrote: >> On 08/02/2011 09:21 PM, Peter Maydell wrote: >>> >>> On 2 August 2011 19:05, Avi Kivity wrote: >>>> On 08/02/2011 08:21 PM, Peter Maydell wrote: >>>>> 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). >>>> >>>> I don't follow - why do we need get_memoryregion? who would call it? >>> >>> The machine model would call it. So you do something like >>> DeviceState *dev = qdev_create(NULL, "whatever"); >>> /* Note the parallel here to the existing >>> * sysbus_mmio_map(sysbus_from_qdev(dev), mmio_idx, addr); >>> */ >>> MemoryRegion *mr = >>> sysbus_mmio_get_memoryregion(sysbus_from_qdev(dev), mmio_idx); >>> omap_gpmc_attach(gpmc, 7, mr); >> >> This is where the gpmc provides the sysbus. It doesn't need to call >> get_memoryregion() on itself. > > Why should the gpmc provide a sysbus? It shouldn't. The bus model is broken in qdev so it has to to make it work today. > It doesn't need it, > all we need to pass it is a MemoryRegion *. But the MemoryRegion * is your bus protocol. While it's useful to have the flexibility to make arbitrary connections for things like Pins, it's better to use higher level interfaces that are more strongly typed. That prevents silly thinks like trying to connect the MemoryRegion of an E1000 PCI card to the OMAP GPMC. > A bus would > imply multiple different things that could all sit on it > at different addresses, whereas if gpmc provided 8 different > sysbuses they'd each have either 0 or 1 child always at > address 0. You don't want a bus here all children are equal. You want: class GPMC : public Device { GPMCChild *slots[8]; }; I feel your pain, but today the way we do this is by having a bus and moving the addressing info into the child as a qdev property. > >>> ie the machine model is where we wire up the subdevices >>> to the gpmc, and at the machine model level what you have is >>> a pointer to an entire device, so you need to be able to >>> convert the (sysbus*, mmio_index) tuple to a MemoryRegion*. >> >> I believe that it is in general unnecessary. A device hands its bus a >> memory region, and the bus does with it what it will (generally mapping it >> into a container, and presenting the container to a parent bus). >> get_memoryregion() implies a third party. > > The third party here is the machine model. The machine model > owns and instantiates both gpmc and the subdevice. It wants > to wire them up. In the same way that you can use qdev_get_gpio_in > and qdev_connect_gpio_out to connect a gpio line from one thing > to another, you need to be able to connect a memory region > from one thing to another. I don't think GPIO is a good example to use because it suffers from the same problem (too low level of an interface). Regards, Anthony Liguori > > -- PMM >