Re: [Qemu-devel] [PATCH 00/15] Memory/IOMMU patches part 4: region ownership

[Paolo Bonzini <pbonzini@redhat.com>]

Il 03/06/2013 11:58, Peter Maydell ha scritto:
> On 3 June 2013 10:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 03/06/2013 11:22, Peter Maydell ha scritto:
>>> arm11mpcore.c does this:
>>>    sysbus_init_mmio(dev, sysbus_mmio_get_region(s->priv, 0));
>>> which I suspect will assert with your patches.
>>
>> Thanks for the pointer.  All other occurrences of
>> sys_bus_mmio_get_region are either in non-qdevified OMAP code, or they
>> do what I said in my patch.
>>
>> I'll fix a11mpcore to use an alias.
> 
> Why? There is no need to -- this should be a perfectly
> reasonable use of MemoryRegion*s. If your reference counting
> code can't deal with it you need to fix the reference
> counting code.

It can:

1) I could set the owner to NULL before calling the sysbus_init_mmio;

2) I could add a variant of sysbus_init_mmio that doesn't set the owner;

3) I could skip setting the owner for sysbus altogether, since it is
only strictly required for unpluggable devices.

What I cannot do is having two owners, where each ref/unref pair will
ref/unref both objects.  I think you agree that it is overkill.

However, I think there is worth in preserving the chain through either
containment or aliasing.  From the nesting point of view,
realview_mpcore is exposing the region.  From the implementor point of
view, arm_gic is implementing the region (and arm_gic is the one that
would be ref/unref'd at execution time).  In either case,
arm11mpcore_priv is not what you want to see.  Its presence is just an
implementation detail of realview_mpcore.

Paolo