All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
@ 2014-01-31 16:09 Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 01/12] trace: [tcg] Add documentation Lluís Vilanova
                   ` (13 more replies)
  0 siblings, 14 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Adds the base ability to specify which events in the "trace-events" file may be
used to trace guest activity in the TCG code (using the "tcg" event propery).

Such events generate an extra set of tracing functions that can be called during
TCG code generation and will automatically redirect a call to the appropriate
backend-dependent tracing functions when the guest code is executed.

Files generating guest code (TCG) must include "trace-tcg.h". Files declaring
per-target helpers ("${target}/helper.h") must include
"trace/generated-helpers.h".

The flow of the generated routines is:


[At translation time]

* trace_${name}_tcg(bool, TCGv)
  Declared: "trace/generated-tcg-tracers.h"
  Defined : "trace/generated-tcg-tracers.h"

* gen_helper_trace_${name}_tcg(bool, TCGv)
  Declared: "trace/generated-helpers.h"
  Defined : "trace/generated-helpers.h"

  Automatically transforms all the arguments and allocates them into the
  appropriate TCG temporary values (which are also freed). Provides a more
  streamlined interface by allowing events in "trace-events" to take a mix of
  tracing-supported types and TCG types.

* gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv)
  Declared: "trace/generated-helpers.h"
  Defined : "trace/generated-helpers.h" (using helper machinery)

  The actual TCG helper function, created using QEMU's TCG helper machinery.


[At execution time]

* helper_trace_${name}_tcg_proxy(uint32_t, uint64_t)
  Declared: "trace/generated-helpers.h"
  Defined : "trace/generated-helpers.c"

  Performs the appropriate argument type casts to match the signature of the
  callee.

* trace_${name}(bool, uint64_t)

  The already-existing tracing function.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---

Lluís Vilanova (12):
      trace: [tcg] Add documentation
      trace: [tracetool,tcg] Allow TCG types in trace event declarations
      trace: [tracetool] Add method 'Event.api' to get the name of public routines
      trace: [tracetool,tcg] Provide TCG-related type transformation rules
      trace: [tracetool] Allow argument types to be transformed
      trace: [tcg] Declare TCG tracing helper routines
      trace: [tcg] Define TCG tracing helper routines
      trace: [tcg] Include TCG-tracing helpers on all helper.h
      trace: [tcg] Generate TCG tracing routines
      trace: [trivial] Include event definitions in "trace.h"
      trace: [tcg] Include TCG-tracing header on all targets
      trace: [all] Add "guest_vmem" event


 .gitignore                               |    3 +
 Makefile                                 |    5 +
 Makefile.objs                            |    8 +-
 Makefile.target                          |    1 
 docs/tracing.txt                         |   36 +++++++
 include/exec/cpu-all.h                   |   58 ++++++-----
 include/exec/def-helper.h                |    9 ++
 include/exec/exec-all.h                  |    3 +
 include/exec/softmmu_header.h            |   17 +++
 include/trace-tcg.h                      |    7 +
 include/trace.h                          |    1 
 scripts/tracetool/__init__.py            |   49 +++++++++
 scripts/tracetool/backend/dtrace.py      |    6 +
 scripts/tracetool/backend/simple.py      |   10 +-
 scripts/tracetool/backend/stderr.py      |    5 +
 scripts/tracetool/backend/tcg.py         |  125 ++++++++++++++++++++++++
 scripts/tracetool/backend/ust.py         |    8 +-
 scripts/tracetool/format/h.py            |    6 +
 scripts/tracetool/format/tcg_h.py        |   47 +++++++++
 scripts/tracetool/format/tcg_helper_c.py |   27 +++++
 scripts/tracetool/format/tcg_helper_h.py |   38 +++++++
 scripts/tracetool/transform.py           |  157 ++++++++++++++++++++++++++++++
 target-alpha/helper.h                    |    2 
 target-alpha/translate.c                 |    3 +
 target-arm/helper.h                      |    2 
 target-arm/translate.c                   |    3 +
 target-cris/helper.h                     |    2 
 target-cris/translate.c                  |    3 +
 target-i386/helper.h                     |    2 
 target-i386/translate.c                  |    3 +
 target-lm32/helper.h                     |    2 
 target-lm32/translate.c                  |    3 +
 target-m68k/helper.h                     |    2 
 target-m68k/translate.c                  |    3 +
 target-microblaze/helper.h               |    2 
 target-microblaze/translate.c            |    3 +
 target-mips/helper.h                     |    2 
 target-mips/translate.c                  |    3 +
 target-openrisc/helper.h                 |    2 
 target-openrisc/translate.c              |    3 +
 target-ppc/helper.h                      |    2 
 target-ppc/translate.c                   |    3 +
 target-s390x/helper.h                    |    2 
 target-s390x/translate.c                 |    2 
 target-sh4/helper.h                      |    2 
 target-sh4/translate.c                   |    3 +
 target-sparc/helper.h                    |    2 
 target-sparc/translate.c                 |    3 +
 target-unicore32/helper.h                |    2 
 target-unicore32/translate.c             |    3 +
 target-xtensa/helper.h                   |    2 
 target-xtensa/translate.c                |    3 +
 tcg/tcg-op.h                             |    8 ++
 tcg/tcg.c                                |    1 
 trace-events                             |   15 +++
 trace/Makefile.objs                      |   45 ++++++++-
 trace/tcg-op-internal.h                  |   55 +++++++++++
 57 files changed, 771 insertions(+), 53 deletions(-)
 create mode 100644 include/trace-tcg.h
 create mode 100644 scripts/tracetool/backend/tcg.py
 create mode 100644 scripts/tracetool/format/tcg_h.py
 create mode 100644 scripts/tracetool/format/tcg_helper_c.py
 create mode 100644 scripts/tracetool/format/tcg_helper_h.py
 create mode 100644 scripts/tracetool/transform.py
 create mode 100644 trace/tcg-op-internal.h

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 01/12] trace: [tcg] Add documentation
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 02/12] trace: [tracetool, tcg] Allow TCG types in trace event declarations Lluís Vilanova
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/tracing.txt |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index bfc261b..9bf7073 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -271,3 +271,39 @@ guard such computations and avoid its compilation when the event is disabled:
 You can check both if the event has been disabled and is dynamically enabled at
 the same time using the 'trace_event_get_state' routine (see header
 "trace/control.h" for more information).
+
+=== "tcg" ===
+
+Guest code generated by TCG can be traced by defining an event with the "tcg"
+event property.
+
+In addition to the regular "trace_<eventname>" routine in the "trace.h" header,
+events with the "tcg" property also provide the routine "trace_<eventname>_tcg"
+in the "trace-tcg.h" header. This routine can be called during TCG code
+generation to automatically generate TCG code to call "trace_<eventname>" during
+TCG code execution.
+
+Events with the "tcg" property can be declared in the "trace-events" file with a
+mix of basic and TCG types. The "trace_<eventname>_tcg" routine will
+transparently take care of turning any non-TCG argument into a TCG value, and
+the "trace_<eventname>" routine will automatically receive non-TCG values.
+
+For example, the event (as would be declared in "trace-events"):
+
+    tcg foo(uint8_t a1, TCGv_i32 a2) "a1=%d a2=%d"
+
+Can be invoked as:
+
+    #include "helper.h"
+    
+    void bar ()
+    {
+        uint8_t a1 = ...;
+        TCGv_i32 a2 = ...;
+        trace_foo_tcg(a1, a2);
+    }
+
+And the intermediate boilerplate code will take care of generating the TCG code
+to call (as would be declared in "trace.h"):
+
+    void trace_foo(uint8_t a1, uint32_t a2);

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 02/12] trace: [tracetool, tcg] Allow TCG types in trace event declarations
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 01/12] trace: [tcg] Add documentation Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 03/12] trace: [tracetool] Add method 'Event.api' to get the name of public routines Lluís Vilanova
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

When found, TCG types are translated into the host native types when declaring
and defining the tracing routines not used during TCG code translation (e.g.,
"trace.h").


Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool/__init__.py |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 175df08..c76f648 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -18,6 +18,7 @@ import sys
 
 import tracetool.format
 import tracetool.backend
+import tracetool.transform
 
 
 def error_write(*lines):
@@ -122,7 +123,7 @@ class Event(object):
 
     _CRE = re.compile("((?P<props>.*)\s+)?(?P<name>[^(\s]+)\((?P<args>[^)]*)\)\s*(?P<fmt>\".*)?")
 
-    _VALID_PROPS = set(["disable"])
+    _VALID_PROPS = set(["disable", "tcg"])
 
     def __init__(self, name, props, fmt, args):
         """
@@ -259,6 +260,11 @@ def generate(fevents, format, backend,
 
     events = _read_events(fevents)
 
+    # transform arguments to host type by default
+    # (otherwise use event.original)
+    events = [e.transform(tracetool.transform.TCG_2_HOST(backend))
+              for e in events]
+
     if backend == "nop":
         ( e.properies.add("disable") for e in events )
 

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 03/12] trace: [tracetool] Add method 'Event.api' to get the name of public routines
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 01/12] trace: [tcg] Add documentation Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 02/12] trace: [tracetool, tcg] Allow TCG types in trace event declarations Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 04/12] trace: [tracetool, tcg] Provide TCG-related type transformation rules Lluís Vilanova
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

This ensures proper naming across tracing backends, even when someone overrides
the value without backends knowing it.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool/__init__.py       |   10 +++++++++-
 scripts/tracetool/backend/dtrace.py |    6 +++---
 scripts/tracetool/backend/simple.py |   10 +++++-----
 scripts/tracetool/backend/stderr.py |    5 +++--
 scripts/tracetool/backend/ust.py    |    8 +++++---
 scripts/tracetool/format/h.py       |    6 +++---
 6 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index c76f648..1170aab 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -6,7 +6,7 @@ Machinery for generating tracing-related intermediate files.
 """
 
 __author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
 __license__    = "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -174,6 +174,14 @@ class Event(object):
                                           self.args,
                                           self.fmt)
 
+    QEMU_TRACE              = "trace_%(name)s"
+
+    def api(self, fmt=None):
+        if fmt is None:
+            fmt = Event.QEMU_TRACE
+        return fmt % {"name": self.name}
+
+
 def _read_events(fobj):
     res = []
     for line in fobj:
diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
index e31bc79..3c369c4 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -6,7 +6,7 @@ DTrace/SystemTAP backend.
 """
 
 __author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
 __license__    = "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -44,10 +44,10 @@ def h(events):
         '')
 
     for e in events:
-        out('static inline void trace_%(name)s(%(args)s) {',
+        out('static inline void %(api)s(%(args)s) {',
             '    QEMU_%(uppername)s(%(argnames)s);',
             '}',
-            name = e.name,
+            api = e.api(),
             args = e.args,
             uppername = e.name.upper(),
             argnames = ", ".join(e.args.names()),
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 3dde372..ca48e12 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -6,7 +6,7 @@ Simple built-in backend.
 """
 
 __author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
 __license__    = "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -34,10 +34,10 @@ def c(events):
         )
 
     for num, event in enumerate(events):
-        out('void trace_%(name)s(%(args)s)',
+        out('void %(api)s(%(args)s)',
             '{',
             '    TraceBufferRecord rec;',
-            name = event.name,
+            api = event.api(),
             args = event.args,
             )
         sizes = []
@@ -95,7 +95,7 @@ def c(events):
 
 def h(events):
     for event in events:
-        out('void trace_%(name)s(%(args)s);',
-            name = event.name,
+        out('void %(api)s(%(args)s);',
+            api = event.api(),
             args = event.args,
             )
diff --git a/scripts/tracetool/backend/stderr.py b/scripts/tracetool/backend/stderr.py
index 6f93dbd..6681e26 100644
--- a/scripts/tracetool/backend/stderr.py
+++ b/scripts/tracetool/backend/stderr.py
@@ -6,7 +6,7 @@ Stderr built-in backend.
 """
 
 __author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
 __license__    = "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -33,13 +33,14 @@ def h(events):
         if len(e.args) > 0:
             argnames = ", " + argnames
 
-        out('static inline void trace_%(name)s(%(args)s)',
+        out('static inline void %(api)s(%(args)s)',
             '{',
             '    bool _state = trace_event_get_state(%(event_id)s);',
             '    if (_state) {',
             '        fprintf(stderr, "%(name)s " %(fmt)s "\\n" %(argnames)s);',
             '    }',
             '}',
+            api = e.api(),
             name = e.name,
             args = e.args,
             event_id = "TRACE_" + e.name.upper(),
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index ea36995..180b1bf 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -6,7 +6,7 @@ LTTng User Space Tracing backend.
 """
 
 __author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
 __license__    = "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -78,7 +78,8 @@ def h(events):
     for e in events:
         if len(e.args) > 0:
             out('DECLARE_TRACE(ust_%(name)s, TP_PROTO(%(args)s), TP_ARGS(%(argnames)s));',
-                '#define trace_%(name)s trace_ust_%(name)s',
+                '#define %(api)s trace_ust_%(name)s',
+                api = e.api(),
                 name = e.name,
                 args = e.args,
                 argnames = ", ".join(e.args.names()),
@@ -86,7 +87,8 @@ def h(events):
 
         else:
             out('_DECLARE_TRACEPOINT_NOARGS(ust_%(name)s);',
-                '#define trace_%(name)s trace_ust_%(name)s',
+                '#define %(api)s trace_ust_%(name)s',
+                api = e.api(),
                 name = e.name,
                 )
 
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 93132fc..9b0903d 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -6,7 +6,7 @@ Generate .h file.
 """
 
 __author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
-__copyright__  = "Copyright 2012, Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
 __license__    = "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -30,9 +30,9 @@ def end(events):
 def nop(events):
     for e in events:
         out('',
-            'static inline void trace_%(name)s(%(args)s)',
+            'static inline void %(api)s(%(args)s)',
             '{',
             '}',
-            name = e.name,
+            api = e.api(),
             args = e.args,
             )

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 04/12] trace: [tracetool, tcg] Provide TCG-related type transformation rules
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (2 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 03/12] trace: [tracetool] Add method 'Event.api' to get the name of public routines Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 05/12] trace: [tracetool] Allow argument types to be transformed Lluís Vilanova
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool/transform.py |  157 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 157 insertions(+)
 create mode 100644 scripts/tracetool/transform.py

diff --git a/scripts/tracetool/transform.py b/scripts/tracetool/transform.py
new file mode 100644
index 0000000..e5fbf92
--- /dev/null
+++ b/scripts/tracetool/transform.py
@@ -0,0 +1,157 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Type-transformation rules.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@linux.vnet.ibm.com"
+
+
+def _transform_type(type_, trans):
+    if isinstance(trans, str):
+        return trans
+    elif isinstance(trans, dict):
+        if type_ in trans:
+            return _transform_type(type_, trans[type_])
+        elif None in trans:
+            return _transform_type(type_, trans[None])
+        else:
+            return type_
+    elif callable(trans):
+        return trans(type_)
+    else:
+        raise ValueError("Invalid type transformation rule: %s" % trans)
+
+
+def transform_type(type_, *trans):
+    """Return a new type transformed according to the given rules.
+
+    Applies each of the transformation rules in trans in order.
+
+    If an element of trans is a string, return it.
+
+    If an element of trans is a function, call it with type_ as its only
+    argument.
+
+    If an element of trans is a dict, search type_ in its keys. If type_ is
+    a key, use the value as a transformation rule for type_. Otherwise, if
+    None is a key use the value as a transformation rule for type_.
+
+    Otherwise, return type_.
+
+    Parameters
+    ----------
+    type_ : str
+        Type to transform.
+    trans : list of function or dict
+        Type transformation rules.
+    """
+    if len(trans) == 0:
+        raise ValueError
+    res = type_
+    for t in trans:
+        res = _transform_type(res, t)
+    return res
+
+
+##################################################
+# host/tcg -> host
+
+def TCG_2_HOST(backend):
+    """Return transformation rules for the given backend."""
+
+    def _tcg_2_host(type_):
+        if type_ == "TCGv":
+            if backend == "tcg":
+                return "target_ulong"
+            else:
+                # force a fixed-size type in trace.{h,c}
+                # (ideally would use a host-specific type)
+                return "uint64_t"
+        else:
+            return type_
+
+    dict_ = {
+        "TCGv_i32": "uint32_t",
+        "TCGv_i64": "uint64_t",
+        "TCGv_ptr": "void *",
+        None: _tcg_2_host,
+        }
+
+    return dict_
+
+##################################################
+# host -> host compatible with tcg sizes
+
+HOST_2_TCG_COMPAT = {
+    "uint8_t": "uint32_t",
+    }
+
+
+##################################################
+# host/tcg -> tcg
+
+def _host_2_tcg(type_):
+    if type_.startswith("TCGv"):
+        return type_
+    raise ValueError("Don't know how to translate '%s' into a TCG type\n" % type_)
+
+HOST_2_TCG = {
+    "uint32_t": "TCGv_i32",
+    "uint64_t": "TCGv_i64",
+    "void *"  : "TCGv_ptr",
+    None: _host_2_tcg,
+    }
+
+
+##################################################
+# tcg -> tcg helper declaration
+
+def _tcg_2_tcg_helper_decl_error(type_):
+    raise ValueError("Don't know how to translate type '%s' into a TCG helper declaration type\n" % type_)
+
+TCG_2_TCG_HELPER_DECL = {
+    "TCGv"    : "tl",
+    "TCGv_ptr": "ptr",
+    "TCGv_i32": "i32",
+    "TCGv_i64": "i64",
+    None: _tcg_2_tcg_helper_decl_error,
+    }
+
+
+##################################################
+# host/tcg -> tcg temporal constant allocation
+
+def _host_2_tcg_tmp_new(type_):
+    if type_.startswith("TCGv"):
+        return "tcg_temp_new_nop"
+    raise ValueError("Don't know how to translate type '%s' into a TCG temporal allocation" % type_)
+
+HOST_2_TCG_TMP_NEW = {
+    "uint32_t": "tcg_const_i32",
+    "uint64_t": "tcg_const_i64",
+    "void *"  : "tcg_const_ptr",
+    None: _host_2_tcg_tmp_new,
+    }
+
+
+##################################################
+# host/tcg -> tcg temporal constant deallocation
+
+def _host_2_tcg_tmp_free(type_):
+    if type_.startswith("TCGv"):
+        return "tcg_temp_free_nop"
+    raise ValueError("Don't know how to translate type '%s' into a TCG temporal deallocation" % type_)
+
+HOST_2_TCG_TMP_FREE = {
+    "uint32_t": "tcg_temp_free_i32",
+    "uint64_t": "tcg_temp_free_i64",
+    "void *"  : "tcg_temp_free_ptr",
+    None: _host_2_tcg_tmp_free,
+    }

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 05/12] trace: [tracetool] Allow argument types to be transformed
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (3 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 04/12] trace: [tracetool, tcg] Provide TCG-related type transformation rules Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 06/12] trace: [tcg] Declare TCG tracing helper routines Lluís Vilanova
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 scripts/tracetool/__init__.py |   30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 1170aab..aab6d31 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -15,6 +15,7 @@ __email__      = "stefanha@linux.vnet.ibm.com"
 
 import re
 import sys
+import weakref
 
 import tracetool.format
 import tracetool.backend
@@ -105,6 +106,18 @@ class Arguments:
         """List of argument types."""
         return [ type_ for type_, _ in self._args ]
 
+    def transform(self, *trans):
+        """Return a new Arguments instance with transformed types.
+
+        The types in the resulting Arguments instance are transformed according
+        to tracetool.transform.transform_type.
+        """
+        res = []
+        for type_, name in self._args:
+            res.append((tracetool.transform.transform_type(type_, *trans),
+                        name))
+        return Arguments(res)
+
 
 class Event(object):
     """Event description.
@@ -125,7 +138,7 @@ class Event(object):
 
     _VALID_PROPS = set(["disable", "tcg"])
 
-    def __init__(self, name, props, fmt, args):
+    def __init__(self, name, props, fmt, args, orig=None):
         """
         Parameters
         ----------
@@ -137,12 +150,19 @@ class Event(object):
             Event printing format.
         args : Arguments
             Event arguments.
+        orig : Event or None
+            Original Event before transformation.
         """
         self.name = name
         self.properties = props
         self.fmt = fmt
         self.args = args
 
+        if orig is None:
+            self.original = weakref.ref(self)
+        else:
+            self.original = orig
+
         unknown_props = set(self.properties) - self._VALID_PROPS
         if len(unknown_props) > 0:
             raise ValueError("Unknown properties: %s" % ", ".join(unknown_props))
@@ -181,6 +201,14 @@ class Event(object):
             fmt = Event.QEMU_TRACE
         return fmt % {"name": self.name}
 
+    def transform(self, *trans):
+        """Return a new Event with transformed Arguments."""
+        return Event(self.name,
+                     self.properties,
+                     self.fmt,
+                     self.args.transform(*trans),
+                     self)
+
 
 def _read_events(fobj):
     res = []

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 06/12] trace: [tcg] Declare TCG tracing helper routines
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (4 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 05/12] trace: [tracetool] Allow argument types to be transformed Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 07/12] trace: [tcg] Define " Lluís Vilanova
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Generate header "trace/generated-helpers.h" with the necessary TCG helper
declarations for tracing events in guest code:

* gen_helper_trace_${event}_tcg

  Routine to transform mixed native and TCG argument types to TCG types and call
  TCG helper 'gen_helper_trace_${event}_tcg_proxy'.

* helper_trace_${event}_tcg_proxy

  TCG helper to cast TCG-compatible argument types to native types and call
  tracing routine 'trace_${event}'.

NOTE: 'gen_gelper_trace_${event}_tcg_proxy' is generated by the TCG helper
      machinery.


Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 .gitignore                               |    1 
 Makefile                                 |    2 +
 include/exec/def-helper.h                |    9 +++
 scripts/tracetool/__init__.py            |    1 
 scripts/tracetool/backend/tcg.py         |   85 ++++++++++++++++++++++++++++++
 scripts/tracetool/format/tcg_helper_h.py |   38 +++++++++++++
 trace/Makefile.objs                      |   24 +++++++-
 7 files changed, 155 insertions(+), 5 deletions(-)
 create mode 100644 scripts/tracetool/backend/tcg.py
 create mode 100644 scripts/tracetool/format/tcg_helper_h.py

diff --git a/.gitignore b/.gitignore
index 1c9d63d..8e61ca1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@ trace/generated-tracers-dtrace.h
 trace/generated-tracers.dtrace
 trace/generated-events.h
 trace/generated-events.c
+trace/generated-helpers.h
 libcacard/trace/generated-tracers.c
 *-timestamp
 *-softmmu
diff --git a/Makefile b/Makefile
index a9b6e39..d1ec29b 100644
--- a/Makefile
+++ b/Makefile
@@ -57,6 +57,8 @@ GENERATED_HEADERS += trace/generated-tracers-dtrace.h
 endif
 GENERATED_SOURCES += trace/generated-tracers.c
 
+GENERATED_HEADERS += trace/generated-helpers.h
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
diff --git a/include/exec/def-helper.h b/include/exec/def-helper.h
index 73d51f9..8e096d1 100644
--- a/include/exec/def-helper.h
+++ b/include/exec/def-helper.h
@@ -13,6 +13,11 @@
    helper.c, defining:
     GEN_HELPER 1 to produce op generation functions (gen_helper_*)
     GEN_HELPER 2 to do runtime registration helper functions.
+
+   After including this header the GEN_HELPER_CODE macro will have the following
+   values:
+    1  : op generation functions were produced (GEN_HELPER was 1)
+    -1 : otherwise
  */
 
 #ifndef DEF_HELPER_H
@@ -157,6 +162,7 @@ dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
 
 #undef GEN_HELPER
 #define GEN_HELPER -1
+#define GEN_HELPER_CODE -1
 
 #elif GEN_HELPER == 1
 /* Gen functions.  */
@@ -236,6 +242,7 @@ static inline void glue(gen_helper_, name)(dh_retvar_decl(ret) \
 
 #undef GEN_HELPER
 #define GEN_HELPER -1
+#define GEN_HELPER_CODE 1
 
 #elif GEN_HELPER == 2
 /* Register helpers.  */
@@ -259,6 +266,7 @@ DEF_HELPER_FLAGS_0(name, flags, ret)
 
 #undef GEN_HELPER
 #define GEN_HELPER -1
+#define GEN_HELPER_CODE -1
 
 #elif GEN_HELPER == -1
 /* Undefine macros.  */
@@ -270,5 +278,6 @@ DEF_HELPER_FLAGS_0(name, flags, ret)
 #undef DEF_HELPER_FLAGS_4
 #undef DEF_HELPER_FLAGS_5
 #undef GEN_HELPER
+#undef GEN_HELPER_CODE
 
 #endif
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index aab6d31..aefdce1 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -195,6 +195,7 @@ class Event(object):
                                           self.fmt)
 
     QEMU_TRACE              = "trace_%(name)s"
+    QEMU_TRACE_TCG          = "trace_%(name)s_tcg"
 
     def api(self, fmt=None):
         if fmt is None:
diff --git a/scripts/tracetool/backend/tcg.py b/scripts/tracetool/backend/tcg.py
new file mode 100644
index 0000000..04aa2b8
--- /dev/null
+++ b/scripts/tracetool/backend/tcg.py
@@ -0,0 +1,85 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Default TCG instrumentation helpers.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+from tracetool.transform import *
+
+
+def tcg_helper_h(events):
+    out('#define tcg_temp_new_nop(v) (v)',
+        '#define tcg_temp_free_nop(v)',
+        '',
+        )
+
+    for e in events:
+        if "tcg" not in e.properties:
+            continue
+
+        # tracetool.generate always transformes types to host
+        e = e.original
+
+        # TCG helper proxy declaration
+        fmt = "DEF_HELPER_FLAGS_%(argc)d(%(name)s, %(flags)svoid%(types)s)"
+        args = e.args.transform(HOST_2_TCG_COMPAT, HOST_2_TCG,
+                                TCG_2_TCG_HELPER_DECL)
+        types = ", ".join(args.types())
+        if types != "":
+            types = ", " + types
+
+        flags = "TCG_CALL_NO_RWG, "
+
+        out(fmt,
+            flags=flags,
+            argc=len(args),
+            name=e.api(e.QEMU_TRACE_TCG) + "_proxy",
+            types=types,
+            )
+
+        # mixed-type to TCG helper bridge
+        args_tcg_compat = e.args.transform(HOST_2_TCG_COMPAT)
+
+        code_new = [
+            "%(tcg_type)s __%(name)s = %(tcg_func)s(%(name)s);" %
+            {"tcg_type": transform_type(type_, HOST_2_TCG),
+             "tcg_func": transform_type(type_, HOST_2_TCG_TMP_NEW),
+             "name": name}
+            for (type_, name) in args_tcg_compat
+        ]
+
+        code_free = [
+            "%(tcg_func)s(__%(name)s);" %
+            {"tcg_func": transform_type(type_, HOST_2_TCG_TMP_FREE),
+             "name": name}
+            for (type_, name) in args_tcg_compat
+        ]
+
+        gen_name = "gen_helper_" + e.api(e.QEMU_TRACE_TCG)
+
+        out(
+            '#if GEN_HELPER_CODE == 1',
+            'static inline void %(name)s(%(args)s)',
+            '{',
+            '    %(code_new)s',
+            '    %(proxy_name)s(%(tmp_names)s);',
+            '    %(code_free)s',
+            '}',
+            '#endif',
+            name=gen_name,
+            args=e.args,
+            proxy_name=gen_name + "_proxy",
+            code_new="\n    ".join(code_new),
+            code_free="\n    ".join(code_free),
+            tmp_names=", ".join(["__%s" % name for _, name in e.args]),
+            )
diff --git a/scripts/tracetool/format/tcg_helper_h.py b/scripts/tracetool/format/tcg_helper_h.py
new file mode 100644
index 0000000..821df71
--- /dev/null
+++ b/scripts/tracetool/format/tcg_helper_h.py
@@ -0,0 +1,38 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generate trace/generated-helpers.h.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+
+
+def begin(events):
+    out('/* This file is autogenerated by tracetool, do not edit. */',
+        '/* You must include this file in your helper.h */',
+        '',
+        )
+
+
+def nop(events):
+    for e in events:
+        if "tcg" not in e.properties:
+            continue
+
+        out('#if GEN_HELPER_CODE == 1',
+            'static inline void %(name)s(%(args)s)',
+            '{',
+            '}',
+            '#endif',
+            name="gen_helper_" + e.api(e.QEMU_TRACE_TCG),
+            args=e.args,
+            )
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 3b88e49..c2a0ac0 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -25,6 +25,9 @@ util-obj-y += generated-events.o
 ######################################################################
 # Auto-generated tracing routines
 
+##################################################
+# Execution level
+
 $(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
 	@cmp -s $< $@ || cp $< $@
 $(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
@@ -33,8 +36,8 @@ $(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 		--backend=$(TRACE_BACKEND) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
-######################################################################
-# Auto-generated tracing routines (non-DTrace)
+##############################
+# non-DTrace
 
 ifneq ($(TRACE_BACKEND),dtrace)
 $(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
@@ -48,9 +51,8 @@ $(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 $(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.h
 endif
 
-
-######################################################################
-# Auto-generated DTrace code
+##############################
+# DTrace
 
 # Normal practice is to name DTrace probe file with a '.d' extension
 # but that gets picked up by QEMU's Makefile as an external dependency
@@ -70,6 +72,18 @@ $(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers.dtrace
 $(obj)/generated-tracers.o: $(obj)/generated-tracers.dtrace
 endif
 
+##################################################
+# Translation level
+
+$(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
+$(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+	$(call quiet-command,$(TRACETOOL) \
+		--format=tcg-helper-h \
+		--backend=tcg \
+		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
+
+
 ######################################################################
 # Backend code
 

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 07/12] trace: [tcg] Define TCG tracing helper routines
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (5 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 06/12] trace: [tcg] Declare TCG tracing helper routines Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 08/12] trace: [tcg] Include TCG-tracing helpers on all helper.h Lluís Vilanova
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Generate file "trace/generated-helpers.c" with the necessary TCG helper routines
for tracing events in guest code:

* helper_trace_${event}_tcg_proxy

  TCG helper implementation to cast TCG-compatible argument types to native
  types and call tracing routine 'trace_${event}'.


Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 .gitignore                               |    1 +
 Makefile                                 |    1 +
 Makefile.objs                            |    8 +++++++-
 Makefile.target                          |    1 +
 scripts/tracetool/backend/tcg.py         |   23 +++++++++++++++++++++++
 scripts/tracetool/format/tcg_helper_c.py |   27 +++++++++++++++++++++++++++
 trace/Makefile.objs                      |   12 ++++++++++++
 7 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 scripts/tracetool/format/tcg_helper_c.py

diff --git a/.gitignore b/.gitignore
index 8e61ca1..418546c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,6 +11,7 @@ trace/generated-tracers.dtrace
 trace/generated-events.h
 trace/generated-events.c
 trace/generated-helpers.h
+trace/generated-helpers.c
 libcacard/trace/generated-tracers.c
 *-timestamp
 *-softmmu
diff --git a/Makefile b/Makefile
index d1ec29b..910fb4f 100644
--- a/Makefile
+++ b/Makefile
@@ -58,6 +58,7 @@ endif
 GENERATED_SOURCES += trace/generated-tracers.c
 
 GENERATED_HEADERS += trace/generated-helpers.h
+GENERATED_SOURCES += trace/generated-helpers.c
 
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
diff --git a/Makefile.objs b/Makefile.objs
index ac1d0e1..c9570de 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,7 +1,7 @@
 #######################################################################
 # Common libraries for tools and emulators
 stub-obj-y = stubs/
-util-obj-y = util/ qobject/ qapi/ trace/
+util-obj-y = util/ qobject/ qapi/
 
 #######################################################################
 # block-obj-y is code used by both qemu system emulation and qemu-img
@@ -105,6 +105,11 @@ version-obj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.o
 version-lobj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.lo
 
 ######################################################################
+# tracing
+util-obj-y +=  trace/
+target-obj-y += trace/
+
+######################################################################
 # guest agent
 
 # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
@@ -124,5 +129,6 @@ nested-vars += \
 	qga-obj-y \
 	qga-vss-dll-obj-y \
 	block-obj-y \
+	target-obj-y \
 	common-obj-y
 dummy := $(call unnest-vars)
diff --git a/Makefile.target b/Makefile.target
index af6ac7e..43896ee 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -145,6 +145,7 @@ nested-vars += obj-y
 include $(SRC_PATH)/Makefile.objs
 
 all-obj-y = $(obj-y)
+all-obj-y += $(target-obj-y)
 all-obj-y += $(addprefix ../, $(common-obj-y))
 
 ifndef CONFIG_HAIKU
diff --git a/scripts/tracetool/backend/tcg.py b/scripts/tracetool/backend/tcg.py
index 04aa2b8..b60fb7e 100644
--- a/scripts/tracetool/backend/tcg.py
+++ b/scripts/tracetool/backend/tcg.py
@@ -17,6 +17,10 @@ from tracetool import out
 from tracetool.transform import *
 
 
+# get the actual set of rules for this backend
+TCG_2_HOST = TCG_2_HOST("tcg")
+
+
 def tcg_helper_h(events):
     out('#define tcg_temp_new_nop(v) (v)',
         '#define tcg_temp_free_nop(v)',
@@ -83,3 +87,22 @@ def tcg_helper_h(events):
             code_free="\n    ".join(code_free),
             tmp_names=", ".join(["__%s" % name for _, name in e.args]),
             )
+
+
+def tcg_helper_c(events):
+    events = (e for e in events
+              if "tcg" in e.properties)
+
+    for e in events:
+        values = ["(%s)%s" % (t, n)
+                  for t, n in e.args.transform(TCG_2_HOST)]
+
+        out('void %(name_tcg)s(%(args)s)',
+            '{',
+            '    %(name)s(%(values)s);',
+            '}',
+            name_tcg="helper_%s_proxy" % e.api(e.QEMU_TRACE_TCG),
+            name=e.api(e.QEMU_TRACE),
+            args=e.args.transform(HOST_2_TCG_COMPAT, TCG_2_HOST),
+            values=", ".join(values),
+            )
diff --git a/scripts/tracetool/format/tcg_helper_c.py b/scripts/tracetool/format/tcg_helper_c.py
new file mode 100644
index 0000000..a3540a6
--- /dev/null
+++ b/scripts/tracetool/format/tcg_helper_c.py
@@ -0,0 +1,27 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generate trace/generated-helpers.c.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+
+
+def begin(events):
+    out('/* This file is autogenerated by tracetool, do not edit. */',
+        '',
+        '#include "qemu-common.h"',
+        '#define TRACE_TCG_HELPER',
+        '#include "trace.h"',
+        '#include "helper.h"',
+        '',
+        )
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index c2a0ac0..de7e9a4 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -83,6 +83,18 @@ $(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
+$(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
+$(obj)/generated-helpers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+	$(call quiet-command,$(TRACETOOL) \
+		--format=tcg-helper-c \
+		--backend=tcg \
+		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
+
+$(obj)/generated-helpers.o: $(obj)/generated-helpers.c
+
+target-obj-y += generated-helpers.o
+
 
 ######################################################################
 # Backend code

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 08/12] trace: [tcg] Include TCG-tracing helpers on all helper.h
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (6 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 07/12] trace: [tcg] Define " Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 09/12] trace: [tcg] Generate TCG tracing routines Lluís Vilanova
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Jia Liu, Alexander Graf, Blue Swirl, Max Filippov,
	Michael Walle, open list:PowerPC, Stefan Hajnoczi,
	Edgar E. Iglesias, Guan Xuetao, Aurelien Jarno,
	Richard Henderson

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 target-alpha/helper.h      |    2 ++
 target-arm/helper.h        |    2 ++
 target-cris/helper.h       |    2 ++
 target-i386/helper.h       |    2 ++
 target-lm32/helper.h       |    2 ++
 target-m68k/helper.h       |    2 ++
 target-microblaze/helper.h |    2 ++
 target-mips/helper.h       |    2 ++
 target-openrisc/helper.h   |    2 ++
 target-ppc/helper.h        |    2 ++
 target-s390x/helper.h      |    2 ++
 target-sh4/helper.h        |    2 ++
 target-sparc/helper.h      |    2 ++
 target-unicore32/helper.h  |    2 ++
 target-xtensa/helper.h     |    2 ++
 15 files changed, 30 insertions(+)

diff --git a/target-alpha/helper.h b/target-alpha/helper.h
index 5a0e78c..5a617fd 100644
--- a/target-alpha/helper.h
+++ b/target-alpha/helper.h
@@ -121,4 +121,6 @@ DEF_HELPER_FLAGS_0(get_walltime, TCG_CALL_NO_RWG, i64)
 DEF_HELPER_FLAGS_2(set_alarm, TCG_CALL_NO_RWG, void, env, i64)
 #endif
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-arm/helper.h b/target-arm/helper.h
index 70872df..a265d13 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -496,4 +496,6 @@ DEF_HELPER_4(crypto_aesmc, void, env, i32, i32, i32)
 #include "helper-a64.h"
 #endif
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-cris/helper.h b/target-cris/helper.h
index 0ac31f5..9437149 100644
--- a/target-cris/helper.h
+++ b/target-cris/helper.h
@@ -26,4 +26,6 @@ DEF_HELPER_FLAGS_3(evaluate_flags_move_2, TCG_CALL_NO_SE, i32, env, i32, i32)
 DEF_HELPER_1(evaluate_flags, void, env)
 DEF_HELPER_1(top_evaluate_flags, void, env)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 3775abe..4428a10 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -220,4 +220,6 @@ DEF_HELPER_3(rclq, tl, env, tl, tl)
 DEF_HELPER_3(rcrq, tl, env, tl, tl)
 #endif
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-lm32/helper.h b/target-lm32/helper.h
index 3ea15a6..2f456d3 100644
--- a/target-lm32/helper.h
+++ b/target-lm32/helper.h
@@ -11,4 +11,6 @@ DEF_HELPER_1(rcsr_ip, i32, env)
 DEF_HELPER_1(rcsr_jtx, i32, env)
 DEF_HELPER_1(rcsr_jrx, i32, env)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-m68k/helper.h b/target-m68k/helper.h
index 2b02450..4722bfe 100644
--- a/target-m68k/helper.h
+++ b/target-m68k/helper.h
@@ -51,4 +51,6 @@ DEF_HELPER_3(set_mac_extu, void, env, i32, i32)
 DEF_HELPER_2(flush_flags, void, env, i32)
 DEF_HELPER_2(raise_exception, void, env, i32)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-microblaze/helper.h b/target-microblaze/helper.h
index 4e51429..b764f52 100644
--- a/target-microblaze/helper.h
+++ b/target-microblaze/helper.h
@@ -38,4 +38,6 @@ DEF_HELPER_2(stackprot, void, env, i32)
 DEF_HELPER_2(get, i32, i32, i32)
 DEF_HELPER_3(put, void, i32, i32, i32)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-mips/helper.h b/target-mips/helper.h
index 1a8b86d..69a2fc3 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -692,4 +692,6 @@ DEF_HELPER_FLAGS_2(rddsp, 0, tl, tl, env)
 
 
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-openrisc/helper.h b/target-openrisc/helper.h
index 2af9790..0b3222c 100644
--- a/target-openrisc/helper.h
+++ b/target-openrisc/helper.h
@@ -67,4 +67,6 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env)
 DEF_HELPER_FLAGS_4(mtspr, 0, void, env, tl, tl, tl)
 DEF_HELPER_FLAGS_4(mfspr, 0, tl, env, tl, tl, tl)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 6d282bb..588369c 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -407,4 +407,6 @@ DEF_HELPER_3(store_601_batl, void, env, i32, tl)
 DEF_HELPER_3(store_601_batu, void, env, i32, tl)
 #endif
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-s390x/helper.h b/target-s390x/helper.h
index 0d80aa0..b5672e7 100644
--- a/target-s390x/helper.h
+++ b/target-s390x/helper.h
@@ -116,4 +116,6 @@ DEF_HELPER_2(lra, i64, env, i64)
 DEF_HELPER_FLAGS_3(stura, TCG_CALL_NO_WG, void, env, i64, i64)
 #endif
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-sh4/helper.h b/target-sh4/helper.h
index 7162448..95e99aa 100644
--- a/target-sh4/helper.h
+++ b/target-sh4/helper.h
@@ -47,4 +47,6 @@ DEF_HELPER_2(ftrc_DT, i32, env, f64)
 DEF_HELPER_3(fipr, void, env, i32, i32)
 DEF_HELPER_2(ftrv, void, env, i32)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 2a771b2..e2bbc5c 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -174,4 +174,6 @@ VIS_CMPHELPER(cmpne)
 DEF_HELPER_1(compute_psr, void, env)
 DEF_HELPER_1(compute_C_icc, i32, env)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-unicore32/helper.h b/target-unicore32/helper.h
index e85ce6c..675ca48 100644
--- a/target-unicore32/helper.h
+++ b/target-unicore32/helper.h
@@ -65,4 +65,6 @@ DEF_HELPER_2(ucf64_si2df, f64, f32, env)
 DEF_HELPER_2(ucf64_sf2si, f32, f32, env)
 DEF_HELPER_2(ucf64_df2si, f32, f64, env)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"
diff --git a/target-xtensa/helper.h b/target-xtensa/helper.h
index 38d7157..11ced0a 100644
--- a/target-xtensa/helper.h
+++ b/target-xtensa/helper.h
@@ -58,4 +58,6 @@ DEF_HELPER_4(ult_s, void, env, i32, f32, f32)
 DEF_HELPER_4(ole_s, void, env, i32, f32, f32)
 DEF_HELPER_4(ule_s, void, env, i32, f32, f32)
 
+#include "trace/generated-helpers.h"
+
 #include "exec/def-helper.h"

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 09/12] trace: [tcg] Generate TCG tracing routines
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (7 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 08/12] trace: [tcg] Include TCG-tracing helpers on all helper.h Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 10/12] trace: [trivial] Include event definitions in "trace.h" Lluís Vilanova
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Generate header "trace/generated-tcg-tracers.h" with the necessary routines for
tracing events in guest code:

* trace_${event}_tcg

  A shorter routine name to call the 'gen_helper_trace_${event}_tcg' routine.

Makes the coding more similar to that of "trace.h", where "trace_*_tcg" is a
common routine name to clearly denote a TCG tracing action.

NOTE: These routines will be renamed to 'trace_${event}_tcg_backend' when TCG
      instrumentation is added.


Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 .gitignore                        |    1 +
 Makefile                          |    2 ++
 include/trace-tcg.h               |    7 ++++++
 scripts/tracetool/backend/tcg.py  |   17 +++++++++++++
 scripts/tracetool/format/tcg_h.py |   47 +++++++++++++++++++++++++++++++++++++
 trace/Makefile.objs               |    9 +++++++
 6 files changed, 83 insertions(+)
 create mode 100644 include/trace-tcg.h
 create mode 100644 scripts/tracetool/format/tcg_h.py

diff --git a/.gitignore b/.gitignore
index 418546c..858a5cb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,6 +12,7 @@ trace/generated-events.h
 trace/generated-events.c
 trace/generated-helpers.h
 trace/generated-helpers.c
+trace/generated-tcg-tracers.h
 libcacard/trace/generated-tracers.c
 *-timestamp
 *-softmmu
diff --git a/Makefile b/Makefile
index 910fb4f..3100439 100644
--- a/Makefile
+++ b/Makefile
@@ -57,6 +57,8 @@ GENERATED_HEADERS += trace/generated-tracers-dtrace.h
 endif
 GENERATED_SOURCES += trace/generated-tracers.c
 
+GENERATED_HEADERS += trace/generated-tcg-tracers.h
+
 GENERATED_HEADERS += trace/generated-helpers.h
 GENERATED_SOURCES += trace/generated-helpers.c
 
diff --git a/include/trace-tcg.h b/include/trace-tcg.h
new file mode 100644
index 0000000..6f6bdbb
--- /dev/null
+++ b/include/trace-tcg.h
@@ -0,0 +1,7 @@
+#ifndef TRACE_TCG_H
+#define TRACE_TCG_H
+
+#include "trace/generated-tcg-tracers.h"
+#include "trace/generated-events.h"
+
+#endif  /* TRACE_TCG_H */
diff --git a/scripts/tracetool/backend/tcg.py b/scripts/tracetool/backend/tcg.py
index b60fb7e..359e5f5 100644
--- a/scripts/tracetool/backend/tcg.py
+++ b/scripts/tracetool/backend/tcg.py
@@ -106,3 +106,20 @@ def tcg_helper_c(events):
             args=e.args.transform(HOST_2_TCG_COMPAT, TCG_2_HOST),
             values=", ".join(values),
             )
+
+
+def tcg_h(events):
+    for e in events:
+        if "tcg" not in e.properties:
+            continue
+
+        e = e.original
+
+        out('static inline void %(name)s(%(args)s)',
+            '{',
+            '    gen_helper_%(name)s(%(argnames)s);',
+            '}',
+            name=e.api(e.QEMU_TRACE_TCG),
+            args=e.args,
+            argnames=", ".join(e.args.names()),
+            )
diff --git a/scripts/tracetool/format/tcg_h.py b/scripts/tracetool/format/tcg_h.py
new file mode 100644
index 0000000..58e992e
--- /dev/null
+++ b/scripts/tracetool/format/tcg_h.py
@@ -0,0 +1,47 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generate .h file for TCG code generation.
+"""
+
+__author__     = "Lluís Vilanova <vilanova@ac.upc.edu>"
+__copyright__  = "Copyright 2012-2014, Lluís Vilanova <vilanova@ac.upc.edu>"
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@linux.vnet.ibm.com"
+
+
+from tracetool import out
+
+
+def begin(events):
+    out('/* This file is autogenerated by tracetool, do not edit. */',
+        '/* You must include this file after the inclusion of helper.h */',
+        '',
+        '#ifndef TRACE__GENERATED_TCG_TRACERS_H',
+        '#define TRACE__GENERATED_TCG_TRACERS_H',
+        '',
+        '#include <stdint.h>',
+        '',
+        )
+
+
+def end(events):
+    out('#endif /* TRACE__GENERATED_TCG_TRACERS_H */')
+
+
+def nop(events):
+    for e in events:
+        if "tcg" not in e.properties:
+            continue
+
+        e = e.original
+
+        out('static inline void %(name)s(%(args)s)',
+            '{',
+            '}',
+            name=e.api(e.QEMU_TRACE_TCG),
+            args=e.args,
+            )
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index de7e9a4..0056aec 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -96,6 +96,15 @@ $(obj)/generated-helpers.o: $(obj)/generated-helpers.c
 target-obj-y += generated-helpers.o
 
 
+$(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
+$(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
+	$(call quiet-command,$(TRACETOOL) \
+		--format=tcg-h \
+		--backend=tcg \
+		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
+
+
 ######################################################################
 # Backend code
 

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 10/12] trace: [trivial] Include event definitions in "trace.h"
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (8 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 09/12] trace: [tcg] Generate TCG tracing routines Lluís Vilanova
@ 2014-01-31 16:09 ` Lluís Vilanova
  2014-01-31 16:10 ` [Qemu-devel] [PATCH 11/12] trace: [tcg] Include TCG-tracing header on all targets Lluís Vilanova
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

Otherwise the user has to explicitly include an auto-generated header.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 include/trace.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/trace.h b/include/trace.h
index c15f498..44a1f1f 100644
--- a/include/trace.h
+++ b/include/trace.h
@@ -2,5 +2,6 @@
 #define TRACE_H
 
 #include "trace/generated-tracers.h"
+#include "trace/generated-events.h"
 
 #endif  /* TRACE_H */

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 11/12] trace: [tcg] Include TCG-tracing header on all targets
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (9 preceding siblings ...)
  2014-01-31 16:09 ` [Qemu-devel] [PATCH 10/12] trace: [trivial] Include event definitions in "trace.h" Lluís Vilanova
@ 2014-01-31 16:10 ` Lluís Vilanova
  2014-01-31 16:10 ` [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event Lluís Vilanova
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Jia Liu, Alexander Graf, Blue Swirl, Max Filippov,
	Michael Walle, open list:PowerPC, Stefan Hajnoczi,
	Edgar E. Iglesias, Guan Xuetao, Aurelien Jarno,
	Richard Henderson

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 target-alpha/translate.c      |    3 +++
 target-arm/translate.c        |    3 +++
 target-cris/translate.c       |    3 +++
 target-i386/translate.c       |    3 +++
 target-lm32/translate.c       |    3 +++
 target-m68k/translate.c       |    3 +++
 target-microblaze/translate.c |    3 +++
 target-mips/translate.c       |    3 +++
 target-openrisc/translate.c   |    3 +++
 target-ppc/translate.c        |    3 +++
 target-s390x/translate.c      |    2 ++
 target-sh4/translate.c        |    3 +++
 target-sparc/translate.c      |    3 +++
 target-unicore32/translate.c  |    3 +++
 target-xtensa/translate.c     |    3 +++
 15 files changed, 44 insertions(+)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 1155e86..a0d5a31 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -26,6 +26,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #undef ALPHA_DEBUG_DISAS
 #define CONFIG_SOFTFLOAT_INLINE
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 8d240e1..c8c0097 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -34,6 +34,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define ENABLE_ARCH_4T    arm_feature(env, ARM_FEATURE_V4T)
 #define ENABLE_ARCH_5     arm_feature(env, ARM_FEATURE_V5)
 /* currently all emulated v5 cores are also v5TE, so don't bother */
diff --git a/target-cris/translate.c b/target-cris/translate.c
index f990d59..1223de5 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -33,6 +33,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define DISAS_CRIS 0
 #if DISAS_CRIS
 #  define LOG_DIS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index b0f2279..2b8a3e6 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -32,6 +32,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define PREFIX_REPZ   0x01
 #define PREFIX_REPNZ  0x02
 #define PREFIX_LOCK   0x04
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 6ea0ecd..f703c09 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -27,6 +27,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define DISAS_LM32 1
 #if DISAS_LM32
 #  define LOG_DIS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index f54b94a..f30fd21 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -27,6 +27,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 //#define DEBUG_DISPATCH 1
 
 /* Fake floating point.  */
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 270138c..275e87b 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -27,6 +27,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define SIM_COMPAT 0
 #define DISAS_GNU 1
 #define DISAS_MB 1
diff --git a/target-mips/translate.c b/target-mips/translate.c
index ef0a2c3..abb5fe5 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -29,6 +29,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define MIPS_DEBUG_DISAS 0
 //#define MIPS_DEBUG_SIGN_EXTENSIONS
 
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index b381477..d2c730c 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -31,6 +31,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define OPENRISC_DISAS
 
 #ifdef OPENRISC_DISAS
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index c5c1108..783034b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -27,6 +27,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define CPU_SINGLE_STEP 0x1
 #define CPU_BRANCH_STEP 0x2
 #define GDBSTUB_SINGLE_STEP 0x4
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index bc99a37..1c73029 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -42,6 +42,8 @@ static TCGv_ptr cpu_env;
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
 
 /* Information that (most) every instruction needs to manipulate.  */
 typedef struct DisasContext DisasContext;
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index 661fc6c..9577203 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -28,6 +28,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 typedef struct DisasContext {
     struct TranslationBlock *tb;
     target_ulong pc;
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 6150b22..705b53a 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -32,6 +32,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 #define DEBUG_DISAS
 
 #define DYNAMIC_PC  1 /* dynamic pc value */
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index 4572890..e0d3275 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -23,6 +23,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 /* internal defines */
 typedef struct DisasContext {
     target_ulong pc;
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 2d2df33..d04b975 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -41,6 +41,9 @@
 #define GEN_HELPER 1
 #include "helper.h"
 
+#include "trace-tcg.h"
+
+
 typedef struct DisasContext {
     const XtensaConfig *config;
     TranslationBlock *tb;

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (10 preceding siblings ...)
  2014-01-31 16:10 ` [Qemu-devel] [PATCH 11/12] trace: [tcg] Include TCG-tracing header on all targets Lluís Vilanova
@ 2014-01-31 16:10 ` Lluís Vilanova
  2014-02-04 15:08   ` Richard Henderson
  2014-02-03 14:40 ` [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Stefan Hajnoczi
  2014-02-04 14:57 ` Richard Henderson
  13 siblings, 1 reply; 30+ messages in thread
From: Lluís Vilanova @ 2014-01-31 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Richard Henderson

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 include/exec/cpu-all.h        |   58 +++++++++++++++++++++--------------------
 include/exec/exec-all.h       |    3 ++
 include/exec/softmmu_header.h |   17 ++++++++++++
 tcg/tcg-op.h                  |    8 ++++++
 tcg/tcg.c                     |    1 +
 trace-events                  |   15 +++++++++++
 trace/tcg-op-internal.h       |   55 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 129 insertions(+), 28 deletions(-)
 create mode 100644 trace/tcg-op-internal.h

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 4cb4b4a..4ecb486 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -250,21 +250,23 @@ extern unsigned long reserved_va;
 
 #if defined(CONFIG_USER_ONLY)
 
+#include "trace.h"
+
 /* if user mode, no other memory access functions */
-#define ldub(p) ldub_raw(p)
-#define ldsb(p) ldsb_raw(p)
-#define lduw(p) lduw_raw(p)
-#define ldsw(p) ldsw_raw(p)
-#define ldl(p) ldl_raw(p)
-#define ldq(p) ldq_raw(p)
-#define ldfl(p) ldfl_raw(p)
-#define ldfq(p) ldfq_raw(p)
-#define stb(p, v) stb_raw(p, v)
-#define stw(p, v) stw_raw(p, v)
-#define stl(p, v) stl_raw(p, v)
-#define stq(p, v) stq_raw(p, v)
-#define stfl(p, v) stfl_raw(p, v)
-#define stfq(p, v) stfq_raw(p, v)
+#define ldub(p)    ({ trace_guest_vmem(p, 1, 0); ldub_raw(p);    })
+#define ldsb(p)    ({ trace_guest_vmem(p, 1, 0); ldsb_raw(p);    })
+#define lduw(p)    ({ trace_guest_vmem(p, 2, 0); lduw_raw(p);    })
+#define ldsw(p)    ({ trace_guest_vmem(p, 2, 0); ldsw_raw(p);    })
+#define ldl(p)     ({ trace_guest_vmem(p, 4, 0); ldl_raw(p);     })
+#define ldq(p)     ({ trace_guest_vmem(p, 8, 0); ldq_raw(p);     })
+#define ldfl(p)    ({ trace_guest_vmem(p, 4, 0); ldfl_raw(p);    })
+#define ldfq(p)    ({ trace_guest_vmem(p, 8, 0); ldfq_raw(p);    })
+#define stb(p, v)  ({ trace_guest_vmem(p, 1, 1); stb_raw(p, v);  })
+#define stw(p, v)  ({ trace_guest_vmem(p, 2, 1); stw_raw(p, v);  })
+#define stl(p, v)  ({ trace_guest_vmem(p, 4, 1); stl_raw(p, v);  })
+#define stq(p, v)  ({ trace_guest_vmem(p, 8, 1); stq_raw(p, v);  })
+#define stfl(p, v) ({ trace_guest_vmem(p, 4, 1); stfl_raw(p, v); })
+#define stfq(p, v) ({ trace_guest_vmem(p, 8, 1); stfq_raw(p, v); })
 
 #define cpu_ldub_code(env1, p) ldub_raw(p)
 #define cpu_ldsb_code(env1, p) ldsb_raw(p)
@@ -295,20 +297,20 @@ extern unsigned long reserved_va;
 #define cpu_stl_kernel(env, addr, data) stl_raw(addr, data)
 #define cpu_stq_kernel(env, addr, data) stq_raw(addr, data)
 
-#define ldub_kernel(p) ldub_raw(p)
-#define ldsb_kernel(p) ldsb_raw(p)
-#define lduw_kernel(p) lduw_raw(p)
-#define ldsw_kernel(p) ldsw_raw(p)
-#define ldl_kernel(p) ldl_raw(p)
-#define ldq_kernel(p) ldq_raw(p)
-#define ldfl_kernel(p) ldfl_raw(p)
-#define ldfq_kernel(p) ldfq_raw(p)
-#define stb_kernel(p, v) stb_raw(p, v)
-#define stw_kernel(p, v) stw_raw(p, v)
-#define stl_kernel(p, v) stl_raw(p, v)
-#define stq_kernel(p, v) stq_raw(p, v)
-#define stfl_kernel(p, v) stfl_raw(p, v)
-#define stfq_kernel(p, vt) stfq_raw(p, v)
+#define ldub_kernel(p)     ({ trace_guest_vmem(p, 1, 0); ldub_raw(p);    })
+#define ldsb_kernel(p)     ({ trace_guest_vmem(p, 1, 0); ldsb_raw(p);    })
+#define lduw_kernel(p)     ({ trace_guest_vmem(p, 2, 0); lduw_raw(p);    })
+#define ldsw_kernel(p)     ({ trace_guest_vmem(p, 2, 0); ldsw_raw(p);    })
+#define ldl_kernel(p)      ({ trace_guest_vmem(p, 4, 0); ldl_raw(p);     })
+#define ldq_kernel(p)      ({ trace_guest_vmem(p, 8, 0); ldq_raw(p);     })
+#define ldfl_kernel(p)     ({ trace_guest_vmem(p, 4, 0); ldfl_raw(p);    })
+#define ldfq_kernel(p)     ({ trace_guest_vmem(p, 8, 0); ldfq_raw(p);    })
+#define stb_kernel(p, v)   ({ trace_guest_vmem(p, 1, 1); stb_raw(p, v);  })
+#define stw_kernel(p, v)   ({ trace_guest_vmem(p, 2, 1); stw_raw(p, v);  })
+#define stl_kernel(p, v)   ({ trace_guest_vmem(p, 4, 1); stl_raw(p, v);  })
+#define stq_kernel(p, v)   ({ trace_guest_vmem(p, 8, 1); stq_raw(p, v);  })
+#define stfl_kernel(p, v)  ({ trace_guest_vmem(p, 4, 1); stfl_raw(p, v); })
+#define stfq_kernel(p, vt) ({ trace_guest_vmem(p, 8, 1); stfq_raw(p, v); })
 
 #define cpu_ldub_data(env, addr) ldub_raw(addr)
 #define cpu_lduw_data(env, addr) lduw_raw(addr)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index ea90b64..f30cc4e 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -339,6 +339,8 @@ uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
 uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
 
 #define ACCESS_TYPE (NB_MMU_MODES + 1)
+/* do not trace '*_code' accesses during instruction disassembly */
+#define TRACE_TCG_CODE_ACCESSOR 1
 #define MEMSUFFIX _code
 
 #define DATA_SIZE 1
@@ -354,6 +356,7 @@ uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
 #include "exec/softmmu_header.h"
 
 #undef ACCESS_TYPE
+#undef TRACE_TCG_CODE_ACCESSOR
 #undef MEMSUFFIX
 
 #endif
diff --git a/include/exec/softmmu_header.h b/include/exec/softmmu_header.h
index d8d9c81..ccd9cb1 100644
--- a/include/exec/softmmu_header.h
+++ b/include/exec/softmmu_header.h
@@ -25,6 +25,11 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+
+#if !defined(TRACE_TCG_CODE_ACCESSOR)
+#include "trace.h"
+#endif
+
 #if DATA_SIZE == 8
 #define SUFFIX q
 #define USUFFIX q
@@ -88,6 +93,10 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
     target_ulong addr;
     int mmu_idx;
 
+#if !defined(TRACE_TCG_CODE_ACCESSOR)
+    trace_guest_vmem(ptr, DATA_SIZE, 0);
+#endif
+
     addr = ptr;
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = CPU_MMU_INDEX;
@@ -109,6 +118,10 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
     target_ulong addr;
     int mmu_idx;
 
+#if !defined(TRACE_TCG_CODE_ACCESSOR)
+    trace_guest_vmem(ptr, DATA_SIZE, 0);
+#endif
+
     addr = ptr;
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = CPU_MMU_INDEX;
@@ -136,6 +149,10 @@ glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr,
     target_ulong addr;
     int mmu_idx;
 
+#if !defined(TRACE_TCG_CODE_ACCESSOR)
+    trace_guest_vmem(ptr, DATA_SIZE, 1);
+#endif
+
     addr = ptr;
     page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
     mmu_idx = CPU_MMU_INDEX;
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 7eabf22..0ce2f81 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -2888,3 +2888,11 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv addr, int mem_index)
 # define tcg_gen_ext_i32_ptr(R, A) \
     tcg_gen_ext_i32_i64(TCGV_PTR_TO_NAT(R), (A))
 #endif /* TCG_TARGET_REG_BITS == 32 */
+
+#if !defined(TCG_OP_NOTRACE_GUEST_MEM)
+/* To avoid a circular dependency with helper.h, overload tcg_gen_qemu_*
+ * routines with preprocessor macros to insert TCG virtual memory access
+ * tracing.
+ */
+#include "trace/tcg-op-internal.h"
+#endif
diff --git a/tcg/tcg.c b/tcg/tcg.c
index acd02b9..7847277 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -47,6 +47,7 @@
 #define NO_CPU_IO_DEFS
 #include "cpu.h"
 
+#define TCG_OP_NOTRACE_GUEST_MEM
 #include "tcg-op.h"
 
 #if UINTPTR_MAX == UINT32_MAX
diff --git a/trace-events b/trace-events
index 1b668d1..3f5a55c 100644
--- a/trace-events
+++ b/trace-events
@@ -1186,3 +1186,18 @@ xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (ad
 # hw/pci/pci_host.c
 pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x -> 0x%x"
 pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) "%s %02u:%u @0x%x <- 0x%x"
+
+
+
+## Guest events, keep at bottom
+
+# @vaddr: Access' virtual address.
+# @size : Access' size (bytes).
+# @write: Whether the access is a write.
+#
+# Start virtual memory access (before any potential access violation).
+#
+# This event can be raised at execution time when running in 'user' mode.
+#
+# Targets: TCG(all)
+disable tcg guest_vmem(TCGv vaddr, uint8_t size, uint8_t write) "vaddr=0x%016"PRIx64" size=%d write=%d"
diff --git a/trace/tcg-op-internal.h b/trace/tcg-op-internal.h
new file mode 100644
index 0000000..fea46fa
--- /dev/null
+++ b/trace/tcg-op-internal.h
@@ -0,0 +1,55 @@
+/* -*- mode: c -*-
+ * Copyright (c) 2012-2014 Lluís Vilanova <vilanova@ac.upc.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/**
+ * @file Capture TCG code generation for virtual memory accesses.
+ *
+ * Assumes that no other lower-level call will be performed by target
+ * architecture disassembly code on TCG instructions for accessing memory.
+ *
+ * Capturing calls to higher-level functions like @tcg_gen_qemu_ld8u would allow
+ * using constants for the access size (instead of computing it from the memory
+ * operand argument), but is harder to maintain.
+ */
+
+#ifndef TRACE__TCG_OP_INTERNAL_H
+#define TRACE__TCG_OP_INTERNAL_H
+
+static inline uint8_t _tcg_memop_size(TCGMemOp op)
+{
+    return 1 << (op & MO_SIZE);
+}
+
+#define tcg_gen_qemu_ld_i32(val, addr, idx, memop)      \
+    do {                                                \
+        uint8_t _memop_size = _tcg_memop_size(memop);   \
+        trace_guest_vmem_tcg(addr, _memop_size, 0);     \
+        tcg_gen_qemu_ld_i32(val, addr, idx, memop);     \
+    } while (0)
+
+#define tcg_gen_qemu_st_i32(val, addr, idx, memop)      \
+    do {                                                \
+        uint8_t _memop_size = _tcg_memop_size(memop);   \
+        trace_guest_vmem_tcg(addr, _memop_size, 1);     \
+        tcg_gen_qemu_st_i32(val, addr, idx, memop);     \
+    } while (0)
+
+#define tcg_gen_qemu_ld_i64(val, addr, idx, memop)      \
+    do {                                                \
+        uint8_t _memop_size = _tcg_memop_size(memop);   \
+        trace_guest_vmem_tcg(addr, _memop_size, 0);     \
+        tcg_gen_qemu_ld_i64(val, addr, idx, memop);     \
+    } while (0)
+
+#define tcg_gen_qemu_st_i64(val, addr, idx, memop)      \
+    do {                                                \
+        uint8_t _memop_size = _tcg_memop_size(memop);   \
+        trace_guest_vmem_tcg(addr, _memop_size, 1);     \
+        tcg_gen_qemu_st_i64(val, addr, idx, memop);     \
+    } while (0)
+
+#endif  /* TRACE__TCG_OP_INTERNAL_H */

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (11 preceding siblings ...)
  2014-01-31 16:10 ` [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event Lluís Vilanova
@ 2014-02-03 14:40 ` Stefan Hajnoczi
  2014-02-03 16:24   ` Lluís Vilanova
  2014-02-04 14:57 ` Richard Henderson
  13 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2014-02-03 14:40 UTC (permalink / raw)
  To: Lluís Vilanova
  Cc: Stefan Weil, Peter Maydell, qemu-devel, Aurelien Jarno,
	Richard Henderson

On Fri, Jan 31, 2014 at 05:09:03PM +0100, Lluís Vilanova wrote:
> Adds the base ability to specify which events in the "trace-events" file may be
> used to trace guest activity in the TCG code (using the "tcg" event propery).
> 
> Such events generate an extra set of tracing functions that can be called during
> TCG code generation and will automatically redirect a call to the appropriate
> backend-dependent tracing functions when the guest code is executed.

I've never worked on TCG but this seems like a good idea: a way to plant
trace events in TCG-generated code.

CCing TCG folks to check they are happy with the approach

Once any high-level discussion is done I'll review the tracing changes.

Stefan

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-03 14:40 ` [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Stefan Hajnoczi
@ 2014-02-03 16:24   ` Lluís Vilanova
  0 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-03 16:24 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Stefan Weil, Peter Maydell, qemu-devel, Aurelien Jarno,
	Richard Henderson

Stefan Hajnoczi writes:

> On Fri, Jan 31, 2014 at 05:09:03PM +0100, Lluís Vilanova wrote:
>> Adds the base ability to specify which events in the "trace-events" file may be
>> used to trace guest activity in the TCG code (using the "tcg" event propery).
>> 
>> Such events generate an extra set of tracing functions that can be called during
>> TCG code generation and will automatically redirect a call to the appropriate
>> backend-dependent tracing functions when the guest code is executed.

> I've never worked on TCG but this seems like a good idea: a way to plant
> trace events in TCG-generated code.

> CCing TCG folks to check they are happy with the approach

> Once any high-level discussion is done I'll review the tracing changes.

I have some other guest code events on other patches (e.g., instruction/BBL
execution), but memory accesses are useful and simple enough to show how it
works.

Another question is whether the tracing functions called during translation
(trace_${name}_tcg) should also be themselves traceable (e.g., to see in a trace
how many times and which instructions are translated by QEMU).


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
                   ` (12 preceding siblings ...)
  2014-02-03 14:40 ` [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Stefan Hajnoczi
@ 2014-02-04 14:57 ` Richard Henderson
  2014-02-04 15:02   ` Peter Maydell
  2014-02-04 20:33   ` Lluís Vilanova
  13 siblings, 2 replies; 30+ messages in thread
From: Richard Henderson @ 2014-02-04 14:57 UTC (permalink / raw)
  To: Lluís Vilanova, qemu-devel; +Cc: Stefan Hajnoczi

On 01/31/2014 08:09 AM, Lluís Vilanova wrote:
> Adds the base ability to specify which events in the "trace-events" file may be
> used to trace guest activity in the TCG code (using the "tcg" event propery).
> 
> Such events generate an extra set of tracing functions that can be called during
> TCG code generation and will automatically redirect a call to the appropriate
> backend-dependent tracing functions when the guest code is executed.
> 
> Files generating guest code (TCG) must include "trace-tcg.h". Files declaring
> per-target helpers ("${target}/helper.h") must include
> "trace/generated-helpers.h".
> 
> The flow of the generated routines is:
> 
> 
> [At translation time]
> 
> * trace_${name}_tcg(bool, TCGv)
>   Declared: "trace/generated-tcg-tracers.h"
>   Defined : "trace/generated-tcg-tracers.h"
> 
> * gen_helper_trace_${name}_tcg(bool, TCGv)
>   Declared: "trace/generated-helpers.h"
>   Defined : "trace/generated-helpers.h"
> 
>   Automatically transforms all the arguments and allocates them into the
>   appropriate TCG temporary values (which are also freed). Provides a more
>   streamlined interface by allowing events in "trace-events" to take a mix of
>   tracing-supported types and TCG types.
> 
> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv)
>   Declared: "trace/generated-helpers.h"
>   Defined : "trace/generated-helpers.h" (using helper machinery)
> 
>   The actual TCG helper function, created using QEMU's TCG helper machinery.

I suppose I have no major objection to the feature, although frankly it's
not especially exciting.  I can't really imagine ever wanting to bulk trace
all of the helpers.  Tracing specific helpers on a target-by-target basis,
sure.  But that can be done just as easily as adding tracing code to any
other bit of C.

If I read these patches right -- and since they're mostly python I'm not
sure that I am -- we go through 5 layers of wrappers to get to the current
trace_foo expansion.  Where trace_foo contains the check to see whether the
tracepoint is actually enabled.

I would strongly suggest this is backward.  One should perform the check for
the tracepoint being enabled at translation time before emitting the call to
the helper in the first place.


r~

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-04 14:57 ` Richard Henderson
@ 2014-02-04 15:02   ` Peter Maydell
  2014-02-04 15:17     ` Richard Henderson
  2014-02-04 20:34     ` Lluís Vilanova
  2014-02-04 20:33   ` Lluís Vilanova
  1 sibling, 2 replies; 30+ messages in thread
From: Peter Maydell @ 2014-02-04 15:02 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Lluís Vilanova, Stefan Hajnoczi, QEMU Developers

On 4 February 2014 14:57, Richard Henderson <rth@twiddle.net> wrote:
> I suppose I have no major objection to the feature, although frankly it's
> not especially exciting.  I can't really imagine ever wanting to bulk trace
> all of the helpers.  Tracing specific helpers on a target-by-target basis,
> sure.  But that can be done just as easily as adding tracing code to any
> other bit of C.

I think the things people seem to actually want (judging
from occasional postings to the list) are things like:
 * trace all guest memory accesses
 * trace all guest instruction executions

Does this patchset get us usefully towards that kind of thing?
Not sure...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event
  2014-01-31 16:10 ` [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event Lluís Vilanova
@ 2014-02-04 15:08   ` Richard Henderson
  2014-02-04 20:01     ` Lluís Vilanova
  0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2014-02-04 15:08 UTC (permalink / raw)
  To: Lluís Vilanova, qemu-devel; +Cc: Stefan Hajnoczi

On 01/31/2014 08:10 AM, Lluís Vilanova wrote:
> +#define ldub(p)    ({ trace_guest_vmem(p, 1, 0); ldub_raw(p);    })

Are you sure you want to log these here?  Uses of these macros are
not restricted to the guest.  Therefore you could wind up with e.g.
PCI device accesses being attributed to the target cpu.

> --- a/include/exec/softmmu_header.h
> +++ b/include/exec/softmmu_header.h
> @@ -25,6 +25,11 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
> +
> +#if !defined(TRACE_TCG_CODE_ACCESSOR)
> +#include "trace.h"
> +#endif
> +
>  #if DATA_SIZE == 8
>  #define SUFFIX q
>  #define USUFFIX q
> @@ -88,6 +93,10 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
>      target_ulong addr;
>      int mmu_idx;
>  
> +#if !defined(TRACE_TCG_CODE_ACCESSOR)
> +    trace_guest_vmem(ptr, DATA_SIZE, 0);
> +#endif

These are going to result in double-logging the same access with

> +#define tcg_gen_qemu_ld_i32(val, addr, idx, memop)      \
> +    do {                                                \
> +        uint8_t _memop_size = _tcg_memop_size(memop);   \
> +        trace_guest_vmem_tcg(addr, _memop_size, 0);     \
> +        tcg_gen_qemu_ld_i32(val, addr, idx, memop);     \
> +    } while (0)

... these.

Of course, those softmmu functions are also used by the system emulators in the
same way the ldub macro above is used for userland emulation.  So again you
have non-target accesses being attributed to the target.

Also, doing this action with macros, here, seems truly backward.  Why not
simply modify the real tcg_gen_qemu_ld_i32 in tcg.c?


r~

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-04 15:02   ` Peter Maydell
@ 2014-02-04 15:17     ` Richard Henderson
  2014-02-04 20:44       ` Lluís Vilanova
  2014-02-04 20:34     ` Lluís Vilanova
  1 sibling, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2014-02-04 15:17 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Lluís Vilanova, Stefan Hajnoczi, QEMU Developers

On 02/04/2014 07:02 AM, Peter Maydell wrote:
> On 4 February 2014 14:57, Richard Henderson <rth@twiddle.net> wrote:
>> I suppose I have no major objection to the feature, although frankly it's
>> not especially exciting.  I can't really imagine ever wanting to bulk trace
>> all of the helpers.  Tracing specific helpers on a target-by-target basis,
>> sure.  But that can be done just as easily as adding tracing code to any
>> other bit of C.
> 
> I think the things people seem to actually want (judging
> from occasional postings to the list) are things like:
>  * trace all guest memory accesses
>  * trace all guest instruction executions
> 
> Does this patchset get us usefully towards that kind of thing?
> Not sure...

If that's the goal, I would suggest that they do not.  One does not need to
hook all of the helpers in order to achieve that.

A hook in tcg_gen_qemu_{ld,st}_i{32,64} to (conditionally) emit a call to a
helper to log the access gets you all (non-execution) guest memory accesses.

Guest instruction executions is quite a bit harder, of course.  But any start
in that direction could be done through a pair of trace events: Log the insn
address range covered by a TB + a uuid at translation time; log the uuid at the
start of execution of the TB.  A script should be able to put the two together
to complete the trace.


r~

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event
  2014-02-04 15:08   ` Richard Henderson
@ 2014-02-04 20:01     ` Lluís Vilanova
  2014-02-06 16:12       ` Richard Henderson
  0 siblings, 1 reply; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-04 20:01 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, Stefan Hajnoczi

Richard Henderson writes:

> On 01/31/2014 08:10 AM, Lluís Vilanova wrote:
>> +#define ldub(p)    ({ trace_guest_vmem(p, 1, 0); ldub_raw(p);    })

> Are you sure you want to log these here?  Uses of these macros are
> not restricted to the guest.  Therefore you could wind up with e.g.
> PCI device accesses being attributed to the target cpu.

These defines are only enabled in user-level mode.

But I wrote them really long ago, and I just realized they are not
up-to-date. The changes should also cover the 'cpu_*_data' and 'cpu_*_kernel'
variants. These macros are mostly used in helpers (e.g., helper_boundl).


>> --- a/include/exec/softmmu_header.h
>> +++ b/include/exec/softmmu_header.h
>> @@ -25,6 +25,11 @@
>> * You should have received a copy of the GNU Lesser General Public
>> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> */
>> +
>> +#if !defined(TRACE_TCG_CODE_ACCESSOR)
>> +#include "trace.h"
>> +#endif
>> +
>> #if DATA_SIZE == 8
>> #define SUFFIX q
>> #define USUFFIX q
>> @@ -88,6 +93,10 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
>> target_ulong addr;
>> int mmu_idx;
>> 
>> +#if !defined(TRACE_TCG_CODE_ACCESSOR)
>> +    trace_guest_vmem(ptr, DATA_SIZE, 0);
>> +#endif

> These are going to result in double-logging the same access with

>> +#define tcg_gen_qemu_ld_i32(val, addr, idx, memop)      \
>> +    do {                                                \
>> +        uint8_t _memop_size = _tcg_memop_size(memop);   \
>> +        trace_guest_vmem_tcg(addr, _memop_size, 0);     \
>> +        tcg_gen_qemu_ld_i32(val, addr, idx, memop);     \
>> +    } while (0)

> ... these.

I don't see how 'tcg_gen_qemu_ld_i32' gets to call 'cpu_ldl_data' (for
example). I also did this long ago, so maybe it changed, but a quick look at the
code shows that in softmmu-mode, a TLB miss performs a slow path access with the
functions in "softmmu_template.h", not "softmmu_exec.h" (which includes
"softmmu_header.h").

Maybe you're referring to some case I've missed.


> Of course, those softmmu functions are also used by the system emulators in the
> same way the ldub macro above is used for userland emulation.  So again you
> have non-target accesses being attributed to the target.

What do you mean by non-target accesses? Accesses not directly "encoded" in the
semantics of a guest instruction? If so, I did this in purpose (like the helper
example on the top).


> Also, doing this action with macros, here, seems truly backward.  Why not
> simply modify the real tcg_gen_qemu_ld_i32 in tcg.c?

The 'trace_guest_vmem_tcg' function just calls a helper generator function in
"helper.h", which cannot be included in "tcg.c". Another possibility is to just
forget about using "helper.h", and instead "manually" generate the call to the
helper; but using macros seems to me it's easier to maintain.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-04 14:57 ` Richard Henderson
  2014-02-04 15:02   ` Peter Maydell
@ 2014-02-04 20:33   ` Lluís Vilanova
  2014-02-06 15:57     ` Richard Henderson
  1 sibling, 1 reply; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-04 20:33 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, Stefan Hajnoczi

Richard Henderson writes:

> On 01/31/2014 08:09 AM, Lluís Vilanova wrote:
>> Adds the base ability to specify which events in the "trace-events" file may be
>> used to trace guest activity in the TCG code (using the "tcg" event propery).
>> 
>> Such events generate an extra set of tracing functions that can be called during
>> TCG code generation and will automatically redirect a call to the appropriate
>> backend-dependent tracing functions when the guest code is executed.
>> 
>> Files generating guest code (TCG) must include "trace-tcg.h". Files declaring
>> per-target helpers ("${target}/helper.h") must include
>> "trace/generated-helpers.h".
>> 
>> The flow of the generated routines is:
>> 
>> 
>> [At translation time]
>> 
>> * trace_${name}_tcg(bool, TCGv)
>> Declared: "trace/generated-tcg-tracers.h"
>> Defined : "trace/generated-tcg-tracers.h"
>> 
>> * gen_helper_trace_${name}_tcg(bool, TCGv)
>> Declared: "trace/generated-helpers.h"
>> Defined : "trace/generated-helpers.h"
>> 
>> Automatically transforms all the arguments and allocates them into the
>> appropriate TCG temporary values (which are also freed). Provides a more
>> streamlined interface by allowing events in "trace-events" to take a mix of
>> tracing-supported types and TCG types.
>> 
>> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv)
>> Declared: "trace/generated-helpers.h"
>> Defined : "trace/generated-helpers.h" (using helper machinery)
>> 
>> The actual TCG helper function, created using QEMU's TCG helper machinery.

> I suppose I have no major objection to the feature, although frankly it's
> not especially exciting.  I can't really imagine ever wanting to bulk trace
> all of the helpers.  Tracing specific helpers on a target-by-target basis,
> sure.  But that can be done just as easily as adding tracing code to any
> other bit of C.

I'm not sure I understand this comment. The patch does not add helper tracing
capabilities, but generates a "trace_foo_tcg" routine that can be called during
TCG code generation, and will call "trace_foo" when that TCG code is
executed.


> If I read these patches right -- and since they're mostly python I'm not
> sure that I am -- we go through 5 layers of wrappers to get to the current
> trace_foo expansion.  Where trace_foo contains the check to see whether the
> tracepoint is actually enabled.

As a side-note, all layers should be optimized by the compiler (except for the
TCG code generation/execution separation), since they're all "static inline".


> I would strongly suggest this is backward.  One should perform the check for
> the tracepoint being enabled at translation time before emitting the call to
> the helper in the first place.

Right, the thing is that dynamically enabling/disabling such events will not
immediately show up in the trace if I do the check at translation time
(trace_foo_tcg), since QEMU will execute the cached TCG translations. I see the
following solutions to ensure traces are accurate:

* Delay the check until execution time.

* Check at translation time; flush translation cache when the tracing state of
  a TCG event changes.

* Check at translation time; use multiple translation caches, one for each
  possible tracing state of all the TCG-enabled events.

This series implements the first approach, since it's correct and much
simpler.

Other patches I did not send implement the third approach, which is quite
efficient if one is dynamically switching the tracing state while executing
mostly-cached code (e.g., profiling the accesses).

I can wait for a later series to send the third option, or even implement the
second, but I just wanted to keep this one as simple as possible. Also,
implementing any of these two last approaches would provide minimal overheads on
builds that have such events enabled at compile time.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-04 15:02   ` Peter Maydell
  2014-02-04 15:17     ` Richard Henderson
@ 2014-02-04 20:34     ` Lluís Vilanova
  1 sibling, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-04 20:34 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Stefan Hajnoczi, Richard Henderson

Peter Maydell writes:

> On 4 February 2014 14:57, Richard Henderson <rth@twiddle.net> wrote:
>> I suppose I have no major objection to the feature, although frankly it's
>> not especially exciting.  I can't really imagine ever wanting to bulk trace
>> all of the helpers.  Tracing specific helpers on a target-by-target basis,
>> sure.  But that can be done just as easily as adding tracing code to any
>> other bit of C.

> I think the things people seem to actually want (judging
> from occasional postings to the list) are things like:
>  * trace all guest memory accesses
>  * trace all guest instruction executions

> Does this patchset get us usefully towards that kind of thing?
> Not sure...

It does trace guest memory accesses (last patch), but I also have other patches
(not sent to keep things simple) for tracing instructions and BBLs executed, as
well as the registers accessed by these.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-04 15:17     ` Richard Henderson
@ 2014-02-04 20:44       ` Lluís Vilanova
  0 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-04 20:44 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Peter Maydell, QEMU Developers, Stefan Hajnoczi

Richard Henderson writes:

> On 02/04/2014 07:02 AM, Peter Maydell wrote:
>> On 4 February 2014 14:57, Richard Henderson <rth@twiddle.net> wrote:
>>> I suppose I have no major objection to the feature, although frankly it's
>>> not especially exciting.  I can't really imagine ever wanting to bulk trace
>>> all of the helpers.  Tracing specific helpers on a target-by-target basis,
>>> sure.  But that can be done just as easily as adding tracing code to any
>>> other bit of C.
>> 
>> I think the things people seem to actually want (judging
>> from occasional postings to the list) are things like:
>> * trace all guest memory accesses
>> * trace all guest instruction executions
>> 
>> Does this patchset get us usefully towards that kind of thing?
>> Not sure...

> If that's the goal, I would suggest that they do not.  One does not need to
> hook all of the helpers in order to achieve that.

> A hook in tcg_gen_qemu_{ld,st}_i{32,64} to (conditionally) emit a call to a
> helper to log the access gets you all (non-execution) guest memory accesses.

That's what this series does, but in a generic way so that you can trace any
event that is "identified" at translation time (i.e., when calling
'trace_foo_tcg', like 'trace_guest_vmem_tcg' in the last patch).


> Guest instruction executions is quite a bit harder, of course.  But any start
> in that direction could be done through a pair of trace events: Log the insn
> address range covered by a TB + a uuid at translation time; log the uuid at the
> start of execution of the TB.  A script should be able to put the two together
> to complete the trace.

Right, that's a common approach to have a much more compact trace (people
usually call it BBL dictionary).

I could extend the patches so that calling 'trace_foo_tcg' also generated a
traceable event. This way every event could be traced at translation and/or
execution time. Thus adding this on "trace-events":

  tcg foo(...) "..."

Would be equivalent to:

  foo(...) "..."
  foo_tcg(...) "..."

If you enable the "foo_tcg" event, you'll see traces from translation time
(calls to 'trace_foo_tcg'). If you enable the "foo" event, 'trace_foo_tcg' will
generate a call to 'trace_foo', and thus you'll see traces from execution time.

Does this make sense?


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-04 20:33   ` Lluís Vilanova
@ 2014-02-06 15:57     ` Richard Henderson
  2014-02-06 19:26       ` Lluís Vilanova
  0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2014-02-06 15:57 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi

On 02/04/2014 12:33 PM, Lluís Vilanova wrote:
> Richard Henderson writes:
> 
>> On 01/31/2014 08:09 AM, Lluís Vilanova wrote:
>>> Adds the base ability to specify which events in the "trace-events" file may be
>>> used to trace guest activity in the TCG code (using the "tcg" event propery).
>>>
>>> Such events generate an extra set of tracing functions that can be called during
>>> TCG code generation and will automatically redirect a call to the appropriate
>>> backend-dependent tracing functions when the guest code is executed.
>>>
>>> Files generating guest code (TCG) must include "trace-tcg.h". Files declaring
>>> per-target helpers ("${target}/helper.h") must include
>>> "trace/generated-helpers.h".
>>>
>>> The flow of the generated routines is:
>>>
>>>
>>> [At translation time]
>>>
>>> * trace_${name}_tcg(bool, TCGv)
>>> Declared: "trace/generated-tcg-tracers.h"
>>> Defined : "trace/generated-tcg-tracers.h"
>>>
>>> * gen_helper_trace_${name}_tcg(bool, TCGv)
>>> Declared: "trace/generated-helpers.h"
>>> Defined : "trace/generated-helpers.h"
>>>
>>> Automatically transforms all the arguments and allocates them into the
>>> appropriate TCG temporary values (which are also freed). Provides a more
>>> streamlined interface by allowing events in "trace-events" to take a mix of
>>> tracing-supported types and TCG types.
>>>
>>> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv)
>>> Declared: "trace/generated-helpers.h"
>>> Defined : "trace/generated-helpers.h" (using helper machinery)
>>>
>>> The actual TCG helper function, created using QEMU's TCG helper machinery.
> 
>> I suppose I have no major objection to the feature, although frankly it's
>> not especially exciting.  I can't really imagine ever wanting to bulk trace
>> all of the helpers.  Tracing specific helpers on a target-by-target basis,
>> sure.  But that can be done just as easily as adding tracing code to any
>> other bit of C.
> 
> I'm not sure I understand this comment. The patch does not add helper tracing
> capabilities, but generates a "trace_foo_tcg" routine that can be called during
> TCG code generation, and will call "trace_foo" when that TCG code is
> executed.

Ah, see, I told you I was probably reading the patches wrong.  With all the
helpers.h changes, I thought this was somehow related to tracing the existing
helpers.  But the existance of trace_foo_tcg is triggered by the trace-events file?

>> I would strongly suggest this is backward.  One should perform the check for
>> the tracepoint being enabled at translation time before emitting the call to
>> the helper in the first place.
> 
> Right, the thing is that dynamically enabling/disabling such events will not
> immediately show up in the trace if I do the check at translation time
> (trace_foo_tcg), since QEMU will execute the cached TCG translations. I see the
> following solutions to ensure traces are accurate:
> 
> * Delay the check until execution time.
> 
> * Check at translation time; flush translation cache when the tracing state of
>   a TCG event changes.
> 
> * Check at translation time; use multiple translation caches, one for each
>   possible tracing state of all the TCG-enabled events.
> 
> This series implements the first approach, since it's correct and much
> simpler.
> 
> Other patches I did not send implement the third approach, which is quite
> efficient if one is dynamically switching the tracing state while executing
> mostly-cached code (e.g., profiling the accesses).

How often do events change state?  My guess is exceedingly rarely.
And by "rare" I mean something well under once per minute.  ;-)

At which point, option 2 would be the best bet, I think.

> I can wait for a later series to send the third option, or even implement the
> second, but I just wanted to keep this one as simple as possible. Also,
> implementing any of these two last approaches would provide minimal overheads on
> builds that have such events enabled at compile time.

Fair enough.


r~

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event
  2014-02-04 20:01     ` Lluís Vilanova
@ 2014-02-06 16:12       ` Richard Henderson
  2014-02-10 13:29         ` Lluís Vilanova
  0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2014-02-06 16:12 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi

On 02/04/2014 12:01 PM, Lluís Vilanova wrote:
> Richard Henderson writes:
> 
>> On 01/31/2014 08:10 AM, Lluís Vilanova wrote:
>>> +#define ldub(p)    ({ trace_guest_vmem(p, 1, 0); ldub_raw(p);    })
> 
>> Are you sure you want to log these here?  Uses of these macros are
>> not restricted to the guest.  Therefore you could wind up with e.g.
>> PCI device accesses being attributed to the target cpu.
> 
> These defines are only enabled in user-level mode.
> 
> But I wrote them really long ago, and I just realized they are not
> up-to-date. The changes should also cover the 'cpu_*_data' and 'cpu_*_kernel'
> variants. These macros are mostly used in helpers (e.g., helper_boundl).

Yes, I know.  But they're also used in non-cpu contexts such as __get_user
(i.e. kernel accesses) or virtio.c (i.e. device accesses).

>> These are going to result in double-logging the same access with
> 
>>> +#define tcg_gen_qemu_ld_i32(val, addr, idx, memop)      \
>>> +    do {                                                \
>>> +        uint8_t _memop_size = _tcg_memop_size(memop);   \
>>> +        trace_guest_vmem_tcg(addr, _memop_size, 0);     \
>>> +        tcg_gen_qemu_ld_i32(val, addr, idx, memop);     \
>>> +    } while (0)
> 
>> ... these.
> 
> I don't see how 'tcg_gen_qemu_ld_i32' gets to call 'cpu_ldl_data'

tcg_gen_qemu_ld_i32 triggers some inline code, with an out of line fallback in
softmmu-template.h.

You're logging both on the main path (before qemu_ld_i32 opcode) and in the
fallback path.  It would be one thing if you logged something different in the
fallback path, but you're not.  You're using the exact same logging routine.

> What do you mean by non-target accesses? Accesses not directly "encoded" in the
> semantics of a guest instruction? If so, I did this in purpose (like the helper
> example on the top).

No, I mean accesses initiated by a device.  Such things are rare, I admit,
since most devices use physical addresses not virtual.  But e.g. old Sparc
hardware used a single mmu to handle both cpu and bus accesses.

> The 'trace_guest_vmem_tcg' function just calls a helper generator function in
> "helper.h", which cannot be included in "tcg.c". Another possibility is to just
> forget about using "helper.h", and instead "manually" generate the call to the
> helper; but using macros seems to me it's easier to maintain.

You simply need to move the declarations somewhere else.  See e.g.
tcg-runtime.h for a set of helpers shared across all translators.


r~

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-06 15:57     ` Richard Henderson
@ 2014-02-06 19:26       ` Lluís Vilanova
  2014-02-07 14:49         ` Richard Henderson
  0 siblings, 1 reply; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-06 19:26 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, Stefan Hajnoczi

Richard Henderson writes:

> On 02/04/2014 12:33 PM, Lluís Vilanova wrote:
>> Richard Henderson writes:
>> 
>>> On 01/31/2014 08:09 AM, Lluís Vilanova wrote:
>>>> Adds the base ability to specify which events in the "trace-events" file may be
>>>> used to trace guest activity in the TCG code (using the "tcg" event propery).
>>>> 
>>>> Such events generate an extra set of tracing functions that can be called during
>>>> TCG code generation and will automatically redirect a call to the appropriate
>>>> backend-dependent tracing functions when the guest code is executed.
>>>> 
>>>> Files generating guest code (TCG) must include "trace-tcg.h". Files declaring
>>>> per-target helpers ("${target}/helper.h") must include
>>>> "trace/generated-helpers.h".
>>>> 
>>>> The flow of the generated routines is:
>>>> 
>>>> 
>>>> [At translation time]
>>>> 
>>>> * trace_${name}_tcg(bool, TCGv)
>>>> Declared: "trace/generated-tcg-tracers.h"
>>>> Defined : "trace/generated-tcg-tracers.h"
>>>> 
>>>> * gen_helper_trace_${name}_tcg(bool, TCGv)
>>>> Declared: "trace/generated-helpers.h"
>>>> Defined : "trace/generated-helpers.h"
>>>> 
>>>> Automatically transforms all the arguments and allocates them into the
>>>> appropriate TCG temporary values (which are also freed). Provides a more
>>>> streamlined interface by allowing events in "trace-events" to take a mix of
>>>> tracing-supported types and TCG types.
>>>> 
>>>> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv)
>>>> Declared: "trace/generated-helpers.h"
>>>> Defined : "trace/generated-helpers.h" (using helper machinery)
>>>> 
>>>> The actual TCG helper function, created using QEMU's TCG helper machinery.
>> 
>>> I suppose I have no major objection to the feature, although frankly it's
>>> not especially exciting.  I can't really imagine ever wanting to bulk trace
>>> all of the helpers.  Tracing specific helpers on a target-by-target basis,
>>> sure.  But that can be done just as easily as adding tracing code to any
>>> other bit of C.
>> 
>> I'm not sure I understand this comment. The patch does not add helper tracing
>> capabilities, but generates a "trace_foo_tcg" routine that can be called during
>> TCG code generation, and will call "trace_foo" when that TCG code is
>> executed.

> Ah, see, I told you I was probably reading the patches wrong.  With all the
> helpers.h changes, I thought this was somehow related to tracing the existing
> helpers.  But the existance of trace_foo_tcg is triggered by the trace-events file?

Yes, defining "tcg foo" in the "trace-events" file will generate the functions
described in this series.


>>> I would strongly suggest this is backward.  One should perform the check for
>>> the tracepoint being enabled at translation time before emitting the call to
>>> the helper in the first place.
>> 
>> Right, the thing is that dynamically enabling/disabling such events will not
>> immediately show up in the trace if I do the check at translation time
>> (trace_foo_tcg), since QEMU will execute the cached TCG translations. I see the
>> following solutions to ensure traces are accurate:
>> 
>> * Delay the check until execution time.
>> 
>> * Check at translation time; flush translation cache when the tracing state of
>> a TCG event changes.
>> 
>> * Check at translation time; use multiple translation caches, one for each
>> possible tracing state of all the TCG-enabled events.
>> 
>> This series implements the first approach, since it's correct and much
>> simpler.
>> 
>> Other patches I did not send implement the third approach, which is quite
>> efficient if one is dynamically switching the tracing state while executing
>> mostly-cached code (e.g., profiling the accesses).

> How often do events change state?  My guess is exceedingly rarely.
> And by "rare" I mean something well under once per minute.  ;-)

> At which point, option 2 would be the best bet, I think.

Right. For the 3rd option I was also thinking about having per-vCPU tracing
states (in the case of guest events), so that you can trace separate events
depending on the vCPU.

Which reminds me, I should add a vCPU pointer to the "guest_vmem" event, since
otherwise you cannot differentiate accesses among different vCPUs.


>> I can wait for a later series to send the third option, or even implement the
>> second, but I just wanted to keep this one as simple as possible. Also,
>> implementing any of these two last approaches would provide minimal overheads on
>> builds that have such events enabled at compile time.

> Fair enough.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-06 19:26       ` Lluís Vilanova
@ 2014-02-07 14:49         ` Richard Henderson
  2014-02-07 15:13           ` Peter Maydell
  0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2014-02-07 14:49 UTC (permalink / raw)
  To: qemu-devel, Stefan Hajnoczi

On 02/06/2014 11:26 AM, Lluís Vilanova wrote:
>> > At which point, option 2 would be the best bet, I think.
> Right. For the 3rd option I was also thinking about having per-vCPU tracing
> states (in the case of guest events), so that you can trace separate events
> depending on the vCPU.
> 
> Which reminds me, I should add a vCPU pointer to the "guest_vmem" event, since
> otherwise you cannot differentiate accesses among different vCPUs.

The TB cache is shared, so you won't be able to do option 3 for different
events for different cpus.  You could only do option 1 for that.


r~

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-07 14:49         ` Richard Henderson
@ 2014-02-07 15:13           ` Peter Maydell
  2014-02-07 15:24             ` Lluís Vilanova
  0 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2014-02-07 15:13 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers, Stefan Hajnoczi

On 7 February 2014 14:49, Richard Henderson <rth@twiddle.net> wrote:
> On 02/06/2014 11:26 AM, Lluís Vilanova wrote:
>>> > At which point, option 2 would be the best bet, I think.
>> Right. For the 3rd option I was also thinking about having per-vCPU tracing
>> states (in the case of guest events), so that you can trace separate events
>> depending on the vCPU.
>>
>> Which reminds me, I should add a vCPU pointer to the "guest_vmem" event, since
>> otherwise you cannot differentiate accesses among different vCPUs.
>
> The TB cache is shared, so you won't be able to do option 3 for different
> events for different cpus.  You could only do option 1 for that

I have a vague plan in the back of my mind that we should make the
TB cache be per-CPU, as a step towards better handling of multithreaded
linux-user guest binaries and also heterogenous multiple CPU support...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code
  2014-02-07 15:13           ` Peter Maydell
@ 2014-02-07 15:24             ` Lluís Vilanova
  0 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-07 15:24 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Stefan Hajnoczi, Richard Henderson

Peter Maydell writes:

> On 7 February 2014 14:49, Richard Henderson <rth@twiddle.net> wrote:
>> On 02/06/2014 11:26 AM, Lluís Vilanova wrote:
>>>> > At which point, option 2 would be the best bet, I think.
>>> Right. For the 3rd option I was also thinking about having per-vCPU tracing
>>> states (in the case of guest events), so that you can trace separate events
>>> depending on the vCPU.
>>> 
>>> Which reminds me, I should add a vCPU pointer to the "guest_vmem" event, since
>>> otherwise you cannot differentiate accesses among different vCPUs.
>> 
>> The TB cache is shared, so you won't be able to do option 3 for different
>> events for different cpus.  You could only do option 1 for that

> I have a vague plan in the back of my mind that we should make the
> TB cache be per-CPU, as a step towards better handling of multithreaded
> linux-user guest binaries and also heterogenous multiple CPU support...

What I implemented is something in between. I have multiple TB caches, and each
CPU selects one of them according to what tcg tracing events they have enabled
(i.e., if two CPUs have the same events, they use the same TB cache).

Although I can see how having one private TB per core could improve TB cache
hits when running multiple applications.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event
  2014-02-06 16:12       ` Richard Henderson
@ 2014-02-10 13:29         ` Lluís Vilanova
  0 siblings, 0 replies; 30+ messages in thread
From: Lluís Vilanova @ 2014-02-10 13:29 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, Stefan Hajnoczi

Richard Henderson writes:

> On 02/04/2014 12:01 PM, Lluís Vilanova wrote:
>> Richard Henderson writes:
>> 
>>> On 01/31/2014 08:10 AM, Lluís Vilanova wrote:
>>>> +#define ldub(p)    ({ trace_guest_vmem(p, 1, 0); ldub_raw(p);    })
>> 
>>> Are you sure you want to log these here?  Uses of these macros are
>>> not restricted to the guest.  Therefore you could wind up with e.g.
>>> PCI device accesses being attributed to the target cpu.
>> 
>> These defines are only enabled in user-level mode.
>> 
>> But I wrote them really long ago, and I just realized they are not
>> up-to-date. The changes should also cover the 'cpu_*_data' and 'cpu_*_kernel'
>> variants. These macros are mostly used in helpers (e.g., helper_boundl).

> Yes, I know.  But they're also used in non-cpu contexts such as __get_user
> (i.e. kernel accesses) or virtio.c (i.e. device accesses).

The macro "__get_user" accesses memory using physical addresses, so it is
not traced.

Grepping at "virtio.c" shows that only "ld*_phys" and "ld*_p" are used (which
are not traced). Similarly happens for stores.

I looked at refereces for ld/st macros (without the "_raw" and "_p" suffixes),
and no non-target code came up (I'm including accesses - directly or indirectly
- initiated from helper functions as target code).

Am I missing something?


>>> These are going to result in double-logging the same access with
>> 
>>>> +#define tcg_gen_qemu_ld_i32(val, addr, idx, memop)      \
>>>> +    do {                                                \
>>>> +        uint8_t _memop_size = _tcg_memop_size(memop);   \
>>>> +        trace_guest_vmem_tcg(addr, _memop_size, 0);     \
>>>> +        tcg_gen_qemu_ld_i32(val, addr, idx, memop);     \
>>>> +    } while (0)
>> 
>>> ... these.
>> 
>> I don't see how 'tcg_gen_qemu_ld_i32' gets to call 'cpu_ldl_data'

> tcg_gen_qemu_ld_i32 triggers some inline code, with an out of line fallback in
> softmmu-template.h.

> You're logging both on the main path (before qemu_ld_i32 opcode) and in the
> fallback path.  It would be one thing if you logged something different in the
> fallback path, but you're not.  You're using the exact same logging routine.

Aha, but I'm only tracing functions in "softmmu_header.h" (e.g.,
"cpu_ldub_kernel"). I've looked at the result of preprocessing
"softmmu_template.h" (from "target-i386/mem_helper.c"), and the functions there
(e.g., "helper_ret_ldub_mmu") perform translation themselves and then access
physical memory (e.g., "ldub_p"). AFAIK, I did not add any tracing to that path.


>> What do you mean by non-target accesses? Accesses not directly "encoded" in the
>> semantics of a guest instruction? If so, I did this in purpose (like the helper
>> example on the top).

> No, I mean accesses initiated by a device.  Such things are rare, I admit,
> since most devices use physical addresses not virtual.  But e.g. old Sparc
> hardware used a single mmu to handle both cpu and bus accesses.

Hmmm, I'll have to take a look at that. Do you have any function or file for the
Sparc case from the top of your head? Admittedly, the whole memory access flow
is quite convoluted in QEMU, specially due to the big number of macros
involved. I guess whether something could be done to simplify all this a little
bit.


>> The 'trace_guest_vmem_tcg' function just calls a helper generator function in
>> "helper.h", which cannot be included in "tcg.c". Another possibility is to just
>> forget about using "helper.h", and instead "manually" generate the call to the
>> helper; but using macros seems to me it's easier to maintain.

> You simply need to move the declarations somewhere else.  See e.g.
> tcg-runtime.h for a set of helpers shared across all translators.

Nice. I'll move them there.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2014-02-10 13:29 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-31 16:09 [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 01/12] trace: [tcg] Add documentation Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 02/12] trace: [tracetool, tcg] Allow TCG types in trace event declarations Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 03/12] trace: [tracetool] Add method 'Event.api' to get the name of public routines Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 04/12] trace: [tracetool, tcg] Provide TCG-related type transformation rules Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 05/12] trace: [tracetool] Allow argument types to be transformed Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 06/12] trace: [tcg] Declare TCG tracing helper routines Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 07/12] trace: [tcg] Define " Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 08/12] trace: [tcg] Include TCG-tracing helpers on all helper.h Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 09/12] trace: [tcg] Generate TCG tracing routines Lluís Vilanova
2014-01-31 16:09 ` [Qemu-devel] [PATCH 10/12] trace: [trivial] Include event definitions in "trace.h" Lluís Vilanova
2014-01-31 16:10 ` [Qemu-devel] [PATCH 11/12] trace: [tcg] Include TCG-tracing header on all targets Lluís Vilanova
2014-01-31 16:10 ` [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event Lluís Vilanova
2014-02-04 15:08   ` Richard Henderson
2014-02-04 20:01     ` Lluís Vilanova
2014-02-06 16:12       ` Richard Henderson
2014-02-10 13:29         ` Lluís Vilanova
2014-02-03 14:40 ` [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code Stefan Hajnoczi
2014-02-03 16:24   ` Lluís Vilanova
2014-02-04 14:57 ` Richard Henderson
2014-02-04 15:02   ` Peter Maydell
2014-02-04 15:17     ` Richard Henderson
2014-02-04 20:44       ` Lluís Vilanova
2014-02-04 20:34     ` Lluís Vilanova
2014-02-04 20:33   ` Lluís Vilanova
2014-02-06 15:57     ` Richard Henderson
2014-02-06 19:26       ` Lluís Vilanova
2014-02-07 14:49         ` Richard Henderson
2014-02-07 15:13           ` Peter Maydell
2014-02-07 15:24             ` Lluís Vilanova

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.