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

[Avi Kivity <avi@redhat.com>]

On 08/02/2011 10:38 PM, Peter Maydell wrote:
> On 2 August 2011 20:11, Avi Kivity<avi@redhat.com>  wrote:
> >  On 08/02/2011 09: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);
> >
> >  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 doesn't need it,
> all we need to pass it is a MemoryRegion *. 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.

The way I see it:

    cpu --> sysbus ---> gpmc ---> [ devices ]

If qdev supports memory regions, we can model this directly.  If not, we 
go via sysbus:

    cpu -> sysbus -> gpmc ---> [ sysbus ---> device ]s

(where the various sysbuses are all different instances)

having both the devices and gpmc sit on the same sysbus doesn't make 
sense to me.

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

So the machine model gives the device its bus (either a qbus embedded in 
gpmc, or a sysbus embedded in gpmc), and the device registers itself in 
its bus.  The machine model doesn't need to stitch individual memory 
regions.

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