qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] replay: add tracing events
@ 2025-11-18 16:25 Paolo Bonzini
  2025-11-18 16:25 ` Paolo Bonzini
  2025-11-19  5:43 ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 4+ messages in thread
From: Paolo Bonzini @ 2025-11-18 16:25 UTC (permalink / raw)
  To: qemu-devel

The replay subsystem does not have any way to see what's going on and
how the replay events interleave with other things happening in QEMU.

Add trace events to improve debuggability; to avoid having too many
events reimplement all functions in terms of (non-traced)
replay_getc and replay_putc and add a single trace event for each
datum that is extracted or written.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build              |  1 +
 replay/trace.h           |  1 +
 replay/replay-internal.c | 70 ++++++++++++++++++++++++++++++----------
 replay/trace-events      | 12 +++++++
 4 files changed, 67 insertions(+), 17 deletions(-)
 create mode 100644 replay/trace.h
 create mode 100644 replay/trace-events

diff --git a/meson.build b/meson.build
index df4460035c3..95fcec9ba15 100644
--- a/meson.build
+++ b/meson.build
@@ -3678,6 +3678,7 @@ if have_system
     'hw/gpio',
     'migration',
     'net',
+    'replay',
     'system',
     'ui',
     'hw/remote',
diff --git a/replay/trace.h b/replay/trace.h
new file mode 100644
index 00000000000..39ff481742e
--- /dev/null
+++ b/replay/trace.h
@@ -0,0 +1 @@
+#include "trace/trace-replay.h"
diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index c2a7200339f..4839f2b6321 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -15,6 +15,7 @@
 #include "replay-internal.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
+#include "trace.h"
 
 /* Mutex to protect reading and writing events to the log.
    data_kind and has_unread_data are also protected
@@ -44,7 +45,7 @@ static void replay_read_error(void)
     exit(1);
 }
 
-void replay_put_byte(uint8_t byte)
+static void replay_putc(uint8_t byte)
 {
     if (replay_file) {
         if (putc(byte, replay_file) == EOF) {
@@ -53,29 +54,45 @@ void replay_put_byte(uint8_t byte)
     }
 }
 
+void replay_put_byte(uint8_t byte)
+{
+    trace_replay_put_byte(byte);
+    replay_putc(byte);
+}
+
 void replay_put_event(uint8_t event)
 {
+    trace_replay_put_event(event);
     assert(event < EVENT_COUNT);
-    replay_put_byte(event);
+    replay_putc(event);
 }
 
 
 void replay_put_word(uint16_t word)
 {
-    replay_put_byte(word >> 8);
-    replay_put_byte(word);
+    trace_replay_put_word(word);
+    replay_putc(word >> 8);
+    replay_putc(word);
 }
 
 void replay_put_dword(uint32_t dword)
 {
-    replay_put_word(dword >> 16);
-    replay_put_word(dword);
+    int i;
+
+    trace_replay_put_dword(dword);
+    for (i = 24; i >= 0; i -= 8) {
+        replay_putc(dword >> i);
+    }
 }
 
 void replay_put_qword(int64_t qword)
 {
-    replay_put_dword(qword >> 32);
-    replay_put_dword(qword);
+    int i;
+
+    trace_replay_put_qword(qword);
+    for (i = 56; i >= 0; i -= 8) {
+        replay_putc(qword >> i);
+    }
 }
 
 void replay_put_array(const uint8_t *buf, size_t size)
@@ -88,7 +105,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
     }
 }
 
-uint8_t replay_get_byte(void)
+static uint8_t replay_getc(void)
 {
     uint8_t byte = 0;
     if (replay_file) {
@@ -101,36 +118,52 @@ uint8_t replay_get_byte(void)
     return byte;
 }
 
+uint8_t replay_get_byte(void)
+{
+    uint8_t byte = replay_getc();
+    trace_replay_get_byte(byte);
+    return byte;
+}
+
 uint16_t replay_get_word(void)
 {
     uint16_t word = 0;
     if (replay_file) {
-        word = replay_get_byte();
-        word = (word << 8) + replay_get_byte();
+        word = replay_getc();
+        word = (word << 8) + replay_getc();
     }
 
+    trace_replay_get_word(word);
     return word;
 }
 
 uint32_t replay_get_dword(void)
 {
     uint32_t dword = 0;
+    int i;
+
     if (replay_file) {
-        dword = replay_get_word();
-        dword = (dword << 16) + replay_get_word();
+        for (i = 24; i >= 0; i -= 8) {
+            dword |= replay_getc() << i;
+        }
     }
 
+    trace_replay_get_dword(dword);
     return dword;
 }
 
 int64_t replay_get_qword(void)
 {
-    int64_t qword = 0;
+    uint64_t qword = 0;
+    int i;
+
     if (replay_file) {
-        qword = replay_get_dword();
-        qword = (qword << 32) + replay_get_dword();
+        for (i = 56; i >= 0; i -= 8) {
+            qword |= (uint64_t)replay_getc() << i;
+        }
     }
 
+    trace_replay_get_qword(qword);
     return qword;
 }
 
@@ -172,10 +205,12 @@ void replay_check_error(void)
 
 void replay_fetch_data_kind(void)
 {
+    trace_replay_fetch_data_kind();
     if (replay_file) {
         if (!replay_state.has_unread_data) {
-            replay_state.data_kind = replay_get_byte();
+            replay_state.data_kind = replay_getc();
             replay_state.current_event++;
+            trace_replay_get_event(replay_state.current_event, replay_state.data_kind);
             if (replay_state.data_kind == EVENT_INSTRUCTION) {
                 replay_state.instruction_count = replay_get_dword();
             }
@@ -246,6 +281,7 @@ void replay_advance_current_icount(uint64_t current_icount)
     int diff = (int)(current_icount - replay_state.current_icount);
 
     /* Time can only go forward */
+    trace_replay_advance_current_icount(replay_state.current_icount, diff);
     assert(diff >= 0);
 
     if (replay_mode == REPLAY_MODE_RECORD) {
diff --git a/replay/trace-events b/replay/trace-events
new file mode 100644
index 00000000000..e9c887a6c57
--- /dev/null
+++ b/replay/trace-events
@@ -0,0 +1,12 @@
+replay_put_byte(uint8_t event) "%02x"
+replay_put_event(uint8_t event) "%02x"
+replay_put_word(uint16_t event) "%04x"
+replay_put_dword(uint32_t event) "%08x"
+replay_put_qword(uint64_t event) "%016" PRIx64
+replay_get_byte(uint8_t byte) "%02x"
+replay_get_word(uint16_t word) "%04x"
+replay_get_dword(uint32_t dword) "%08x"
+replay_get_qword(uint64_t qword) "%016" PRIx64
+replay_fetch_data_kind(void) ""
+replay_get_event(uint32_t current, uint8_t data) "#%u data=%02x"
+replay_advance_current_icount(uint64_t current_icount, int diff) "current=%" PRIu64 " diff=%d"
-- 
2.51.1



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

* [PATCH] replay: add tracing events
  2025-11-18 16:25 [PATCH] replay: add tracing events Paolo Bonzini
@ 2025-11-18 16:25 ` Paolo Bonzini
  2025-11-19  5:43 ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2025-11-18 16:25 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build              |  1 +
 replay/trace.h           |  1 +
 replay/replay-internal.c | 70 ++++++++++++++++++++++++++++++----------
 replay/trace-events      | 12 +++++++
 4 files changed, 67 insertions(+), 17 deletions(-)
 create mode 100644 replay/trace.h
 create mode 100644 replay/trace-events

diff --git a/meson.build b/meson.build
index df4460035c3..95fcec9ba15 100644
--- a/meson.build
+++ b/meson.build
@@ -3678,6 +3678,7 @@ if have_system
     'hw/gpio',
     'migration',
     'net',
+    'replay',
     'system',
     'ui',
     'hw/remote',
diff --git a/replay/trace.h b/replay/trace.h
new file mode 100644
index 00000000000..39ff481742e
--- /dev/null
+++ b/replay/trace.h
@@ -0,0 +1 @@
+#include "trace/trace-replay.h"
diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index c2a7200339f..4839f2b6321 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -15,6 +15,7 @@
 #include "replay-internal.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
+#include "trace.h"
 
 /* Mutex to protect reading and writing events to the log.
    data_kind and has_unread_data are also protected
@@ -44,7 +45,7 @@ static void replay_read_error(void)
     exit(1);
 }
 
-void replay_put_byte(uint8_t byte)
+static void replay_putc(uint8_t byte)
 {
     if (replay_file) {
         if (putc(byte, replay_file) == EOF) {
@@ -53,29 +54,45 @@ void replay_put_byte(uint8_t byte)
     }
 }
 
+void replay_put_byte(uint8_t byte)
+{
+    trace_replay_put_byte(byte);
+    replay_putc(byte);
+}
+
 void replay_put_event(uint8_t event)
 {
+    trace_replay_put_event(event);
     assert(event < EVENT_COUNT);
-    replay_put_byte(event);
+    replay_putc(event);
 }
 
 
 void replay_put_word(uint16_t word)
 {
-    replay_put_byte(word >> 8);
-    replay_put_byte(word);
+    trace_replay_put_word(word);
+    replay_putc(word >> 8);
+    replay_putc(word);
 }
 
 void replay_put_dword(uint32_t dword)
 {
-    replay_put_word(dword >> 16);
-    replay_put_word(dword);
+    int i;
+
+    trace_replay_put_dword(dword);
+    for (i = 24; i >= 0; i -= 8) {
+        replay_putc(dword >> i);
+    }
 }
 
 void replay_put_qword(int64_t qword)
 {
-    replay_put_dword(qword >> 32);
-    replay_put_dword(qword);
+    int i;
+
+    trace_replay_put_qword(qword);
+    for (i = 56; i >= 0; i -= 8) {
+        replay_putc(qword >> i);
+    }
 }
 
 void replay_put_array(const uint8_t *buf, size_t size)
@@ -88,7 +105,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
     }
 }
 
-uint8_t replay_get_byte(void)
+static uint8_t replay_getc(void)
 {
     uint8_t byte = 0;
     if (replay_file) {
@@ -101,36 +118,52 @@ uint8_t replay_get_byte(void)
     return byte;
 }
 
+uint8_t replay_get_byte(void)
+{
+    uint8_t byte = replay_getc();
+    trace_replay_get_byte(byte);
+    return byte;
+}
+
 uint16_t replay_get_word(void)
 {
     uint16_t word = 0;
     if (replay_file) {
-        word = replay_get_byte();
-        word = (word << 8) + replay_get_byte();
+        word = replay_getc();
+        word = (word << 8) + replay_getc();
     }
 
+    trace_replay_get_word(word);
     return word;
 }
 
 uint32_t replay_get_dword(void)
 {
     uint32_t dword = 0;
+    int i;
+
     if (replay_file) {
-        dword = replay_get_word();
-        dword = (dword << 16) + replay_get_word();
+        for (i = 24; i >= 0; i -= 8) {
+            dword |= replay_getc() << i;
+        }
     }
 
+    trace_replay_get_dword(dword);
     return dword;
 }
 
 int64_t replay_get_qword(void)
 {
-    int64_t qword = 0;
+    uint64_t qword = 0;
+    int i;
+
     if (replay_file) {
-        qword = replay_get_dword();
-        qword = (qword << 32) + replay_get_dword();
+        for (i = 56; i >= 0; i -= 8) {
+            qword |= (uint64_t)replay_getc() << i;
+        }
     }
 
+    trace_replay_get_qword(qword);
     return qword;
 }
 
@@ -172,10 +205,12 @@ void replay_check_error(void)
 
 void replay_fetch_data_kind(void)
 {
+    trace_replay_fetch_data_kind();
     if (replay_file) {
         if (!replay_state.has_unread_data) {
-            replay_state.data_kind = replay_get_byte();
+            replay_state.data_kind = replay_getc();
             replay_state.current_event++;
+            trace_replay_get_event(replay_state.current_event, replay_state.data_kind);
             if (replay_state.data_kind == EVENT_INSTRUCTION) {
                 replay_state.instruction_count = replay_get_dword();
             }
@@ -246,6 +281,7 @@ void replay_advance_current_icount(uint64_t current_icount)
     int diff = (int)(current_icount - replay_state.current_icount);
 
     /* Time can only go forward */
+    trace_replay_advance_current_icount(replay_state.current_icount, diff);
     assert(diff >= 0);
 
     if (replay_mode == REPLAY_MODE_RECORD) {
diff --git a/replay/trace-events b/replay/trace-events
new file mode 100644
index 00000000000..e9c887a6c57
--- /dev/null
+++ b/replay/trace-events
@@ -0,0 +1,12 @@
+replay_put_byte(uint8_t event) "%02x"
+replay_put_event(uint8_t event) "%02x"
+replay_put_word(uint16_t event) "%04x"
+replay_put_dword(uint32_t event) "%08x"
+replay_put_qword(uint64_t event) "%016" PRIx64
+replay_get_byte(uint8_t byte) "%02x"
+replay_get_word(uint16_t word) "%04x"
+replay_get_dword(uint32_t dword) "%08x"
+replay_get_qword(uint64_t qword) "%016" PRIx64
+replay_fetch_data_kind(void) ""
+replay_get_event(uint32_t current, uint8_t data) "#%u data=%02x"
+replay_advance_current_icount(uint64_t current_icount, int diff) "current=%" PRIu64 " diff=%d"
-- 
2.51.1



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

* Re: [PATCH] replay: add tracing events
  2025-11-18 16:25 [PATCH] replay: add tracing events Paolo Bonzini
  2025-11-18 16:25 ` Paolo Bonzini
@ 2025-11-19  5:43 ` Philippe Mathieu-Daudé
  2025-11-19  7:11   ` Paolo Bonzini
  1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-11-19  5:43 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 18/11/25 17:25, Paolo Bonzini wrote:
> The replay subsystem does not have any way to see what's going on and
> how the replay events interleave with other things happening in QEMU.
> 
> Add trace events to improve debuggability; to avoid having too many
> events reimplement all functions in terms of (non-traced)
> replay_getc and replay_putc and add a single trace event for each
> datum that is extracted or written.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   meson.build              |  1 +
>   replay/trace.h           |  1 +
>   replay/replay-internal.c | 70 ++++++++++++++++++++++++++++++----------
>   replay/trace-events      | 12 +++++++
>   4 files changed, 67 insertions(+), 17 deletions(-)
>   create mode 100644 replay/trace.h
>   create mode 100644 replay/trace-events


>   void replay_put_dword(uint32_t dword)
>   {
> -    replay_put_word(dword >> 16);
> -    replay_put_word(dword);
> +    int i;
> +
> +    trace_replay_put_dword(dword);
> +    for (i = 24; i >= 0; i -= 8) {

Matter of taste, this looks more natural to me:

        for (i = 32; i > 0; i -= 8) {

> +        replay_putc(dword >> i);
> +    }
>   }
>   
>   void replay_put_qword(int64_t qword)
>   {
> -    replay_put_dword(qword >> 32);
> -    replay_put_dword(qword);
> +    int i;
> +
> +    trace_replay_put_qword(qword);
> +    for (i = 56; i >= 0; i -= 8) {

Ditto, etc.

> +        replay_putc(qword >> i);
> +    }
>   }

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Also, still taste, I'd have used trace_replay_{put,get}{8,16,32,64} ;)


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

* Re: [PATCH] replay: add tracing events
  2025-11-19  5:43 ` Philippe Mathieu-Daudé
@ 2025-11-19  7:11   ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2025-11-19  7:11 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]

Il mer 19 nov 2025, 06:44 Philippe Mathieu-Daudé <philmd@linaro.org> ha
scritto:

> On 18/11/25 17:25, Paolo Bonzini wrote:
> > The replay subsystem does not have any way to see what's going on and
> > how the replay events interleave with other things happening in QEMU.
> >
> > Add trace events to improve debuggability; to avoid having too many
> > events reimplement all functions in terms of (non-traced)
> > replay_getc and replay_putc and add a single trace event for each
> > datum that is extracted or written.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   meson.build              |  1 +
> >   replay/trace.h           |  1 +
> >   replay/replay-internal.c | 70 ++++++++++++++++++++++++++++++----------
> >   replay/trace-events      | 12 +++++++
> >   4 files changed, 67 insertions(+), 17 deletions(-)
> >   create mode 100644 replay/trace.h
> >   create mode 100644 replay/trace-events
>
>
> >   void replay_put_dword(uint32_t dword)
> >   {
> > -    replay_put_word(dword >> 16);
> > -    replay_put_word(dword);
> > +    int i;
> > +
> > +    trace_replay_put_dword(dword);
> > +    for (i = 24; i >= 0; i -= 8) {
>
> Matter of taste, this looks more natural to me:
>
>         for (i = 32; i > 0; i -= 8) {
>

But ">> (i - 8)" is uglier... Or you have to put i -= 8 in the loop body.

Paolo


> > +        replay_putc(dword >> i);
> > +    }
> >   }
> >
> >   void replay_put_qword(int64_t qword)
> >   {
> > -    replay_put_dword(qword >> 32);
> > -    replay_put_dword(qword);
> > +    int i;
> > +
> > +    trace_replay_put_qword(qword);
> > +    for (i = 56; i >= 0; i -= 8) {
>
> Ditto, etc.
>
> > +        replay_putc(qword >> i);
> > +    }
> >   }
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Also, still taste, I'd have used trace_replay_{put,get}{8,16,32,64} ;)
>
>

[-- Attachment #2: Type: text/html, Size: 3080 bytes --]

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

end of thread, other threads:[~2025-11-19  7:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 16:25 [PATCH] replay: add tracing events Paolo Bonzini
2025-11-18 16:25 ` Paolo Bonzini
2025-11-19  5:43 ` Philippe Mathieu-Daudé
2025-11-19  7:11   ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).