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

[Anthony Liguori <anthony@codemonkey.ws>]

On 08/02/2011 01:21 PM, Peter Maydell wrote:
> On 2 August 2011 19:05, Avi Kivity<avi@redhat.com>  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
>