On Sun, Jun 25, 2017 at 07:58:04PM +0100, Mark Cave-Ayland wrote: > 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 don't understand this part. When and why would the check become 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? Well, if we can't do object_property_add_child() on ->instance_init() and doing it on ->realize() would require a more complex solution involving QOM links, I believe the simplest solution is to provide a helper function. > > 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. I think both manually mapping+linking from board code or calling a helper function from board code would be acceptable. -- Eduardo
