Re: [Qemu-devel] [PATCH v2 06/17] sysbus: add sysbus_pass_mmio

[Peter Maydell <peter.maydell@linaro.org>]

On 4 June 2013 15:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 04/06/2013 16:11, Peter Maydell ha scritto:
>> We've already got a working implementation, in the shape
>> of sysbus_mmio_get_region(). This is exactly the right way
>> to do this API -- we have one API which says "give me a
>> MemoryRegion*" and one which says "I have a MemoryRegion*,
>> please expose it". All I'm asking you to do is not break it.
>
> I can add a conditional to sysbus_add_mmio if you prefer.  I think it's
> uglier, but I can live with it.

No, I just think it shouldn't be setting the owner.

>> As a concrete example, if somebody submitted cirrus_vga
>> as a new driver, I have no idea how to tell that it needs
>> to set the owner for its memory regions, when 99% of
>> other devices don't. I think this is going to result in
>> "forgot to set owner" bugs.
>
> Because cirrus is adding regions directly to address_space_memory/io.
> As documented:
>
>  * The device must set the owner itself
>  * only if it uses memory_region_add_subregion directly on some address
>  * space, or after the parent region is passed to the bus (for example
>  * dynamically while the device runs).

OK, so why doesn't your patchset make the places in
hw/arm/omap1.c which add memory regions directly
to a subregion set the owner of the region? (or any
of the many other places where we do similar things).

thanks
-- PMM