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

[Anthony Liguori]

On 08/02/2011 01: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);
>
> 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*.

Hrm, this looks like badness to me.

You're effectively using MemoryRegions to implement an ad-hoc interface. 
 This is not what MemoryRegions are meant to do though.  You want 
something like:

class WhateverDevice : public Device, implements SimpleDevice
{
   MemoryRegion *get_memory_region(void);
};

class OmapGmc : public Device
{
   SimpleDevice *slots[8];
};

In qdev of today, you should implement something other than SysBus as a 
base class and make OmapGmc a bus.

> > > [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.]
> >
> > I think I see.  Perhaps you're describing qdev/MemoryRegion integration.
>
> I think qdev devices need to be able to expose MemoryRegions
> as first class named 'properties' or 'plugs' or 'sockets' or
> whatever we want to call them, yes. (Ditto gpio/irq, which at
> the moment we can kind of expose but not by name.)

I disagree in this case.  I think MemoryRegion is a bit too low level of 
a connecting point.  I think an interface would be a stronger interface 
to use.

What's the relationship between the omap_gpmc and the devices in real 
hardware?  Are the devices designed to connect to the GPMC explicitly 
via a common set of pins?  Is there an intermediate bridge chip or 
something?

Regards,

Anthony Liguori

> -- PMM