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

[Jan Kiszka]

On 2011-08-02 19:21, Peter Maydell wrote:
> On 2 August 2011 16:58, Avi Kivity <address@hidden> wrote:
>> On 08/02/2011 06:47 PM, Peter Maydell wrote:
>>> This kind of "I want to manage the memory layout of a pile of other
>>> things" seems like what the hierarchical memory API should provide,
>>> but there's a bit of a difficulty here in that sysbus MMIOs aren't
>>> necessarily MemoryRegions (and sysbus doesn't provide a "MemoryRegion*
>>> sysbus_get_memoryregion(SysBusDevice dev, int regionnumber)"
>>> anyway). How are we planning to move forward here? Will all sysbus
>>> MMIOs eventually be converted to MemoryRegions?
>>
>> Yes.
> 
> Cool. I guess this means struct SysBusDevice's struct { ... } mmio[]
> will turn into a MemoryRegion* mmio[] ?
> 
>>> Secondly, I'm not sure how to implement the gpmc size registers with
>>> the memory API: memory_region_add_subregion() lets you specify the
>>> offset you want to put the subregion at, but doesn't provide any way
>>> to say "limit the size of the mapping to this many bytes, regardless
>>> of how big the underlying device thinks it is".
>>
>> You can interpose you own container region:
>>
>>
>>  system_memory
>>     |
>>     +--- cs_region-0
>>             |
>>             +--- device-connected-to-that-region
>>
>> cs-region-0 will clip anything under it.
> 
> OK, and when we change the size of CS0 we delete the container
> region, create a new one of the new size and reconnect the
> device-region to the new container? That's a bit clumsy but it
> will work.

That's why I was asking for a memory_region_update service + region
description via some struct, not (only) via function arguments.

> 
>>> So maybe I'm approaching the problem wrong -- how should I be doing
>>> this?
>>
>> I don't think those devices should be connected to the sysbus (since they
>> aren't on real hardware).  Connect them to your gpmc instead.  If the
>> devices are already designed for sysbus, maybe we can dual-bus them, or make
>> gpmc have eight sysbuses hanging off it.
> 
> Actually I think in an ideal world omap_gpmc_attach() would
> simply take a MemoryRegion* :
>  void omap_gpmc_attach(DeviceState *gpmc, int cs,
>                        MemoryRegion *subdevice)
> 
> (for "NOR-like" devices, with a second separate one for NAND-like
> devices:
>  void omap_gpmc_attach_nand(DeviceState *gpmc, int cs,
>                             DeviceState *nanddevice)
> ...because for a NAND like device we need to do nand_setpins(),
> nand_setio(), etc, but for a NOR-like device we just need to map
> its memory.)
> 
> 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).
> 
> Then in some future new-qemu-object-model world when devices
> just directly expose their MemoryRegions it will be easy to
> just pass mydevice->registers to omap_gpmc_attach().
> 
> [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.]

And that's why we need GPIO/IRQ services at qdev level, not exclusively
for sysbus club members.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux