On 4 June 2013 15:27, Paolo Bonzini <pbonz...@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
