qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 03/11] hw/core: create Resettable QOM interface


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v7 03/11] hw/core: create Resettable QOM interface
Date: Thu, 16 Jan 2020 03:12:23 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2

On 1/16/20 2:59 AM, Philippe Mathieu-Daudé wrote:
On 1/15/20 1:36 PM, Damien Hedde wrote:
This commit defines an interface allowing multi-phase reset. This aims
to solve a problem of the actual single-phase reset (built in
DeviceClass and BusClass): reset behavior is dependent on the order
in which reset handlers are called. In particular doing external
side-effect (like setting an qemu_irq) is problematic because receiving
object may not be reset yet.

The Resettable interface divides the reset in 3 well defined phases.
To reset an object tree, all 1st phases are executed then all 2nd then
all 3rd. See the comments in include/hw/resettable.h for a more complete
description. The interface defines 3 phases to let the future
possibility of holding an object into reset for some time.

The qdev/qbus reset in DeviceClass and BusClass will be modified in
following commits to use this interface. A mechanism is provided
to allow executing a transitional reset handler in place of the 2nd
phase which is executed in children-then-parent order inside a tree.
This will allow to transition devices and buses smoothly while
keeping the exact current qdev/qbus reset behavior for now.

Documentation will be added in a following commit.

Signed-off-by: Damien Hedde <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
---

v7 update: un-nest struct ResettablePhases
---
  Makefile.objs           |   1 +
  include/hw/resettable.h | 211 +++++++++++++++++++++++++++++++++++
  hw/core/resettable.c    | 238 ++++++++++++++++++++++++++++++++++++++++
  hw/core/Makefile.objs   |   1 +
  hw/core/trace-events    |  17 +++
  5 files changed, 468 insertions(+)
  create mode 100644 include/hw/resettable.h
  create mode 100644 hw/core/resettable.c

diff --git a/Makefile.objs b/Makefile.objs
index 7c1e50f9d6..9752d549b4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -191,6 +191,7 @@ trace-events-subdirs += migration
  trace-events-subdirs += net
  trace-events-subdirs += ui
  endif
+trace-events-subdirs += hw/core

TL;DR Duplicating this line breaks using the LTTng Userspace Tracer library (UST backend).

Probably because you started this series before commit 26b8e6dc42b got merged, Jun 13 2019!

Indeed Oct 02 2018...
https://www.mail-archive.com/address@hidden/msg564153.html

The problem is you (correctly) sorted alphabetically while Alexey appended at the end.

  trace-events-subdirs += hw/display
  trace-events-subdirs += qapi
  trace-events-subdirs += qom
[...]
diff --git a/hw/core/trace-events b/hw/core/trace-events
index a375aa88a4..a2e43f1120 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -9,3 +9,20 @@ 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, const char *objtype, void *oldp, const char *oldptype, void *newp, const char *newptype) "obj=%p(%s) old_parent=%p(%s) new_parent=%p(%s)"
+
+# resettable.c
+resettable_reset(void *obj, int cold) "obj=%p cold=%d"
+resettable_reset_assert_begin(void *obj, int cold) "obj=%p cold=%d"
+resettable_reset_assert_end(void *obj) "obj=%p"
+resettable_reset_release_begin(void *obj, int cold) "obj=%p cold=%d"
+resettable_reset_release_end(void *obj) "obj=%p"
+resettable_phase_enter_begin(void *obj, const char *objtype, uint32_t count, int type) "obj=%p(%s) count=%" PRIu32 " type=%d" +resettable_phase_enter_exec(void *obj, const char *objtype, int type, int has_method) "obj=%p(%s) type=%d method=%d" +resettable_phase_enter_end(void *obj, const char *objtype, uint32_t count) "obj=%p(%s) count=%" PRIu32 +resettable_phase_hold_begin(void *obj, const char *objtype, uint32_t count, int type) "obj=%p(%s) count=%" PRIu32 " type=%d" +resettable_phase_hold_exec(void *obj, const char *objtype, int has_method) "obj=%p(%s) method=%d" +resettable_phase_hold_end(void *obj, const char *objtype, uint32_t count) "obj=%p(%s) count=%" PRIu32 +resettable_phase_exit_begin(void *obj, const char *objtype, uint32_t count, int type) "obj=%p(%s) count=%" PRIu32 " type=%d" +resettable_phase_exit_exec(void *obj, const char *objtype, int has_method) "obj=%p(%s) method=%d" +resettable_phase_exit_end(void *obj, const char *objtype, uint32_t count) "obj=%p(%s) count=%" PRIu32 +resettable_transitional_function(void *obj, const char *objtype) "obj=%p(%s)"

Something here breaks ./configure --enable-trace-backends=ust:

   CC      trace-ust-all.o
In file included from trace-ust-all.h:13,
                  from trace-ust-all.c:13:
trace-ust-all.h:35151:1: error: redefinition of ‘__tracepoint_cb_qemu___loader_write_rom’
35151 | TRACEPOINT_EVENT(
       | ^~~~~~~~~~~~~~~~
trace-ust-all.h:31791:1: note: previous definition of ‘__tracepoint_cb_qemu___loader_write_rom’ was here
31791 | TRACEPOINT_EVENT(
       | ^~~~~~~~~~~~~~~~
...
./trace-ust-all.h:35388:1: error: redefinition of ‘__tp_event_signature___qemu___resettable_transitional_function’
35388 | TRACEPOINT_EVENT(
       | ^~~~~~~~~~~~~~~~
./trace-ust-all.h:32028:1: note: previous definition of ‘__tp_event_signature___qemu___resettable_transitional_function’ was here
32028 | TRACEPOINT_EVENT(
       | ^~~~~~~~~~~~~~~~
In file included from /usr/include/lttng/tracepoint-event.h:58,
                  from trace-ust-all.h:35401,
                  from trace-ust-all.c:13:

Indeed:

32028 TRACEPOINT_EVENT(
32029    qemu,
32030    resettable_transitional_function,
32031    TP_ARGS(void *, obj, const char *, objtype),
32032    TP_FIELDS(
32033        ctf_integer_hex(void *, obj, obj)
32034        ctf_string(objtype, objtype)
32035    )
32036 )
32037
...
35388 TRACEPOINT_EVENT(
35389    qemu,
35390    resettable_transitional_function,
35391    TP_ARGS(void *, obj, const char *, objtype),
35392    TP_FIELDS(
35393        ctf_integer_hex(void *, obj, obj)
35394        ctf_string(objtype, objtype)
35395    )
35396 )
35397
35398 #endif /* TRACE_ALL_GENERATED_UST_H */

Ah! I was going to say "no clue what could be wrong, so Cc'ing Stefan" but got it:

$ git grep hw/core Makefile.objs
Makefile.objs:194:trace-events-subdirs += hw/core
Makefile.objs:207:trace-events-subdirs += hw/core

We might already have a 'uniq' makefile function to do:

trace-events-subdirs = $(call uniq $(trace-events-subdirs))

or maybe was it with $filter? I can't find it/remember, too tired.

So the fix is:

-- >8 --
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -191,7 +191,6 @@ trace-events-subdirs += migration
  trace-events-subdirs += net
  trace-events-subdirs += ui
  endif
-trace-events-subdirs += hw/core
  trace-events-subdirs += hw/display
  trace-events-subdirs += qapi
  trace-events-subdirs += qom
---

Forgot to add, with trace-events-subdirs fixed:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]