Message ID | 95099c6d2c53520148512094676ca637f53eabd6.1401181024.git.peter.crosthwaite@xilinx.com |
---|---|
State | New |
Headers | show |
On Tue, May 27, 2014 at 7:03 PM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > 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 owner field, which is much more akin to an actual QOM > parent. > > Setting the .parent ("container") will cause the memory subregion adding > to happen. Nullifying or changing the .parent will delete or relocate > (to a different container) the subregion resp. > > Setting or changing the address will relocate the memory region. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > changed since v1: > Converted container to low level link property and added subregion setup. > > memory.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) > > diff --git a/memory.c b/memory.c > index d9d3c07..a95bb1e 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" > @@ -861,9 +862,104 @@ void memory_region_init(MemoryRegion *mr, > mr->name = g_strdup(name); > } > > +static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + MemoryRegion *mr = MEMORY_REGION(obj); > + Error *local_err = NULL; > + uint64_t value = mr->addr; > + > + visit_type_uint64(v, &value, name, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + } > +} > + > +static void memory_region_set_addr(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + MemoryRegion *mr = MEMORY_REGION(obj); > + Error *local_err = NULL; > + uint64_t value; > + > + visit_type_uint64(v, &value, name, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + > + memory_region_set_address(mr, value); > +} > + > +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_parent = mr->parent; > + MemoryRegion *new_parent = NULL; > + char *path = NULL; > + > + visit_type_str(v, &path, name, &local_err); > + > + if (!local_err && strcmp(path, "") != 0) { > + new_parent = MEMORY_REGION(object_resolve_link(obj, name, path, > + &local_err)); > + } > + > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + > + object_ref(OBJECT(new_parent)); > + > + memory_region_transaction_begin(); > + memory_region_ref(mr); > + if (old_parent) { > + memory_region_del_subregion(old_parent, mr); > + } > + mr->parent = new_parent; > + if (new_parent) { > + do_memory_region_add_subregion_common(mr); > + } > + memory_region_unref(mr); > + memory_region_transaction_commit(); > + > + object_unref(OBJECT(old_parent)); > +} > + > +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->parent) { > + path = object_get_canonical_path(OBJECT(mr->parent)); > + } > + visit_type_str(v, &path, name, errp); > + if (mr->parent) { > + g_free(path); > + } > +} > + > +static void memory_region_release_container(Object *obj, const char *name, > + void *opaque) > +{ > + MemoryRegion *mr = MEMORY_REGION(obj); > + > + if (mr->parent) { > + memory_region_del_subregion(mr->parent, mr); > + object_unref(OBJECT(mr->parent)); > + } > +} > + > static void memory_region_initfn(Object *obj) > { > MemoryRegion *mr = MEMORY_REGION(obj); > + gchar *container_link_type = g_strdup_printf("link<%s>", > + TYPE_MEMORY_REGION); Since TYPE_MEMORY_REGION is a literal string constant, this can be done with regular "" "" style string concatenation. Dropped the strdup in V3. Regards, Peter > > mr->ops = &unassigned_mem_ops; > mr->enabled = true; > @@ -872,6 +968,18 @@ static void memory_region_initfn(Object *obj) > QTAILQ_INIT(&mr->subregions); > memset(&mr->subregions_link, 0, sizeof mr->subregions_link); > QTAILQ_INIT(&mr->coalesced); > + > + object_property_add(OBJECT(mr), "container", container_link_type, > + memory_region_get_container, > + memory_region_set_container, > + memory_region_release_container, > + NULL, &error_abort); > + g_free(container_link_type); > + > + object_property_add(OBJECT(mr), "addr", "uint64", > + memory_region_get_addr, > + memory_region_set_addr, > + NULL, NULL, &error_abort); > } > > static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, > -- > 1.9.3.1.ga73a6ad >
Il 02/06/2014 04:40, Peter Crosthwaite ha scritto: >> > { >> > MemoryRegion *mr = MEMORY_REGION(obj); >> > + gchar *container_link_type = g_strdup_printf("link<%s>", >> > + TYPE_MEMORY_REGION); > Since TYPE_MEMORY_REGION is a literal string constant, this can be > done with regular "" "" style string concatenation. Dropped the strdup > in V3. Note that object_resolve_path_component expects link<FOO> properties to have a LinkProperty stored in prop->opaque. Does this hold in your case? Perhaps we can instead add a new ->resolve function pointer to properties. Paolo
On Tue, Jun 3, 2014 at 5:51 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 02/06/2014 04:40, Peter Crosthwaite ha scritto: > >>> > { >>> > MemoryRegion *mr = MEMORY_REGION(obj); >>> > + gchar *container_link_type = g_strdup_printf("link<%s>", >>> > + TYPE_MEMORY_REGION); >> >> Since TYPE_MEMORY_REGION is a literal string constant, this can be >> done with regular "" "" style string concatenation. Dropped the strdup >> in V3. > > > Note that object_resolve_path_component expects link<FOO> properties to have > a LinkProperty stored in prop->opaque. Does this hold in your case? > Probably not. Using canonical paths through links would not work in this case. Nice catch. > Perhaps we can instead add a new ->resolve function pointer to properties. > The other option is we open up the check fn from object_property_add_link as being usable for setter side-effects. No change of code, just I guess "check" would then be a wrong name. Then there's no need for a low-level link. Probably easier than yet another fn hook. Regards, Peter > Paolo >
diff --git a/memory.c b/memory.c index d9d3c07..a95bb1e 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" @@ -861,9 +862,104 @@ void memory_region_init(MemoryRegion *mr, mr->name = g_strdup(name); } +static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + Error *local_err = NULL; + uint64_t value = mr->addr; + + visit_type_uint64(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + } +} + +static void memory_region_set_addr(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + Error *local_err = NULL; + uint64_t value; + + visit_type_uint64(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + memory_region_set_address(mr, value); +} + +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_parent = mr->parent; + MemoryRegion *new_parent = NULL; + char *path = NULL; + + visit_type_str(v, &path, name, &local_err); + + if (!local_err && strcmp(path, "") != 0) { + new_parent = MEMORY_REGION(object_resolve_link(obj, name, path, + &local_err)); + } + + if (local_err) { + error_propagate(errp, local_err); + return; + } + + object_ref(OBJECT(new_parent)); + + memory_region_transaction_begin(); + memory_region_ref(mr); + if (old_parent) { + memory_region_del_subregion(old_parent, mr); + } + mr->parent = new_parent; + if (new_parent) { + do_memory_region_add_subregion_common(mr); + } + memory_region_unref(mr); + memory_region_transaction_commit(); + + object_unref(OBJECT(old_parent)); +} + +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->parent) { + path = object_get_canonical_path(OBJECT(mr->parent)); + } + visit_type_str(v, &path, name, errp); + if (mr->parent) { + g_free(path); + } +} + +static void memory_region_release_container(Object *obj, const char *name, + void *opaque) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + + if (mr->parent) { + memory_region_del_subregion(mr->parent, mr); + object_unref(OBJECT(mr->parent)); + } +} + static void memory_region_initfn(Object *obj) { MemoryRegion *mr = MEMORY_REGION(obj); + gchar *container_link_type = g_strdup_printf("link<%s>", + TYPE_MEMORY_REGION); mr->ops = &unassigned_mem_ops; mr->enabled = true; @@ -872,6 +968,18 @@ static void memory_region_initfn(Object *obj) QTAILQ_INIT(&mr->subregions); memset(&mr->subregions_link, 0, sizeof mr->subregions_link); QTAILQ_INIT(&mr->coalesced); + + object_property_add(OBJECT(mr), "container", container_link_type, + memory_region_get_container, + memory_region_set_container, + memory_region_release_container, + NULL, &error_abort); + g_free(container_link_type); + + object_property_add(OBJECT(mr), "addr", "uint64", + memory_region_get_addr, + memory_region_set_addr, + NULL, NULL, &error_abort); } static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
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 owner field, which is much more akin to an actual QOM parent. Setting the .parent ("container") will cause the memory subregion adding to happen. Nullifying or changing the .parent will delete or relocate (to a different container) the subregion resp. Setting or changing the address will relocate the memory region. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- changed since v1: Converted container to low level link property and added subregion setup. memory.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)