|
From: | Anthony Liguori |
Subject: | Re: [Qemu-devel] modelling omap_gpmc with the hierarchical memory API |
Date: | Tue, 02 Aug 2011 16:00:59 -0500 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10 |
On 08/02/2011 02:38 PM, Peter Maydell wrote:
On 2 August 2011 20:11, Avi Kivity<address@hidden> wrote:On 08/02/2011 09:21 PM, Peter Maydell wrote:On 2 August 2011 19:05, Avi Kivity<address@hidden> 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
[Prev in Thread] | Current Thread | [Next in Thread] |