| Message ID | 20191018150630.31099-3-damien.hedde@greensocs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
On 10/18/19 8:06 AM, Damien Hedde wrote: > Adds trace events to reset procedure and when updating the parent > bus of a device. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/qdev.c | 27 ++++++++++++++++++++++++--- > hw/core/trace-events | 9 +++++++++ > 2 files changed, 33 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 10/18/19 5:06 PM, Damien Hedde wrote: > Adds trace events to reset procedure and when updating the parent > bus of a device. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/qdev.c | 27 ++++++++++++++++++++++++--- > hw/core/trace-events | 9 +++++++++ > 2 files changed, 33 insertions(+), 3 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 60be2f2fef..f230063189 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -38,6 +38,7 @@ > #include "hw/boards.h" > #include "hw/sysbus.h" > #include "migration/vmstate.h" > +#include "trace.h" > > bool qdev_hotplug = false; > static bool qdev_hot_added = false; > @@ -98,7 +99,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) > bool replugging = dev->parent_bus != NULL; > > if (replugging) { > - /* Keep a reference to the device while it's not plugged into > + trace_qdev_update_parent_bus(dev, dev->parent_bus, bus); Nitpicking, if you respin, can you add object_get_typename(OBJECT(dev))) and object_get_typename(OBJECT(bus)))? With/without it: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + /* > + * Keep a reference to the device while it's not plugged into > * any bus, to avoid it potentially evaporating when it is > * dereffed in bus_remove_child(). > */ > @@ -272,6 +275,18 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > return hotplug_ctrl; > } > > +static int qdev_prereset(DeviceState *dev, void *opaque) > +{ > + trace_qdev_reset_tree(dev, object_get_typename(OBJECT(dev))); > + return 0; > +} > + > +static int qbus_prereset(BusState *bus, void *opaque) > +{ > + trace_qbus_reset_tree(bus, object_get_typename(OBJECT(bus))); > + return 0; > +} > + > static int qdev_reset_one(DeviceState *dev, void *opaque) > { > device_legacy_reset(dev); > @@ -282,6 +297,7 @@ static int qdev_reset_one(DeviceState *dev, void *opaque) > static int qbus_reset_one(BusState *bus, void *opaque) > { > BusClass *bc = BUS_GET_CLASS(bus); > + trace_qbus_reset(bus, object_get_typename(OBJECT(bus))); > if (bc->reset) { > bc->reset(bus); > } > @@ -290,7 +306,9 @@ static int qbus_reset_one(BusState *bus, void *opaque) > > void qdev_reset_all(DeviceState *dev) > { > - qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL); > + trace_qdev_reset_all(dev, object_get_typename(OBJECT(dev))); > + qdev_walk_children(dev, qdev_prereset, qbus_prereset, > + qdev_reset_one, qbus_reset_one, NULL); > } > > void qdev_reset_all_fn(void *opaque) > @@ -300,7 +318,9 @@ void qdev_reset_all_fn(void *opaque) > > void qbus_reset_all(BusState *bus) > { > - qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL); > + trace_qbus_reset_all(bus, object_get_typename(OBJECT(bus))); > + qbus_walk_children(bus, qdev_prereset, qbus_prereset, > + qdev_reset_one, qbus_reset_one, NULL); > } > > void qbus_reset_all_fn(void *opaque) > @@ -1108,6 +1128,7 @@ void device_legacy_reset(DeviceState *dev) > { > DeviceClass *klass = DEVICE_GET_CLASS(dev); > > + trace_qdev_reset(dev, object_get_typename(OBJECT(dev))); > if (klass->reset) { > klass->reset(dev); > } > diff --git a/hw/core/trace-events b/hw/core/trace-events > index fe47a9c8cb..1a669be0ea 100644 > --- a/hw/core/trace-events > +++ b/hw/core/trace-events > @@ -1,2 +1,11 @@ > # loader.c > loader_write_rom(const char *name, uint64_t gpa, uint64_t size, bool isrom) "%s: @0x%"PRIx64" size=0x%"PRIx64" ROM=%d" > + > +# qdev.c > +qdev_reset(void *obj, const char *objtype) "obj=%p(%s)" > +qdev_reset_all(void *obj, const char *objtype) "obj=%p(%s)" > +qdev_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" > +qbus_reset(void *obj, const char *objtype) "obj=%p(%s)" > +qbus_reset_all(void *obj, const char *objtype) "obj=%p(%s)" > +qbus_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" > +qdev_update_parent_bus(void *obj, void *oldp, void *newp) "obj=%p old_parent=%p new_parent=%p" >
On 11/1/19 12:23 AM, Philippe Mathieu-Daudé wrote: > On 10/18/19 5:06 PM, Damien Hedde wrote: >> Adds trace events to reset procedure and when updating the parent >> bus of a device. >> >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >> --- >> hw/core/qdev.c | 27 ++++++++++++++++++++++++--- >> hw/core/trace-events | 9 +++++++++ >> 2 files changed, 33 insertions(+), 3 deletions(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 60be2f2fef..f230063189 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -38,6 +38,7 @@ >> #include "hw/boards.h" >> #include "hw/sysbus.h" >> #include "migration/vmstate.h" >> +#include "trace.h" >> bool qdev_hotplug = false; >> static bool qdev_hot_added = false; >> @@ -98,7 +99,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState >> *bus) >> bool replugging = dev->parent_bus != NULL; >> if (replugging) { >> - /* Keep a reference to the device while it's not plugged into >> + trace_qdev_update_parent_bus(dev, dev->parent_bus, bus); > > Nitpicking, if you respin, can you add object_get_typename(OBJECT(dev))) > and object_get_typename(OBJECT(bus)))? sure. I was wondering if having some kind of qom object support to trace would be feasible. Because it's a bit tedious to add this each time. And IMO it would be more useful to have the path, but we can't reasonably compute it as a trace_..() arguments. > > With/without it: > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> + /* >> + * Keep a reference to the device while it's not plugged into >> * any bus, to avoid it potentially evaporating when it is >> * dereffed in bus_remove_child(). >> */ >> @@ -272,6 +275,18 @@ HotplugHandler >> *qdev_get_hotplug_handler(DeviceState *dev) >> return hotplug_ctrl; >> } >> +static int qdev_prereset(DeviceState *dev, void *opaque) >> +{ >> + trace_qdev_reset_tree(dev, object_get_typename(OBJECT(dev))); >> + return 0; >> +} >> + >> +static int qbus_prereset(BusState *bus, void *opaque) >> +{ >> + trace_qbus_reset_tree(bus, object_get_typename(OBJECT(bus))); >> + return 0; >> +} >> + >> static int qdev_reset_one(DeviceState *dev, void *opaque) >> { >> device_legacy_reset(dev); >> @@ -282,6 +297,7 @@ static int qdev_reset_one(DeviceState *dev, void >> *opaque) >> static int qbus_reset_one(BusState *bus, void *opaque) >> { >> BusClass *bc = BUS_GET_CLASS(bus); >> + trace_qbus_reset(bus, object_get_typename(OBJECT(bus))); >> if (bc->reset) { >> bc->reset(bus); >> } >> @@ -290,7 +306,9 @@ static int qbus_reset_one(BusState *bus, void >> *opaque) >> void qdev_reset_all(DeviceState *dev) >> { >> - qdev_walk_children(dev, NULL, NULL, qdev_reset_one, >> qbus_reset_one, NULL); >> + trace_qdev_reset_all(dev, object_get_typename(OBJECT(dev))); >> + qdev_walk_children(dev, qdev_prereset, qbus_prereset, >> + qdev_reset_one, qbus_reset_one, NULL); >> } >> void qdev_reset_all_fn(void *opaque) >> @@ -300,7 +318,9 @@ void qdev_reset_all_fn(void *opaque) >> void qbus_reset_all(BusState *bus) >> { >> - qbus_walk_children(bus, NULL, NULL, qdev_reset_one, >> qbus_reset_one, NULL); >> + trace_qbus_reset_all(bus, object_get_typename(OBJECT(bus))); >> + qbus_walk_children(bus, qdev_prereset, qbus_prereset, >> + qdev_reset_one, qbus_reset_one, NULL); >> } >> void qbus_reset_all_fn(void *opaque) >> @@ -1108,6 +1128,7 @@ void device_legacy_reset(DeviceState *dev) >> { >> DeviceClass *klass = DEVICE_GET_CLASS(dev); >> + trace_qdev_reset(dev, object_get_typename(OBJECT(dev))); >> if (klass->reset) { >> klass->reset(dev); >> } >> diff --git a/hw/core/trace-events b/hw/core/trace-events >> index fe47a9c8cb..1a669be0ea 100644 >> --- a/hw/core/trace-events >> +++ b/hw/core/trace-events >> @@ -1,2 +1,11 @@ >> # loader.c >> loader_write_rom(const char *name, uint64_t gpa, uint64_t size, bool >> isrom) "%s: @0x%"PRIx64" size=0x%"PRIx64" ROM=%d" >> + >> +# qdev.c >> +qdev_reset(void *obj, const char *objtype) "obj=%p(%s)" >> +qdev_reset_all(void *obj, const char *objtype) "obj=%p(%s)" >> +qdev_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" >> +qbus_reset(void *obj, const char *objtype) "obj=%p(%s)" >> +qbus_reset_all(void *obj, const char *objtype) "obj=%p(%s)" >> +qbus_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" >> +qdev_update_parent_bus(void *obj, void *oldp, void *newp) "obj=%p >> old_parent=%p new_parent=%p" >>
On 11/4/19 1:16 PM, Damien Hedde wrote: > On 11/1/19 12:23 AM, Philippe Mathieu-Daudé wrote: >> On 10/18/19 5:06 PM, Damien Hedde wrote: >>> Adds trace events to reset procedure and when updating the parent >>> bus of a device. >>> >>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >>> --- >>> hw/core/qdev.c | 27 ++++++++++++++++++++++++--- >>> hw/core/trace-events | 9 +++++++++ >>> 2 files changed, 33 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>> index 60be2f2fef..f230063189 100644 >>> --- a/hw/core/qdev.c >>> +++ b/hw/core/qdev.c >>> @@ -38,6 +38,7 @@ >>> #include "hw/boards.h" >>> #include "hw/sysbus.h" >>> #include "migration/vmstate.h" >>> +#include "trace.h" >>> bool qdev_hotplug = false; >>> static bool qdev_hot_added = false; >>> @@ -98,7 +99,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState >>> *bus) >>> bool replugging = dev->parent_bus != NULL; >>> if (replugging) { >>> - /* Keep a reference to the device while it's not plugged into >>> + trace_qdev_update_parent_bus(dev, dev->parent_bus, bus); >> >> Nitpicking, if you respin, can you add object_get_typename(OBJECT(dev))) >> and object_get_typename(OBJECT(bus)))? > > sure. I was wondering if having some kind of qom object support to trace > would be feasible. Because it's a bit tedious to add this each time. And > IMO it would be more useful to have the path, but we can't reasonably > compute it as a trace_..() arguments. Meanwhile you can use: if (trace_event_get_state_backends(TRACE_QDEV_UPDATE_PARENT_BUS)) { ... >> >> With/without it: >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> >>> + /* >>> + * Keep a reference to the device while it's not plugged into >>> * any bus, to avoid it potentially evaporating when it is >>> * dereffed in bus_remove_child(). >>> */ >>> @@ -272,6 +275,18 @@ HotplugHandler >>> *qdev_get_hotplug_handler(DeviceState *dev) >>> return hotplug_ctrl; >>> } >>> +static int qdev_prereset(DeviceState *dev, void *opaque) >>> +{ >>> + trace_qdev_reset_tree(dev, object_get_typename(OBJECT(dev))); >>> + return 0; >>> +} >>> + >>> +static int qbus_prereset(BusState *bus, void *opaque) >>> +{ >>> + trace_qbus_reset_tree(bus, object_get_typename(OBJECT(bus))); >>> + return 0; >>> +} >>> + >>> static int qdev_reset_one(DeviceState *dev, void *opaque) >>> { >>> device_legacy_reset(dev); >>> @@ -282,6 +297,7 @@ static int qdev_reset_one(DeviceState *dev, void >>> *opaque) >>> static int qbus_reset_one(BusState *bus, void *opaque) >>> { >>> BusClass *bc = BUS_GET_CLASS(bus); >>> + trace_qbus_reset(bus, object_get_typename(OBJECT(bus))); >>> if (bc->reset) { >>> bc->reset(bus); >>> } >>> @@ -290,7 +306,9 @@ static int qbus_reset_one(BusState *bus, void >>> *opaque) >>> void qdev_reset_all(DeviceState *dev) >>> { >>> - qdev_walk_children(dev, NULL, NULL, qdev_reset_one, >>> qbus_reset_one, NULL); >>> + trace_qdev_reset_all(dev, object_get_typename(OBJECT(dev))); >>> + qdev_walk_children(dev, qdev_prereset, qbus_prereset, >>> + qdev_reset_one, qbus_reset_one, NULL); >>> } >>> void qdev_reset_all_fn(void *opaque) >>> @@ -300,7 +318,9 @@ void qdev_reset_all_fn(void *opaque) >>> void qbus_reset_all(BusState *bus) >>> { >>> - qbus_walk_children(bus, NULL, NULL, qdev_reset_one, >>> qbus_reset_one, NULL); >>> + trace_qbus_reset_all(bus, object_get_typename(OBJECT(bus))); >>> + qbus_walk_children(bus, qdev_prereset, qbus_prereset, >>> + qdev_reset_one, qbus_reset_one, NULL); >>> } >>> void qbus_reset_all_fn(void *opaque) >>> @@ -1108,6 +1128,7 @@ void device_legacy_reset(DeviceState *dev) >>> { >>> DeviceClass *klass = DEVICE_GET_CLASS(dev); >>> + trace_qdev_reset(dev, object_get_typename(OBJECT(dev))); >>> if (klass->reset) { >>> klass->reset(dev); >>> } >>> diff --git a/hw/core/trace-events b/hw/core/trace-events >>> index fe47a9c8cb..1a669be0ea 100644 >>> --- a/hw/core/trace-events >>> +++ b/hw/core/trace-events >>> @@ -1,2 +1,11 @@ >>> # loader.c >>> loader_write_rom(const char *name, uint64_t gpa, uint64_t size, bool >>> isrom) "%s: @0x%"PRIx64" size=0x%"PRIx64" ROM=%d" >>> + >>> +# qdev.c >>> +qdev_reset(void *obj, const char *objtype) "obj=%p(%s)" >>> +qdev_reset_all(void *obj, const char *objtype) "obj=%p(%s)" >>> +qdev_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" >>> +qbus_reset(void *obj, const char *objtype) "obj=%p(%s)" >>> +qbus_reset_all(void *obj, const char *objtype) "obj=%p(%s)" >>> +qbus_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" >>> +qdev_update_parent_bus(void *obj, void *oldp, void *newp) "obj=%p >>> old_parent=%p new_parent=%p" >>>
On Fri, 18 Oct 2019 17:06:19 +0200 Damien Hedde <damien.hedde@greensocs.com> wrote: > Adds trace events to reset procedure and when updating the parent > bus of a device. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/qdev.c | 27 ++++++++++++++++++++++++--- > hw/core/trace-events | 9 +++++++++ > 2 files changed, 33 insertions(+), 3 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 60be2f2fef..f230063189 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -38,6 +38,7 @@ #include "hw/boards.h" #include "hw/sysbus.h" #include "migration/vmstate.h" +#include "trace.h" bool qdev_hotplug = false; static bool qdev_hot_added = false; @@ -98,7 +99,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) bool replugging = dev->parent_bus != NULL; if (replugging) { - /* Keep a reference to the device while it's not plugged into + trace_qdev_update_parent_bus(dev, dev->parent_bus, bus); + /* + * Keep a reference to the device while it's not plugged into * any bus, to avoid it potentially evaporating when it is * dereffed in bus_remove_child(). */ @@ -272,6 +275,18 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) return hotplug_ctrl; } +static int qdev_prereset(DeviceState *dev, void *opaque) +{ + trace_qdev_reset_tree(dev, object_get_typename(OBJECT(dev))); + return 0; +} + +static int qbus_prereset(BusState *bus, void *opaque) +{ + trace_qbus_reset_tree(bus, object_get_typename(OBJECT(bus))); + return 0; +} + static int qdev_reset_one(DeviceState *dev, void *opaque) { device_legacy_reset(dev); @@ -282,6 +297,7 @@ static int qdev_reset_one(DeviceState *dev, void *opaque) static int qbus_reset_one(BusState *bus, void *opaque) { BusClass *bc = BUS_GET_CLASS(bus); + trace_qbus_reset(bus, object_get_typename(OBJECT(bus))); if (bc->reset) { bc->reset(bus); } @@ -290,7 +306,9 @@ static int qbus_reset_one(BusState *bus, void *opaque) void qdev_reset_all(DeviceState *dev) { - qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL); + trace_qdev_reset_all(dev, object_get_typename(OBJECT(dev))); + qdev_walk_children(dev, qdev_prereset, qbus_prereset, + qdev_reset_one, qbus_reset_one, NULL); } void qdev_reset_all_fn(void *opaque) @@ -300,7 +318,9 @@ void qdev_reset_all_fn(void *opaque) void qbus_reset_all(BusState *bus) { - qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL); + trace_qbus_reset_all(bus, object_get_typename(OBJECT(bus))); + qbus_walk_children(bus, qdev_prereset, qbus_prereset, + qdev_reset_one, qbus_reset_one, NULL); } void qbus_reset_all_fn(void *opaque) @@ -1108,6 +1128,7 @@ void device_legacy_reset(DeviceState *dev) { DeviceClass *klass = DEVICE_GET_CLASS(dev); + trace_qdev_reset(dev, object_get_typename(OBJECT(dev))); if (klass->reset) { klass->reset(dev); } diff --git a/hw/core/trace-events b/hw/core/trace-events index fe47a9c8cb..1a669be0ea 100644 --- a/hw/core/trace-events +++ b/hw/core/trace-events @@ -1,2 +1,11 @@ # loader.c loader_write_rom(const char *name, uint64_t gpa, uint64_t size, bool isrom) "%s: @0x%"PRIx64" size=0x%"PRIx64" ROM=%d" + +# qdev.c +qdev_reset(void *obj, const char *objtype) "obj=%p(%s)" +qdev_reset_all(void *obj, const char *objtype) "obj=%p(%s)" +qdev_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" +qbus_reset(void *obj, const char *objtype) "obj=%p(%s)" +qbus_reset_all(void *obj, const char *objtype) "obj=%p(%s)" +qbus_reset_tree(void *obj, const char *objtype) "obj=%p(%s)" +qdev_update_parent_bus(void *obj, void *oldp, void *newp) "obj=%p old_parent=%p new_parent=%p"
Adds trace events to reset procedure and when updating the parent bus of a device. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- hw/core/qdev.c | 27 ++++++++++++++++++++++++--- hw/core/trace-events | 9 +++++++++ 2 files changed, 33 insertions(+), 3 deletions(-)