Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem

[Jan Kiszka]

On 2012-08-27 20:20, Avi Kivity wrote:
> On 08/27/2012 11:17 AM, Jan Kiszka wrote:
>> On 2012-08-27 20:09, Avi Kivity wrote:
>>> On 08/27/2012 10:14 AM, Jan Kiszka wrote:
>>>>>
>>>>> Deregistration is fine, the problem is destruction.
>>>>>
>>>>
>>>> It isn't as you access memory region states that can change after
>>>> deregistration. Devices can remove memory regions from the mapping,
>>>> alter and then reinsert them. The last to steps must not happen while
>>>> anyone is still using a reference to that region.
>>>>
>>>
>>> Why not?  If the guest is accessing an mmio region while reconfiguring
>>> it in a way that changes its meaning, either the previous or the next
>>> meaning is valid.
>>
>> If the memory region owner sets the content to zero or even releases it
>> (nothing states a memory region can only live inside a device
>> structure), we will crash. Restricting how a memory region can be
>> created and handled after it was once registered somewhere is an
>> unnatural interface, waiting to cause subtle bugs.
> 
> Using an Object * allows the simple case to be really simple (object ==
> device) and the hard cases to be doable.
> 
> What would you suggest as a better interface?

To protect the life cycle of the object we manage in the memory layer:
regions. We don't manage devices there. If there is any implementation
benefit in having a full QOM object, then make memory regions objects.

I simply don't like this indirection, having the memory layer pick up
the opaque value of the region and interpret it. Even worse, apply
restrictions on how the dispatched objects, the regions, have to be
treated because of this.

Also, using memory regions to control the locking behaviour allows for
more fine-grained control. A device may expose certain regions with
self-managed locking while others, less time critical ones, can still be
handled under BQL for simplicity reasons.

Example: regions that translate MMIO to PIO (alpha-pci.c, every user of
isa_mmio.c, ...). If PIO dispatching runs outside of the BQL, these
regions must not be protected by the BQL anymore. At the same time, we
may not want to convert the device exposing the region to its own
locking scheme (yet). And we surely don't want to take the per-device
lock for this state-less PIO dispatching.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux