From ef7c7ff6d4ca1955278af1bc5025f47044183250 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 18 Jun 2014 17:58:28 +0800 Subject: [PATCH 01/15] qom: add object_property_add_alias() Sometimes an object needs to present a property which is actually on another object, or it needs to provide an alias name for an existing property. Examples: a.foo -> b.foo a.old_name -> a.new_name The new object_property_add_alias() API allows objects to alias a property on the same object or another object. The source and target names can be different. Signed-off-by: Stefan Hajnoczi Reviewed-by: Peter Crosthwaite --- include/qom/object.h | 20 +++++++++++++++++ qom/object.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/include/qom/object.h b/include/qom/object.h index b882ccc85f..44c513f985 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1230,6 +1230,26 @@ void object_property_add_uint32_ptr(Object *obj, const char *name, void object_property_add_uint64_ptr(Object *obj, const char *name, const uint64_t *v, Error **Errp); +/** + * object_property_add_alias: + * @obj: the object to add a property to + * @name: the name of the property + * @target_obj: the object to forward property access to + * @target_name: the name of the property on the forwarded object + * @errp: if an error occurs, a pointer to an area to store the error + * + * Add an alias for a property on an object. This function will add a property + * of the same type as the forwarded property. + * + * The caller must ensure that @target_obj stays alive as long as + * this property exists. In the case of a child object or an alias on the same + * object this will be the case. For aliases to other objects the caller is + * responsible for taking a reference. + */ +void object_property_add_alias(Object *obj, const char *name, + Object *target_obj, const char *target_name, + Error **errp); + /** * object_child_foreach: * @obj: the object whose children will be navigated diff --git a/qom/object.c b/qom/object.c index 3876618c2e..a760514787 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1550,6 +1550,57 @@ void object_property_add_uint64_ptr(Object *obj, const char *name, NULL, NULL, (void *)v, errp); } +typedef struct { + Object *target_obj; + const char *target_name; +} AliasProperty; + +static void property_get_alias(Object *obj, struct Visitor *v, void *opaque, + const char *name, Error **errp) +{ + AliasProperty *prop = opaque; + + object_property_get(prop->target_obj, v, prop->target_name, errp); +} + +static void property_set_alias(Object *obj, struct Visitor *v, void *opaque, + const char *name, Error **errp) +{ + AliasProperty *prop = opaque; + + object_property_set(prop->target_obj, v, prop->target_name, errp); +} + +static void property_release_alias(Object *obj, const char *name, void *opaque) +{ + AliasProperty *prop = opaque; + + g_free(prop); +} + +void object_property_add_alias(Object *obj, const char *name, + Object *target_obj, const char *target_name, + Error **errp) +{ + AliasProperty *prop; + ObjectProperty *target_prop; + + target_prop = object_property_find(target_obj, target_name, errp); + if (!target_prop) { + return; + } + + prop = g_malloc(sizeof(*prop)); + prop->target_obj = target_obj; + prop->target_name = target_name; + + object_property_add(obj, name, target_prop->type, + property_get_alias, + property_set_alias, + property_release_alias, + prop, errp); +} + static void object_instance_init(Object *obj) { object_property_add_str(obj, "type", qdev_get_type, NULL, NULL); From 64607d088132abdb25bf30d93e97d0c8df7b364c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 5 Jun 2014 13:11:51 +0200 Subject: [PATCH 02/15] qom: add a generic mechanism to resolve paths It may be desirable to have custom link<> properties that do more than just store an object. Even the addition of a "check" function is not enough if setting the link has side effects or if a non-standard reference counting is preferrable. Avoid the assumption that the opaque field of a link<> is a LinkProperty struct, by adding a generic "resolve" callback to ObjectProperty. This fixes aliases of link properties. Signed-off-by: Paolo Bonzini --- include/qom/object.h | 34 +++++++++++++++--- qom/object.c | 82 ++++++++++++++++++++++++++++---------------- 2 files changed, 82 insertions(+), 34 deletions(-) diff --git a/include/qom/object.h b/include/qom/object.h index 44c513f985..8a05a81a99 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -303,6 +303,25 @@ typedef void (ObjectPropertyAccessor)(Object *obj, const char *name, Error **errp); +/** + * ObjectPropertyResolve: + * @obj: the object that owns the property + * @opaque: the opaque registered with the property + * @part: the name of the property + * + * Resolves the #Object corresponding to property @part. + * + * The returned object can also be used as a starting point + * to resolve a relative path starting with "@part". + * + * Returns: If @path is the path that led to @obj, the function + * returns the #Object corresponding to "@path/@part". + * If "@path/@part" is not a valid object path, it returns #NULL. + */ +typedef Object *(ObjectPropertyResolve)(Object *obj, + void *opaque, + const char *part); + /** * ObjectPropertyRelease: * @obj: the object that owns the property @@ -321,6 +340,7 @@ typedef struct ObjectProperty gchar *type; ObjectPropertyAccessor *get; ObjectPropertyAccessor *set; + ObjectPropertyResolve *resolve; ObjectPropertyRelease *release; void *opaque; @@ -787,12 +807,16 @@ void object_unref(Object *obj); * destruction. This may be NULL. * @opaque: an opaque pointer to pass to the callbacks for the property * @errp: returns an error if this function fails + * + * Returns: The #ObjectProperty; this can be used to set the @resolve + * callback for child and link properties. */ -void object_property_add(Object *obj, const char *name, const char *type, - ObjectPropertyAccessor *get, - ObjectPropertyAccessor *set, - ObjectPropertyRelease *release, - void *opaque, Error **errp); +ObjectProperty *object_property_add(Object *obj, const char *name, + const char *type, + ObjectPropertyAccessor *get, + ObjectPropertyAccessor *set, + ObjectPropertyRelease *release, + void *opaque, Error **errp); void object_property_del(Object *obj, const char *name, Error **errp); diff --git a/qom/object.c b/qom/object.c index a760514787..7a892ef4a6 100644 --- a/qom/object.c +++ b/qom/object.c @@ -356,11 +356,6 @@ static inline bool object_property_is_child(ObjectProperty *prop) return strstart(prop->type, "child<", NULL); } -static inline bool object_property_is_link(ObjectProperty *prop) -{ - return strstart(prop->type, "link<", NULL); -} - static void object_property_del_all(Object *obj) { while (!QTAILQ_EMPTY(&obj->properties)) { @@ -728,11 +723,12 @@ void object_unref(Object *obj) } } -void object_property_add(Object *obj, const char *name, const char *type, - ObjectPropertyAccessor *get, - ObjectPropertyAccessor *set, - ObjectPropertyRelease *release, - void *opaque, Error **errp) +ObjectProperty * +object_property_add(Object *obj, const char *name, const char *type, + ObjectPropertyAccessor *get, + ObjectPropertyAccessor *set, + ObjectPropertyRelease *release, + void *opaque, Error **errp) { ObjectProperty *prop; @@ -741,7 +737,7 @@ void object_property_add(Object *obj, const char *name, const char *type, error_setg(errp, "attempt to add duplicate property '%s'" " to object (type '%s')", name, object_get_typename(obj)); - return; + return NULL; } } @@ -756,6 +752,7 @@ void object_property_add(Object *obj, const char *name, const char *type, prop->opaque = opaque; QTAILQ_INSERT_TAIL(&obj->properties, prop, node); + return prop; } ObjectProperty *object_property_find(Object *obj, const char *name, @@ -1028,6 +1025,11 @@ static void object_get_child_property(Object *obj, Visitor *v, void *opaque, g_free(path); } +static Object *object_resolve_child_property(Object *parent, void *opaque, const gchar *part) +{ + return opaque; +} + static void object_finalize_child_property(Object *obj, const char *name, void *opaque) { @@ -1041,15 +1043,18 @@ void object_property_add_child(Object *obj, const char *name, { Error *local_err = NULL; gchar *type; + ObjectProperty *op; type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); - object_property_add(obj, name, type, object_get_child_property, NULL, - object_finalize_child_property, child, &local_err); + op = object_property_add(obj, name, type, object_get_child_property, NULL, + object_finalize_child_property, child, &local_err); if (local_err) { error_propagate(errp, local_err); goto out; } + + op->resolve = object_resolve_child_property; object_ref(child); g_assert(child->parent == NULL); child->parent = obj; @@ -1163,6 +1168,13 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, } } +static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) +{ + LinkProperty *lprop = opaque; + + return *lprop->child; +} + static void object_release_link_property(Object *obj, const char *name, void *opaque) { @@ -1184,6 +1196,7 @@ void object_property_add_link(Object *obj, const char *name, Error *local_err = NULL; LinkProperty *prop = g_malloc(sizeof(*prop)); gchar *full_type; + ObjectProperty *op; prop->child = child; prop->check = check; @@ -1191,17 +1204,21 @@ void object_property_add_link(Object *obj, const char *name, full_type = g_strdup_printf("link<%s>", type); - object_property_add(obj, name, full_type, - object_get_link_property, - check ? object_set_link_property : NULL, - object_release_link_property, - prop, - &local_err); + op = object_property_add(obj, name, full_type, + object_get_link_property, + check ? object_set_link_property : NULL, + object_release_link_property, + prop, + &local_err); if (local_err) { error_propagate(errp, local_err); g_free(prop); + goto out; } + op->resolve = object_resolve_link_property; + +out: g_free(full_type); } @@ -1260,11 +1277,8 @@ Object *object_resolve_path_component(Object *parent, const gchar *part) return NULL; } - if (object_property_is_link(prop)) { - LinkProperty *lprop = prop->opaque; - return *lprop->child; - } else if (object_property_is_child(prop)) { - return prop->opaque; + if (prop->resolve) { + return prop->resolve(parent, prop->opaque, part); } else { return NULL; } @@ -1571,6 +1585,14 @@ static void property_set_alias(Object *obj, struct Visitor *v, void *opaque, object_property_set(prop->target_obj, v, prop->target_name, errp); } +static Object *property_resolve_alias(Object *obj, void *opaque, + const gchar *part) +{ + AliasProperty *prop = opaque; + + return object_resolve_path_component(prop->target_obj, prop->target_name); +} + static void property_release_alias(Object *obj, const char *name, void *opaque) { AliasProperty *prop = opaque; @@ -1583,6 +1605,7 @@ void object_property_add_alias(Object *obj, const char *name, Error **errp) { AliasProperty *prop; + ObjectProperty *op; ObjectProperty *target_prop; target_prop = object_property_find(target_obj, target_name, errp); @@ -1594,11 +1617,12 @@ void object_property_add_alias(Object *obj, const char *name, prop->target_obj = target_obj; prop->target_name = target_name; - object_property_add(obj, name, target_prop->type, - property_get_alias, - property_set_alias, - property_release_alias, - prop, errp); + op = object_property_add(obj, name, target_prop->type, + property_get_alias, + property_set_alias, + property_release_alias, + prop, errp); + op->resolve = property_resolve_alias; } static void object_instance_init(Object *obj) From d190698e6f806198da42c05c35b623760b6e1f00 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jun 2014 11:17:35 +0200 Subject: [PATCH 03/15] qom: allow creating an alias of a child<> property Child properties must be unique. Fix this problem by turning their aliases into links. The resolve function that forwards to the target property does not have any knowledge of the target property's type, so it works fine. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- qom/object.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/qom/object.c b/qom/object.c index 7a892ef4a6..f49335f0cf 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1607,22 +1607,32 @@ void object_property_add_alias(Object *obj, const char *name, AliasProperty *prop; ObjectProperty *op; ObjectProperty *target_prop; + gchar *prop_type; target_prop = object_property_find(target_obj, target_name, errp); if (!target_prop) { return; } + if (object_property_is_child(target_prop)) { + prop_type = g_strdup_printf("link%s", + target_prop->type + strlen("child")); + } else { + prop_type = g_strdup(target_prop->type); + } + prop = g_malloc(sizeof(*prop)); prop->target_obj = target_obj; prop->target_name = target_name; - op = object_property_add(obj, name, target_prop->type, + op = object_property_add(obj, name, prop_type, property_get_alias, property_set_alias, property_release_alias, prop, errp); op->resolve = property_resolve_alias; + + g_free(prop_type); } static void object_instance_init(Object *obj) From 654a36d857ff949e0d1989904b76f53fded9dc83 Mon Sep 17 00:00:00 2001 From: Marcelo Tosatti Date: Wed, 4 Jun 2014 14:52:03 -0300 Subject: [PATCH 04/15] mc146818rtc: add "rtc-time" link to "/machine/rtc" Add a link to rtc under /machine providing a stable location for management apps to query the value of the time. The link should be added by any object that sends RTC_TIME_CHANGE events. {"execute":"qom-get","arguments":{"path":"/machine","property":"rtc-time"} } Suggested by Paolo Bonzini and Andreas Faerber. Signed-off-by: Marcelo Tosatti Signed-off-by: Paolo Bonzini --- hw/timer/mc146818rtc.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index 307732c744..9d817cab78 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -909,6 +909,9 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) object_property_add(OBJECT(s), "date", "struct tm", rtc_get_date, NULL, NULL, s, NULL); + + object_property_add_alias(qdev_get_machine(), "rtc-time", + OBJECT(s), "date", NULL); } ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq) @@ -950,11 +953,17 @@ static void rtc_class_initfn(ObjectClass *klass, void *data) dc->cannot_instantiate_with_device_add_yet = true; } +static void rtc_finalize(Object *obj) +{ + object_property_del(qdev_get_machine(), "rtc", NULL); +} + static const TypeInfo mc146818rtc_info = { .name = TYPE_MC146818_RTC, .parent = TYPE_ISA_DEVICE, .instance_size = sizeof(RTCState), .class_init = rtc_class_initfn, + .instance_finalize = rtc_finalize, }; static void mc146818rtc_register_types(void) From c28322d10cd5f0529605b48684d2b82c9eb9c020 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Mon, 26 May 2014 17:39:51 -0700 Subject: [PATCH 05/15] qom: object: remove parent pointer when unparenting Certain parts of the QOM framework test this pointer to determine if an object is parented. Nuke it when the object is unparented to allow for reuse of an object after unparenting. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- qom/object.c | 1 + 1 file changed, 1 insertion(+) diff --git a/qom/object.c b/qom/object.c index f49335f0cf..d5de8f6062 100644 --- a/qom/object.c +++ b/qom/object.c @@ -397,6 +397,7 @@ void object_unparent(Object *obj) } if (obj->parent) { object_property_del_child(obj->parent, obj, NULL); + obj->parent = NULL; } object_unref(obj); } From 8ffad850ef5ae14287d0e185d478c9a35820482c Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Thu, 5 Jun 2014 23:13:36 -0700 Subject: [PATCH 06/15] qom: object: Ignore refs/unrefs of NULL Just do nothing if passed NULL for a ref or unref. This avoids call sites that manage a combination of NULL or non-NULL pointers having to add iffery around every ref and unref. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- qom/object.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/qom/object.c b/qom/object.c index d5de8f6062..0e8267bc2a 100644 --- a/qom/object.c +++ b/qom/object.c @@ -711,11 +711,17 @@ GSList *object_class_get_list(const char *implements_type, void object_ref(Object *obj) { + if (!obj) { + return; + } atomic_inc(&obj->ref); } void object_unref(Object *obj) { + if (!obj) { + return; + } g_assert(obj->ref > 0); /* parent always holds a reference to its children */ @@ -1160,13 +1166,9 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, return; } - if (new_target) { - object_ref(new_target); - } + object_ref(new_target); *child = new_target; - if (old_target != NULL) { - object_unref(old_target); - } + object_unref(old_target); } static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part) From 563890c7c7e977842e2a35afe7a24d06d2103242 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jun 2014 09:35:18 +0200 Subject: [PATCH 07/15] libqtest: escape strings in QMP commands, fix leak libqtest is using g_strdup_printf to format QMP commands, but this does not work if the argument strings need to be escaped. Instead, use the fancy %-formatting functionality of QObject. The only change required in tests is that strings have to be formatted as %s, not '%s' or \"%s\". Luckily this usage of parameterized QMP commands is not that frequent. The leak is in socket_sendf. Since we are extracting the send loop to a new function, fix it now. Signed-off-by: Paolo Bonzini --- tests/fdc-test.c | 2 +- tests/libqtest.c | 47 +++++++++++++++++++++++++++++++++++---------- tests/qom-test.c | 6 +++--- tests/tmp105-test.c | 4 ++-- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 37096dcc13..c8e1e7bd18 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -291,7 +291,7 @@ static void test_media_insert(void) /* Insert media in drive. DSKCHK should not be reset until a step pulse * is sent. */ qmp_discard_response("{'execute':'change', 'arguments':{" - " 'device':'floppy0', 'target': '%s' }}", + " 'device':'floppy0', 'target': %s }}", test_image); qmp_discard_response(""); /* ignore event (FIXME open -> open transition?!) */ diff --git a/tests/libqtest.c b/tests/libqtest.c index 71468ac9c7..98e8f4b648 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -30,8 +30,9 @@ #include "qemu/compiler.h" #include "qemu/osdep.h" -#include "qapi/qmp/json-streamer.h" #include "qapi/qmp/json-parser.h" +#include "qapi/qmp/json-streamer.h" +#include "qapi/qmp/qjson.h" #define MAX_IRQ 256 #define SOCKET_TIMEOUT 5 @@ -220,19 +221,15 @@ void qtest_quit(QTestState *s) g_free(s); } -static void socket_sendf(int fd, const char *fmt, va_list ap) +static void socket_send(int fd, const char *buf, size_t size) { - gchar *str; - size_t size, offset; - - str = g_strdup_vprintf(fmt, ap); - size = strlen(str); + size_t offset; offset = 0; while (offset < size) { ssize_t len; - len = write(fd, str + offset, size - offset); + len = write(fd, buf + offset, size - offset); if (len == -1 && errno == EINTR) { continue; } @@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt, va_list ap) } } +static void socket_sendf(int fd, const char *fmt, va_list ap) +{ + gchar *str = g_strdup_vprintf(fmt, ap); + size_t size = strlen(str); + + socket_send(fd, str, size); + g_free(str); +} + static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...) { va_list ap; @@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s) QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap) { - /* Send QMP request */ - socket_sendf(s->qmp_fd, fmt, ap); + va_list ap_copy; + QObject *qobj; + + /* Going through qobject ensures we escape strings properly. + * This seemingly unnecessary copy is required in case va_list + * is an array type. + */ + va_copy(ap_copy, ap); + qobj = qobject_from_jsonv(fmt, &ap_copy); + va_end(ap_copy); + + /* No need to send anything for an empty QObject. */ + if (qobj) { + QString *qstr = qobject_to_json(qobj); + const char *str = qstring_get_str(qstr); + size_t size = qstring_get_length(qstr); + + /* Send QMP request */ + socket_send(s->qmp_fd, str, size); + + QDECREF(qstr); + qobject_decref(qobj); + } /* Receive reply */ return qtest_qmp_receive(s); diff --git a/tests/qom-test.c b/tests/qom-test.c index d8d1d8d9ff..4246382d38 100644 --- a/tests/qom-test.c +++ b/tests/qom-test.c @@ -53,7 +53,7 @@ static void test_properties(const char *path, bool recurse) g_test_message("Obtaining properties of %s", path); response = qmp("{ 'execute': 'qom-list'," - " 'arguments': { 'path': '%s' } }", path); + " 'arguments': { 'path': %s } }", path); g_assert(response); if (!recurse) { @@ -76,8 +76,8 @@ static void test_properties(const char *path, bool recurse) const char *prop = qdict_get_str(tuple, "name"); g_test_message("Testing property %s.%s", path, prop); response = qmp("{ 'execute': 'qom-get'," - " 'arguments': { 'path': '%s'," - " 'property': '%s' } }", + " 'arguments': { 'path': %s," + " 'property': %s } }", path, prop); /* qom-get may fail but should not, e.g., segfault. */ g_assert(response); diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c index 15ddaf38d4..99db538191 100644 --- a/tests/tmp105-test.c +++ b/tests/tmp105-test.c @@ -69,7 +69,7 @@ static int qmp_tmp105_get_temperature(const char *id) QDict *response; int ret; - response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': '%s', " + response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, " "'property': 'temperature' } }", id); g_assert(qdict_haskey(response, "return")); ret = qdict_get_int(response, "return"); @@ -81,7 +81,7 @@ static void qmp_tmp105_set_temperature(const char *id, int value) { QDict *response; - response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': '%s', " + response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, " "'property': 'temperature', 'value': %d } }", id, value); g_assert(qdict_haskey(response, "return")); QDECREF(response); From b5c2c3d0c81ea97ac8443113b9a7a0c0ce25368e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 11 Jun 2014 10:52:32 +0200 Subject: [PATCH 08/15] memory: MemoryRegion: use /machine as default owner This will be added (after QOMification) as the QOM parent. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- memory.c | 2 +- vl.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/memory.c b/memory.c index b91a60a921..7eaa1e9fd4 100644 --- a/memory.c +++ b/memory.c @@ -849,7 +849,7 @@ void memory_region_init(MemoryRegion *mr, { mr->ops = &unassigned_mem_ops; mr->opaque = NULL; - mr->owner = owner; + mr->owner = owner ? owner : qdev_get_machine(); mr->iommu_ops = NULL; mr->container = NULL; mr->size = int128_make64(size); diff --git a/vl.c b/vl.c index 41ddcd2678..88feeabd63 100644 --- a/vl.c +++ b/vl.c @@ -3986,12 +3986,11 @@ int main(int argc, char **argv, char **envp) exit(1); } - cpu_exec_init_all(); - current_machine = MACHINE(object_new(object_class_get_name( OBJECT_CLASS(machine_class)))); object_property_add_child(object_get_root(), "machine", OBJECT(current_machine), &error_abort); + cpu_exec_init_all(); if (machine_class->hw_version) { qemu_set_version(machine_class->hw_version); From b4fefef9d52003b6d09866501275a9a57995c6b0 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Thu, 5 Jun 2014 23:15:52 -0700 Subject: [PATCH 09/15] memory: MemoryRegion: QOMify QOMify memory regions as an Object. The former init() and destroy() routines become instance_init() and instance_finalize() resp. memory_region_init() is re-implemented to be: object_initialize() + set fields memory_region_destroy() is re-implemented to call unparent(). Signed-off-by: Peter Crosthwaite [Add newly-created MR as child, unparent on destruction. - Paolo] Signed-off-by: Paolo Bonzini --- exec.c | 4 +- include/exec/memory.h | 6 ++ memory.c | 129 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 113 insertions(+), 26 deletions(-) diff --git a/exec.c b/exec.c index 18d6c35942..df4a080619 100644 --- a/exec.c +++ b/exec.c @@ -883,7 +883,7 @@ static void phys_section_destroy(MemoryRegion *mr) if (mr->subpage) { subpage_t *subpage = container_of(mr, subpage_t, iomem); - memory_region_destroy(&subpage->iomem); + object_unref(OBJECT(&subpage->iomem)); g_free(subpage); } } @@ -1768,7 +1768,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base) mmio->as = as; mmio->base = base; memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio, - "subpage", TARGET_PAGE_SIZE); + NULL, TARGET_PAGE_SIZE); mmio->iomem.subpage = true; #if defined(DEBUG_SUBPAGE) printf("%s: %p base " TARGET_FMT_plx " len %08x\n", __func__, diff --git a/include/exec/memory.h b/include/exec/memory.h index 3d778d70f0..85b56e2e0c 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -32,10 +32,15 @@ #include "qemu/int128.h" #include "qemu/notify.h" #include "qapi/error.h" +#include "qom/object.h" #define MAX_PHYS_ADDR_SPACE_BITS 62 #define MAX_PHYS_ADDR (((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 1) +#define TYPE_MEMORY_REGION "qemu:memory-region" +#define MEMORY_REGION(obj) \ + OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION) + typedef struct MemoryRegionOps MemoryRegionOps; typedef struct MemoryRegionMmio MemoryRegionMmio; @@ -131,6 +136,7 @@ typedef struct CoalescedMemoryRange CoalescedMemoryRange; typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd; struct MemoryRegion { + Object parent_obj; /* All fields are private - violators will be prosecuted */ const MemoryRegionOps *ops; const MemoryRegionIOMMUOps *iommu_ops; diff --git a/memory.c b/memory.c index 7eaa1e9fd4..9397fecd2c 100644 --- a/memory.c +++ b/memory.c @@ -842,40 +842,94 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr) qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK); } +static bool memory_region_need_escape(char c) +{ + return c == '/' || c == '[' || c == '\\' || c == ']'; +} + +static char *memory_region_escape_name(const char *name) +{ + const char *p; + char *escaped, *q; + uint8_t c; + size_t bytes = 0; + + for (p = name; *p; p++) { + bytes += memory_region_need_escape(*p) ? 4 : 1; + } + if (bytes == p - name) { + return g_memdup(name, bytes + 1); + } + + escaped = g_malloc(bytes + 1); + for (p = name, q = escaped; *p; p++) { + c = *p; + if (unlikely(memory_region_need_escape(c))) { + *q++ = '\\'; + *q++ = 'x'; + *q++ = "0123456789abcdef"[c >> 4]; + c = "0123456789abcdef"[c & 15]; + } + *q++ = c; + } + *q = 0; + return escaped; +} + +static void object_property_add_child_array(Object *owner, + const char *name, + Object *child) +{ + int i; + char *base_name = memory_region_escape_name(name); + + for (i = 0; ; i++) { + char *full_name = g_strdup_printf("%s[%d]", base_name, i); + Error *local_err = NULL; + + object_property_add_child(owner, full_name, child, &local_err); + g_free(full_name); + if (!local_err) { + break; + } + + error_free(local_err); + } + + g_free(base_name); +} + + void memory_region_init(MemoryRegion *mr, Object *owner, const char *name, uint64_t size) { - mr->ops = &unassigned_mem_ops; - mr->opaque = NULL; + object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION); + mr->owner = owner ? owner : qdev_get_machine(); - mr->iommu_ops = NULL; - mr->container = NULL; mr->size = int128_make64(size); if (size == UINT64_MAX) { mr->size = int128_2_64(); } - mr->addr = 0; - mr->subpage = false; - mr->enabled = true; - mr->terminates = false; - mr->ram = false; - mr->romd_mode = true; - mr->readonly = false; - mr->rom_device = false; - mr->destructor = memory_region_destructor_none; - mr->priority = 0; - mr->may_overlap = false; - mr->alias = NULL; - QTAILQ_INIT(&mr->subregions); - memset(&mr->subregions_link, 0, sizeof mr->subregions_link); - QTAILQ_INIT(&mr->coalesced); mr->name = g_strdup(name); - mr->dirty_log_mask = 0; - mr->ioeventfd_nb = 0; - mr->ioeventfds = NULL; - mr->flush_coalesced_mmio = false; + + if (name) { + object_property_add_child_array(mr->owner, name, OBJECT(mr)); + object_unref(OBJECT(mr)); + } +} + +static void memory_region_initfn(Object *obj) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + + mr->ops = &unassigned_mem_ops; + mr->enabled = true; + mr->romd_mode = true; + mr->destructor = memory_region_destructor_none; + QTAILQ_INIT(&mr->subregions); + QTAILQ_INIT(&mr->coalesced); } static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, @@ -1113,8 +1167,10 @@ void memory_region_init_reservation(MemoryRegion *mr, memory_region_init_io(mr, owner, &unassigned_mem_ops, mr, name, size); } -void memory_region_destroy(MemoryRegion *mr) +static void memory_region_finalize(Object *obj) { + MemoryRegion *mr = MEMORY_REGION(obj); + assert(QTAILQ_EMPTY(&mr->subregions)); assert(memory_region_transaction_depth == 0); mr->destructor(mr); @@ -1123,6 +1179,12 @@ void memory_region_destroy(MemoryRegion *mr) g_free(mr->ioeventfds); } +void memory_region_destroy(MemoryRegion *mr) +{ + object_unparent(OBJECT(mr)); +} + + Object *memory_region_owner(MemoryRegion *mr) { return mr->owner; @@ -1132,6 +1194,8 @@ void memory_region_ref(MemoryRegion *mr) { if (mr && mr->owner) { object_ref(mr->owner); + } else { + object_ref(OBJECT(mr)); } } @@ -1139,6 +1203,8 @@ void memory_region_unref(MemoryRegion *mr) { if (mr && mr->owner) { object_unref(mr->owner); + } else { + object_unref(OBJECT(mr)); } } @@ -1946,3 +2012,18 @@ void mtree_info(fprintf_function mon_printf, void *f) g_free(ml); } } + +static const TypeInfo memory_region_info = { + .parent = TYPE_OBJECT, + .name = TYPE_MEMORY_REGION, + .instance_size = sizeof(MemoryRegion), + .instance_init = memory_region_initfn, + .instance_finalize = memory_region_finalize, +}; + +static void memory_register_types(void) +{ + type_register_static(&memory_region_info); +} + +type_init(memory_register_types) From 22a893e4f55344f96e1aafc66f5fedc491a5ca97 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 11 Jun 2014 10:58:06 +0200 Subject: [PATCH 10/15] memory: MemoryRegion: replace owner field with QOM parent The two are now the same. Reviewed-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 1 - memory.c | 35 +++++++++++++++++++++++++---------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 85b56e2e0c..0c7e825822 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -141,7 +141,6 @@ struct MemoryRegion { const MemoryRegionOps *ops; const MemoryRegionIOMMUOps *iommu_ops; void *opaque; - struct Object *owner; MemoryRegion *container; Int128 size; hwaddr addr; diff --git a/memory.c b/memory.c index 9397fecd2c..8970081336 100644 --- a/memory.c +++ b/memory.c @@ -905,9 +905,11 @@ void memory_region_init(MemoryRegion *mr, const char *name, uint64_t size) { - object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION); + if (!owner) { + owner = qdev_get_machine(); + } - mr->owner = owner ? owner : qdev_get_machine(); + object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION); mr->size = int128_make64(size); if (size == UINT64_MAX) { mr->size = int128_2_64(); @@ -915,7 +917,7 @@ void memory_region_init(MemoryRegion *mr, mr->name = g_strdup(name); if (name) { - object_property_add_child_array(mr->owner, name, OBJECT(mr)); + object_property_add_child_array(owner, name, OBJECT(mr)); object_unref(OBJECT(mr)); } } @@ -1187,24 +1189,37 @@ void memory_region_destroy(MemoryRegion *mr) Object *memory_region_owner(MemoryRegion *mr) { - return mr->owner; + Object *obj = OBJECT(mr); + return obj->parent; } void memory_region_ref(MemoryRegion *mr) { - if (mr && mr->owner) { - object_ref(mr->owner); + /* MMIO callbacks most likely will access data that belongs + * to the owner, hence the need to ref/unref the owner whenever + * the memory region is in use. + * + * The memory region is a child of its owner. As long as the + * owner doesn't call unparent itself on the memory region, + * ref-ing the owner will also keep the memory region alive. + * Memory regions without an owner are supposed to never go away, + * but we still ref/unref them for debugging purposes. + */ + Object *obj = OBJECT(mr); + if (obj && obj->parent) { + object_ref(obj->parent); } else { - object_ref(OBJECT(mr)); + object_ref(obj); } } void memory_region_unref(MemoryRegion *mr) { - if (mr && mr->owner) { - object_unref(mr->owner); + Object *obj = OBJECT(mr); + if (obj && obj->parent) { + object_unref(obj->parent); } else { - object_unref(OBJECT(mr)); + object_unref(obj); } } From 409ddd0139c101f813d16e8ebaa7c7d4b4afb96e Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Thu, 5 Jun 2014 23:16:27 -0700 Subject: [PATCH 11/15] memory: MemoryRegion: Add container and addr props Expose the already existing .parent and .addr fields as QOM properties. .parent (i.e. the field describing the memory region that contains this one in Memory hierachy) is renamed "container". This is to avoid confusion with the QOM parent. Signed-off-by: Peter Crosthwaite [Remove setters. Do not unref parent on releasing the property. Clean up error propagation. - Paolo] Signed-off-by: Paolo Bonzini --- memory.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/memory.c b/memory.c index 8970081336..04dc933603 100644 --- a/memory.c +++ b/memory.c @@ -16,6 +16,7 @@ #include "exec/memory.h" #include "exec/address-spaces.h" #include "exec/ioport.h" +#include "qapi/visitor.h" #include "qemu/bitops.h" #include "qom/object.h" #include "trace.h" @@ -922,9 +923,42 @@ void memory_region_init(MemoryRegion *mr, } } +static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + uint64_t value = mr->addr; + + visit_type_uint64(v, &value, name, errp); +} + +static void memory_region_get_container(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + gchar *path = (gchar *)""; + + if (mr->container) { + path = object_get_canonical_path(OBJECT(mr->container)); + } + visit_type_str(v, &path, name, errp); + if (mr->container) { + g_free(path); + } +} + +static Object *memory_region_resolve_container(Object *obj, void *opaque, + const char *part) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + + return OBJECT(mr->container); +} + static void memory_region_initfn(Object *obj) { MemoryRegion *mr = MEMORY_REGION(obj); + ObjectProperty *op; mr->ops = &unassigned_mem_ops; mr->enabled = true; @@ -932,6 +966,18 @@ static void memory_region_initfn(Object *obj) mr->destructor = memory_region_destructor_none; QTAILQ_INIT(&mr->subregions); QTAILQ_INIT(&mr->coalesced); + + op = object_property_add(OBJECT(mr), "container", + "link<" TYPE_MEMORY_REGION ">", + memory_region_get_container, + NULL, /* memory_region_set_container */ + NULL, NULL, &error_abort); + op->resolve = memory_region_resolve_container; + + object_property_add(OBJECT(mr), "addr", "uint64", + memory_region_get_addr, + NULL, /* memory_region_set_addr */ + NULL, NULL, &error_abort); } static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, From d33382da9abe21cb42f26b55945845b56ce9324a Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Thu, 5 Jun 2014 23:17:01 -0700 Subject: [PATCH 12/15] memory: MemoryRegion: Add may-overlap and priority props QOM propertyify the .may-overlap and .priority fields. The setters will re-add the memory as a subregion if needed (i.e. the values change when the memory region is already contained). Signed-off-by: Peter Crosthwaite [Remove setters. - Paolo] Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 2 +- memory.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 0c7e825822..e2c8e3e0a6 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -157,7 +157,7 @@ struct MemoryRegion { bool flush_coalesced_mmio; MemoryRegion *alias; hwaddr alias_offset; - int priority; + int32_t priority; bool may_overlap; QTAILQ_HEAD(subregions, MemoryRegion) subregions; QTAILQ_ENTRY(MemoryRegion) subregions_link; diff --git a/memory.c b/memory.c index 04dc933603..834959b0a3 100644 --- a/memory.c +++ b/memory.c @@ -955,6 +955,22 @@ static Object *memory_region_resolve_container(Object *obj, void *opaque, return OBJECT(mr->container); } +static void memory_region_get_priority(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + int32_t value = mr->priority; + + visit_type_int32(v, &value, name, errp); +} + +static bool memory_region_get_may_overlap(Object *obj, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + + return mr->may_overlap; +} + static void memory_region_initfn(Object *obj) { MemoryRegion *mr = MEMORY_REGION(obj); @@ -978,6 +994,14 @@ static void memory_region_initfn(Object *obj) memory_region_get_addr, NULL, /* memory_region_set_addr */ NULL, NULL, &error_abort); + object_property_add(OBJECT(mr), "priority", "uint32", + memory_region_get_priority, + NULL, /* memory_region_set_priority */ + NULL, NULL, &error_abort); + object_property_add_bool(OBJECT(mr), "may-overlap", + memory_region_get_may_overlap, + NULL, /* memory_region_set_may_overlap */ + &error_abort); } static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, From 52aef7bba74a65d0c9fbec7a6559cc8385ab0e17 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Thu, 5 Jun 2014 23:17:35 -0700 Subject: [PATCH 13/15] memory: MemoryRegion: Add size property To allow devices to dynamically resize the device. The motivation is to allow devices with variable size to init their memory_region without size early and then correctly populate size at realize() time. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- memory.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/memory.c b/memory.c index 834959b0a3..64d7176193 100644 --- a/memory.c +++ b/memory.c @@ -971,6 +971,15 @@ static bool memory_region_get_may_overlap(Object *obj, Error **errp) return mr->may_overlap; } +static void memory_region_get_size(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + uint64_t value = memory_region_size(mr); + + visit_type_uint64(v, &value, name, errp); +} + static void memory_region_initfn(Object *obj) { MemoryRegion *mr = MEMORY_REGION(obj); @@ -1002,6 +1011,10 @@ static void memory_region_initfn(Object *obj) memory_region_get_may_overlap, NULL, /* memory_region_set_may_overlap */ &error_abort); + object_property_add(OBJECT(mr), "size", "uint64", + memory_region_get_size, + NULL, /* memory_region_set_size, */ + NULL, NULL, &error_abort); } static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, From 1f6245e5aba94ff7acd34f8514da7dfb9712935d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jun 2014 10:48:06 +0200 Subject: [PATCH 14/15] memory: do not give a name to the internal exec.c regions There is no need to have them visible under /machine. Signed-off-by: Paolo Bonzini --- exec.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exec.c b/exec.c index df4a080619..5a2a25e851 100644 --- a/exec.c +++ b/exec.c @@ -1801,13 +1801,13 @@ MemoryRegion *iotlb_to_region(AddressSpace *as, hwaddr index) static void io_mem_init(void) { - memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, "rom", UINT64_MAX); + memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX); memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL, - "unassigned", UINT64_MAX); + NULL, UINT64_MAX); memory_region_init_io(&io_mem_notdirty, NULL, ¬dirty_mem_ops, NULL, - "notdirty", UINT64_MAX); + NULL, UINT64_MAX); memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL, - "watch", UINT64_MAX); + NULL, UINT64_MAX); } static void mem_begin(MemoryListener *listener) From 352e8da743f26948cb12d0ee53c455f328f59bbe Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 26 Jun 2014 15:10:03 +0200 Subject: [PATCH 15/15] qdev: correctly send DEVICE_DELETED for recursively-deleted devices When a device is unparented (i.e. made completely hidden from management) we want to send a DEVICE_DELETED event only if the device actually was realized. This avoids raising DEVICE_DELETED events when device_add fails. However, this does not work right for recursively-deleted devices: the whole tree is _first_ unrealized, _then_ unparented. Then device_unparent sees realized==false and fails to trigger the event. The solution is simply to move have_realized into the DeviceState struct. If device_add fails, we never set the new field to true and DEVICE_DELETED is not sent. Fixes qemu-iotests testcase 067 (broken by commit 5942a19, though that commit in turn fixed a possible segfault in the same test). Reported-by: Markus Armbruster Reviewed-by: Markus Armbruster Signed-off-by: Paolo Bonzini --- hw/core/qdev.c | 5 +++-- include/hw/qdev-core.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index d1eba3cc3d..c52041543d 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -848,6 +848,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp) if (dev->hotplugged && local_err == NULL) { device_reset(dev); } + dev->pending_deleted_event = false; } else if (!value && dev->realized) { QLIST_FOREACH(bus, &dev->child_bus, sibling) { object_property_set_bool(OBJECT(bus), false, "realized", @@ -862,6 +863,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp) if (dc->unrealize && local_err == NULL) { dc->unrealize(dev, &local_err); } + dev->pending_deleted_event = true; } if (local_err != NULL) { @@ -972,7 +974,6 @@ static void device_unparent(Object *obj) { DeviceState *dev = DEVICE(obj); BusState *bus; - bool have_realized = dev->realized; if (dev->realized) { object_property_set_bool(obj, false, "realized", NULL); @@ -988,7 +989,7 @@ static void device_unparent(Object *obj) } /* Only send event if the device had been completely realized */ - if (have_realized) { + if (dev->pending_deleted_event) { gchar *path = object_get_canonical_path(OBJECT(dev)); qapi_event_send_device_deleted(!!dev->id, dev->id, path, &error_abort); diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 9221cfc879..0799ff29b0 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -156,6 +156,7 @@ struct DeviceState { const char *id; bool realized; + bool pending_deleted_event; QemuOpts *opts; int hotplugged; BusState *parent_bus;