Re: [Qemu-devel] [PATCHv6 3/5] fw_cfg: move assert() and linking of fw_cfg device to the machine into instance_init()

[Mark Cave-Ayland]

On 23/06/17 19:50, Eduardo Habkost wrote:

>> Really, please go back to the earlier discussion around fw_cfg_init1()
>> and you'll see my original point (which matches what you just voiced).
> 
> Yep.  I was just not sure validation on realize was necessary or
> convenient.  It looks like we agree it is just convenient.
> 
> 
>>
>>> All that said, I don't have a strong argument against doing it on
>>> realize, except my gut feeling that this is not how qdev was
>>> designed[1].  If doing it on realize is the simplest way to do it, I
>>> won't be the one complaining.
>>>
>>> [1] I believe the original intent was to make every single device
>>>     user-creatable and define boards in a declarative way in config
>>>     files.  We are very far from that goal.
>>
>> I'm fine either way, I just wanted to accommodate Mark's preference,
>> because he seemed to strongly dislike having to call any helper
>> functions from board code, beyond initing and realizing the device.
>>
>> This is my recollection of the earlier discussion anyway.
> 
> I think we are in agreement, then.  If there are no objections from
> others against doing object_property_add_child() on realize, I'm also OK
> with that.

Just to clarify here that my objection wasn't so much about calling
helper functions from board code, it was that as the current patch
exposes the MemoryRegions in TYPE_FW_CFG_IO and TYPE_FW_CFG_MEM then as
per my example where the machine link is omitted then the check becomes
useless.

I can see that device_set_realized() will always set the device parent
to /machine/unattached before calling the realize function if the device
doesn't have a parent. So is it even possible to add the device via
object_property_add_child() to the machine during realize? Or could it
be done by making /machine/fw_cfg an alias to its real location in the
QOM tree at realize time without breaking the object_resolve_path_type()
check?

The other interesting option to explore is that since fw_cfg already has
a machine_ready notifier, the check could be moved there similar to as
done in hw/core/machine.c's error_on_sysbus_device() if the check
shouldn't be present in realize. That still doesn't answer the question
as to how to enforce that the device is correctly linked to the machine
though.


ATB,

Mark.