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

[Avi Kivity]

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.

> 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.

> >>  [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.)

Let's hope some sucker gets volunteered into it.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.