On Mon, Apr 27, 2015 at 8:01 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > > On 27/04/2015 16:56, Eric Auger wrote: >> Peter, Paolo, >> >> After your feedbacks, I feel I need to spend some more time on the >> original check() track. I would prefer not to introduce any patch that >> will make issue in the future. > > Peter, see the other threads between me and Eric. See in particular > http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02749.html > starting at "The notifier actually is not even necessary" and the > replies from there. > Thanks, I see the problem with check. In this reply http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02956.html Eric says that the problem with the check hook is it happens before the setting. I think this can be solved with a RYO link setter for GPIOs. We almost have an in-tree precedent with MemoryRegion and the container property (memory.c): 960 op = object_property_add(OBJECT(mr), "container", 961 "link<" TYPE_MEMORY_REGION ">", 962 memory_region_get_container, 963 NULL, /* memory_region_set_container */ 964 NULL, NULL, &error_abort); Now in reality we could have done this link normal style as it is only a trivial getter, but the reason the link was done this way in the first place, is because I have a follow up patch to memory.c that adds a customer Link setter: +static void memory_region_set_container(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + Error *local_err = NULL; + MemoryRegion *old_container = mr->container; + MemoryRegion *new_container = NULL; + char *path = NULL; + + visit_type_str(v, &path, name, &local_err); + + if (!local_err && strcmp(path, "") != 0) { + new_container = MEMORY_REGION(object_resolve_link(obj, name, path, + &local_err)); + while (new_container->alias) { + new_container = new_container->alias; + } + } + + if (local_err) { + error_propagate(errp, local_err); + return; + } + + object_ref(OBJECT(new_container)); + + memory_region_transaction_begin(); + memory_region_ref(mr); + if (old_container) { + memory_region_del_subregion(old_container, mr); + } + mr->container = new_container; + if (new_container) { + memory_region_update_container_subregions(mr); + } + memory_region_unref(mr); + memory_region_transaction_commit(); + + object_unref(OBJECT(old_container)); +} + op = object_property_add(OBJECT(mr), "container", "link<" TYPE_MEMORY_REGION ">", memory_region_get_container, - NULL, /* memory_region_set_container */ + memory_region_set_container, NULL, NULL, &error_abort); The function does the normal link setting - similar to object_set_link_property (not to be confused with object_property_set_link!) but is surrounded by class specific side effects. Specifically in this case, it does memory_region_transaction_begin/ref/unref/commit etc for the MR. For this GPIO case, you could create a custom setter that does the normal set, then calls the DeviceClass installed hook (or you can install the hook to the object and init it at qdev_init_gpio_out_named time as suggested in the eariler thread). The callback will happen after the link is populated. To reduce verbosity, I suggest making object_set_link_property() a visible API, then RYO link setters can call it surrounded by custom behavior e.g: foo_object_set_bar_property(...) { pre_set_link_side_effects(); object_set_link_property(); post_set_link_side_effects(); } object_set_link_property() would need to be coreified and wrapped to remove it's awareness of LinkProperty type (as that doesn't exist in RYO properties) in this case. Regards, Peter > If you have any idea, please help. > > Paolo > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
