[RFC PATCH 00/14] tracetool: add Rust support

[RFC PATCH 00/14] tracetool: add Rust support

[Paolo Bonzini]

This is the result of the summer project of Tanish Desai.  It mostly
consists of changes to tracetool, which I used to add tracepoint support
to the pl011 device.  All the backends are supported except dtrace and
ust; support for Linux in dtrace should be easy using the "probe" crate,
the rest (ust, and dtrace on macOS or Solaris) less so.

Patches 1-2 are cleanups that could be committed separately.

Patches 3-5 are tracetool patches that have been posted before, now
rebased on top of the "make check-tracetool" series.  Their purpose
is to simplify .h code generation for tracetool backends, and these
simplifications translate directly to the new .rs code generation.

Patches 6-7 add the minimal support for adding tracepoint functions
in Rust, albeit with no content and thus no actual tracing.

Patches 8-9 add back tracepoints to the Rust pl011 device model.

Patches 10-14 finally add Rust code generation to the supported
tracing backends.

Many thanks to Daniel for his tracetool testsuite.  It was very
useful and did indeed improve reviewability!

Paolo

Based-on: <20250819161053.464641-1-berrange@redhat.com>

Paolo Bonzini (6):
  treewide: write "unsigned long int" instead of "long unsigned int"
  rust: move dependencies to rust/Cargo.toml
  trace/ftrace: move snprintf+write from tracepoints to ftrace.c
  rust: qdev: add minimal clock bindings
  rust: pl011: add tracepoints
  log: change qemu_loglevel to unsigned

Tanish Desai (8):
  tracetool: add CHECK_TRACE_EVENT_GET_STATE
  tracetool/backend: remove redundant trace event checks
  tracetool: Add Rust format support
  rust: add trace crate
  tracetool/simple: add Rust support
  tracetool/log: add Rust support
  tracetool/ftrace: add Rust support
  tracetool/syslog: add Rust support

 include/qemu/log-for-trace.h          |   4 +-
 include/qemu/log.h                    |  44 ++++----
 rust/qemu-api/wrapper.h               |   1 +
 tests/tracetool/ftrace.h              |  28 +----
 tests/tracetool/log.h                 |  16 ++-
 trace/ftrace.h                        |   1 +
 crypto/pbkdf-gcrypt.c                 |   2 +-
 crypto/pbkdf-gnutls.c                 |   2 +-
 crypto/pbkdf-nettle.c                 |   2 +-
 hw/display/exynos4210_fimd.c          |   2 +-
 hw/misc/imx7_src.c                    |   4 +-
 hw/net/can/can_sja1000.c              |   4 +-
 trace/ftrace.c                        |  15 +++
 util/log.c                            |   2 +-
 hw/char/trace-events                  |  14 +--
 hw/xen/trace-events                   |   4 +-
 rust/Cargo.lock                       |   8 ++
 rust/Cargo.toml                       |   6 +
 rust/hw/char/pl011/Cargo.toml         |   1 +
 rust/hw/char/pl011/meson.build        |   1 +
 rust/hw/char/pl011/src/device.rs      |  57 +++++++---
 rust/meson.build                      |   2 +-
 rust/qemu-api/Cargo.toml              |   6 +-
 rust/qemu-api/src/log.rs              |   2 +-
 rust/qemu-api/src/qdev.rs             |  33 ++++++
 rust/trace/Cargo.toml                 |  19 ++++
 rust/trace/meson.build                |  19 ++++
 rust/trace/src/lib.rs                 |  27 +++++
 scripts/tracetool/__init__.py         | 156 ++++++++++++++++++++++++++
 scripts/tracetool/backend/__init__.py |  39 +++++--
 scripts/tracetool/backend/ftrace.py   |  24 ++--
 scripts/tracetool/backend/log.py      |  20 ++--
 scripts/tracetool/backend/simple.py   |  15 ++-
 scripts/tracetool/backend/syslog.py   |  15 +--
 scripts/tracetool/format/h.py         |  11 +-
 scripts/tracetool/format/rs.py        |  76 +++++++++++++
 tests/tracetool/ftrace.rs             |  41 +++++++
 tests/tracetool/log.rs                |  45 ++++++++
 tests/tracetool/simple.rs             |  41 +++++++
 tests/tracetool/syslog.rs             |  41 +++++++
 tests/tracetool/tracetool-test.py     |   2 +
 trace/meson.build                     |   8 +-
 42 files changed, 718 insertions(+), 142 deletions(-)
 create mode 100644 rust/trace/Cargo.toml
 create mode 100644 rust/trace/meson.build
 create mode 100644 rust/trace/src/lib.rs
 create mode 100644 scripts/tracetool/format/rs.py
 create mode 100644 tests/tracetool/ftrace.rs
 create mode 100644 tests/tracetool/log.rs
 create mode 100644 tests/tracetool/simple.rs
 create mode 100644 tests/tracetool/syslog.rs

-- 
2.50.1

[PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Paolo Bonzini]

Putting "unsigned" in anything but the first position is weird.  As such,
tracetool's Rust type conversion will not support it.  Remove it from
the whole of QEMU's source code, not just trace-events.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 crypto/pbkdf-gcrypt.c        | 2 +-
 crypto/pbkdf-gnutls.c        | 2 +-
 crypto/pbkdf-nettle.c        | 2 +-
 hw/display/exynos4210_fimd.c | 2 +-
 hw/misc/imx7_src.c           | 4 ++--
 hw/net/can/can_sja1000.c     | 4 ++--
 hw/xen/trace-events          | 4 ++--
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
index e89b8b1c768..f93996f674c 100644
--- a/crypto/pbkdf-gcrypt.c
+++ b/crypto/pbkdf-gcrypt.c
@@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
     if (iterations > ULONG_MAX) {
         error_setg_errno(errp, ERANGE,
                          "PBKDF iterations %llu must be less than %lu",
-                         (long long unsigned)iterations, ULONG_MAX);
+                         (unsigned long long)iterations, ULONG_MAX);
         return -1;
     }
 
diff --git a/crypto/pbkdf-gnutls.c b/crypto/pbkdf-gnutls.c
index f34423f918b..46a3a869994 100644
--- a/crypto/pbkdf-gnutls.c
+++ b/crypto/pbkdf-gnutls.c
@@ -62,7 +62,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
     if (iterations > ULONG_MAX) {
         error_setg_errno(errp, ERANGE,
                          "PBKDF iterations %llu must be less than %lu",
-                         (long long unsigned)iterations, ULONG_MAX);
+                         (unsigned long long)iterations, ULONG_MAX);
         return -1;
     }
 
diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c
index 3ef9c1b52c4..3c8bbaf9f17 100644
--- a/crypto/pbkdf-nettle.c
+++ b/crypto/pbkdf-nettle.c
@@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
     if (iterations > UINT_MAX) {
         error_setg_errno(errp, ERANGE,
                          "PBKDF iterations %llu must be less than %u",
-                         (long long unsigned)iterations, UINT_MAX);
+                         (unsigned long long)iterations, ULONG_MAX);
         return -1;
     }
 
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index c61e0280a7c..5632aa1388c 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1380,7 +1380,7 @@ static void exynos4210_fimd_write(void *opaque, hwaddr offset,
     uint32_t old_value;
 
     DPRINT_L2("write offset 0x%08x, value=%llu(0x%08llx)\n", offset,
-            (long long unsigned int)val, (long long unsigned int)val);
+            (unsigned long long)val, (unsigned long long)val);
 
     switch (offset) {
     case FIMD_VIDCON0:
diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
index df0b0a69057..817c95bf65b 100644
--- a/hw/misc/imx7_src.c
+++ b/hw/misc/imx7_src.c
@@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
 {
     IMX7SRCState *s = (IMX7SRCState *)opaque;
     uint32_t index = offset >> 2;
-    long unsigned int change_mask;
+    uint32_t change_mask;
     uint32_t current_value = value;
 
     if (index >= SRC_MAX) {
@@ -180,7 +180,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
 
     trace_imx7_src_write(imx7_src_reg_name(SRC_A7RCR0), s->regs[SRC_A7RCR0]);
 
-    change_mask = s->regs[index] ^ (uint32_t)current_value;
+    change_mask = s->regs[index] ^ current_value;
 
     switch (index) {
     case SRC_A7RCR0:
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 5b6ba9df6c4..545c520c3b4 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
             break;
         }
     }
-    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
-            (int)addr, size, (long unsigned int)temp);
+    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02x\n",
+            (int)addr, size, (unsigned)temp);
 
     return temp;
 }
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index b67942d07b4..3b71ee641ff 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -57,8 +57,8 @@ cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uin
 cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
 cpu_get_ioreq_from_shared_memory_req_not_ready(int state, int data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O request not ready: 0x%x, ptr: 0x%x, port: 0x%"PRIx64", data: 0x%"PRIx64", count: %u, size: %u"
 xen_main_loop_prepare_init_cpu(int id, void *cpu) "cpu_by_vcpu_id[%d]=%p"
-xen_map_ioreq_server_shared_page(long unsigned int ioreq_pfn) "shared page at pfn 0x%lx"
-xen_map_ioreq_server_buffered_io_page(long unsigned int ioreq_pfn) "buffered io page at pfn 0x%lx"
+xen_map_ioreq_server_shared_page(unsigned long int ioreq_pfn) "shared page at pfn 0x%lx"
+xen_map_ioreq_server_buffered_io_page(unsigned long int ioreq_pfn) "buffered io page at pfn 0x%lx"
 xen_map_ioreq_server_buffered_io_evtchn(int bufioreq_evtchn) "buffered io evtchn is 0x%x"
 destroy_hvm_domain_cannot_acquire_handle(void) "Cannot acquire xenctrl handle"
 destroy_hvm_domain_failed_action(const char *action, int sts, char *errno_s) "xc_domain_shutdown failed to issue %s, sts %d, %s"
-- 
2.50.1

[PATCH 02/14] rust: move dependencies to rust/Cargo.toml

[Paolo Bonzini]

As more crates start using the same dependencies, it's better to not
repeat the versions and move the dependency declarations to the workspace.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/Cargo.toml          | 5 +++++
 rust/qemu-api/Cargo.toml | 6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 682184cb158..99c275f2d9f 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -15,6 +15,11 @@ license = "GPL-2.0-or-later"
 repository = "https://gitlab.com/qemu-project/qemu/"
 rust-version = "1.83.0"
 
+[workspace.dependencies]
+anyhow = "~1.0"
+foreign = "~0.3.1"
+libc = "0.2.162"
+
 [workspace.lints.rust]
 unexpected_cfgs = { level = "deny", check-cfg = [
     'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index db7000dee44..c07a17a28b0 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -15,9 +15,9 @@ rust-version.workspace = true
 
 [dependencies]
 qemu_api_macros = { path = "../qemu-api-macros" }
-anyhow = "~1.0"
-libc = "0.2.162"
-foreign = "~0.3.1"
+anyhow = { workspace = true }
+foreign = { workspace = true }
+libc = { workspace = true }
 
 [features]
 default = ["debug_cell"]
-- 
2.50.1

[PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Paolo Bonzini]

This simplifies the Python code and reduces the size of the tracepoints.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/tracetool/ftrace.h            | 28 ++++++----------------------
 trace/ftrace.h                      |  1 +
 trace/ftrace.c                      | 15 +++++++++++++++
 scripts/tracetool/backend/ftrace.py | 12 ++----------
 4 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/tests/tracetool/ftrace.h b/tests/tracetool/ftrace.h
index fe22ea0f09f..1dfe4239413 100644
--- a/tests/tracetool/ftrace.h
+++ b/tests/tracetool/ftrace.h
@@ -21,18 +21,10 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
 
 static inline void trace_test_blah(void *context, const char *filename)
 {
-    {
-        char ftrace_buf[MAX_TRACE_STRLEN];
-        int unused __attribute__ ((unused));
-        int trlen;
-        if (trace_event_get_state(TRACE_TEST_BLAH)) {
+    if (trace_event_get_state(TRACE_TEST_BLAH)) {
 #line 4 "trace-events"
-            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
-                             "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
-#line 33 "ftrace.h"
-            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
-            unused = write(trace_marker_fd, ftrace_buf, trlen);
-        }
+        ftrace_write("test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
+#line 28 "ftrace.h"
     }
 }
 
@@ -42,18 +34,10 @@ static inline void trace_test_blah(void *context, const char *filename)
 
 static inline void trace_test_wibble(void *context, int value)
 {
-    {
-        char ftrace_buf[MAX_TRACE_STRLEN];
-        int unused __attribute__ ((unused));
-        int trlen;
-        if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
+    if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
 #line 5 "trace-events"
-            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
-                             "test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
-#line 54 "ftrace.h"
-            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
-            unused = write(trace_marker_fd, ftrace_buf, trlen);
-        }
+        ftrace_write("test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
+#line 41 "ftrace.h"
     }
 }
 #endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */
diff --git a/trace/ftrace.h b/trace/ftrace.h
index cb5e35d2171..16c122816d1 100644
--- a/trace/ftrace.h
+++ b/trace/ftrace.h
@@ -8,5 +8,6 @@
 extern int trace_marker_fd;
 
 bool ftrace_init(void);
+G_GNUC_PRINTF(1, 2) void ftrace_write(const char *fmt, ...);
 
 #endif /* TRACE_FTRACE_H */
diff --git a/trace/ftrace.c b/trace/ftrace.c
index 9749543d9b2..6875faedb9c 100644
--- a/trace/ftrace.c
+++ b/trace/ftrace.c
@@ -38,6 +38,21 @@ static int find_mount(char *mount_point, const char *fstype)
     return ret;
 }
 
+void ftrace_write(const char *fmt, ...)
+{
+    char ftrace_buf[MAX_TRACE_STRLEN];
+    int unused __attribute__ ((unused));
+    int trlen;
+    va_list ap;
+
+    va_start(ap, fmt);
+    trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
+    va_end(ap);
+
+    trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
+    unused = write(trace_marker_fd, ftrace_buf, trlen);
+}
+
 bool ftrace_init(void)
 {
     char mount_point[PATH_MAX];
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 5fa30ccc08e..a07f8a9dfd8 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -28,18 +28,10 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    out('    {',
-        '        char ftrace_buf[MAX_TRACE_STRLEN];',
-        '        int unused __attribute__ ((unused));',
-        '        int trlen;',
-        '        if (trace_event_get_state(%(event_id)s)) {',
+    out('    if (trace_event_get_state(%(event_id)s)) {',
         '#line %(event_lineno)d "%(event_filename)s"',
-        '            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
-        '                             "%(name)s " %(fmt)s "\\n" %(argnames)s);',
+        '        ftrace_write("%(name)s " %(fmt)s "\\n" %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
-        '            unused = write(trace_marker_fd, ftrace_buf, trlen);',
-        '        }',
         '    }',
         name=event.name,
         args=event.args,
-- 
2.50.1

[PATCH 04/14] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

Add a new attribute CHECK_TRACE_EVENT_GET_STATE to the backends.
When present and True, the code generated by the generate function
is wrapped in a conditional that checks whether the event is enabled;
this removes the need for repeating the same conditional in multiple
backends.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/tracetool/backend/__init__.py | 39 ++++++++++++++++++---------
 scripts/tracetool/format/h.py         | 11 +++++---
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index 7bfcc86cc53..273c8bbba3b 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -19,11 +19,15 @@
 Backend attributes
 ------------------
 
-========= ====================================================================
-Attribute Description
-========= ====================================================================
-PUBLIC    If exists and is set to 'True', the backend is considered "public".
-========= ====================================================================
+=========================== ====================================================
+Attribute                   Description
+=========================== ====================================================
+PUBLIC                      If exists and is set to 'True', the backend is
+                            considered "public".
+CHECK_TRACE_EVENT_GET_STATE If exists and is set to 'True', the backend-specific
+                            code inside the tracepoint is emitted within an
+                            ``if trace_event_get_state()`` conditional.
+=========================== ====================================================
 
 
 Backend functions
@@ -101,22 +105,33 @@ class Wrapper:
     def __init__(self, backends, format):
         self._backends = [backend.replace("-", "_") for backend in backends]
         self._format = format.replace("-", "_")
+        self.check_trace_event_get_state = False
         for backend in self._backends:
             assert exists(backend)
         assert tracetool.format.exists(self._format)
+        for backend in self.backend_modules():
+            check_trace_event_get_state = getattr(backend, "CHECK_TRACE_EVENT_GET_STATE", False)
+            self.check_trace_event_get_state = self.check_trace_event_get_state or check_trace_event_get_state
 
-    def _run_function(self, name, *args, **kwargs):
+    def backend_modules(self):
         for backend in self._backends:
-            func = tracetool.try_import("tracetool.backend." + backend,
-                                        name % self._format, None)[1]
-            if func is not None:
-                func(*args, **kwargs)
+             module = tracetool.try_import("tracetool.backend." + backend)[1]
+             if module is not None:
+                 yield module
+
+    def _run_function(self, name, *args, check_trace_event_get_state=None, **kwargs):
+        for backend in self.backend_modules():
+            func = getattr(backend, name % self._format, None)
+            if func is not None and \
+                (check_trace_event_get_state is None or
+                 check_trace_event_get_state == getattr(backend, 'CHECK_TRACE_EVENT_GET_STATE', False)):
+                    func(*args, **kwargs)
 
     def generate_begin(self, events, group):
         self._run_function("generate_%s_begin", events, group)
 
-    def generate(self, event, group):
-        self._run_function("generate_%s", event, group)
+    def generate(self, event, group, check_trace_event_get_state=None):
+        self._run_function("generate_%s", event, group, check_trace_event_get_state=check_trace_event_get_state)
 
     def generate_backend_dstate(self, event, group):
         self._run_function("generate_%s_backend_dstate", event, group)
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index b42a8268a81..20cbacfe843 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -60,7 +60,6 @@ def generate(events, backend, group):
 
         out('    false)')
 
-        # tracer without checks
         out('',
             'static inline void %(api)s(%(args)s)',
             '{',
@@ -68,11 +67,17 @@ def generate(events, backend, group):
             args=e.args)
 
         if "disable" not in e.properties:
-            backend.generate(e, group)
+            backend.generate(e, group, check_trace_event_get_state=False)
 
+            if backend.check_trace_event_get_state:
+                event_id = 'TRACE_' + e.name.upper()
+                cond = "trace_event_get_state(%s)" % event_id
+                out('    if (%(cond)s) {',
+                        cond=cond)
+                backend.generate(e, group, check_trace_event_get_state=True)
+                out('    }')
         out('}')
 
-
     backend.generate_end(events, group)
 
     out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
-- 
2.50.1

[PATCH 05/14] tracetool/backend: remove redundant trace event checks

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

Use CHECK_TRACE_EVENT_GET_STATE in log, syslog, dtrace and simple
backend, so that the "if (trace_event_get_state)" is created from common
code and unified when multiple backends are active.

When a single backend is active there is no code change (except
for the log backend, as shown in tests/tracetool/log.h), but the
code in the backends is simpler.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/tracetool/log.h               | 16 ++++++++++------
 scripts/tracetool/backend/ftrace.py |  6 ++----
 scripts/tracetool/backend/log.py    | 10 ++++------
 scripts/tracetool/backend/simple.py |  8 ++------
 scripts/tracetool/backend/syslog.py |  8 ++------
 5 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/tests/tracetool/log.h b/tests/tracetool/log.h
index edcc7f9d47c..c7795871f85 100644
--- a/tests/tracetool/log.h
+++ b/tests/tracetool/log.h
@@ -21,10 +21,12 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
 
 static inline void trace_test_blah(void *context, const char *filename)
 {
-    if (trace_event_get_state(TRACE_TEST_BLAH) && qemu_loglevel_mask(LOG_TRACE)) {
+    if (trace_event_get_state(TRACE_TEST_BLAH)) {
+        if (qemu_loglevel_mask(LOG_TRACE)) {
 #line 4 "trace-events"
-        qemu_log("test_blah " "Blah context=%p filename=%s" "\n", context, filename);
-#line 28 "log.h"
+            qemu_log("test_blah " "Blah context=%p filename=%s" "\n", context, filename);
+#line 29 "log.h"
+        }
     }
 }
 
@@ -34,10 +36,12 @@ static inline void trace_test_blah(void *context, const char *filename)
 
 static inline void trace_test_wibble(void *context, int value)
 {
-    if (trace_event_get_state(TRACE_TEST_WIBBLE) && qemu_loglevel_mask(LOG_TRACE)) {
+    if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
+        if (qemu_loglevel_mask(LOG_TRACE)) {
 #line 5 "trace-events"
-        qemu_log("test_wibble " "Wibble context=%p value=%d" "\n", context, value);
-#line 41 "log.h"
+            qemu_log("test_wibble " "Wibble context=%p value=%d" "\n", context, value);
+#line 44 "log.h"
+        }
     }
 }
 #endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index a07f8a9dfd8..432f216ea2b 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def generate_h_begin(events, group):
@@ -28,14 +29,11 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    out('    if (trace_event_get_state(%(event_id)s)) {',
-        '#line %(event_lineno)d "%(event_filename)s"',
+    out('#line %(event_lineno)d "%(event_filename)s"',
         '        ftrace_write("%(name)s " %(fmt)s "\\n" %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '    }',
         name=event.name,
         args=event.args,
-        event_id="TRACE_" + event.name.upper(),
         event_lineno=event.lineno,
         event_filename=event.filename,
         fmt=event.fmt.rstrip("\n"),
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index eb50ceea34c..2aa180f4b47 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def generate_h_begin(events, group):
@@ -28,14 +29,11 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
-
-    out('    if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {',
+    out('        if (qemu_loglevel_mask(LOG_TRACE)) {',
         '#line %(event_lineno)d "%(event_filename)s"',
-        '        qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
+        '            qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '    }',
-        cond=cond,
+        '        }',
         event_lineno=event.lineno,
         event_filename=event.filename,
         name=event.name,
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 7c84c06b200..a8afc977a20 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def is_string(arg):
@@ -36,13 +37,8 @@ def generate_h_begin(events, group):
 
 
 def generate_h(event, group):
-    event_id = 'TRACE_' + event.name.upper()
-    cond = "trace_event_get_state(%s)" % event_id
-    out('    if (%(cond)s) {',
-        '        _simple_%(api)s(%(args)s);',
-        '    }',
+    out('        _simple_%(api)s(%(args)s);',
         api=event.api(),
-        cond=cond,
         args=", ".join(event.args.names()))
 
 
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 3f82e54aabf..04ec85717a3 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def generate_h_begin(events, group):
@@ -28,14 +29,9 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
-
-    out('    if (%(cond)s) {',
-        '#line %(event_lineno)d "%(event_filename)s"',
+    out('#line %(event_lineno)d "%(event_filename)s"',
         '        syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '    }',
-        cond=cond,
         event_lineno=event.lineno,
         event_filename=event.filename,
         name=event.name,
-- 
2.50.1

[PATCH 06/14] tracetool: Add Rust format support

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

Generating .rs files makes it possible to support tracing in rust.
This support comprises a new format, and common code that converts
the C expressions in trace-events to Rust.  In particular, types
need to be converted, and PRI macros expanded.  Fortunately, all
common platforms have a known mapping of 8/16/32/64-bit integers
to char/short/int/"long long": even if int64_t is equal to long,
it is fine to change the format string from PRIx64's expansion
"%lx" to "%llx".  This makes it possible to have a static mapping
from PRI macros to their expansion.

As of this commit no backend generates Rust code, but it is already
possible to use tracetool to generate Rust sources; they are not
functional but they compile and contain tracepoint functions.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
[Move Rust argument conversion from Event to Arguments; string
 support. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/tracetool/__init__.py  | 156 +++++++++++++++++++++++++++++++++
 scripts/tracetool/format/rs.py |  76 ++++++++++++++++
 2 files changed, 232 insertions(+)
 create mode 100644 scripts/tracetool/format/rs.py

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 1d5238a0843..0b8ec707332 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -31,6 +31,49 @@ def error(*lines):
     error_write(*lines)
     sys.exit(1)
 
+FMT_TOKEN = re.compile(r'''(?:
+                       " ( (?: [^"\\] | \\[\\"abfnrt] |            # a string literal
+                               \\x[0-9a-fA-F][0-9a-fA-F]) *? ) "
+                       | ( PRI [duixX] (?:8|16|32|64|PTR|MAX) )    # a PRIxxx macro
+                       | \s+                                       # spaces (ignored)
+                       )''', re.X)
+
+PRI_SIZE_MAP = {
+    '8':  'hh',
+    '16': 'h',
+    '32': '',
+    '64': 'll',
+    'PTR': 't',
+    'MAX': 'j',
+}
+
+def expand_format_string(c_fmt, prefix=""):
+    def pri_macro_to_fmt(pri_macro):
+        assert pri_macro.startswith("PRI")
+        fmt_type = pri_macro[3]  # 'd', 'i', 'u', or 'x'
+        fmt_size = pri_macro[4:]  # '8', '16', '32', '64', 'PTR', 'MAX'
+
+        size = PRI_SIZE_MAP.get(fmt_size, None)
+        if size is None:
+            raise Exception(f"unknown macro {pri_macro}")
+        return size + fmt_type
+
+    result = prefix
+    pos = 0
+    while pos < len(c_fmt):
+        m = FMT_TOKEN.match(c_fmt, pos)
+        if not m:
+            print("No match at position", pos, ":", repr(c_fmt[pos:]), file=sys.stderr)
+            raise Exception("syntax error in trace file")
+        if m[1]:
+            substr = m[1]
+        elif m[2]:
+            substr = pri_macro_to_fmt(m[2])
+        else:
+            substr = ""
+        result += substr
+        pos = m.end()
+    return result
 
 out_lineno = 1
 out_filename = '<none>'
@@ -90,6 +133,48 @@ def out(*lines, **kwargs):
     "ptrdiff_t",
 ]
 
+C_TYPE_KEYWORDS = {"int", "short", "long", "unsigned", "char"}
+
+C_TO_RUST_TYPE_MAP = {
+    "int": "std::ffi::c_int",
+    "long": "std::ffi::c_long",
+    "long long": "std::ffi::c_longlong",
+    "short": "std::ffi::c_short",
+    "char": "std::ffi::c_char",
+    "bool": "bool",
+    "unsigned": "std::ffi::c_uint",
+    "unsigned long": "std::ffi::c_long",
+    "unsigned long long": "std::ffi::c_ulonglong",
+    "unsigned short": "std::ffi::c_ushort",
+    "unsigned char": "u8",
+    "int8_t": "i8",
+    "uint8_t": "u8",
+    "int16_t": "i16",
+    "uint16_t": "u16",
+    "int32_t": "i32",
+    "uint32_t": "u32",
+    "int64_t": "i64",
+    "uint64_t": "u64",
+    "void": "()",
+    "size_t": "usize",
+    "ssize_t": "isize",
+    "uintptr_t": "usize",
+    "ptrdiff_t": "isize",
+}
+
+# Rust requires manual casting of <32-bit types when passing them to
+# variable-argument functions.
+RUST_VARARGS_SMALL_TYPES = {
+    "std::ffi::c_short",
+    "std::ffi::c_ushort",
+    "std::ffi::c_char",
+    "i8",
+    "u8",
+    "i16",
+    "u16",
+    "bool",
+}
+
 def validate_type(name):
     bits = name.split(" ")
     for bit in bits:
@@ -105,6 +190,40 @@ def validate_type(name):
                              "other complex pointer types should be "
                              "declared as 'void *'" % name)
 
+def c_type_to_rust(name):
+    ptr = False
+    const = False
+    name = name.rstrip()
+    if name[-1] == '*':
+        name = name[:-1].rstrip()
+        ptr = True
+        if name[-1] == '*':
+            # pointers to pointers are the same as void*
+            name = "void"
+
+    bits = iter(name.split())
+    bit = next(bits)
+    if bit == "const":
+        const = True
+        bit = next(bits)
+
+    if bit in C_TYPE_KEYWORDS:
+        if bit == 'signed':
+            bit = ''
+        rest = list(bits)
+        if rest and rest[-1] == 'int':
+            rest = rest[:-1]
+        name = bit + ' ' + ' '.join(rest)
+    else:
+        if list(bits):
+            raise ValueError("Invalid type '%s'." % name)
+        name = bit
+
+    ty = C_TO_RUST_TYPE_MAP[name.strip()]
+    if ptr:
+        ty = f'*{"const" if const else "mut"} {ty}'
+    return ty
+
 class Arguments:
     """Event arguments description."""
 
@@ -197,6 +316,43 @@ def casted(self):
         """List of argument names casted to their type."""
         return ["(%s)%s" % (type_, name) for type_, name in self._args]
 
+    def rust_decl_extern(self):
+        """Return a Rust argument list for an extern "C" function"""
+        return ", ".join((f"_{name}: {c_type_to_rust(type_)}"
+                          for type_, name in self._args))
+
+    def rust_decl(self):
+        """Return a Rust argument list for a tracepoint function"""
+        def decl_type(type_):
+            if type_ == "const char *":
+                return "&std::ffi::CStr"
+            return c_type_to_rust(type_)
+
+        return ", ".join((f"_{name}: {decl_type(type_)}"
+                          for type_, name in self._args))
+
+    def rust_call_extern(self):
+        """Return a Rust argument list for a call to an extern "C" function"""
+        def rust_cast(name, type_):
+            if type_ == "const char *":
+                return f"_{name}.as_ptr()"
+            return "_{name}"
+
+        return ", ".join((rust_cast(name, type_) for type_, name in self._args))
+
+    def rust_call_varargs(self):
+        """Return a Rust argument list for a call to a C varargs function"""
+        def rust_cast(name, type_):
+            if type_ == "const char *":
+                return f"_{name}.as_ptr()"
+
+            type_ = c_type_to_rust(type_)
+            if type_ in RUST_VARARGS_SMALL_TYPES:
+                return f"_{name} as std::ffi::c_int"
+            return f"_{name} /* as {type_} */"
+
+        return ", ".join((rust_cast(name, type_) for type_, name in self._args))
+
 
 class Event(object):
     """Event description.
diff --git a/scripts/tracetool/format/rs.py b/scripts/tracetool/format/rs.py
new file mode 100644
index 00000000000..bc8b2be5971
--- /dev/null
+++ b/scripts/tracetool/format/rs.py
@@ -0,0 +1,76 @@
+# -*- coding: utf-8 -*-
+
+"""
+trace-DIR.rs
+"""
+
+__author__     = "Tanish Desai <tanishdesai37@gmail.com>"
+__copyright__  = "Copyright 2025, Tanish Desai <tanishdesai37@gmail.com>"
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__      = "stefanha@redhat.com"
+
+
+from tracetool import out
+
+
+def generate(events, backend, group):
+    out('// This file is autogenerated by tracetool, do not edit.',
+        '',
+        '#[allow(unused_imports)]',
+        'use std::ffi::c_char;',
+        '#[allow(unused_imports)]',
+        'use qemu_api::bindings;',
+        '',
+        '#[inline(always)]',
+        'fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {',
+        '    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }',
+        '}',
+        '',
+        'extern "C" {',
+        '    static mut trace_events_enabled_count: u32;',
+        '}',)
+
+    out('extern "C" {')
+
+    for e in events:
+        out('    static mut %s: u16;' % e.api(e.QEMU_DSTATE))
+    out('}')
+
+    # static state
+    for e in events:
+        if 'disable' in e.properties:
+            enabled = "false"
+        else:
+            enabled = "true"
+        if "tcg-exec" in e.properties:
+            # a single define for the two "sub-events"
+            out('const _TRACE_%(name)s_ENABLED: bool = %(enabled)s;',
+                name=e.original.name.upper(),
+				enabled=enabled)
+        out('const _TRACE_%s_ENABLED: bool = %s;' % (e.name.upper(), enabled))
+
+    backend.generate_begin(events, group)
+
+    for e in events:
+        out('',
+			'#[inline(always)]',
+            '#[allow(dead_code)]',
+            'pub fn %(api)s(%(args)s)',
+            '{',
+            api=e.api(e.QEMU_TRACE),
+            args=e.args.rust_decl())
+
+        if "disable" not in e.properties:
+            backend.generate(e, group, check_trace_event_get_state=False)
+            if backend.check_trace_event_get_state:
+                event_id = 'TRACE_' + e.name.upper()
+                out('    if trace_event_get_state_dynamic_by_id(unsafe { _%(event_id)s_DSTATE}) {',
+                    event_id = event_id,
+                    api=e.api())
+                backend.generate(e, group, check_trace_event_get_state=True)
+                out('    }')
+        out('}')
+
+    backend.generate_end(events, group)
-- 
2.50.1

[PATCH 07/14] rust: add trace crate

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

The trace crate is a minimal container for dependencies of tracepoints
(so that they do not have to be imported in all the crates that use
tracepoints); it also contains a macro called "include_trace!" that is
able to find the right include file from the trace/ directory.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
[Write commit message. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/Cargo.lock        |  4 ++++
 rust/Cargo.toml        |  1 +
 rust/meson.build       |  2 +-
 rust/trace/Cargo.toml  | 16 ++++++++++++++++
 rust/trace/meson.build | 19 +++++++++++++++++++
 rust/trace/src/lib.rs  | 23 +++++++++++++++++++++++
 trace/meson.build      |  8 +++++++-
 7 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 rust/trace/Cargo.toml
 create mode 100644 rust/trace/meson.build
 create mode 100644 rust/trace/src/lib.rs

diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index b785c718f31..aa13ab2a99a 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -164,6 +164,10 @@ dependencies = [
  "unicode-ident",
 ]
 
+[[package]]
+name = "trace"
+version = "0.1.0"
+
 [[package]]
 name = "unicode-ident"
 version = "1.0.12"
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 99c275f2d9f..2be9f886113 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -4,6 +4,7 @@ members = [
     "bits",
     "qemu-api-macros",
     "qemu-api",
+    "trace",
     "hw/char/pl011",
     "hw/timer/hpet",
 ]
diff --git a/rust/meson.build b/rust/meson.build
index 45936a0a731..2878bd8bc8d 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -23,7 +23,7 @@ genrs = []
 subdir('qemu-api-macros')
 subdir('bits')
 subdir('qemu-api')
-
+subdir('trace')
 subdir('hw')
 
 cargo = find_program('cargo', required: false)
diff --git a/rust/trace/Cargo.toml b/rust/trace/Cargo.toml
new file mode 100644
index 00000000000..913010e9787
--- /dev/null
+++ b/rust/trace/Cargo.toml
@@ -0,0 +1,16 @@
+[package]
+name = "trace"
+version = "0.1.0"
+authors = ["Tanish Desai<tanishdesai37@gmail.com>"]
+description = "rust trace library"
+resolver = "2"
+publish = false
+
+edition.workspace = true
+homepage.workspace = true
+license.workspace = true
+repository.workspace = true
+rust-version.workspace = true
+
+[lints]
+workspace = true
diff --git a/rust/trace/meson.build b/rust/trace/meson.build
new file mode 100644
index 00000000000..adca57e5507
--- /dev/null
+++ b/rust/trace/meson.build
@@ -0,0 +1,19 @@
+rust = import('rust')
+
+lib_rs = configure_file(
+  input: 'src/lib.rs',
+  output: 'lib.rs',
+  configuration: {
+    'MESON_BUILD_ROOT': meson.project_build_root(),
+  })
+
+_trace_rs = static_library(
+  'trace',             # Library name,
+  lib_rs,
+  trace_rs_targets,         # List of generated `.rs` custom targets
+  override_options: ['rust_std=2021', 'build.rust_std=2021'],
+  dependencies: [libc_rs],
+  rust_abi: 'rust',
+)
+
+trace_rs = declare_dependency(link_with: _trace_rs)
diff --git a/rust/trace/src/lib.rs b/rust/trace/src/lib.rs
new file mode 100644
index 00000000000..9b931ddf1de
--- /dev/null
+++ b/rust/trace/src/lib.rs
@@ -0,0 +1,23 @@
+//! This crate provides macros that aid in using QEMU's tracepoint
+//! functionality.
+
+#[macro_export]
+/// Define the trace-points from the named directory (which should have slashes
+/// replaced by underscore characters) as functions in a module called `trace`.
+///
+/// ```ignore
+/// ::trace::include_trace!("hw_char");
+/// // ...
+/// trace::trace_pl011_read_fifo_rx_full();
+/// ```
+macro_rules! include_trace {
+    ($name:literal) => {
+        mod trace {
+            #[cfg(not(MESON))]
+            include!(concat!(env!("MESON_BUILD_ROOT"), "/trace/", $name, ".rs"));
+
+            #[cfg(MESON)]
+            include!(concat!("@MESON_BUILD_ROOT@/trace/", $name, ".rs"));
+        }
+    };
+}
diff --git a/trace/meson.build b/trace/meson.build
index 9c42a57a053..d89a0db82a1 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -1,5 +1,5 @@
 system_ss.add(files('control-target.c', 'trace-hmp-cmds.c'))
-
+trace_rs_targets = []
 trace_events_files = []
 foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
   if item in qapi_trace_events
@@ -24,6 +24,11 @@ foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
                           input: trace_events_file,
                           command: [ tracetool, group, '--format=c', '@INPUT@', '@OUTPUT@' ],
                           depend_files: tracetool_depends)
+  trace_rs = custom_target(fmt.format('trace', 'rs'),
+                          output: fmt.format('trace', 'rs'),
+                          input: trace_events_file,
+                          command: [ tracetool, group, '--format=rs', '@INPUT@', '@OUTPUT@' ],
+                          depend_files: tracetool_depends)
   if 'ust' in get_option('trace_backends')
     trace_ust_h = custom_target(fmt.format('trace-ust', 'h'),
                                 output: fmt.format('trace-ust', 'h'),
@@ -34,6 +39,7 @@ foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
     genh += trace_ust_h
   endif
   trace_ss.add(trace_h, trace_c)
+  trace_rs_targets += trace_rs
   if 'dtrace' in get_option('trace_backends')
     trace_dtrace = custom_target(fmt.format('trace-dtrace', 'dtrace'),
                                  output: fmt.format('trace-dtrace', 'dtrace'),
-- 
2.50.1

[PATCH 08/14] rust: qdev: add minimal clock bindings

[Paolo Bonzini]

Add the minimal support that is needed by pl011's event and tracepoint.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/qdev.rs | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 6d619661ba4..64b961136b2 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -449,6 +449,39 @@ fn init_gpio_out(&self, pins: &[InterruptSource]) {
 
 impl<R: ObjectDeref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
 
+impl Clock {
+    pub const PERIOD_1SEC: u64 = bindings::CLOCK_PERIOD_1SEC;
+
+    pub const fn period_from_ns(ns: u64) -> u64 {
+        ns * Self::PERIOD_1SEC / 1_000_000_000
+    }
+
+    pub const fn period_from_hz(hz: u64) -> u64 {
+        if hz == 0 {
+            0
+        } else {
+            Self::PERIOD_1SEC / hz
+        }
+    }
+
+    pub const fn period_to_hz(period: u64) -> u64 {
+        if period == 0 {
+            0
+        } else {
+            Self::PERIOD_1SEC / period
+        }
+    }
+
+    pub const fn get(&self) -> u64 {
+        // SAFETY: Clock is returned by init_clock_in with zero value for period
+        unsafe { &*self.0.as_ptr() }.period
+    }
+
+    pub const fn get_hz(&self) -> u64 {
+        Self::period_to_hz(self.get())
+    }
+}
+
 unsafe impl ObjectType for Clock {
     type Class = ObjectClass;
     const TYPE_NAME: &'static CStr =
-- 
2.50.1

[PATCH 09/14] rust: pl011: add tracepoints

[Paolo Bonzini]

Finally bring parity between C and Rust versions of the PL011 device model.
Changing some types of the arguments makes for nicer Rust code; C does not
care. :)

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/trace-events             | 14 ++++----
 rust/Cargo.lock                  |  1 +
 rust/hw/char/pl011/Cargo.toml    |  1 +
 rust/hw/char/pl011/meson.build   |  1 +
 rust/hw/char/pl011/src/device.rs | 57 ++++++++++++++++++++++----------
 5 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/hw/char/trace-events b/hw/char/trace-events
index 05a33036c12..9e74be2c14f 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -58,15 +58,15 @@ imx_serial_write(const char *chrname, uint64_t addr, uint64_t value) "%s:[0x%03"
 imx_serial_put_data(const char *chrname, uint32_t value) "%s: 0x%" PRIx32
 
 # pl011.c
-pl011_irq_state(int level) "irq state %d"
-pl011_read(uint32_t addr, uint32_t value, const char *regname) "addr 0x%03x value 0x%08x reg %s"
-pl011_read_fifo(unsigned rx_fifo_used, size_t rx_fifo_depth) "RX FIFO read, used %u/%zu"
-pl011_write(uint32_t addr, uint32_t value, const char *regname) "addr 0x%03x value 0x%08x reg %s"
-pl011_can_receive(uint32_t lcr, unsigned rx_fifo_used, size_t rx_fifo_depth, unsigned rx_fifo_available) "LCR 0x%02x, RX FIFO used %u/%zu, can_receive %u chars"
-pl011_fifo_rx_put(uint32_t c, unsigned read_count, size_t rx_fifo_depth) "RX FIFO push char [0x%02x] %d/%zu depth used"
+pl011_irq_state(bool level) "irq state %d"
+pl011_read(uint64_t addr, uint32_t value, const char *regname) "addr 0x%03" PRIx64 " value 0x%08x reg %s"
+pl011_read_fifo(unsigned rx_fifo_used, unsigned rx_fifo_depth) "RX FIFO read, used %u/%u"
+pl011_write(uint64_t addr, uint32_t value, const char *regname) "addr 0x%03" PRIx64 " value 0x%08x reg %s"
+pl011_can_receive(uint32_t lcr, unsigned rx_fifo_used, unsigned rx_fifo_depth, unsigned rx_fifo_available) "LCR 0x%02x, RX FIFO used %u/%u, can_receive %u chars"
+pl011_fifo_rx_put(uint32_t c, unsigned read_count, unsigned rx_fifo_depth) "RX FIFO push char [0x%02x] %d/%u depth used"
 pl011_fifo_rx_full(void) "RX FIFO now full, RXFF set"
 pl011_baudrate_change(unsigned int baudrate, uint64_t clock, uint32_t ibrd, uint32_t fbrd) "new baudrate %u (clk: %" PRIu64 "hz, ibrd: %" PRIu32 ", fbrd: %" PRIu32 ")"
-pl011_receive(int size) "recv %d chars"
+pl011_receive(size_t size) "recv %zd chars"
 
 # cmsdk-apb-uart.c
 cmsdk_apb_uart_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB UART read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index aa13ab2a99a..6ed838f863f 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -91,6 +91,7 @@ dependencies = [
  "bits",
  "qemu_api",
  "qemu_api_macros",
+ "trace",
 ]
 
 [[package]]
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index 88ef110507d..d71b881f4e1 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -18,6 +18,7 @@ bilge-impl = { version = "0.2.0" }
 bits = { path = "../../../bits" }
 qemu_api = { path = "../../../qemu-api" }
 qemu_api_macros = { path = "../../../qemu-api-macros" }
+trace = { path = "../../../trace" }
 
 [lints]
 workspace = true
diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
index 16acf12f7cc..5083701d6a7 100644
--- a/rust/hw/char/pl011/meson.build
+++ b/rust/hw/char/pl011/meson.build
@@ -9,6 +9,7 @@ _libpl011_rs = static_library(
     bits_rs,
     qemu_api_rs,
     qemu_api_macros,
+    trace_rs
   ],
 )
 
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 8411db8d00c..bd30b117e92 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -4,6 +4,8 @@
 
 use std::{ffi::CStr, mem::size_of};
 
+::trace::include_trace!("trace-hw_char");
+
 use qemu_api::{
     chardev::{CharBackend, Chardev, Event},
     impl_vmstate_forward, impl_vmstate_struct,
@@ -210,13 +212,7 @@ pub(self) fn read(&mut self, offset: RegisterOffset) -> (bool, u32) {
         (update, result)
     }
 
-    pub(self) fn write(
-        &mut self,
-        offset: RegisterOffset,
-        value: u32,
-        char_backend: &CharBackend,
-    ) -> bool {
-        // eprintln!("write offset {offset} value {value}");
+    pub(self) fn write(&mut self, offset: RegisterOffset, value: u32, device: &PL011State) -> bool {
         use RegisterOffset::*;
         match offset {
             DR => return self.write_data_register(value),
@@ -231,9 +227,11 @@ pub(self) fn write(
             }
             IBRD => {
                 self.ibrd = value;
+                device.trace_baudrate_change(self.ibrd, self.fbrd);
             }
             FBRD => {
                 self.fbrd = value;
+                device.trace_baudrate_change(self.ibrd, self.fbrd);
             }
             LCR_H => {
                 let new_val: registers::LineControl = value.into();
@@ -244,7 +242,7 @@ pub(self) fn write(
                 }
                 let update = (self.line_control.send_break() != new_val.send_break()) && {
                     let break_enable = new_val.send_break();
-                    let _ = char_backend.send_break(break_enable);
+                    let _ = device.char_backend.send_break(break_enable);
                     self.loopback_break(break_enable)
                 };
                 self.line_control = new_val;
@@ -281,12 +279,13 @@ pub(self) fn write(
     }
 
     fn read_data_register(&mut self, update: &mut bool) -> u32 {
+        let depth = self.fifo_depth();
         self.flags.set_receive_fifo_full(false);
         let c = self.read_fifo[self.read_pos];
 
         if self.read_count > 0 {
             self.read_count -= 1;
-            self.read_pos = (self.read_pos + 1) & (self.fifo_depth() - 1);
+            self.read_pos = (self.read_pos + 1) & (depth - 1);
         }
         if self.read_count == 0 {
             self.flags.set_receive_fifo_empty(true);
@@ -294,6 +293,7 @@ fn read_data_register(&mut self, update: &mut bool) -> u32 {
         if self.read_count + 1 == self.read_trigger {
             self.int_level &= !Interrupt::RX;
         }
+        trace::trace_pl011_read_fifo(self.read_count, depth);
         self.receive_status_error_clear.set_from_data(c);
         *update = true;
         u32::from(c)
@@ -449,7 +449,9 @@ pub fn fifo_rx_put(&mut self, value: registers::Data) -> bool {
         self.read_fifo[slot] = value;
         self.read_count += 1;
         self.flags.set_receive_fifo_empty(false);
+        trace::trace_pl011_fifo_rx_put(value.into(), self.read_count, depth);
         if self.read_count == depth {
+            trace::trace_pl011_fifo_rx_full();
             self.flags.set_receive_fifo_full(true);
         }
 
@@ -518,8 +520,21 @@ unsafe fn init(mut this: ParentInit<Self>) {
         uninit_field_mut!(*this, clock).write(clock);
     }
 
-    const fn clock_update(&self, _event: ClockEvent) {
-        /* pl011_trace_baudrate_change(s); */
+    pub fn trace_baudrate_change(&self, ibrd: u32, fbrd: u32) {
+        let divider = 4.0 / f64::from(ibrd * (FBRD_MASK + 1) + fbrd);
+        let hz = self.clock.get_hz();
+        let rate = if ibrd == 0 {
+            0
+        } else {
+            ((hz as f64) * divider) as u32
+        };
+        trace::trace_pl011_baudrate_change(rate, hz, ibrd, fbrd);
+    }
+
+    fn clock_update(&self, _event: ClockEvent) {
+        let regs = self.regs.borrow();
+        let (ibrd, fbrd) = (regs.ibrd, regs.fbrd);
+        self.trace_baudrate_change(ibrd, fbrd)
     }
 
     pub fn clock_needed(&self) -> bool {
@@ -545,6 +560,7 @@ fn read(&self, offset: hwaddr, _size: u32) -> u64 {
             }
             Ok(field) => {
                 let (update_irq, result) = self.regs.borrow_mut().read(field);
+                trace::trace_pl011_read(offset, result, c"");
                 if update_irq {
                     self.update();
                     self.char_backend.accept_input();
@@ -559,6 +575,7 @@ fn write(&self, offset: hwaddr, value: u64, _size: u32) {
         if let Ok(field) = RegisterOffset::try_from(offset) {
             // qemu_chr_fe_write_all() calls into the can_receive
             // callback, so handle writes before entering PL011Registers.
+            trace::trace_pl011_write(offset, value as u32, c"");
             if field == RegisterOffset::DR {
                 // ??? Check if transmitter is enabled.
                 let ch: [u8; 1] = [value as u8];
@@ -567,10 +584,7 @@ fn write(&self, offset: hwaddr, value: u64, _size: u32) {
                 let _ = self.char_backend.write_all(&ch);
             }
 
-            update_irq = self
-                .regs
-                .borrow_mut()
-                .write(field, value as u32, &self.char_backend);
+            update_irq = self.regs.borrow_mut().write(field, value as u32, &self);
         } else {
             log_mask_ln!(
                 Log::GuestError,
@@ -584,11 +598,19 @@ fn write(&self, offset: hwaddr, value: u64, _size: u32) {
 
     fn can_receive(&self) -> u32 {
         let regs = self.regs.borrow();
-        // trace_pl011_can_receive(s->lcr, s->read_count, r);
-        regs.fifo_depth() - regs.read_count
+        let fifo_available = regs.fifo_depth() - regs.read_count;
+        trace::trace_pl011_can_receive(
+            regs.line_control.into(),
+            regs.read_count,
+            regs.fifo_depth(),
+            fifo_available,
+        );
+        fifo_available
     }
 
     fn receive(&self, buf: &[u8]) {
+        trace::trace_pl011_receive(buf.len());
+
         let mut regs = self.regs.borrow_mut();
         if regs.loopback_enabled() {
             // In loopback mode, the RX input signal is internally disconnected
@@ -637,6 +659,7 @@ fn reset_hold(&self, _type: ResetType) {
     fn update(&self) {
         let regs = self.regs.borrow();
         let flags = regs.int_level & regs.int_enabled;
+        trace::trace_pl011_irq_state(flags != 0);
         for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
             irq.set(flags.any_set(i));
         }
-- 
2.50.1

[PATCH 10/14] tracetool/simple: add Rust support

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/tracetool/__init__.py       |  2 +-
 scripts/tracetool/backend/simple.py |  7 +++++
 tests/tracetool/simple.rs           | 41 +++++++++++++++++++++++++++++
 tests/tracetool/tracetool-test.py   |  2 ++
 4 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 tests/tracetool/simple.rs

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 0b8ec707332..7542757799e 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -336,7 +336,7 @@ def rust_call_extern(self):
         def rust_cast(name, type_):
             if type_ == "const char *":
                 return f"_{name}.as_ptr()"
-            return "_{name}"
+            return f"_{name}"
 
         return ", ".join((rust_cast(name, type_) for type_, name in self._args))
 
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index a8afc977a20..7acbd6cc69d 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -98,3 +98,10 @@ def generate_c(event, group):
     out('    trace_record_finish(&rec);',
         '}',
         '')
+
+def generate_rs(event, group):
+    out('        extern "C" { fn _simple_%(api)s(%(rust_args)s); }',
+        '        unsafe { _simple_%(api)s(%(args)s); }',
+        api=event.api(),
+        rust_args=event.args.rust_decl_extern(),
+        args=event.args.rust_call_extern())
diff --git a/tests/tracetool/simple.rs b/tests/tracetool/simple.rs
new file mode 100644
index 00000000000..895096088dc
--- /dev/null
+++ b/tests/tracetool/simple.rs
@@ -0,0 +1,41 @@
+// This file is autogenerated by tracetool, do not edit.
+
+#[allow(unused_imports)]
+use std::ffi::c_char;
+#[allow(unused_imports)]
+use qemu_api::bindings;
+
+#[inline(always)]
+fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {
+    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }
+}
+
+extern "C" {
+    static mut trace_events_enabled_count: u32;
+}
+extern "C" {
+    static mut _TRACE_TEST_BLAH_DSTATE: u16;
+    static mut _TRACE_TEST_WIBBLE_DSTATE: u16;
+}
+const _TRACE_TEST_BLAH_ENABLED: bool = true;
+const _TRACE_TEST_WIBBLE_ENABLED: bool = true;
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_blah(_context: *mut (), _filename: &std::ffi::CStr)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_BLAH_DSTATE}) {
+        extern "C" { fn _simple_trace_test_blah(_context: *mut (), _filename: *const std::ffi::c_char); }
+        unsafe { _simple_trace_test_blah(_context, _filename.as_ptr()); }
+    }
+}
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_wibble(_context: *mut (), _value: std::ffi::c_int)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_WIBBLE_DSTATE}) {
+        extern "C" { fn _simple_trace_test_wibble(_context: *mut (), _value: std::ffi::c_int); }
+        unsafe { _simple_trace_test_wibble(_context, _value); }
+    }
+}
diff --git a/tests/tracetool/tracetool-test.py b/tests/tracetool/tracetool-test.py
index a420597fc48..ad7dd667288 100755
--- a/tests/tracetool/tracetool-test.py
+++ b/tests/tracetool/tracetool-test.py
@@ -13,6 +13,8 @@ def get_formats(backend):
         "c",
         "h",
     ]
+    if backend in {"simple"}:
+        formats += ["rs"]
     if backend == "dtrace":
         formats += [
             "d",
-- 
2.50.1

[PATCH 11/14] log: change qemu_loglevel to unsigned

[Paolo Bonzini]

Bindgen makes the LOG_* constants unsigned, even if they are defined as
(1 << 15):

   pub const LOG_TRACE: u32 = 32768;

Make them unsigned in C as well, and also change the type of the variable
that they are used with.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/log-for-trace.h |  4 ++--
 include/qemu/log.h           | 44 ++++++++++++++++++------------------
 util/log.c                   |  2 +-
 rust/qemu-api/src/log.rs     |  2 +-
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/qemu/log-for-trace.h b/include/qemu/log-for-trace.h
index d47c9cd4462..f3a8791f1d4 100644
--- a/include/qemu/log-for-trace.h
+++ b/include/qemu/log-for-trace.h
@@ -19,9 +19,9 @@
 #define QEMU_LOG_FOR_TRACE_H
 
 /* Private global variable, don't use */
-extern int qemu_loglevel;
+extern unsigned qemu_loglevel;
 
-#define LOG_TRACE          (1 << 15)
+#define LOG_TRACE          (1u << 15)
 
 /* Returns true if a bit is set in the current loglevel mask */
 static inline bool qemu_loglevel_mask(int mask)
diff --git a/include/qemu/log.h b/include/qemu/log.h
index aae72985f0d..7effba4da4c 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -14,30 +14,30 @@ bool qemu_log_enabled(void);
 /* Returns true if qemu_log() will write somewhere other than stderr. */
 bool qemu_log_separate(void);
 
-#define CPU_LOG_TB_OUT_ASM (1 << 0)
-#define CPU_LOG_TB_IN_ASM  (1 << 1)
-#define CPU_LOG_TB_OP      (1 << 2)
-#define CPU_LOG_TB_OP_OPT  (1 << 3)
-#define CPU_LOG_INT        (1 << 4)
-#define CPU_LOG_EXEC       (1 << 5)
-#define CPU_LOG_PCALL      (1 << 6)
-#define CPU_LOG_TB_CPU     (1 << 8)
-#define CPU_LOG_RESET      (1 << 9)
-#define LOG_UNIMP          (1 << 10)
-#define LOG_GUEST_ERROR    (1 << 11)
-#define CPU_LOG_MMU        (1 << 12)
-#define CPU_LOG_TB_NOCHAIN (1 << 13)
-#define CPU_LOG_PAGE       (1 << 14)
+#define CPU_LOG_TB_OUT_ASM (1u << 0)
+#define CPU_LOG_TB_IN_ASM  (1u << 1)
+#define CPU_LOG_TB_OP      (1u << 2)
+#define CPU_LOG_TB_OP_OPT  (1u << 3)
+#define CPU_LOG_INT        (1u << 4)
+#define CPU_LOG_EXEC       (1u << 5)
+#define CPU_LOG_PCALL      (1u << 6)
+#define CPU_LOG_TB_CPU     (1u << 8)
+#define CPU_LOG_RESET      (1u << 9)
+#define LOG_UNIMP          (1u << 10)
+#define LOG_GUEST_ERROR    (1u << 11)
+#define CPU_LOG_MMU        (1u << 12)
+#define CPU_LOG_TB_NOCHAIN (1u << 13)
+#define CPU_LOG_PAGE       (1u << 14)
 /* LOG_TRACE (1 << 15) is defined in log-for-trace.h */
-#define CPU_LOG_TB_OP_IND  (1 << 16)
-#define CPU_LOG_TB_FPU     (1 << 17)
-#define CPU_LOG_PLUGIN     (1 << 18)
+#define CPU_LOG_TB_OP_IND  (1u << 16)
+#define CPU_LOG_TB_FPU     (1u << 17)
+#define CPU_LOG_PLUGIN     (1u << 18)
 /* LOG_STRACE is used for user-mode strace logging. */
-#define LOG_STRACE         (1 << 19)
-#define LOG_PER_THREAD     (1 << 20)
-#define CPU_LOG_TB_VPU     (1 << 21)
-#define LOG_TB_OP_PLUGIN   (1 << 22)
-#define LOG_INVALID_MEM    (1 << 23)
+#define LOG_STRACE         (1u << 19)
+#define LOG_PER_THREAD     (1u << 20)
+#define CPU_LOG_TB_VPU     (1u << 21)
+#define LOG_TB_OP_PLUGIN   (1u << 22)
+#define LOG_INVALID_MEM    (1u << 23)
 
 /* Lock/unlock output. */
 
diff --git a/util/log.c b/util/log.c
index abdcb6b3111..41f78ce86b2 100644
--- a/util/log.c
+++ b/util/log.c
@@ -44,7 +44,7 @@ static FILE *global_file;
 static __thread FILE *thread_file;
 static __thread Notifier qemu_log_thread_cleanup_notifier;
 
-int qemu_loglevel;
+unsigned qemu_loglevel;
 static bool log_per_thread;
 static GArray *debug_regions;
 
diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs
index a441b8c1f2e..fe43e30104c 100644
--- a/rust/qemu-api/src/log.rs
+++ b/rust/qemu-api/src/log.rs
@@ -140,7 +140,7 @@ macro_rules! log_mask_ln {
         let _: Log = $mask;
 
         if unsafe {
-            (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
+            (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_uint)) != 0
         } {
             _ = ::qemu_api::log::LogGuard::log_fmt(
                 format_args!("{}\n", format_args!($fmt $($args)*)));
-- 
2.50.1

[PATCH 12/14] tracetool/log: add Rust support

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/tracetool/backend/log.py  | 10 ++++++-
 tests/tracetool/log.rs            | 45 +++++++++++++++++++++++++++++++
 tests/tracetool/tracetool-test.py |  2 +-
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 tests/tracetool/log.rs

diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index 2aa180f4b47..c167f30cf2c 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -12,7 +12,7 @@
 __email__      = "stefanha@redhat.com"
 
 
-from tracetool import out
+from tracetool import out, expand_format_string
 
 
 PUBLIC = True
@@ -44,3 +44,11 @@ def generate_h(event, group):
 def generate_h_backend_dstate(event, group):
     out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
         event_id="TRACE_" + event.name.upper())
+
+def generate_rs(event, group):
+    out('        let format_string = c"%(fmt)s\\n";',
+        '        if (unsafe { bindings::qemu_loglevel } & bindings::LOG_TRACE) != 0 {',
+        '            unsafe { bindings::qemu_log(format_string.as_ptr() as *const c_char, %(args)s);}',
+        '        }',
+        fmt=expand_format_string(event.fmt, event.name + " "),
+        args=event.args.rust_call_varargs())
diff --git a/tests/tracetool/log.rs b/tests/tracetool/log.rs
new file mode 100644
index 00000000000..fc95adafa46
--- /dev/null
+++ b/tests/tracetool/log.rs
@@ -0,0 +1,45 @@
+// This file is autogenerated by tracetool, do not edit.
+
+#[allow(unused_imports)]
+use std::ffi::c_char;
+#[allow(unused_imports)]
+use qemu_api::bindings;
+
+#[inline(always)]
+fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {
+    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }
+}
+
+extern "C" {
+    static mut trace_events_enabled_count: u32;
+}
+extern "C" {
+    static mut _TRACE_TEST_BLAH_DSTATE: u16;
+    static mut _TRACE_TEST_WIBBLE_DSTATE: u16;
+}
+const _TRACE_TEST_BLAH_ENABLED: bool = true;
+const _TRACE_TEST_WIBBLE_ENABLED: bool = true;
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_blah(_context: *mut (), _filename: &std::ffi::CStr)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_BLAH_DSTATE}) {
+        let format_string = c"test_blah Blah context=%p filename=%s\n";
+        if (unsafe { bindings::qemu_loglevel } & bindings::LOG_TRACE) != 0 {
+            unsafe { bindings::qemu_log(format_string.as_ptr() as *const c_char, _context, _filename.as_ptr());}
+        }
+    }
+}
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_wibble(_context: *mut (), _value: std::ffi::c_int)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_WIBBLE_DSTATE}) {
+        let format_string = c"test_wibble Wibble context=%p value=%d\n";
+        if (unsafe { bindings::qemu_loglevel } & bindings::LOG_TRACE) != 0 {
+            unsafe { bindings::qemu_log(format_string.as_ptr() as *const c_char, _context, _value);}
+        }
+    }
+}
diff --git a/tests/tracetool/tracetool-test.py b/tests/tracetool/tracetool-test.py
index ad7dd667288..5e5b16e2856 100755
--- a/tests/tracetool/tracetool-test.py
+++ b/tests/tracetool/tracetool-test.py
@@ -13,7 +13,7 @@ def get_formats(backend):
         "c",
         "h",
     ]
-    if backend in {"simple"}:
+    if backend in {"log", "simple"}:
         formats += ["rs"]
     if backend == "dtrace":
         formats += [
-- 
2.50.1

[PATCH 13/14] tracetool/ftrace: add Rust support

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/wrapper.h             |  1 +
 scripts/tracetool/backend/ftrace.py |  8 +++++-
 tests/tracetool/ftrace.rs           | 41 +++++++++++++++++++++++++++++
 tests/tracetool/tracetool-test.py   |  2 +-
 4 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 tests/tracetool/ftrace.rs

diff --git a/rust/qemu-api/wrapper.h b/rust/qemu-api/wrapper.h
index 15a1b19847f..5227838292b 100644
--- a/rust/qemu-api/wrapper.h
+++ b/rust/qemu-api/wrapper.h
@@ -69,3 +69,4 @@ typedef enum memory_order {
 #include "qemu/timer.h"
 #include "system/address-spaces.h"
 #include "hw/char/pl011.h"
+#include "trace/ftrace.h"
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 432f216ea2b..51d978f998f 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -12,7 +12,7 @@
 __email__      = "stefanha@redhat.com"
 
 
-from tracetool import out
+from tracetool import out, expand_format_string
 
 
 PUBLIC = True
@@ -43,3 +43,9 @@ def generate_h(event, group):
 def generate_h_backend_dstate(event, group):
     out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
         event_id="TRACE_" + event.name.upper())
+
+def generate_rs(event, group):
+    out('        let format_string = c"%(fmt)s";',
+        '        unsafe {bindings::ftrace_write(format_string.as_ptr() as *const c_char, %(args)s);}',
+        fmt=expand_format_string(event.fmt),
+        args=event.args.rust_call_varargs())
diff --git a/tests/tracetool/ftrace.rs b/tests/tracetool/ftrace.rs
new file mode 100644
index 00000000000..950a89534af
--- /dev/null
+++ b/tests/tracetool/ftrace.rs
@@ -0,0 +1,41 @@
+// This file is autogenerated by tracetool, do not edit.
+
+#[allow(unused_imports)]
+use std::ffi::c_char;
+#[allow(unused_imports)]
+use qemu_api::bindings;
+
+#[inline(always)]
+fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {
+    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }
+}
+
+extern "C" {
+    static mut trace_events_enabled_count: u32;
+}
+extern "C" {
+    static mut _TRACE_TEST_BLAH_DSTATE: u16;
+    static mut _TRACE_TEST_WIBBLE_DSTATE: u16;
+}
+const _TRACE_TEST_BLAH_ENABLED: bool = true;
+const _TRACE_TEST_WIBBLE_ENABLED: bool = true;
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_blah(_context: *mut (), _filename: &std::ffi::CStr)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_BLAH_DSTATE}) {
+        let format_string = c"Blah context=%p filename=%s";
+        unsafe {bindings::ftrace_write(format_string.as_ptr() as *const c_char, _context, _filename.as_ptr());}
+    }
+}
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_wibble(_context: *mut (), _value: std::ffi::c_int)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_WIBBLE_DSTATE}) {
+        let format_string = c"Wibble context=%p value=%d";
+        unsafe {bindings::ftrace_write(format_string.as_ptr() as *const c_char, _context, _value);}
+    }
+}
diff --git a/tests/tracetool/tracetool-test.py b/tests/tracetool/tracetool-test.py
index 5e5b16e2856..a91d635910a 100755
--- a/tests/tracetool/tracetool-test.py
+++ b/tests/tracetool/tracetool-test.py
@@ -13,7 +13,7 @@ def get_formats(backend):
         "c",
         "h",
     ]
-    if backend in {"log", "simple"}:
+    if backend in {"ftrace", "log", "simple"}:
         formats += ["rs"]
     if backend == "dtrace":
         formats += [
-- 
2.50.1

[PATCH 14/14] tracetool/syslog: add Rust support

[Paolo Bonzini]

From: Tanish Desai <tanishdesai37@gmail.com>

The syslog backend needs the syslog function from libc and the LOG_INFO enum
value; they are re-exported as "::trace::syslog" and "::trace::LOG_INFO"
so that device crates do not all have to add the libc dependency, but
otherwise there is nothing special.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/Cargo.lock                     |  3 +++
 rust/trace/Cargo.toml               |  3 +++
 rust/trace/src/lib.rs               |  4 +++
 scripts/tracetool/backend/syslog.py |  7 ++++-
 tests/tracetool/syslog.rs           | 41 +++++++++++++++++++++++++++++
 tests/tracetool/tracetool-test.py   |  2 +-
 6 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 tests/tracetool/syslog.rs

diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 6ed838f863f..e5f10cb358a 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -168,6 +168,9 @@ dependencies = [
 [[package]]
 name = "trace"
 version = "0.1.0"
+dependencies = [
+ "libc",
+]
 
 [[package]]
 name = "unicode-ident"
diff --git a/rust/trace/Cargo.toml b/rust/trace/Cargo.toml
index 913010e9787..5f0f370e523 100644
--- a/rust/trace/Cargo.toml
+++ b/rust/trace/Cargo.toml
@@ -12,5 +12,8 @@ license.workspace = true
 repository.workspace = true
 rust-version.workspace = true
 
+[dependencies]
+libc = { workspace = true }
+
 [lints]
 workspace = true
diff --git a/rust/trace/src/lib.rs b/rust/trace/src/lib.rs
index 9b931ddf1de..966991432e3 100644
--- a/rust/trace/src/lib.rs
+++ b/rust/trace/src/lib.rs
@@ -1,6 +1,10 @@
 //! This crate provides macros that aid in using QEMU's tracepoint
 //! functionality.
 
+#[doc(hidden)]
+/// Re-exported items to avoid adding libc as a dependency everywhere.
+pub use libc::{syslog, LOG_INFO};
+
 #[macro_export]
 /// Define the trace-points from the named directory (which should have slashes
 /// replaced by underscore characters) as functions in a module called `trace`.
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 04ec85717a3..acaa56ba073 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -12,7 +12,7 @@
 __email__      = "stefanha@redhat.com"
 
 
-from tracetool import out
+from tracetool import out, expand_format_string
 
 
 PUBLIC = True
@@ -38,6 +38,11 @@ def generate_h(event, group):
         fmt=event.fmt.rstrip("\n"),
         argnames=argnames)
 
+def generate_rs(event, group):
+    out('        let format_string = c"%(fmt)s";',
+        '        unsafe {::trace::syslog(::trace::LOG_INFO, format_string.as_ptr() as *const c_char, %(args)s);}',
+        fmt=expand_format_string(event.fmt),
+        args=event.args.rust_call_varargs())
 
 def generate_h_backend_dstate(event, group):
     out('    trace_event_get_state_dynamic_by_id(%(event_id)s) || \\',
diff --git a/tests/tracetool/syslog.rs b/tests/tracetool/syslog.rs
new file mode 100644
index 00000000000..33a8ae7c1aa
--- /dev/null
+++ b/tests/tracetool/syslog.rs
@@ -0,0 +1,41 @@
+// This file is autogenerated by tracetool, do not edit.
+
+#[allow(unused_imports)]
+use std::ffi::c_char;
+#[allow(unused_imports)]
+use qemu_api::bindings;
+
+#[inline(always)]
+fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {
+    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }
+}
+
+extern "C" {
+    static mut trace_events_enabled_count: u32;
+}
+extern "C" {
+    static mut _TRACE_TEST_BLAH_DSTATE: u16;
+    static mut _TRACE_TEST_WIBBLE_DSTATE: u16;
+}
+const _TRACE_TEST_BLAH_ENABLED: bool = true;
+const _TRACE_TEST_WIBBLE_ENABLED: bool = true;
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_blah(_context: *mut (), _filename: &std::ffi::CStr)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_BLAH_DSTATE}) {
+        let format_string = c"Blah context=%p filename=%s";
+        unsafe {::trace::syslog(::trace::LOG_INFO, format_string.as_ptr() as *const c_char, _context, _filename.as_ptr());}
+    }
+}
+
+#[inline(always)]
+#[allow(dead_code)]
+pub fn trace_test_wibble(_context: *mut (), _value: std::ffi::c_int)
+{
+    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_WIBBLE_DSTATE}) {
+        let format_string = c"Wibble context=%p value=%d";
+        unsafe {::trace::syslog(::trace::LOG_INFO, format_string.as_ptr() as *const c_char, _context, _value);}
+    }
+}
diff --git a/tests/tracetool/tracetool-test.py b/tests/tracetool/tracetool-test.py
index a91d635910a..d317e047891 100755
--- a/tests/tracetool/tracetool-test.py
+++ b/tests/tracetool/tracetool-test.py
@@ -13,7 +13,7 @@ def get_formats(backend):
         "c",
         "h",
     ]
-    if backend in {"ftrace", "log", "simple"}:
+    if backend in {"ftrace", "log", "simple", "syslog"}:
         formats += ["rs"]
     if backend == "dtrace":
         formats += [
-- 
2.50.1

Re: [PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Manos Pitsidianakis]

On Fri, Aug 22, 2025 at 3:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Putting "unsigned" in anything but the first position is weird.  As such,
> tracetool's Rust type conversion will not support it.  Remove it from
> the whole of QEMU's source code, not just trace-events.
>

Hm weird C quirk indeed.

Why can't tracetool support this? Can't we just add the permutations
in the C_TO_RUST_TYPE_MAP dict in "[PATCH 06/14] tracetool: Add Rust
format support"?

+    "unsigned long long": "std::ffi::c_ulonglong",
+    "long unsigned long": "std::ffi::c_ulonglong",
+    "long long unsigned": "std::ffi::c_ulonglong",



> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  crypto/pbkdf-gcrypt.c        | 2 +-
>  crypto/pbkdf-gnutls.c        | 2 +-
>  crypto/pbkdf-nettle.c        | 2 +-
>  hw/display/exynos4210_fimd.c | 2 +-
>  hw/misc/imx7_src.c           | 4 ++--
>  hw/net/can/can_sja1000.c     | 4 ++--
>  hw/xen/trace-events          | 4 ++--
>  7 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
> index e89b8b1c768..f93996f674c 100644
> --- a/crypto/pbkdf-gcrypt.c
> +++ b/crypto/pbkdf-gcrypt.c
> @@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
>      if (iterations > ULONG_MAX) {
>          error_setg_errno(errp, ERANGE,
>                           "PBKDF iterations %llu must be less than %lu",
> -                         (long long unsigned)iterations, ULONG_MAX);
> +                         (unsigned long long)iterations, ULONG_MAX);
>          return -1;
>      }
>
> diff --git a/crypto/pbkdf-gnutls.c b/crypto/pbkdf-gnutls.c
> index f34423f918b..46a3a869994 100644
> --- a/crypto/pbkdf-gnutls.c
> +++ b/crypto/pbkdf-gnutls.c
> @@ -62,7 +62,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
>      if (iterations > ULONG_MAX) {
>          error_setg_errno(errp, ERANGE,
>                           "PBKDF iterations %llu must be less than %lu",
> -                         (long long unsigned)iterations, ULONG_MAX);
> +                         (unsigned long long)iterations, ULONG_MAX);
>          return -1;
>      }
>
> diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c
> index 3ef9c1b52c4..3c8bbaf9f17 100644
> --- a/crypto/pbkdf-nettle.c
> +++ b/crypto/pbkdf-nettle.c
> @@ -66,7 +66,7 @@ int qcrypto_pbkdf2(QCryptoHashAlgo hash,
>      if (iterations > UINT_MAX) {
>          error_setg_errno(errp, ERANGE,
>                           "PBKDF iterations %llu must be less than %u",
> -                         (long long unsigned)iterations, UINT_MAX);
> +                         (unsigned long long)iterations, ULONG_MAX);
>          return -1;
>      }
>
> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
> index c61e0280a7c..5632aa1388c 100644
> --- a/hw/display/exynos4210_fimd.c
> +++ b/hw/display/exynos4210_fimd.c
> @@ -1380,7 +1380,7 @@ static void exynos4210_fimd_write(void *opaque, hwaddr offset,
>      uint32_t old_value;
>
>      DPRINT_L2("write offset 0x%08x, value=%llu(0x%08llx)\n", offset,
> -            (long long unsigned int)val, (long long unsigned int)val);
> +            (unsigned long long)val, (unsigned long long)val);
>
>      switch (offset) {
>      case FIMD_VIDCON0:
> diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
> index df0b0a69057..817c95bf65b 100644
> --- a/hw/misc/imx7_src.c
> +++ b/hw/misc/imx7_src.c
> @@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
>  {
>      IMX7SRCState *s = (IMX7SRCState *)opaque;
>      uint32_t index = offset >> 2;
> -    long unsigned int change_mask;
> +    uint32_t change_mask;
>      uint32_t current_value = value;
>
>      if (index >= SRC_MAX) {
> @@ -180,7 +180,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
>
>      trace_imx7_src_write(imx7_src_reg_name(SRC_A7RCR0), s->regs[SRC_A7RCR0]);
>
> -    change_mask = s->regs[index] ^ (uint32_t)current_value;
> +    change_mask = s->regs[index] ^ current_value;
>
>      switch (index) {
>      case SRC_A7RCR0:
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 5b6ba9df6c4..545c520c3b4 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
>              break;
>          }
>      }
> -    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> -            (int)addr, size, (long unsigned int)temp);
> +    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02x\n",
> +            (int)addr, size, (unsigned)temp);
>
>      return temp;
>  }
> diff --git a/hw/xen/trace-events b/hw/xen/trace-events
> index b67942d07b4..3b71ee641ff 100644
> --- a/hw/xen/trace-events
> +++ b/hw/xen/trace-events
> @@ -57,8 +57,8 @@ cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uin
>  cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
>  cpu_get_ioreq_from_shared_memory_req_not_ready(int state, int data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O request not ready: 0x%x, ptr: 0x%x, port: 0x%"PRIx64", data: 0x%"PRIx64", count: %u, size: %u"
>  xen_main_loop_prepare_init_cpu(int id, void *cpu) "cpu_by_vcpu_id[%d]=%p"
> -xen_map_ioreq_server_shared_page(long unsigned int ioreq_pfn) "shared page at pfn 0x%lx"
> -xen_map_ioreq_server_buffered_io_page(long unsigned int ioreq_pfn) "buffered io page at pfn 0x%lx"
> +xen_map_ioreq_server_shared_page(unsigned long int ioreq_pfn) "shared page at pfn 0x%lx"
> +xen_map_ioreq_server_buffered_io_page(unsigned long int ioreq_pfn) "buffered io page at pfn 0x%lx"
>  xen_map_ioreq_server_buffered_io_evtchn(int bufioreq_evtchn) "buffered io evtchn is 0x%x"
>  destroy_hvm_domain_cannot_acquire_handle(void) "Cannot acquire xenctrl handle"
>  destroy_hvm_domain_failed_action(const char *action, int sts, char *errno_s) "xc_domain_shutdown failed to issue %s, sts %d, %s"
> --
> 2.50.1
>
>

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Re: [PATCH 06/14] tracetool: Add Rust format support

[Manos Pitsidianakis]

On Fri, Aug 22, 2025 at 3:31 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Tanish Desai <tanishdesai37@gmail.com>
>
> Generating .rs files makes it possible to support tracing in rust.
> This support comprises a new format, and common code that converts
> the C expressions in trace-events to Rust.  In particular, types
> need to be converted, and PRI macros expanded.  Fortunately, all
> common platforms have a known mapping of 8/16/32/64-bit integers
> to char/short/int/"long long": even if int64_t is equal to long,
> it is fine to change the format string from PRIx64's expansion
> "%lx" to "%llx".  This makes it possible to have a static mapping
> from PRI macros to their expansion.
>
> As of this commit no backend generates Rust code, but it is already
> possible to use tracetool to generate Rust sources; they are not
> functional but they compile and contain tracepoint functions.
>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> [Move Rust argument conversion from Event to Arguments; string
>  support. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/tracetool/__init__.py  | 156 +++++++++++++++++++++++++++++++++
>  scripts/tracetool/format/rs.py |  76 ++++++++++++++++
>  2 files changed, 232 insertions(+)
>  create mode 100644 scripts/tracetool/format/rs.py
>
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 1d5238a0843..0b8ec707332 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -31,6 +31,49 @@ def error(*lines):
>      error_write(*lines)
>      sys.exit(1)
>
> +FMT_TOKEN = re.compile(r'''(?:
> +                       " ( (?: [^"\\] | \\[\\"abfnrt] |            # a string literal
> +                               \\x[0-9a-fA-F][0-9a-fA-F]) *? ) "
> +                       | ( PRI [duixX] (?:8|16|32|64|PTR|MAX) )    # a PRIxxx macro
> +                       | \s+                                       # spaces (ignored)
> +                       )''', re.X)
> +
> +PRI_SIZE_MAP = {
> +    '8':  'hh',
> +    '16': 'h',
> +    '32': '',
> +    '64': 'll',
> +    'PTR': 't',
> +    'MAX': 'j',
> +}
> +
> +def expand_format_string(c_fmt, prefix=""):

(Pedantic comment) let's put type annotations in function signatures
when possible, these seem to handle `str` mostly so this should be
simple. This should catch basic errors like passing/returning `str |
None` or wrong types altogether.

> +    def pri_macro_to_fmt(pri_macro):
> +        assert pri_macro.startswith("PRI")
> +        fmt_type = pri_macro[3]  # 'd', 'i', 'u', or 'x'
> +        fmt_size = pri_macro[4:]  # '8', '16', '32', '64', 'PTR', 'MAX'
> +
> +        size = PRI_SIZE_MAP.get(fmt_size, None)
> +        if size is None:
> +            raise Exception(f"unknown macro {pri_macro}")
> +        return size + fmt_type
> +
> +    result = prefix
> +    pos = 0
> +    while pos < len(c_fmt):
> +        m = FMT_TOKEN.match(c_fmt, pos)
> +        if not m:
> +            print("No match at position", pos, ":", repr(c_fmt[pos:]), file=sys.stderr)
> +            raise Exception("syntax error in trace file")
> +        if m[1]:
> +            substr = m[1]
> +        elif m[2]:
> +            substr = pri_macro_to_fmt(m[2])
> +        else:
> +            substr = ""
> +        result += substr
> +        pos = m.end()
> +    return result
>
>  out_lineno = 1
>  out_filename = '<none>'
> @@ -90,6 +133,48 @@ def out(*lines, **kwargs):
>      "ptrdiff_t",
>  ]
>
> +C_TYPE_KEYWORDS = {"int", "short", "long", "unsigned", "char"}
> +
> +C_TO_RUST_TYPE_MAP = {
> +    "int": "std::ffi::c_int",
> +    "long": "std::ffi::c_long",
> +    "long long": "std::ffi::c_longlong",
> +    "short": "std::ffi::c_short",
> +    "char": "std::ffi::c_char",
> +    "bool": "bool",
> +    "unsigned": "std::ffi::c_uint",
> +    "unsigned long": "std::ffi::c_long",
> +    "unsigned long long": "std::ffi::c_ulonglong",
> +    "unsigned short": "std::ffi::c_ushort",
> +    "unsigned char": "u8",
> +    "int8_t": "i8",
> +    "uint8_t": "u8",
> +    "int16_t": "i16",
> +    "uint16_t": "u16",
> +    "int32_t": "i32",
> +    "uint32_t": "u32",
> +    "int64_t": "i64",
> +    "uint64_t": "u64",
> +    "void": "()",
> +    "size_t": "usize",
> +    "ssize_t": "isize",
> +    "uintptr_t": "usize",
> +    "ptrdiff_t": "isize",
> +}
> +
> +# Rust requires manual casting of <32-bit types when passing them to
> +# variable-argument functions.
> +RUST_VARARGS_SMALL_TYPES = {
> +    "std::ffi::c_short",
> +    "std::ffi::c_ushort",
> +    "std::ffi::c_char",
> +    "i8",
> +    "u8",
> +    "i16",
> +    "u16",
> +    "bool",
> +}
> +
>  def validate_type(name):
>      bits = name.split(" ")
>      for bit in bits:
> @@ -105,6 +190,40 @@ def validate_type(name):
>                               "other complex pointer types should be "
>                               "declared as 'void *'" % name)
>
> +def c_type_to_rust(name):
> +    ptr = False
> +    const = False
> +    name = name.rstrip()
> +    if name[-1] == '*':
> +        name = name[:-1].rstrip()
> +        ptr = True
> +        if name[-1] == '*':
> +            # pointers to pointers are the same as void*
> +            name = "void"
> +
> +    bits = iter(name.split())
> +    bit = next(bits)
> +    if bit == "const":
> +        const = True
> +        bit = next(bits)
> +
> +    if bit in C_TYPE_KEYWORDS:
> +        if bit == 'signed':
> +            bit = ''
> +        rest = list(bits)
> +        if rest and rest[-1] == 'int':
> +            rest = rest[:-1]
> +        name = bit + ' ' + ' '.join(rest)
> +    else:
> +        if list(bits):
> +            raise ValueError("Invalid type '%s'." % name)
> +        name = bit
> +
> +    ty = C_TO_RUST_TYPE_MAP[name.strip()]
> +    if ptr:
> +        ty = f'*{"const" if const else "mut"} {ty}'
> +    return ty
> +
>  class Arguments:
>      """Event arguments description."""
>
> @@ -197,6 +316,43 @@ def casted(self):
>          """List of argument names casted to their type."""
>          return ["(%s)%s" % (type_, name) for type_, name in self._args]
>
> +    def rust_decl_extern(self):
> +        """Return a Rust argument list for an extern "C" function"""
> +        return ", ".join((f"_{name}: {c_type_to_rust(type_)}"
> +                          for type_, name in self._args))
> +
> +    def rust_decl(self):
> +        """Return a Rust argument list for a tracepoint function"""
> +        def decl_type(type_):
> +            if type_ == "const char *":
> +                return "&std::ffi::CStr"
> +            return c_type_to_rust(type_)
> +
> +        return ", ".join((f"_{name}: {decl_type(type_)}"
> +                          for type_, name in self._args))
> +
> +    def rust_call_extern(self):
> +        """Return a Rust argument list for a call to an extern "C" function"""
> +        def rust_cast(name, type_):
> +            if type_ == "const char *":
> +                return f"_{name}.as_ptr()"
> +            return "_{name}"
> +
> +        return ", ".join((rust_cast(name, type_) for type_, name in self._args))
> +
> +    def rust_call_varargs(self):
> +        """Return a Rust argument list for a call to a C varargs function"""
> +        def rust_cast(name, type_):
> +            if type_ == "const char *":
> +                return f"_{name}.as_ptr()"
> +
> +            type_ = c_type_to_rust(type_)
> +            if type_ in RUST_VARARGS_SMALL_TYPES:
> +                return f"_{name} as std::ffi::c_int"
> +            return f"_{name} /* as {type_} */"
> +
> +        return ", ".join((rust_cast(name, type_) for type_, name in self._args))
> +
>
>  class Event(object):
>      """Event description.
> diff --git a/scripts/tracetool/format/rs.py b/scripts/tracetool/format/rs.py
> new file mode 100644
> index 00000000000..bc8b2be5971
> --- /dev/null
> +++ b/scripts/tracetool/format/rs.py
> @@ -0,0 +1,76 @@
> +# -*- coding: utf-8 -*-

This was probably copied verbatim from other tracetool files, but IIUC
it's not needed, utf-8 is the default encoding if there's no `coding`
declaration.

Also, missing SPDX header as a new file

> +
> +"""
> +trace-DIR.rs

(what's "trace-DIR"?)

> +"""
> +
> +__author__     = "Tanish Desai <tanishdesai37@gmail.com>"
> +__copyright__  = "Copyright 2025, Tanish Desai <tanishdesai37@gmail.com>"
> +__license__    = "GPL version 2 or (at your option) any later version"
> +
> +__maintainer__ = "Stefan Hajnoczi"
> +__email__      = "stefanha@redhat.com"
> +
> +
> +from tracetool import out
> +
> +
> +def generate(events, backend, group):
> +    out('// This file is autogenerated by tracetool, do not edit.',

Let's use `@generated comments` https://generated.at/

> +        '',
> +        '#[allow(unused_imports)]',
> +        'use std::ffi::c_char;',
> +        '#[allow(unused_imports)]',
> +        'use qemu_api::bindings;',
> +        '',
> +        '#[inline(always)]',
> +        'fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {',
> +        '    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }',
> +        '}',
> +        '',
> +        'extern "C" {',
> +        '    static mut trace_events_enabled_count: u32;',
> +        '}',)
> +
> +    out('extern "C" {')
> +
> +    for e in events:
> +        out('    static mut %s: u16;' % e.api(e.QEMU_DSTATE))
> +    out('}')
> +
> +    # static state
> +    for e in events:
> +        if 'disable' in e.properties:
> +            enabled = "false"
> +        else:
> +            enabled = "true"
> +        if "tcg-exec" in e.properties:
> +            # a single define for the two "sub-events"
> +            out('const _TRACE_%(name)s_ENABLED: bool = %(enabled)s;',
> +                name=e.original.name.upper(),

What's the difference between e.original.name and e.name?

> +                               enabled=enabled)
> +        out('const _TRACE_%s_ENABLED: bool = %s;' % (e.name.upper(), enabled))
> +
> +    backend.generate_begin(events, group)
> +
> +    for e in events:
> +        out('',
> +                       '#[inline(always)]',
> +            '#[allow(dead_code)]',
> +            'pub fn %(api)s(%(args)s)',
> +            '{',
> +            api=e.api(e.QEMU_TRACE),
> +            args=e.args.rust_decl())
> +
> +        if "disable" not in e.properties:
> +            backend.generate(e, group, check_trace_event_get_state=False)
> +            if backend.check_trace_event_get_state:
> +                event_id = 'TRACE_' + e.name.upper()
> +                out('    if trace_event_get_state_dynamic_by_id(unsafe { _%(event_id)s_DSTATE}) {',
> +                    event_id = event_id,
> +                    api=e.api())
> +                backend.generate(e, group, check_trace_event_get_state=True)
> +                out('    }')
> +        out('}')
> +
> +    backend.generate_end(events, group)
> --
> 2.50.1
>
>

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Re: [PATCH 02/14] rust: move dependencies to rust/Cargo.toml

[Manos Pitsidianakis]

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

On Fri, Aug 22, 2025 at 3:34 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> As more crates start using the same dependencies, it's better to not
> repeat the versions and move the dependency declarations to the workspace.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/Cargo.toml          | 5 +++++
>  rust/qemu-api/Cargo.toml | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> index 682184cb158..99c275f2d9f 100644
> --- a/rust/Cargo.toml
> +++ b/rust/Cargo.toml
> @@ -15,6 +15,11 @@ license = "GPL-2.0-or-later"
>  repository = "https://gitlab.com/qemu-project/qemu/"
>  rust-version = "1.83.0"
>
> +[workspace.dependencies]
> +anyhow = "~1.0"
> +foreign = "~0.3.1"
> +libc = "0.2.162"
> +
>  [workspace.lints.rust]
>  unexpected_cfgs = { level = "deny", check-cfg = [
>      'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
> diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
> index db7000dee44..c07a17a28b0 100644
> --- a/rust/qemu-api/Cargo.toml
> +++ b/rust/qemu-api/Cargo.toml
> @@ -15,9 +15,9 @@ rust-version.workspace = true
>
>  [dependencies]
>  qemu_api_macros = { path = "../qemu-api-macros" }
> -anyhow = "~1.0"
> -libc = "0.2.162"
> -foreign = "~0.3.1"
> +anyhow = { workspace = true }
> +foreign = { workspace = true }
> +libc = { workspace = true }
>
>  [features]
>  default = ["debug_cell"]
> --
> 2.50.1
>
>

Re: [PATCH 11/14] log: change qemu_loglevel to unsigned

[Manos Pitsidianakis]

On Fri, Aug 22, 2025 at 3:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Bindgen makes the LOG_* constants unsigned, even if they are defined as
> (1 << 15):
>
>    pub const LOG_TRACE: u32 = 32768;
>
> Make them unsigned in C as well, and also change the type of the variable
> that they are used with.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  include/qemu/log-for-trace.h |  4 ++--
>  include/qemu/log.h           | 44 ++++++++++++++++++------------------
>  util/log.c                   |  2 +-
>  rust/qemu-api/src/log.rs     |  2 +-
>  4 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/include/qemu/log-for-trace.h b/include/qemu/log-for-trace.h
> index d47c9cd4462..f3a8791f1d4 100644
> --- a/include/qemu/log-for-trace.h
> +++ b/include/qemu/log-for-trace.h
> @@ -19,9 +19,9 @@
>  #define QEMU_LOG_FOR_TRACE_H
>
>  /* Private global variable, don't use */
> -extern int qemu_loglevel;
> +extern unsigned qemu_loglevel;
>
> -#define LOG_TRACE          (1 << 15)
> +#define LOG_TRACE          (1u << 15)
>
>  /* Returns true if a bit is set in the current loglevel mask */
>  static inline bool qemu_loglevel_mask(int mask)
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index aae72985f0d..7effba4da4c 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -14,30 +14,30 @@ bool qemu_log_enabled(void);
>  /* Returns true if qemu_log() will write somewhere other than stderr. */
>  bool qemu_log_separate(void);
>
> -#define CPU_LOG_TB_OUT_ASM (1 << 0)
> -#define CPU_LOG_TB_IN_ASM  (1 << 1)
> -#define CPU_LOG_TB_OP      (1 << 2)
> -#define CPU_LOG_TB_OP_OPT  (1 << 3)
> -#define CPU_LOG_INT        (1 << 4)
> -#define CPU_LOG_EXEC       (1 << 5)
> -#define CPU_LOG_PCALL      (1 << 6)
> -#define CPU_LOG_TB_CPU     (1 << 8)
> -#define CPU_LOG_RESET      (1 << 9)
> -#define LOG_UNIMP          (1 << 10)
> -#define LOG_GUEST_ERROR    (1 << 11)
> -#define CPU_LOG_MMU        (1 << 12)
> -#define CPU_LOG_TB_NOCHAIN (1 << 13)
> -#define CPU_LOG_PAGE       (1 << 14)
> +#define CPU_LOG_TB_OUT_ASM (1u << 0)
> +#define CPU_LOG_TB_IN_ASM  (1u << 1)
> +#define CPU_LOG_TB_OP      (1u << 2)
> +#define CPU_LOG_TB_OP_OPT  (1u << 3)
> +#define CPU_LOG_INT        (1u << 4)
> +#define CPU_LOG_EXEC       (1u << 5)
> +#define CPU_LOG_PCALL      (1u << 6)
> +#define CPU_LOG_TB_CPU     (1u << 8)
> +#define CPU_LOG_RESET      (1u << 9)
> +#define LOG_UNIMP          (1u << 10)
> +#define LOG_GUEST_ERROR    (1u << 11)
> +#define CPU_LOG_MMU        (1u << 12)
> +#define CPU_LOG_TB_NOCHAIN (1u << 13)
> +#define CPU_LOG_PAGE       (1u << 14)
>  /* LOG_TRACE (1 << 15) is defined in log-for-trace.h */
> -#define CPU_LOG_TB_OP_IND  (1 << 16)
> -#define CPU_LOG_TB_FPU     (1 << 17)
> -#define CPU_LOG_PLUGIN     (1 << 18)
> +#define CPU_LOG_TB_OP_IND  (1u << 16)
> +#define CPU_LOG_TB_FPU     (1u << 17)
> +#define CPU_LOG_PLUGIN     (1u << 18)
>  /* LOG_STRACE is used for user-mode strace logging. */
> -#define LOG_STRACE         (1 << 19)
> -#define LOG_PER_THREAD     (1 << 20)
> -#define CPU_LOG_TB_VPU     (1 << 21)
> -#define LOG_TB_OP_PLUGIN   (1 << 22)
> -#define LOG_INVALID_MEM    (1 << 23)
> +#define LOG_STRACE         (1u << 19)
> +#define LOG_PER_THREAD     (1u << 20)
> +#define CPU_LOG_TB_VPU     (1u << 21)
> +#define LOG_TB_OP_PLUGIN   (1u << 22)
> +#define LOG_INVALID_MEM    (1u << 23)
>
>  /* Lock/unlock output. */
>
> diff --git a/util/log.c b/util/log.c
> index abdcb6b3111..41f78ce86b2 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -44,7 +44,7 @@ static FILE *global_file;
>  static __thread FILE *thread_file;
>  static __thread Notifier qemu_log_thread_cleanup_notifier;
>
> -int qemu_loglevel;
> +unsigned qemu_loglevel;
>  static bool log_per_thread;
>  static GArray *debug_regions;
>
> diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs
> index a441b8c1f2e..fe43e30104c 100644
> --- a/rust/qemu-api/src/log.rs
> +++ b/rust/qemu-api/src/log.rs
> @@ -140,7 +140,7 @@ macro_rules! log_mask_ln {
>          let _: Log = $mask;
>
>          if unsafe {
> -            (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
> +            (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_uint)) != 0
>          } {
>              _ = ::qemu_api::log::LogGuard::log_fmt(
>                  format_args!("{}\n", format_args!($fmt $($args)*)));
> --
> 2.50.1
>
>

Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Manos Pitsidianakis]

On Fri, Aug 22, 2025 at 3:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> This simplifies the Python code and reduces the size of the tracepoints.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Nice.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  tests/tracetool/ftrace.h            | 28 ++++++----------------------
>  trace/ftrace.h                      |  1 +
>  trace/ftrace.c                      | 15 +++++++++++++++
>  scripts/tracetool/backend/ftrace.py | 12 ++----------
>  4 files changed, 24 insertions(+), 32 deletions(-)
>
> diff --git a/tests/tracetool/ftrace.h b/tests/tracetool/ftrace.h
> index fe22ea0f09f..1dfe4239413 100644
> --- a/tests/tracetool/ftrace.h
> +++ b/tests/tracetool/ftrace.h
> @@ -21,18 +21,10 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
>
>  static inline void trace_test_blah(void *context, const char *filename)
>  {
> -    {
> -        char ftrace_buf[MAX_TRACE_STRLEN];
> -        int unused __attribute__ ((unused));
> -        int trlen;
> -        if (trace_event_get_state(TRACE_TEST_BLAH)) {
> +    if (trace_event_get_state(TRACE_TEST_BLAH)) {
>  #line 4 "trace-events"
> -            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> -                             "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> -#line 33 "ftrace.h"
> -            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> -            unused = write(trace_marker_fd, ftrace_buf, trlen);
> -        }
> +        ftrace_write("test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> +#line 28 "ftrace.h"
>      }
>  }
>
> @@ -42,18 +34,10 @@ static inline void trace_test_blah(void *context, const char *filename)
>
>  static inline void trace_test_wibble(void *context, int value)
>  {
> -    {
> -        char ftrace_buf[MAX_TRACE_STRLEN];
> -        int unused __attribute__ ((unused));
> -        int trlen;
> -        if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
> +    if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
>  #line 5 "trace-events"
> -            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> -                             "test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> -#line 54 "ftrace.h"
> -            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> -            unused = write(trace_marker_fd, ftrace_buf, trlen);
> -        }
> +        ftrace_write("test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> +#line 41 "ftrace.h"
>      }
>  }
>  #endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */
> diff --git a/trace/ftrace.h b/trace/ftrace.h
> index cb5e35d2171..16c122816d1 100644
> --- a/trace/ftrace.h
> +++ b/trace/ftrace.h
> @@ -8,5 +8,6 @@
>  extern int trace_marker_fd;
>
>  bool ftrace_init(void);
> +G_GNUC_PRINTF(1, 2) void ftrace_write(const char *fmt, ...);
>
>  #endif /* TRACE_FTRACE_H */
> diff --git a/trace/ftrace.c b/trace/ftrace.c
> index 9749543d9b2..6875faedb9c 100644
> --- a/trace/ftrace.c
> +++ b/trace/ftrace.c
> @@ -38,6 +38,21 @@ static int find_mount(char *mount_point, const char *fstype)
>      return ret;
>  }
>
> +void ftrace_write(const char *fmt, ...)
> +{
> +    char ftrace_buf[MAX_TRACE_STRLEN];
> +    int unused __attribute__ ((unused));
> +    int trlen;
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> +    va_end(ap);
> +
> +    trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> +    unused = write(trace_marker_fd, ftrace_buf, trlen);
> +}
> +
>  bool ftrace_init(void)
>  {
>      char mount_point[PATH_MAX];
> diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
> index 5fa30ccc08e..a07f8a9dfd8 100644
> --- a/scripts/tracetool/backend/ftrace.py
> +++ b/scripts/tracetool/backend/ftrace.py
> @@ -28,18 +28,10 @@ def generate_h(event, group):
>      if len(event.args) > 0:
>          argnames = ", " + argnames
>
> -    out('    {',
> -        '        char ftrace_buf[MAX_TRACE_STRLEN];',
> -        '        int unused __attribute__ ((unused));',
> -        '        int trlen;',
> -        '        if (trace_event_get_state(%(event_id)s)) {',
> +    out('    if (trace_event_get_state(%(event_id)s)) {',
>          '#line %(event_lineno)d "%(event_filename)s"',
> -        '            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
> -        '                             "%(name)s " %(fmt)s "\\n" %(argnames)s);',
> +        '        ftrace_write("%(name)s " %(fmt)s "\\n" %(argnames)s);',
>          '#line %(out_next_lineno)d "%(out_filename)s"',
> -        '            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
> -        '            unused = write(trace_marker_fd, ftrace_buf, trlen);',
> -        '        }',
>          '    }',
>          name=event.name,
>          args=event.args,
> --
> 2.50.1
>
>

Re: [PATCH 07/14] rust: add trace crate

[Manos Pitsidianakis]

On Fri, Aug 22, 2025 at 3:30 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Tanish Desai <tanishdesai37@gmail.com>
>
> The trace crate is a minimal container for dependencies of tracepoints
> (so that they do not have to be imported in all the crates that use
> tracepoints); it also contains a macro called "include_trace!" that is
> able to find the right include file from the trace/ directory.
>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> [Write commit message. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/Cargo.lock        |  4 ++++
>  rust/Cargo.toml        |  1 +
>  rust/meson.build       |  2 +-
>  rust/trace/Cargo.toml  | 16 ++++++++++++++++
>  rust/trace/meson.build | 19 +++++++++++++++++++
>  rust/trace/src/lib.rs  | 23 +++++++++++++++++++++++
>  trace/meson.build      |  8 +++++++-
>  7 files changed, 71 insertions(+), 2 deletions(-)
>  create mode 100644 rust/trace/Cargo.toml
>  create mode 100644 rust/trace/meson.build
>  create mode 100644 rust/trace/src/lib.rs
>
> diff --git a/rust/Cargo.lock b/rust/Cargo.lock
> index b785c718f31..aa13ab2a99a 100644
> --- a/rust/Cargo.lock
> +++ b/rust/Cargo.lock
> @@ -164,6 +164,10 @@ dependencies = [
>   "unicode-ident",
>  ]
>
> +[[package]]
> +name = "trace"
> +version = "0.1.0"
> +
>  [[package]]
>  name = "unicode-ident"
>  version = "1.0.12"
> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> index 99c275f2d9f..2be9f886113 100644
> --- a/rust/Cargo.toml
> +++ b/rust/Cargo.toml
> @@ -4,6 +4,7 @@ members = [
>      "bits",
>      "qemu-api-macros",
>      "qemu-api",
> +    "trace",
>      "hw/char/pl011",
>      "hw/timer/hpet",
>  ]
> diff --git a/rust/meson.build b/rust/meson.build
> index 45936a0a731..2878bd8bc8d 100644
> --- a/rust/meson.build
> +++ b/rust/meson.build
> @@ -23,7 +23,7 @@ genrs = []
>  subdir('qemu-api-macros')
>  subdir('bits')
>  subdir('qemu-api')
> -
> +subdir('trace')
>  subdir('hw')
>
>  cargo = find_program('cargo', required: false)
> diff --git a/rust/trace/Cargo.toml b/rust/trace/Cargo.toml
> new file mode 100644
> index 00000000000..913010e9787
> --- /dev/null
> +++ b/rust/trace/Cargo.toml
> @@ -0,0 +1,16 @@
> +[package]
> +name = "trace"
> +version = "0.1.0"
> +authors = ["Tanish Desai<tanishdesai37@gmail.com>"]

Missing space between name and angle bracket

> +description = "rust trace library"

Suggestion:

> +description = "QEMU tracing infrastructure support"

> +resolver = "2"
> +publish = false
> +
> +edition.workspace = true
> +homepage.workspace = true
> +license.workspace = true
> +repository.workspace = true
> +rust-version.workspace = true
> +
> +[lints]
> +workspace = true
> diff --git a/rust/trace/meson.build b/rust/trace/meson.build
> new file mode 100644
> index 00000000000..adca57e5507
> --- /dev/null
> +++ b/rust/trace/meson.build
> @@ -0,0 +1,19 @@
> +rust = import('rust')
> +
> +lib_rs = configure_file(
> +  input: 'src/lib.rs',
> +  output: 'lib.rs',
> +  configuration: {
> +    'MESON_BUILD_ROOT': meson.project_build_root(),
> +  })
> +
> +_trace_rs = static_library(
> +  'trace',             # Library name,
> +  lib_rs,
> +  trace_rs_targets,         # List of generated `.rs` custom targets
> +  override_options: ['rust_std=2021', 'build.rust_std=2021'],
> +  dependencies: [libc_rs],
> +  rust_abi: 'rust',
> +)
> +
> +trace_rs = declare_dependency(link_with: _trace_rs)
> diff --git a/rust/trace/src/lib.rs b/rust/trace/src/lib.rs
> new file mode 100644
> index 00000000000..9b931ddf1de
> --- /dev/null
> +++ b/rust/trace/src/lib.rs
> @@ -0,0 +1,23 @@
> +//! This crate provides macros that aid in using QEMU's tracepoint
> +//! functionality.

Missing SPDX headers

> +
> +#[macro_export]
> +/// Define the trace-points from the named directory (which should have slashes

Pedantic: s/trace-points/tracepoints

> +/// replaced by underscore characters) as functions in a module called `trace`.
> +///
> +/// ```ignore
> +/// ::trace::include_trace!("hw_char");
> +/// // ...
> +/// trace::trace_pl011_read_fifo_rx_full();
> +/// ```
> +macro_rules! include_trace {
> +    ($name:literal) => {
> +        mod trace {
> +            #[cfg(not(MESON))]
> +            include!(concat!(env!("MESON_BUILD_ROOT"), "/trace/", $name, ".rs"));
> +
> +            #[cfg(MESON)]
> +            include!(concat!("@MESON_BUILD_ROOT@/trace/", $name, ".rs"));
> +        }
> +    };
> +}
> diff --git a/trace/meson.build b/trace/meson.build
> index 9c42a57a053..d89a0db82a1 100644
> --- a/trace/meson.build
> +++ b/trace/meson.build
> @@ -1,5 +1,5 @@
>  system_ss.add(files('control-target.c', 'trace-hmp-cmds.c'))
> -
> +trace_rs_targets = []
>  trace_events_files = []
>  foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
>    if item in qapi_trace_events
> @@ -24,6 +24,11 @@ foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
>                            input: trace_events_file,
>                            command: [ tracetool, group, '--format=c', '@INPUT@', '@OUTPUT@' ],
>                            depend_files: tracetool_depends)
> +  trace_rs = custom_target(fmt.format('trace', 'rs'),
> +                          output: fmt.format('trace', 'rs'),
> +                          input: trace_events_file,
> +                          command: [ tracetool, group, '--format=rs', '@INPUT@', '@OUTPUT@' ],
> +                          depend_files: tracetool_depends)
>    if 'ust' in get_option('trace_backends')
>      trace_ust_h = custom_target(fmt.format('trace-ust', 'h'),
>                                  output: fmt.format('trace-ust', 'h'),
> @@ -34,6 +39,7 @@ foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events
>      genh += trace_ust_h
>    endif
>    trace_ss.add(trace_h, trace_c)
> +  trace_rs_targets += trace_rs
>    if 'dtrace' in get_option('trace_backends')
>      trace_dtrace = custom_target(fmt.format('trace-dtrace', 'dtrace'),
>                                   output: fmt.format('trace-dtrace', 'dtrace'),
> --
> 2.50.1
>
>

Re: [PATCH 06/14] tracetool: Add Rust format support

[Manos Pitsidianakis]

On Fri, Aug 22, 2025 at 3:31 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Tanish Desai <tanishdesai37@gmail.com>
>
> Generating .rs files makes it possible to support tracing in rust.
> This support comprises a new format, and common code that converts
> the C expressions in trace-events to Rust.  In particular, types
> need to be converted, and PRI macros expanded.  Fortunately, all
> common platforms have a known mapping of 8/16/32/64-bit integers
> to char/short/int/"long long": even if int64_t is equal to long,
> it is fine to change the format string from PRIx64's expansion
> "%lx" to "%llx".  This makes it possible to have a static mapping
> from PRI macros to their expansion.
>
> As of this commit no backend generates Rust code, but it is already
> possible to use tracetool to generate Rust sources; they are not
> functional but they compile and contain tracepoint functions.
>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> [Move Rust argument conversion from Event to Arguments; string
>  support. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/tracetool/__init__.py  | 156 +++++++++++++++++++++++++++++++++
>  scripts/tracetool/format/rs.py |  76 ++++++++++++++++
>  2 files changed, 232 insertions(+)
>  create mode 100644 scripts/tracetool/format/rs.py
>
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 1d5238a0843..0b8ec707332 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -31,6 +31,49 @@ def error(*lines):
>      error_write(*lines)
>      sys.exit(1)
>
> +FMT_TOKEN = re.compile(r'''(?:
> +                       " ( (?: [^"\\] | \\[\\"abfnrt] |            # a string literal
> +                               \\x[0-9a-fA-F][0-9a-fA-F]) *? ) "
> +                       | ( PRI [duixX] (?:8|16|32|64|PTR|MAX) )    # a PRIxxx macro
> +                       | \s+                                       # spaces (ignored)
> +                       )''', re.X)
> +
> +PRI_SIZE_MAP = {
> +    '8':  'hh',
> +    '16': 'h',
> +    '32': '',
> +    '64': 'll',
> +    'PTR': 't',
> +    'MAX': 'j',
> +}
> +
> +def expand_format_string(c_fmt, prefix=""):
> +    def pri_macro_to_fmt(pri_macro):
> +        assert pri_macro.startswith("PRI")
> +        fmt_type = pri_macro[3]  # 'd', 'i', 'u', or 'x'
> +        fmt_size = pri_macro[4:]  # '8', '16', '32', '64', 'PTR', 'MAX'
> +
> +        size = PRI_SIZE_MAP.get(fmt_size, None)
> +        if size is None:
> +            raise Exception(f"unknown macro {pri_macro}")
> +        return size + fmt_type
> +
> +    result = prefix
> +    pos = 0
> +    while pos < len(c_fmt):
> +        m = FMT_TOKEN.match(c_fmt, pos)
> +        if not m:
> +            print("No match at position", pos, ":", repr(c_fmt[pos:]), file=sys.stderr)
> +            raise Exception("syntax error in trace file")
> +        if m[1]:
> +            substr = m[1]
> +        elif m[2]:
> +            substr = pri_macro_to_fmt(m[2])
> +        else:
> +            substr = ""
> +        result += substr
> +        pos = m.end()
> +    return result
>
>  out_lineno = 1
>  out_filename = '<none>'
> @@ -90,6 +133,48 @@ def out(*lines, **kwargs):
>      "ptrdiff_t",
>  ]
>
> +C_TYPE_KEYWORDS = {"int", "short", "long", "unsigned", "char"}
> +
> +C_TO_RUST_TYPE_MAP = {
> +    "int": "std::ffi::c_int",
> +    "long": "std::ffi::c_long",
> +    "long long": "std::ffi::c_longlong",
> +    "short": "std::ffi::c_short",
> +    "char": "std::ffi::c_char",
> +    "bool": "bool",
> +    "unsigned": "std::ffi::c_uint",
> +    "unsigned long": "std::ffi::c_long",
> +    "unsigned long long": "std::ffi::c_ulonglong",
> +    "unsigned short": "std::ffi::c_ushort",
> +    "unsigned char": "u8",
> +    "int8_t": "i8",
> +    "uint8_t": "u8",
> +    "int16_t": "i16",
> +    "uint16_t": "u16",
> +    "int32_t": "i32",
> +    "uint32_t": "u32",
> +    "int64_t": "i64",
> +    "uint64_t": "u64",
> +    "void": "()",
> +    "size_t": "usize",
> +    "ssize_t": "isize",
> +    "uintptr_t": "usize",
> +    "ptrdiff_t": "isize",
> +}
> +
> +# Rust requires manual casting of <32-bit types when passing them to
> +# variable-argument functions.
> +RUST_VARARGS_SMALL_TYPES = {
> +    "std::ffi::c_short",
> +    "std::ffi::c_ushort",
> +    "std::ffi::c_char",
> +    "i8",
> +    "u8",
> +    "i16",
> +    "u16",
> +    "bool",
> +}
> +
>  def validate_type(name):
>      bits = name.split(" ")
>      for bit in bits:
> @@ -105,6 +190,40 @@ def validate_type(name):
>                               "other complex pointer types should be "
>                               "declared as 'void *'" % name)
>
> +def c_type_to_rust(name):
> +    ptr = False
> +    const = False
> +    name = name.rstrip()
> +    if name[-1] == '*':
> +        name = name[:-1].rstrip()
> +        ptr = True
> +        if name[-1] == '*':
> +            # pointers to pointers are the same as void*
> +            name = "void"
> +
> +    bits = iter(name.split())
> +    bit = next(bits)
> +    if bit == "const":
> +        const = True
> +        bit = next(bits)
> +
> +    if bit in C_TYPE_KEYWORDS:
> +        if bit == 'signed':
> +            bit = ''
> +        rest = list(bits)
> +        if rest and rest[-1] == 'int':
> +            rest = rest[:-1]
> +        name = bit + ' ' + ' '.join(rest)
> +    else:
> +        if list(bits):
> +            raise ValueError("Invalid type '%s'." % name)
> +        name = bit
> +
> +    ty = C_TO_RUST_TYPE_MAP[name.strip()]
> +    if ptr:
> +        ty = f'*{"const" if const else "mut"} {ty}'
> +    return ty
> +
>  class Arguments:
>      """Event arguments description."""
>
> @@ -197,6 +316,43 @@ def casted(self):
>          """List of argument names casted to their type."""
>          return ["(%s)%s" % (type_, name) for type_, name in self._args]
>
> +    def rust_decl_extern(self):
> +        """Return a Rust argument list for an extern "C" function"""
> +        return ", ".join((f"_{name}: {c_type_to_rust(type_)}"
> +                          for type_, name in self._args))
> +
> +    def rust_decl(self):
> +        """Return a Rust argument list for a tracepoint function"""
> +        def decl_type(type_):
> +            if type_ == "const char *":
> +                return "&std::ffi::CStr"
> +            return c_type_to_rust(type_)
> +
> +        return ", ".join((f"_{name}: {decl_type(type_)}"
> +                          for type_, name in self._args))
> +
> +    def rust_call_extern(self):
> +        """Return a Rust argument list for a call to an extern "C" function"""
> +        def rust_cast(name, type_):
> +            if type_ == "const char *":
> +                return f"_{name}.as_ptr()"
> +            return "_{name}"

Missing f-prefix for f-string

> +
> +        return ", ".join((rust_cast(name, type_) for type_, name in self._args))
> +
> +    def rust_call_varargs(self):
> +        """Return a Rust argument list for a call to a C varargs function"""
> +        def rust_cast(name, type_):
> +            if type_ == "const char *":
> +                return f"_{name}.as_ptr()"
> +
> +            type_ = c_type_to_rust(type_)
> +            if type_ in RUST_VARARGS_SMALL_TYPES:
> +                return f"_{name} as std::ffi::c_int"
> +            return f"_{name} /* as {type_} */"
> +
> +        return ", ".join((rust_cast(name, type_) for type_, name in self._args))
> +
>
>  class Event(object):
>      """Event description.
> diff --git a/scripts/tracetool/format/rs.py b/scripts/tracetool/format/rs.py
> new file mode 100644
> index 00000000000..bc8b2be5971
> --- /dev/null
> +++ b/scripts/tracetool/format/rs.py
> @@ -0,0 +1,76 @@
> +# -*- coding: utf-8 -*-
> +
> +"""
> +trace-DIR.rs
> +"""
> +
> +__author__     = "Tanish Desai <tanishdesai37@gmail.com>"
> +__copyright__  = "Copyright 2025, Tanish Desai <tanishdesai37@gmail.com>"
> +__license__    = "GPL version 2 or (at your option) any later version"
> +
> +__maintainer__ = "Stefan Hajnoczi"
> +__email__      = "stefanha@redhat.com"
> +
> +
> +from tracetool import out
> +
> +
> +def generate(events, backend, group):
> +    out('// This file is autogenerated by tracetool, do not edit.',
> +        '',
> +        '#[allow(unused_imports)]',
> +        'use std::ffi::c_char;',
> +        '#[allow(unused_imports)]',
> +        'use qemu_api::bindings;',
> +        '',
> +        '#[inline(always)]',
> +        'fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {',
> +        '    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }',
> +        '}',
> +        '',
> +        'extern "C" {',
> +        '    static mut trace_events_enabled_count: u32;',
> +        '}',)
> +
> +    out('extern "C" {')
> +
> +    for e in events:
> +        out('    static mut %s: u16;' % e.api(e.QEMU_DSTATE))
> +    out('}')
> +
> +    # static state
> +    for e in events:
> +        if 'disable' in e.properties:
> +            enabled = "false"
> +        else:
> +            enabled = "true"
> +        if "tcg-exec" in e.properties:
> +            # a single define for the two "sub-events"
> +            out('const _TRACE_%(name)s_ENABLED: bool = %(enabled)s;',
> +                name=e.original.name.upper(),
> +                               enabled=enabled)
> +        out('const _TRACE_%s_ENABLED: bool = %s;' % (e.name.upper(), enabled))
> +
> +    backend.generate_begin(events, group)
> +
> +    for e in events:
> +        out('',
> +                       '#[inline(always)]',
> +            '#[allow(dead_code)]',
> +            'pub fn %(api)s(%(args)s)',
> +            '{',
> +            api=e.api(e.QEMU_TRACE),
> +            args=e.args.rust_decl())
> +
> +        if "disable" not in e.properties:
> +            backend.generate(e, group, check_trace_event_get_state=False)
> +            if backend.check_trace_event_get_state:
> +                event_id = 'TRACE_' + e.name.upper()
> +                out('    if trace_event_get_state_dynamic_by_id(unsafe { _%(event_id)s_DSTATE}) {',
> +                    event_id = event_id,
> +                    api=e.api())
> +                backend.generate(e, group, check_trace_event_get_state=True)
> +                out('    }')
> +        out('}')
> +
> +    backend.generate_end(events, group)
> --
> 2.50.1
>
>

Re: [PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Zhao Liu]

On Fri, Aug 22, 2025 at 02:26:42PM +0200, Paolo Bonzini wrote:
> Date: Fri, 22 Aug 2025 14:26:42 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 01/14] treewide: write "unsigned long int" instead of "long
>  unsigned int"
> X-Mailer: git-send-email 2.50.1
> 
> Putting "unsigned" in anything but the first position is weird.

I think one reason may be gcc uses something like ‘long unsigned int *‘
by default?

../hw/misc/imx7_src.c: In function ‘imx7_src_write’:
../hw/misc/imx7_src.c:218:42: error: passing argument 2 of ‘clear_bit’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  218 |             clear_bit(R_CORE1_RST_SHIFT, &change_mask);
      |                                          ^~~~~~~~~~~~
      |                                          |
      |                                          uint32_t * {aka unsigned int *}
In file included from /media/liuzhao/data/qemu-cook/include/qemu/bitmap.h:16,
                 from /media/liuzhao/data/qemu-cook/include/hw/qdev-core.h:6,
                 from /media/liuzhao/data/qemu-cook/include/hw/sysbus.h:6,
                 from /media/liuzhao/data/qemu-cook/include/hw/misc/imx7_src.h:13,
                 from ../hw/misc/imx7_src.c:12:
/qemu/include/qemu/bitops.h:93:54: note: expected ‘long unsigned int *’ but argument is of type ‘uint32_t *’ {aka ‘unsigned int *’}
   93 | static inline void clear_bit(long nr, unsigned long *addr)
      |                                       ~~~~~~~~~~~~~~~^~~~
cc1: all warnings being treated as errors

> As such,
> tracetool's Rust type conversion will not support it.  Remove it from
> the whole of QEMU's source code, not just trace-events.

But I also agree it's a good idea to clean this up.

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  crypto/pbkdf-gcrypt.c        | 2 +-
>  crypto/pbkdf-gnutls.c        | 2 +-
>  crypto/pbkdf-nettle.c        | 2 +-
>  hw/display/exynos4210_fimd.c | 2 +-
>  hw/misc/imx7_src.c           | 4 ++--
>  hw/net/can/can_sja1000.c     | 4 ++--
>  hw/xen/trace-events          | 4 ++--
>  7 files changed, 10 insertions(+), 10 deletions(-)

...

> diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
> index df0b0a69057..817c95bf65b 100644
> --- a/hw/misc/imx7_src.c
> +++ b/hw/misc/imx7_src.c
> @@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
>  {
>      IMX7SRCState *s = (IMX7SRCState *)opaque;
>      uint32_t index = offset >> 2;
> -    long unsigned int change_mask;
> +    uint32_t change_mask;

We needs "unsigned long", otherwise, there'll be the error as I listed
above.

>      uint32_t current_value = value;
>  
>      if (index >= SRC_MAX) {

...

> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 5b6ba9df6c4..545c520c3b4 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
>              break;
>          }
>      }
> -    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> -            (int)addr, size, (long unsigned int)temp);

tmep is "uint64_t", so there's no need to convert its type?

We can just drop `(long unsigned int)` directly.

> +    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02x\n",
> +            (int)addr, size, (unsigned)temp);
>      return temp;
>  }

Others look fine to me. With the nits fixed,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 02/14] rust: move dependencies to rust/Cargo.toml

[Zhao Liu]

On Fri, Aug 22, 2025 at 02:26:43PM +0200, Paolo Bonzini wrote:
> Date: Fri, 22 Aug 2025 14:26:43 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 02/14] rust: move dependencies to rust/Cargo.toml
> X-Mailer: git-send-email 2.50.1
> 
> As more crates start using the same dependencies, it's better to not
> repeat the versions and move the dependency declarations to the workspace.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/Cargo.toml          | 5 +++++
>  rust/qemu-api/Cargo.toml | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 08/14] rust: qdev: add minimal clock bindings

[Manos Pitsidianakis]

On Mon, Aug 25, 2025 at 10:30 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> On Fri, Aug 22, 2025 at 02:26:49PM +0200, Paolo Bonzini wrote:
> > Date: Fri, 22 Aug 2025 14:26:49 +0200
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH 08/14] rust: qdev: add minimal clock bindings
> > X-Mailer: git-send-email 2.50.1
> >
> > Add the minimal support that is needed by pl011's event and tracepoint.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  rust/qemu-api/src/qdev.rs | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
>
> ...
>
> > +    pub const fn get(&self) -> u64 {
>
> get() sounds too general for Clock obj...maybe get_period()?

(Or just clock.period())

>
> > +        // SAFETY: Clock is returned by init_clock_in with zero value for period
> > +        unsafe { &*self.0.as_ptr() }.period
> > +    }
>
> Otherwise,
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
>

Re: [PATCH 08/14] rust: qdev: add minimal clock bindings

[Zhao Liu]

On Fri, Aug 22, 2025 at 02:26:49PM +0200, Paolo Bonzini wrote:
> Date: Fri, 22 Aug 2025 14:26:49 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 08/14] rust: qdev: add minimal clock bindings
> X-Mailer: git-send-email 2.50.1
> 
> Add the minimal support that is needed by pl011's event and tracepoint.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/qdev.rs | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+) 

...

> +    pub const fn get(&self) -> u64 {

get() sounds too general for Clock obj...maybe get_period()?

> +        // SAFETY: Clock is returned by init_clock_in with zero value for period
> +        unsafe { &*self.0.as_ptr() }.period
> +    }

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 11/14] log: change qemu_loglevel to unsigned

[Zhao Liu]

On Fri, Aug 22, 2025 at 02:26:52PM +0200, Paolo Bonzini wrote:
> Date: Fri, 22 Aug 2025 14:26:52 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 11/14] log: change qemu_loglevel to unsigned
> X-Mailer: git-send-email 2.50.1
> 
> Bindgen makes the LOG_* constants unsigned, even if they are defined as
> (1 << 15):
> 
>    pub const LOG_TRACE: u32 = 32768;
> 
> Make them unsigned in C as well, and also change the type of the variable
> that they are used with.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/log-for-trace.h |  4 ++--
>  include/qemu/log.h           | 44 ++++++++++++++++++------------------
>  util/log.c                   |  2 +-
>  rust/qemu-api/src/log.rs     |  2 +-
>  4 files changed, 26 insertions(+), 26 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Zhao Liu]

On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> Date: Fri, 22 Aug 2025 14:26:44 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints
>  to ftrace.c
> X-Mailer: git-send-email 2.50.1
> 
> This simplifies the Python code and reduces the size of the tracepoints.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/tracetool/ftrace.h            | 28 ++++++----------------------
>  trace/ftrace.h                      |  1 +
>  trace/ftrace.c                      | 15 +++++++++++++++
>  scripts/tracetool/backend/ftrace.py | 12 ++----------
>  4 files changed, 24 insertions(+), 32 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Paolo Bonzini]

On Mon, Aug 25, 2025 at 8:40 AM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> Why can't tracetool support this? Can't we just add the permutations
> in the C_TO_RUST_TYPE_MAP dict in "[PATCH 06/14] tracetool: Add Rust
> format support"?
>
> +    "unsigned long long": "std::ffi::c_ulonglong",
> +    "long unsigned long": "std::ffi::c_ulonglong",
> +    "long long unsigned": "std::ffi::c_ulonglong",

Just laziness. I guess I can just sort the keywords.

Paolo

Re: [PATCH 06/14] tracetool: Add Rust format support

[Paolo Bonzini]

On 8/25/25 09:03, Manos Pitsidianakis wrote:
>> +def expand_format_string(c_fmt, prefix=""):
> 
> (Pedantic comment) let's put type annotations in function signatures
> when possible, these seem to handle `str` mostly so this should be
> simple. This should catch basic errors like passing/returning `str |
> None` or wrong types altogether.

Not pedantic, I have a full conversion of tracetool to add mypy 
annotations... but it's on top of these patches. :)

I can reorder them and post the conversion first, depending on what the 
maintainers prefer.

>> @@ -0,0 +1,76 @@
>> +# -*- coding: utf-8 -*-
> 
> This was probably copied verbatim from other tracetool files, but IIUC
> it's not needed, utf-8 is the default encoding if there's no `coding`
> declaration.
> 
> Also, missing SPDX header as a new file

Ok I can do that for all files, too.

>> +        if "tcg-exec" in e.properties:
>> +            # a single define for the two "sub-events"
>> +            out('const _TRACE_%(name)s_ENABLED: bool = %(enabled)s;',
>> +                name=e.original.name.upper(),
> 
> What's the difference between e.original.name and e.name?

Good point---anything to do with e.original is dead code.  I'll add a 
patch in front to drop it.

Paolo

Re: [PATCH 07/14] rust: add trace crate

[Zhao Liu]

> diff --git a/rust/trace/src/lib.rs b/rust/trace/src/lib.rs
> new file mode 100644
> index 00000000000..9b931ddf1de
> --- /dev/null
> +++ b/rust/trace/src/lib.rs
> @@ -0,0 +1,23 @@
> +//! This crate provides macros that aid in using QEMU's tracepoint
> +//! functionality.
> +
> +#[macro_export]
> +/// Define the trace-points from the named directory (which should have slashes
> +/// replaced by underscore characters) as functions in a module called `trace`.
> +///
> +/// ```ignore
> +/// ::trace::include_trace!("hw_char");
> +/// // ...
> +/// trace::trace_pl011_read_fifo_rx_full();
> +/// ```
> +macro_rules! include_trace {
> +    ($name:literal) => {
> +        mod trace {
> +            #[cfg(not(MESON))]
> +            include!(concat!(env!("MESON_BUILD_ROOT"), "/trace/", $name, ".rs"));

nit: missing the "trace-" prefix~

include!(concat!(env!("MESON_BUILD_ROOT"), "/trace/", "trace-", $name, ".rs"));

> +            #[cfg(MESON)]
> +            include!(concat!("@MESON_BUILD_ROOT@/trace/", $name, ".rs"));

ditto

> +        }
> +    };
> +}

this is the nice work!

(but it breaks the doing `cargo build` directly without meson virt env.
 For `cfd(not(MESON)) case`, could we support placing trace files under
 local folder? like

 include!(concat!(env!("CARGO_MANIFEST_DIR"), "src/", "trace-", $name, ".rs"));

 Or should we add a build.rs like qemu-api does?)

Thanks,
Zhao

Re: [PATCH 08/14] rust: qdev: add minimal clock bindings

[Paolo Bonzini]

On Mon, Aug 25, 2025 at 9:33 AM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> > get() sounds too general for Clock obj...maybe get_period()?
>
> (Or just clock.period())

Will do period() and hz().

Paolo

Re: [PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Daniel P. Berrangé]

On Mon, Aug 25, 2025 at 03:24:01PM +0800, Zhao Liu wrote:
> On Fri, Aug 22, 2025 at 02:26:42PM +0200, Paolo Bonzini wrote:
> > Date: Fri, 22 Aug 2025 14:26:42 +0200
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH 01/14] treewide: write "unsigned long int" instead of "long
> >  unsigned int"
> > X-Mailer: git-send-email 2.50.1
> > 
> > Putting "unsigned" in anything but the first position is weird.
> 
> I think one reason may be gcc uses something like ‘long unsigned int *‘
> by default?
> 
> ../hw/misc/imx7_src.c: In function ‘imx7_src_write’:
> ../hw/misc/imx7_src.c:218:42: error: passing argument 2 of ‘clear_bit’ from incompatible pointer type [-Werror=incompatible-pointer-types]
>   218 |             clear_bit(R_CORE1_RST_SHIFT, &change_mask);
>       |                                          ^~~~~~~~~~~~
>       |                                          |
>       |                                          uint32_t * {aka unsigned int *}
> In file included from /media/liuzhao/data/qemu-cook/include/qemu/bitmap.h:16,
>                  from /media/liuzhao/data/qemu-cook/include/hw/qdev-core.h:6,
>                  from /media/liuzhao/data/qemu-cook/include/hw/sysbus.h:6,
>                  from /media/liuzhao/data/qemu-cook/include/hw/misc/imx7_src.h:13,
>                  from ../hw/misc/imx7_src.c:12:
> /qemu/include/qemu/bitops.h:93:54: note: expected ‘long unsigned int *’ but argument is of type ‘uint32_t *’ {aka ‘unsigned int *’}
>    93 | static inline void clear_bit(long nr, unsigned long *addr)
>       |                                       ~~~~~~~~~~~~~~~^~~~
> cc1: all warnings being treated as errors
> 
> > As such,
> > tracetool's Rust type conversion will not support it.  Remove it from
> > the whole of QEMU's source code, not just trace-events.
> 
> But I also agree it's a good idea to clean this up.
> 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  crypto/pbkdf-gcrypt.c        | 2 +-
> >  crypto/pbkdf-gnutls.c        | 2 +-
> >  crypto/pbkdf-nettle.c        | 2 +-
> >  hw/display/exynos4210_fimd.c | 2 +-
> >  hw/misc/imx7_src.c           | 4 ++--
> >  hw/net/can/can_sja1000.c     | 4 ++--
> >  hw/xen/trace-events          | 4 ++--
> >  7 files changed, 10 insertions(+), 10 deletions(-)
> 
> ...
> 
> > diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c
> > index df0b0a69057..817c95bf65b 100644
> > --- a/hw/misc/imx7_src.c
> > +++ b/hw/misc/imx7_src.c
> > @@ -169,7 +169,7 @@ static void imx7_src_write(void *opaque, hwaddr offset, uint64_t value,
> >  {
> >      IMX7SRCState *s = (IMX7SRCState *)opaque;
> >      uint32_t index = offset >> 2;
> > -    long unsigned int change_mask;
> > +    uint32_t change_mask;
> 
> We needs "unsigned long", otherwise, there'll be the error as I listed
> above.
> 
> >      uint32_t current_value = value;
> >  
> >      if (index >= SRC_MAX) {
> 
> ...
> 
> > diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> > index 5b6ba9df6c4..545c520c3b4 100644
> > --- a/hw/net/can/can_sja1000.c
> > +++ b/hw/net/can/can_sja1000.c
> > @@ -750,8 +750,8 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
> >              break;
> >          }
> >      }
> > -    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> > -            (int)addr, size, (long unsigned int)temp);
> 
> tmep is "uint64_t", so there's no need to convert its type?

We can't assume 'uint64_t' is a match for '%lx' - the
format string can be changed to PRIx64 though which
would let us drop the cast.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 01/14] treewide: write "unsigned long int" instead of "long unsigned int"

[Peter Maydell]

On Tue, 26 Aug 2025 at 12:16, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Aug 25, 2025 at 03:24:01PM +0800, Zhao Liu wrote:
> > > -    DPRINTF("read addr 0x%02x, %d bytes, content 0x%02lx\n",
> > > -            (int)addr, size, (long unsigned int)temp);
> >
> > tmep is "uint64_t", so there's no need to convert its type?
>
> We can't assume 'uint64_t' is a match for '%lx' - the
> format string can be changed to PRIx64 though which
> would let us drop the cast.

Yes, we have quite a few uses of casts in printfs which
are really only there because the standard way to print
a uint64_t is so awkward and ugly...

-- PMM

Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Daniel P. Berrangé]

On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> This simplifies the Python code and reduces the size of the tracepoints.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/tracetool/ftrace.h            | 28 ++++++----------------------
>  trace/ftrace.h                      |  1 +
>  trace/ftrace.c                      | 15 +++++++++++++++
>  scripts/tracetool/backend/ftrace.py | 12 ++----------
>  4 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/tracetool/ftrace.h b/tests/tracetool/ftrace.h
> index fe22ea0f09f..1dfe4239413 100644
> --- a/tests/tracetool/ftrace.h
> +++ b/tests/tracetool/ftrace.h
> @@ -21,18 +21,10 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
>  
>  static inline void trace_test_blah(void *context, const char *filename)
>  {
> -    {
> -        char ftrace_buf[MAX_TRACE_STRLEN];
> -        int unused __attribute__ ((unused));
> -        int trlen;
> -        if (trace_event_get_state(TRACE_TEST_BLAH)) {
> +    if (trace_event_get_state(TRACE_TEST_BLAH)) {
>  #line 4 "trace-events"
> -            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> -                             "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> -#line 33 "ftrace.h"
> -            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> -            unused = write(trace_marker_fd, ftrace_buf, trlen);
> -        }
> +        ftrace_write("test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> +#line 28 "ftrace.h"
>      }
>  }
>  
> @@ -42,18 +34,10 @@ static inline void trace_test_blah(void *context, const char *filename)
>  
>  static inline void trace_test_wibble(void *context, int value)
>  {
> -    {
> -        char ftrace_buf[MAX_TRACE_STRLEN];
> -        int unused __attribute__ ((unused));
> -        int trlen;
> -        if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
> +    if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
>  #line 5 "trace-events"
> -            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> -                             "test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> -#line 54 "ftrace.h"
> -            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> -            unused = write(trace_marker_fd, ftrace_buf, trlen);
> -        }
> +        ftrace_write("test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> +#line 41 "ftrace.h"
>      }
>  }
>  #endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */

snip

> diff --git a/trace/ftrace.c b/trace/ftrace.c
> index 9749543d9b2..6875faedb9c 100644
> --- a/trace/ftrace.c
> +++ b/trace/ftrace.c
> @@ -38,6 +38,21 @@ static int find_mount(char *mount_point, const char *fstype)
>      return ret;
>  }
>  
> +void ftrace_write(const char *fmt, ...)
> +{
> +    char ftrace_buf[MAX_TRACE_STRLEN];
> +    int unused __attribute__ ((unused));
> +    int trlen;
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> +    va_end(ap);
> +
> +    trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> +    unused = write(trace_marker_fd, ftrace_buf, trlen);

You're just copying the existing code pattern which is fine for now so

   Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


More generally though, IMHO, QEMU would be better off bringing in
gnulib's 'ignore_value' macro, but simplified since we don't care
about ancient GCC

  #define ignore_value(x) \
      (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))

so that we don't need to play games with extra variables. eg

   ignore_value(write(trace_marker_fd, ftrace_buf, trlen));

With regards,
Daniel

[1] https://github.com/coreutils/gnulib/blob/master/lib/ignore-value.h#L38
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 04/14] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Daniel P. Berrangé]

On Fri, Aug 22, 2025 at 02:26:45PM +0200, Paolo Bonzini wrote:
> From: Tanish Desai <tanishdesai37@gmail.com>
> 
> Add a new attribute CHECK_TRACE_EVENT_GET_STATE to the backends.
> When present and True, the code generated by the generate function
> is wrapped in a conditional that checks whether the event is enabled;
> this removes the need for repeating the same conditional in multiple
> backends.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/tracetool/backend/__init__.py | 39 ++++++++++++++++++---------
>  scripts/tracetool/format/h.py         | 11 +++++---
>  2 files changed, 35 insertions(+), 15 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 05/14] tracetool/backend: remove redundant trace event checks

[Daniel P. Berrangé]

On Fri, Aug 22, 2025 at 02:26:46PM +0200, Paolo Bonzini wrote:
> From: Tanish Desai <tanishdesai37@gmail.com>
> 
> Use CHECK_TRACE_EVENT_GET_STATE in log, syslog, dtrace and simple
> backend, so that the "if (trace_event_get_state)" is created from common
> code and unified when multiple backends are active.
> 
> When a single backend is active there is no code change (except
> for the log backend, as shown in tests/tracetool/log.h), but the
> code in the backends is simpler.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/tracetool/log.h               | 16 ++++++++++------
>  scripts/tracetool/backend/ftrace.py |  6 ++----
>  scripts/tracetool/backend/log.py    | 10 ++++------
>  scripts/tracetool/backend/simple.py |  8 ++------
>  scripts/tracetool/backend/syslog.py |  8 ++------
>  5 files changed, 20 insertions(+), 28 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Peter Maydell]

On Tue, 26 Aug 2025 at 12:42, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> > This simplifies the Python code and reduces the size of the tracepoints.

> > +void ftrace_write(const char *fmt, ...)
> > +{
> > +    char ftrace_buf[MAX_TRACE_STRLEN];
> > +    int unused __attribute__ ((unused));
> > +    int trlen;
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> > +    va_end(ap);
> > +
> > +    trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> > +    unused = write(trace_marker_fd, ftrace_buf, trlen);
>
> You're just copying the existing code pattern which is fine for now so
>
>    Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> More generally though, IMHO, QEMU would be better off bringing in
> gnulib's 'ignore_value' macro, but simplified since we don't care
> about ancient GCC
>
>   #define ignore_value(x) \
>       (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
>
> so that we don't need to play games with extra variables. eg
>
>    ignore_value(write(trace_marker_fd, ftrace_buf, trlen));

Isn't there a way to write that that explicitly tells
the compiler "this is unused" (i.e. with the 'unused'
attribute) rather than relying on a particular construct
to not trigger a warning?

-- PMM

Re: [PATCH 06/14] tracetool: Add Rust format support

[Daniel P. Berrangé]

On Fri, Aug 22, 2025 at 02:26:47PM +0200, Paolo Bonzini wrote:
> From: Tanish Desai <tanishdesai37@gmail.com>
> 
> Generating .rs files makes it possible to support tracing in rust.
> This support comprises a new format, and common code that converts
> the C expressions in trace-events to Rust.  In particular, types
> need to be converted, and PRI macros expanded.  Fortunately, all
> common platforms have a known mapping of 8/16/32/64-bit integers
> to char/short/int/"long long": even if int64_t is equal to long,
> it is fine to change the format string from PRIx64's expansion
> "%lx" to "%llx".  This makes it possible to have a static mapping
> from PRI macros to their expansion.
> 
> As of this commit no backend generates Rust code, but it is already
> possible to use tracetool to generate Rust sources; they are not
> functional but they compile and contain tracepoint functions.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> [Move Rust argument conversion from Event to Arguments; string
>  support. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/tracetool/__init__.py  | 156 +++++++++++++++++++++++++++++++++
>  scripts/tracetool/format/rs.py |  76 ++++++++++++++++
>  2 files changed, 232 insertions(+)
>  create mode 100644 scripts/tracetool/format/rs.py


> diff --git a/scripts/tracetool/format/rs.py b/scripts/tracetool/format/rs.py
> new file mode 100644
> index 00000000000..bc8b2be5971
> --- /dev/null
> +++ b/scripts/tracetool/format/rs.py
> @@ -0,0 +1,76 @@
> +# -*- coding: utf-8 -*-
> +
> +"""
> +trace-DIR.rs
> +"""
> +
> +__author__     = "Tanish Desai <tanishdesai37@gmail.com>"
> +__copyright__  = "Copyright 2025, Tanish Desai <tanishdesai37@gmail.com>"
> +__license__    = "GPL version 2 or (at your option) any later version"
> +
> +__maintainer__ = "Stefan Hajnoczi"
> +__email__      = "stefanha@redhat.com"
> +
> +
> +from tracetool import out
> +
> +
> +def generate(events, backend, group):
> +    out('// This file is autogenerated by tracetool, do not edit.',

Should add

        '/* SPDX-License-Identifier: GPL-2.0-or-later */',

and update the reference output in the later patch.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 10/14] tracetool/simple: add Rust support

[Daniel P. Berrangé]

On Fri, Aug 22, 2025 at 02:26:51PM +0200, Paolo Bonzini wrote:
> From: Tanish Desai <tanishdesai37@gmail.com>
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/tracetool/__init__.py       |  2 +-
>  scripts/tracetool/backend/simple.py |  7 +++++
>  tests/tracetool/simple.rs           | 41 +++++++++++++++++++++++++++++
>  tests/tracetool/tracetool-test.py   |  2 ++
>  4 files changed, 51 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tracetool/simple.rs
> 
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 0b8ec707332..7542757799e 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -336,7 +336,7 @@ def rust_call_extern(self):
>          def rust_cast(name, type_):
>              if type_ == "const char *":
>                  return f"_{name}.as_ptr()"
> -            return "_{name}"
> +            return f"_{name}"
>  
>          return ", ".join((rust_cast(name, type_) for type_, name in self._args))
>

This should be squashed into the earlier patch that introduced the mistake

> diff --git a/tests/tracetool/simple.rs b/tests/tracetool/simple.rs
> new file mode 100644
> index 00000000000..895096088dc
> --- /dev/null
> +++ b/tests/tracetool/simple.rs
> @@ -0,0 +1,41 @@
> +// This file is autogenerated by tracetool, do not edit.
> +
> +#[allow(unused_imports)]
> +use std::ffi::c_char;
> +#[allow(unused_imports)]
> +use qemu_api::bindings;
> +
> +#[inline(always)]
> +fn trace_event_get_state_dynamic_by_id(_id: u16) -> bool {
> +    unsafe { (trace_events_enabled_count != 0) && (_id != 0) }
> +}
> +
> +extern "C" {
> +    static mut trace_events_enabled_count: u32;
> +}
> +extern "C" {
> +    static mut _TRACE_TEST_BLAH_DSTATE: u16;
> +    static mut _TRACE_TEST_WIBBLE_DSTATE: u16;
> +}
> +const _TRACE_TEST_BLAH_ENABLED: bool = true;
> +const _TRACE_TEST_WIBBLE_ENABLED: bool = true;

Does rust have any policy reserving use of leading underscore in
identifiers  ?


> +
> +#[inline(always)]
> +#[allow(dead_code)]
> +pub fn trace_test_blah(_context: *mut (), _filename: &std::ffi::CStr)
> +{
> +    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_BLAH_DSTATE}) {
> +        extern "C" { fn _simple_trace_test_blah(_context: *mut (), _filename: *const std::ffi::c_char); }
> +        unsafe { _simple_trace_test_blah(_context, _filename.as_ptr()); }
> +    }
> +}
> +
> +#[inline(always)]
> +#[allow(dead_code)]
> +pub fn trace_test_wibble(_context: *mut (), _value: std::ffi::c_int)
> +{
> +    if trace_event_get_state_dynamic_by_id(unsafe { _TRACE_TEST_WIBBLE_DSTATE}) {
> +        extern "C" { fn _simple_trace_test_wibble(_context: *mut (), _value: std::ffi::c_int); }
> +        unsafe { _simple_trace_test_wibble(_context, _value); }
> +    }
> +}

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Daniel P. Berrangé]

On Tue, Aug 26, 2025 at 12:45:42PM +0100, Peter Maydell wrote:
> On Tue, 26 Aug 2025 at 12:42, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> > > This simplifies the Python code and reduces the size of the tracepoints.
> 
> > > +void ftrace_write(const char *fmt, ...)
> > > +{
> > > +    char ftrace_buf[MAX_TRACE_STRLEN];
> > > +    int unused __attribute__ ((unused));
> > > +    int trlen;
> > > +    va_list ap;
> > > +
> > > +    va_start(ap, fmt);
> > > +    trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> > > +    va_end(ap);
> > > +
> > > +    trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> > > +    unused = write(trace_marker_fd, ftrace_buf, trlen);
> >
> > You're just copying the existing code pattern which is fine for now so
> >
> >    Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >
> >
> > More generally though, IMHO, QEMU would be better off bringing in
> > gnulib's 'ignore_value' macro, but simplified since we don't care
> > about ancient GCC
> >
> >   #define ignore_value(x) \
> >       (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
> >
> > so that we don't need to play games with extra variables. eg
> >
> >    ignore_value(write(trace_marker_fd, ftrace_buf, trlen));
> 
> Isn't there a way to write that that explicitly tells
> the compiler "this is unused" (i.e. with the 'unused'
> attribute) rather than relying on a particular construct
> to not trigger a warning?

I think what the code is already doing is the closest to making it
explicit, but that needs the extra variable which is unpleasant.

It is annoyng that 'warn_unused_result' doesn't let you just cast
to (void) to discard a result without warnings :-(

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 11/14] log: change qemu_loglevel to unsigned

[Philippe Mathieu-Daudé]

On 22/8/25 14:26, Paolo Bonzini wrote:
> Bindgen makes the LOG_* constants unsigned, even if they are defined as
> (1 << 15):
> 
>     pub const LOG_TRACE: u32 = 32768;
> 
> Make them unsigned in C as well, and also change the type of the variable
> that they are used with.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   include/qemu/log-for-trace.h |  4 ++--
>   include/qemu/log.h           | 44 ++++++++++++++++++------------------
>   util/log.c                   |  2 +-
>   rust/qemu-api/src/log.rs     |  2 +-
>   4 files changed, 26 insertions(+), 26 deletions(-)


> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index aae72985f0d..7effba4da4c 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -14,30 +14,30 @@ bool qemu_log_enabled(void);
>   /* Returns true if qemu_log() will write somewhere other than stderr. */
>   bool qemu_log_separate(void);
>   
> -#define CPU_LOG_TB_OUT_ASM (1 << 0)
> -#define CPU_LOG_TB_IN_ASM  (1 << 1)
> -#define CPU_LOG_TB_OP      (1 << 2)
> -#define CPU_LOG_TB_OP_OPT  (1 << 3)
> -#define CPU_LOG_INT        (1 << 4)
> -#define CPU_LOG_EXEC       (1 << 5)
> -#define CPU_LOG_PCALL      (1 << 6)
> -#define CPU_LOG_TB_CPU     (1 << 8)
> -#define CPU_LOG_RESET      (1 << 9)
> -#define LOG_UNIMP          (1 << 10)
> -#define LOG_GUEST_ERROR    (1 << 11)
> -#define CPU_LOG_MMU        (1 << 12)
> -#define CPU_LOG_TB_NOCHAIN (1 << 13)
> -#define CPU_LOG_PAGE       (1 << 14)
> +#define CPU_LOG_TB_OUT_ASM (1u << 0)
> +#define CPU_LOG_TB_IN_ASM  (1u << 1)
> +#define CPU_LOG_TB_OP      (1u << 2)
> +#define CPU_LOG_TB_OP_OPT  (1u << 3)
> +#define CPU_LOG_INT        (1u << 4)
> +#define CPU_LOG_EXEC       (1u << 5)
> +#define CPU_LOG_PCALL      (1u << 6)
> +#define CPU_LOG_TB_CPU     (1u << 8)
> +#define CPU_LOG_RESET      (1u << 9)
> +#define LOG_UNIMP          (1u << 10)
> +#define LOG_GUEST_ERROR    (1u << 11)
> +#define CPU_LOG_MMU        (1u << 12)
> +#define CPU_LOG_TB_NOCHAIN (1u << 13)
> +#define CPU_LOG_PAGE       (1u << 14)
>   /* LOG_TRACE (1 << 15) is defined in log-for-trace.h */
> -#define CPU_LOG_TB_OP_IND  (1 << 16)
> -#define CPU_LOG_TB_FPU     (1 << 17)
> -#define CPU_LOG_PLUGIN     (1 << 18)
> +#define CPU_LOG_TB_OP_IND  (1u << 16)
> +#define CPU_LOG_TB_FPU     (1u << 17)
> +#define CPU_LOG_PLUGIN     (1u << 18)
>   /* LOG_STRACE is used for user-mode strace logging. */
> -#define LOG_STRACE         (1 << 19)
> -#define LOG_PER_THREAD     (1 << 20)
> -#define CPU_LOG_TB_VPU     (1 << 21)
> -#define LOG_TB_OP_PLUGIN   (1 << 22)
> -#define LOG_INVALID_MEM    (1 << 23)
> +#define LOG_STRACE         (1u << 19)
> +#define LOG_PER_THREAD     (1u << 20)
> +#define CPU_LOG_TB_VPU     (1u << 21)
> +#define LOG_TB_OP_PLUGIN   (1u << 22)
> +#define LOG_INVALID_MEM    (1u << 23)

Since changing these, alternatively use BIT() from "qemu/bitops.h",
as "easier for the eyes". Anyhow,

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

Re: [PATCH 02/14] rust: move dependencies to rust/Cargo.toml

[Stefan Hajnoczi]

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

On Fri, Aug 22, 2025 at 02:26:43PM +0200, Paolo Bonzini wrote:
> As more crates start using the same dependencies, it's better to not
> repeat the versions and move the dependency declarations to the workspace.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/Cargo.toml          | 5 +++++
>  rust/qemu-api/Cargo.toml | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

[Stefan Hajnoczi]

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

On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> This simplifies the Python code and reduces the size of the tracepoints.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/tracetool/ftrace.h            | 28 ++++++----------------------
>  trace/ftrace.h                      |  1 +
>  trace/ftrace.c                      | 15 +++++++++++++++
>  scripts/tracetool/backend/ftrace.py | 12 ++----------
>  4 files changed, 24 insertions(+), 32 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 04/14] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Stefan Hajnoczi]

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

On Fri, Aug 22, 2025 at 02:26:45PM +0200, Paolo Bonzini wrote:
> From: Tanish Desai <tanishdesai37@gmail.com>
> 
> Add a new attribute CHECK_TRACE_EVENT_GET_STATE to the backends.
> When present and True, the code generated by the generate function
> is wrapped in a conditional that checks whether the event is enabled;
> this removes the need for repeating the same conditional in multiple
> backends.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/tracetool/backend/__init__.py | 39 ++++++++++++++++++---------
>  scripts/tracetool/format/h.py         | 11 +++++---
>  2 files changed, 35 insertions(+), 15 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 05/14] tracetool/backend: remove redundant trace event checks

[Stefan Hajnoczi]

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

On Fri, Aug 22, 2025 at 02:26:46PM +0200, Paolo Bonzini wrote:
> From: Tanish Desai <tanishdesai37@gmail.com>
> 
> Use CHECK_TRACE_EVENT_GET_STATE in log, syslog, dtrace and simple
> backend, so that the "if (trace_event_get_state)" is created from common
> code and unified when multiple backends are active.
> 
> When a single backend is active there is no code change (except
> for the log backend, as shown in tests/tracetool/log.h), but the
> code in the backends is simpler.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/tracetool/log.h               | 16 ++++++++++------
>  scripts/tracetool/backend/ftrace.py |  6 ++----
>  scripts/tracetool/backend/log.py    | 10 ++++------
>  scripts/tracetool/backend/simple.py |  8 ++------
>  scripts/tracetool/backend/syslog.py |  8 ++------
>  5 files changed, 20 insertions(+), 28 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]