qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] modelling omap_gpmc with the hierarchical memory API


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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]