qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] plugins: reduce total number of build objects
@ 2025-02-25 11:08 Alex Bennée
  2025-02-25 11:08 ` [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK Alex Bennée
                   ` (10 more replies)
  0 siblings, 11 replies; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

As we move towards a more modular build this series converts both
loader and api to build once objects. For both objects the only real
difference is between user mode and system emulation so those bits
have been hived off into those source sets.

The remaining core plugin is more intimately aligned with the TCG
backend so requires definitions like TCG_TARGET_LONG. Hopefully this
can been cleaned up once Richards TCG rationalisation code is added.

Please review.

Alex.

Alex Bennée (10):
  plugins/api: use tcg_ctx to get TARGET_PAGE_MASK
  plugins/loader: populate target_name with target_name()
  include/qemu: plugin-memory.h doesn't need cpu-defs.h
  plugins/api: clean-up the includes
  plugins/plugin.h: include queue.h
  plugins/loader: compile loader only once
  plugins/api: split out binary path/start/end/entry code
  plugins/api: split out the vaddr/hwaddr helpers
  plugins/api: split out time control helpers
  plugins/api: build only once

 include/qemu/plugin-memory.h |   1 -
 plugins/plugin.h             |   7 ++
 linux-user/plugin-api.c      |  43 +++++++++
 plugins/api-system.c         | 131 +++++++++++++++++++++++++++
 plugins/api-user.c           |  57 ++++++++++++
 plugins/api.c                | 170 +----------------------------------
 plugins/loader.c             |  15 +---
 plugins/system.c             |  24 +++++
 plugins/user.c               |  19 ++++
 linux-user/meson.build       |   1 +
 plugins/meson.build          |   8 +-
 11 files changed, 292 insertions(+), 184 deletions(-)
 create mode 100644 linux-user/plugin-api.c
 create mode 100644 plugins/api-system.c
 create mode 100644 plugins/api-user.c
 create mode 100644 plugins/system.c
 create mode 100644 plugins/user.c

-- 
2.39.5



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

* [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 14:57   ` Philippe Mathieu-Daudé
  2025-02-25 16:00   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 02/10] plugins/loader: populate target_name with target_name() Alex Bennée
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

Requiring TARGET_PAGE_MASK to be defined gets in the way of building
this unit once. As tcg_ctx has the value lets use it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/api.c b/plugins/api.c
index cf8cdf076a..10b258b08d 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -287,7 +287,7 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn)
 void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
 {
     const DisasContextBase *db = tcg_ctx->plugin_db;
-    vaddr page0_last = db->pc_first | ~TARGET_PAGE_MASK;
+    vaddr page0_last = db->pc_first | ~tcg_ctx->page_mask;
 
     if (db->fake_insn) {
         return NULL;
-- 
2.39.5



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

* [PATCH 02/10] plugins/loader: populate target_name with target_name()
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
  2025-02-25 11:08 ` [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 14:58   ` Philippe Mathieu-Daudé
  2025-02-25 18:04   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h Alex Bennée
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

We have a function we can call for this, lets not rely on macros that
stop us building once.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/loader.c b/plugins/loader.c
index 99686b5466..827473c8b6 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -297,7 +297,7 @@ int qemu_plugin_load_list(QemuPluginList *head, Error **errp)
     struct qemu_plugin_desc *desc, *next;
     g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
 
-    info->target_name = TARGET_NAME;
+    info->target_name = target_name();
     info->version.min = QEMU_PLUGIN_MIN_VERSION;
     info->version.cur = QEMU_PLUGIN_VERSION;
 #ifndef CONFIG_USER_ONLY
-- 
2.39.5



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

* [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
  2025-02-25 11:08 ` [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK Alex Bennée
  2025-02-25 11:08 ` [PATCH 02/10] plugins/loader: populate target_name with target_name() Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:10   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 04/10] plugins/api: clean-up the includes Alex Bennée
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

hwaddr is a fixed size on all builds.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/plugin-memory.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/qemu/plugin-memory.h b/include/qemu/plugin-memory.h
index 71c1123308..6065ec7aaf 100644
--- a/include/qemu/plugin-memory.h
+++ b/include/qemu/plugin-memory.h
@@ -9,7 +9,6 @@
 #ifndef PLUGIN_MEMORY_H
 #define PLUGIN_MEMORY_H
 
-#include "exec/cpu-defs.h"
 #include "exec/hwaddr.h"
 
 struct qemu_plugin_hwaddr {
-- 
2.39.5



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

* [PATCH 04/10] plugins/api: clean-up the includes
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (2 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:14   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 05/10] plugins/plugin.h: include queue.h Alex Bennée
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

Thanks to re-factoring and clean-up work (especially to exec-all) we
no longer need such broad headers for the api.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/api.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/plugins/api.c b/plugins/api.c
index 10b258b08d..3e1aac7bfb 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -39,9 +39,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/plugin.h"
 #include "qemu/log.h"
-#include "qemu/timer.h"
 #include "tcg/tcg.h"
-#include "exec/exec-all.h"
 #include "exec/gdbstub.h"
 #include "exec/translation-block.h"
 #include "exec/translator.h"
@@ -50,7 +48,6 @@
 #ifndef CONFIG_USER_ONLY
 #include "qapi/error.h"
 #include "migration/blocker.h"
-#include "exec/ram_addr.h"
 #include "qemu/plugin-memory.h"
 #include "hw/boards.h"
 #else
-- 
2.39.5



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

* [PATCH 05/10] plugins/plugin.h: include queue.h
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (3 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 04/10] plugins/api: clean-up the includes Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:15   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 06/10] plugins/loader: compile loader only once Alex Bennée
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

Headers should bring in what they need so don't rely on getting
queue.h by side effects. This will help with clean-ups in the
following patches.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/plugin.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/plugins/plugin.h b/plugins/plugin.h
index 30e2299a54..9ed20b5c41 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -13,6 +13,7 @@
 #define PLUGIN_H
 
 #include <gmodule.h>
+#include "qemu/queue.h"
 #include "qemu/qht.h"
 
 #define QEMU_PLUGIN_MIN_VERSION 2
-- 
2.39.5



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

* [PATCH 06/10] plugins/loader: compile loader only once
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (4 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 05/10] plugins/plugin.h: include queue.h Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:18   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 07/10] plugins/api: split out binary path/start/end/entry code Alex Bennée
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

There is very little in loader that is different between builds save
for a tiny user/system mode difference in the plugin_info structure.
Create two new files, user and system to hold mode specific helpers
and move loader into common_ss.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/plugin.h    |  6 ++++++
 plugins/loader.c    | 13 ++-----------
 plugins/system.c    | 24 ++++++++++++++++++++++++
 plugins/user.c      | 19 +++++++++++++++++++
 plugins/meson.build |  7 ++++++-
 5 files changed, 57 insertions(+), 12 deletions(-)
 create mode 100644 plugins/system.c
 create mode 100644 plugins/user.c

diff --git a/plugins/plugin.h b/plugins/plugin.h
index 9ed20b5c41..6fbc443b96 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -119,4 +119,10 @@ struct qemu_plugin_scoreboard *plugin_scoreboard_new(size_t element_size);
 
 void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score);
 
+/**
+ * qemu_plugin_fillin_mode_info() - populate mode specific info
+ * info: pointer to qemu_info_t structure
+ */
+void qemu_plugin_fillin_mode_info(qemu_info_t *info);
+
 #endif /* PLUGIN_H */
diff --git a/plugins/loader.c b/plugins/loader.c
index 827473c8b6..7523d554f0 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -31,9 +31,6 @@
 #include "qemu/memalign.h"
 #include "hw/core/cpu.h"
 #include "exec/tb-flush.h"
-#ifndef CONFIG_USER_ONLY
-#include "hw/boards.h"
-#endif
 
 #include "plugin.h"
 
@@ -300,14 +297,8 @@ int qemu_plugin_load_list(QemuPluginList *head, Error **errp)
     info->target_name = target_name();
     info->version.min = QEMU_PLUGIN_MIN_VERSION;
     info->version.cur = QEMU_PLUGIN_VERSION;
-#ifndef CONFIG_USER_ONLY
-    MachineState *ms = MACHINE(qdev_get_machine());
-    info->system_emulation = true;
-    info->system.smp_vcpus = ms->smp.cpus;
-    info->system.max_vcpus = ms->smp.max_cpus;
-#else
-    info->system_emulation = false;
-#endif
+
+    qemu_plugin_fillin_mode_info(info);
 
     QTAILQ_FOREACH_SAFE(desc, head, entry, next) {
         int err;
diff --git a/plugins/system.c b/plugins/system.c
new file mode 100644
index 0000000000..b3ecc33ba5
--- /dev/null
+++ b/plugins/system.c
@@ -0,0 +1,24 @@
+/*
+ * QEMU Plugin system-emulation helpers
+ *
+ * Helpers that are specific to system emulation.
+ *
+ * Copyright (C) 2017, Emilio G. Cota <cota@braap.org>
+ * Copyright (C) 2019-2025, Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/plugin.h"
+#include "hw/boards.h"
+
+#include "plugin.h"
+
+void qemu_plugin_fillin_mode_info(qemu_info_t *info)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    info->system_emulation = true;
+    info->system.smp_vcpus = ms->smp.cpus;
+    info->system.max_vcpus = ms->smp.max_cpus;
+}
diff --git a/plugins/user.c b/plugins/user.c
new file mode 100644
index 0000000000..250d542502
--- /dev/null
+++ b/plugins/user.c
@@ -0,0 +1,19 @@
+/*
+ * QEMU Plugin user-mode helpers
+ *
+ * Helpers that are specific to user-mode.
+ *
+ * Copyright (C) 2017, Emilio G. Cota <cota@braap.org>
+ * Copyright (C) 2019-2025, Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/plugin.h"
+#include "plugin.h"
+
+void qemu_plugin_fillin_mode_info(qemu_info_t *info)
+{
+    info->system_emulation = false;
+}
diff --git a/plugins/meson.build b/plugins/meson.build
index d60be2a4d6..f7820806d3 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -57,8 +57,13 @@ if host_os == 'windows'
     command: dlltool_cmd
   )
 endif
+
+user_ss.add(files('user.c'))
+system_ss.add(files('system.c'))
+
+common_ss.add(files('loader.c'))
+
 specific_ss.add(files(
-  'loader.c',
   'core.c',
   'api.c',
 ))
-- 
2.39.5



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

* [PATCH 07/10] plugins/api: split out binary path/start/end/entry code
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (5 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 06/10] plugins/loader: compile loader only once Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:32   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers Alex Bennée
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

To move the main api.c to a single build compilation object we need to
start splitting out user and system specific code. As we need to grob
around host headers we move these particular helpers into the *-user
mode directories.

The binary/start/end/entry helpers are all NOPs for system mode.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 linux-user/plugin-api.c | 43 +++++++++++++++++++++++++++++++++++++++++
 plugins/api-system.c    | 39 +++++++++++++++++++++++++++++++++++++
 plugins/api.c           | 43 -----------------------------------------
 linux-user/meson.build  |  1 +
 plugins/meson.build     |  2 +-
 5 files changed, 84 insertions(+), 44 deletions(-)
 create mode 100644 linux-user/plugin-api.c
 create mode 100644 plugins/api-system.c

diff --git a/linux-user/plugin-api.c b/linux-user/plugin-api.c
new file mode 100644
index 0000000000..42e977c339
--- /dev/null
+++ b/linux-user/plugin-api.c
@@ -0,0 +1,43 @@
+/*
+ * QEMU Plugin API - linux-user-mode only implementations
+ *
+ * Common user-mode only APIs are in plugins/api-user. These helpers
+ * are only specific to linux-user.
+ *
+ * Copyright (C) 2017, Emilio G. Cota <cota@braap.org>
+ * Copyright (C) 2019-2025, Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/plugin.h"
+#include "qemu.h"
+
+/*
+ * Binary path, start and end locations. Host specific due to TaskState.
+ */
+const char *qemu_plugin_path_to_binary(void)
+{
+    TaskState *ts = get_task_state(current_cpu);
+    return g_strdup(ts->bprm->filename);
+}
+
+uint64_t qemu_plugin_start_code(void)
+{
+    TaskState *ts = get_task_state(current_cpu);
+    return ts->info->start_code;
+}
+
+uint64_t qemu_plugin_end_code(void)
+{
+    TaskState *ts = get_task_state(current_cpu);
+    return ts->info->end_code;
+}
+
+uint64_t qemu_plugin_entry_code(void)
+{
+    TaskState *ts = get_task_state(current_cpu);
+    return ts->info->entry;
+}
diff --git a/plugins/api-system.c b/plugins/api-system.c
new file mode 100644
index 0000000000..cb0dd8f730
--- /dev/null
+++ b/plugins/api-system.c
@@ -0,0 +1,39 @@
+/*
+ * QEMU Plugin API - System specific implementations
+ *
+ * This provides the APIs that have a specific system implementation
+ * or are only relevant to system-mode.
+ *
+ * Copyright (C) 2017, Emilio G. Cota <cota@braap.org>
+ * Copyright (C) 2019-2025, Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/plugin.h"
+
+/*
+ * In system mode we cannot trace the binary being executed so the
+ * helpers all return NULL/0.
+ */
+const char *qemu_plugin_path_to_binary(void)
+{
+    return NULL;
+}
+
+uint64_t qemu_plugin_start_code(void)
+{
+    return 0;
+}
+
+uint64_t qemu_plugin_end_code(void)
+{
+    return 0;
+}
+
+uint64_t qemu_plugin_entry_code(void)
+{
+    return 0;
+}
diff --git a/plugins/api.c b/plugins/api.c
index 3e1aac7bfb..12a3b6a66d 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -470,49 +470,6 @@ bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
     return name && value && qapi_bool_parse(name, value, ret, NULL);
 }
 
-/*
- * Binary path, start and end locations
- */
-const char *qemu_plugin_path_to_binary(void)
-{
-    char *path = NULL;
-#ifdef CONFIG_USER_ONLY
-    TaskState *ts = get_task_state(current_cpu);
-    path = g_strdup(ts->bprm->filename);
-#endif
-    return path;
-}
-
-uint64_t qemu_plugin_start_code(void)
-{
-    uint64_t start = 0;
-#ifdef CONFIG_USER_ONLY
-    TaskState *ts = get_task_state(current_cpu);
-    start = ts->info->start_code;
-#endif
-    return start;
-}
-
-uint64_t qemu_plugin_end_code(void)
-{
-    uint64_t end = 0;
-#ifdef CONFIG_USER_ONLY
-    TaskState *ts = get_task_state(current_cpu);
-    end = ts->info->end_code;
-#endif
-    return end;
-}
-
-uint64_t qemu_plugin_entry_code(void)
-{
-    uint64_t entry = 0;
-#ifdef CONFIG_USER_ONLY
-    TaskState *ts = get_task_state(current_cpu);
-    entry = ts->info->entry;
-#endif
-    return entry;
-}
-
 /*
  * Create register handles.
  *
diff --git a/linux-user/meson.build b/linux-user/meson.build
index f75b4fe0e3..f47a213ca3 100644
--- a/linux-user/meson.build
+++ b/linux-user/meson.build
@@ -27,6 +27,7 @@ linux_user_ss.add(libdw)
 linux_user_ss.add(when: 'TARGET_HAS_BFLT', if_true: files('flatload.c'))
 linux_user_ss.add(when: 'TARGET_I386', if_true: files('vm86.c'))
 linux_user_ss.add(when: 'CONFIG_ARM_COMPATIBLE_SEMIHOSTING', if_true: files('semihost.c'))
+linux_user_ss.add(when: 'CONFIG_TCG_PLUGINS', if_true: files('plugin-api.c'))
 
 syscall_nr_generators = {}
 
diff --git a/plugins/meson.build b/plugins/meson.build
index f7820806d3..9c9bc9e5bb 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -59,7 +59,7 @@ if host_os == 'windows'
 endif
 
 user_ss.add(files('user.c'))
-system_ss.add(files('system.c'))
+system_ss.add(files('system.c', 'api-system.c'))
 
 common_ss.add(files('loader.c'))
 
-- 
2.39.5



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

* [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (6 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 07/10] plugins/api: split out binary path/start/end/entry code Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:40   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 09/10] plugins/api: split out time control helpers Alex Bennée
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

These only work for system-mode and are NOPs for user-mode.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/api-system.c | 58 ++++++++++++++++++++++++++++++++++++
 plugins/api-user.c   | 40 +++++++++++++++++++++++++
 plugins/api.c        | 70 --------------------------------------------
 plugins/meson.build  |  2 +-
 4 files changed, 99 insertions(+), 71 deletions(-)
 create mode 100644 plugins/api-user.c

diff --git a/plugins/api-system.c b/plugins/api-system.c
index cb0dd8f730..38560de342 100644
--- a/plugins/api-system.c
+++ b/plugins/api-system.c
@@ -12,6 +12,10 @@
 
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
+#include "qapi/error.h"
+#include "migration/blocker.h"
+#include "hw/boards.h"
+#include "qemu/plugin-memory.h"
 #include "qemu/plugin.h"
 
 /*
@@ -37,3 +41,57 @@ uint64_t qemu_plugin_entry_code(void)
 {
     return 0;
 }
+
+/*
+ * Virtual Memory queries
+ */
+
+static __thread struct qemu_plugin_hwaddr hwaddr_info;
+
+struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
+                                                  uint64_t vaddr)
+{
+    CPUState *cpu = current_cpu;
+    unsigned int mmu_idx = get_mmuidx(info);
+    enum qemu_plugin_mem_rw rw = get_plugin_meminfo_rw(info);
+    hwaddr_info.is_store = (rw & QEMU_PLUGIN_MEM_W) != 0;
+
+    assert(mmu_idx < NB_MMU_MODES);
+
+    if (!tlb_plugin_lookup(cpu, vaddr, mmu_idx,
+                           hwaddr_info.is_store, &hwaddr_info)) {
+        error_report("invalid use of qemu_plugin_get_hwaddr");
+        return NULL;
+    }
+
+    return &hwaddr_info;
+}
+
+bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
+{
+    return haddr->is_io;
+}
+
+uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr)
+{
+    if (haddr) {
+        return haddr->phys_addr;
+    }
+    return 0;
+}
+
+const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
+{
+    if (h && h->is_io) {
+        MemoryRegion *mr = h->mr;
+        if (!mr->name) {
+            unsigned maddr = (uintptr_t)mr;
+            g_autofree char *temp = g_strdup_printf("anon%08x", maddr);
+            return g_intern_string(temp);
+        } else {
+            return g_intern_string(mr->name);
+        }
+    } else {
+        return g_intern_static_string("RAM");
+    }
+}
diff --git a/plugins/api-user.c b/plugins/api-user.c
new file mode 100644
index 0000000000..867b420339
--- /dev/null
+++ b/plugins/api-user.c
@@ -0,0 +1,40 @@
+/*
+ * QEMU Plugin API - user-mode only implementations
+ *
+ * This provides the APIs that have a user-mode specific
+ * implementations or are only relevant to user-mode.
+ *
+ * Copyright (C) 2017, Emilio G. Cota <cota@braap.org>
+ * Copyright (C) 2019-2025, Linaro
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/plugin.h"
+
+/*
+ * Virtual Memory queries - these are all NOPs for user-mode which
+ * only ever has visibility of virtual addresses.
+ */
+
+struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
+                                                  uint64_t vaddr)
+{
+    return NULL;
+}
+
+bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
+{
+    return false;
+}
+
+uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr)
+{
+    return 0;
+}
+
+const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
+{
+    return g_intern_static_string("Invalid");
+}
diff --git a/plugins/api.c b/plugins/api.c
index 12a3b6a66d..b04577424f 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -382,76 +382,6 @@ qemu_plugin_mem_value qemu_plugin_mem_get_value(qemu_plugin_meminfo_t info)
     return value;
 }
 
-/*
- * Virtual Memory queries
- */
-
-#ifdef CONFIG_SOFTMMU
-static __thread struct qemu_plugin_hwaddr hwaddr_info;
-#endif
-
-struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
-                                                  uint64_t vaddr)
-{
-#ifdef CONFIG_SOFTMMU
-    CPUState *cpu = current_cpu;
-    unsigned int mmu_idx = get_mmuidx(info);
-    enum qemu_plugin_mem_rw rw = get_plugin_meminfo_rw(info);
-    hwaddr_info.is_store = (rw & QEMU_PLUGIN_MEM_W) != 0;
-
-    assert(mmu_idx < NB_MMU_MODES);
-
-    if (!tlb_plugin_lookup(cpu, vaddr, mmu_idx,
-                           hwaddr_info.is_store, &hwaddr_info)) {
-        error_report("invalid use of qemu_plugin_get_hwaddr");
-        return NULL;
-    }
-
-    return &hwaddr_info;
-#else
-    return NULL;
-#endif
-}
-
-bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
-{
-#ifdef CONFIG_SOFTMMU
-    return haddr->is_io;
-#else
-    return false;
-#endif
-}
-
-uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr)
-{
-#ifdef CONFIG_SOFTMMU
-    if (haddr) {
-        return haddr->phys_addr;
-    }
-#endif
-    return 0;
-}
-
-const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
-{
-#ifdef CONFIG_SOFTMMU
-    if (h && h->is_io) {
-        MemoryRegion *mr = h->mr;
-        if (!mr->name) {
-            unsigned maddr = (uintptr_t)mr;
-            g_autofree char *temp = g_strdup_printf("anon%08x", maddr);
-            return g_intern_string(temp);
-        } else {
-            return g_intern_string(mr->name);
-        }
-    } else {
-        return g_intern_static_string("RAM");
-    }
-#else
-    return g_intern_static_string("Invalid");
-#endif
-}
-
 int qemu_plugin_num_vcpus(void)
 {
     return plugin_num_vcpus();
diff --git a/plugins/meson.build b/plugins/meson.build
index 9c9bc9e5bb..942b59e904 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -58,7 +58,7 @@ if host_os == 'windows'
   )
 endif
 
-user_ss.add(files('user.c'))
+user_ss.add(files('user.c', 'api-user.c'))
 system_ss.add(files('system.c', 'api-system.c'))
 
 common_ss.add(files('loader.c'))
-- 
2.39.5



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

* [PATCH 09/10] plugins/api: split out time control helpers
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (7 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:45   ` Richard Henderson
  2025-02-25 11:08 ` [PATCH 10/10] plugins/api: build only once Alex Bennée
  2025-02-25 19:21 ` [PATCH 00/10] plugins: reduce total number of build objects Pierrick Bouvier
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

These are only usable in system mode where we control the timer. For
user-mode make them NOPs.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/api-system.c | 34 ++++++++++++++++++++++++++++++++++
 plugins/api-user.c   | 17 +++++++++++++++++
 plugins/api.c        | 41 -----------------------------------------
 3 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/plugins/api-system.c b/plugins/api-system.c
index 38560de342..cc190b167e 100644
--- a/plugins/api-system.c
+++ b/plugins/api-system.c
@@ -95,3 +95,37 @@ const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
         return g_intern_static_string("RAM");
     }
 }
+
+/*
+ * Time control
+ */
+static bool has_control;
+static Error *migration_blocker;
+
+const void *qemu_plugin_request_time_control(void)
+{
+    if (!has_control) {
+        has_control = true;
+        error_setg(&migration_blocker,
+                   "TCG plugin time control does not support migration");
+        migrate_add_blocker(&migration_blocker, NULL);
+        return &has_control;
+    }
+    return NULL;
+}
+
+static void advance_virtual_time__async(CPUState *cpu, run_on_cpu_data data)
+{
+    int64_t new_time = data.host_ulong;
+    qemu_clock_advance_virtual_time(new_time);
+}
+
+void qemu_plugin_update_ns(const void *handle, int64_t new_time)
+{
+    if (handle == &has_control) {
+        /* Need to execute out of cpu_exec, so bql can be locked. */
+        async_run_on_cpu(current_cpu,
+                         advance_virtual_time__async,
+                         RUN_ON_CPU_HOST_ULONG(new_time));
+    }
+}
diff --git a/plugins/api-user.c b/plugins/api-user.c
index 867b420339..28704a89e8 100644
--- a/plugins/api-user.c
+++ b/plugins/api-user.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/plugin.h"
+#include "exec/log.h"
 
 /*
  * Virtual Memory queries - these are all NOPs for user-mode which
@@ -38,3 +39,19 @@ const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h)
 {
     return g_intern_static_string("Invalid");
 }
+
+/*
+ * Time control - for user mode the only real time is wall clock time
+ * so realistically all you can do in user mode is slow down execution
+ * which doesn't require the ability to mess with the clock.
+ */
+
+const void *qemu_plugin_request_time_control(void)
+{
+    return NULL;
+}
+
+void qemu_plugin_update_ns(const void *handle, int64_t new_time)
+{
+    qemu_log_mask(LOG_UNIMP, "user-mode can't control time");
+}
diff --git a/plugins/api.c b/plugins/api.c
index b04577424f..61480d3dc1 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -525,44 +525,3 @@ uint64_t qemu_plugin_u64_sum(qemu_plugin_u64 entry)
     return total;
 }
 
-/*
- * Time control
- */
-static bool has_control;
-#ifdef CONFIG_SOFTMMU
-static Error *migration_blocker;
-#endif
-
-const void *qemu_plugin_request_time_control(void)
-{
-    if (!has_control) {
-        has_control = true;
-#ifdef CONFIG_SOFTMMU
-        error_setg(&migration_blocker,
-                   "TCG plugin time control does not support migration");
-        migrate_add_blocker(&migration_blocker, NULL);
-#endif
-        return &has_control;
-    }
-    return NULL;
-}
-
-#ifdef CONFIG_SOFTMMU
-static void advance_virtual_time__async(CPUState *cpu, run_on_cpu_data data)
-{
-    int64_t new_time = data.host_ulong;
-    qemu_clock_advance_virtual_time(new_time);
-}
-#endif
-
-void qemu_plugin_update_ns(const void *handle, int64_t new_time)
-{
-#ifdef CONFIG_SOFTMMU
-    if (handle == &has_control) {
-        /* Need to execute out of cpu_exec, so bql can be locked. */
-        async_run_on_cpu(current_cpu,
-                         advance_virtual_time__async,
-                         RUN_ON_CPU_HOST_ULONG(new_time));
-    }
-#endif
-}
-- 
2.39.5



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

* [PATCH 10/10] plugins/api: build only once
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (8 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 09/10] plugins/api: split out time control helpers Alex Bennée
@ 2025-02-25 11:08 ` Alex Bennée
  2025-02-25 18:46   ` Richard Henderson
  2025-02-25 19:21 ` [PATCH 00/10] plugins: reduce total number of build objects Pierrick Bouvier
  10 siblings, 1 reply; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 11:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Laurent Vivier, Pierrick Bouvier,
	Alexandre Iooss, Mahmoud Mandour

Now all the softmmu/user-mode stuff has been split out we can build
this compilation unit only once.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/api.c       | 11 -----------
 plugins/meson.build |  3 +--
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/plugins/api.c b/plugins/api.c
index 61480d3dc1..a292b5bff3 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -45,17 +45,6 @@
 #include "exec/translator.h"
 #include "disas/disas.h"
 #include "plugin.h"
-#ifndef CONFIG_USER_ONLY
-#include "qapi/error.h"
-#include "migration/blocker.h"
-#include "qemu/plugin-memory.h"
-#include "hw/boards.h"
-#else
-#include "qemu.h"
-#ifdef CONFIG_LINUX
-#include "loader.h"
-#endif
-#endif
 
 /* Uninstall and Reset handlers */
 
diff --git a/plugins/meson.build b/plugins/meson.build
index 942b59e904..d27220d5ff 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -61,9 +61,8 @@ endif
 user_ss.add(files('user.c', 'api-user.c'))
 system_ss.add(files('system.c', 'api-system.c'))
 
-common_ss.add(files('loader.c'))
+common_ss.add(files('loader.c', 'api.c'))
 
 specific_ss.add(files(
   'core.c',
-  'api.c',
 ))
-- 
2.39.5



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

* Re: [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK
  2025-02-25 11:08 ` [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK Alex Bennée
@ 2025-02-25 14:57   ` Philippe Mathieu-Daudé
  2025-02-25 16:00   ` Richard Henderson
  1 sibling, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-02-25 14:57 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Laurent Vivier, Pierrick Bouvier, Alexandre Iooss,
	Mahmoud Mandour

On 25/2/25 12:08, Alex Bennée wrote:
> Requiring TARGET_PAGE_MASK to be defined gets in the way of building
> this unit once. As tcg_ctx has the value lets use it.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   plugins/api.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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



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

* Re: [PATCH 02/10] plugins/loader: populate target_name with target_name()
  2025-02-25 11:08 ` [PATCH 02/10] plugins/loader: populate target_name with target_name() Alex Bennée
@ 2025-02-25 14:58   ` Philippe Mathieu-Daudé
  2025-02-25 18:04   ` Richard Henderson
  1 sibling, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-02-25 14:58 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Laurent Vivier, Pierrick Bouvier, Alexandre Iooss,
	Mahmoud Mandour

On 25/2/25 12:08, Alex Bennée wrote:
> We have a function we can call for this, lets not rely on macros that
> stop us building once.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   plugins/loader.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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



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

* Re: [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK
  2025-02-25 11:08 ` [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK Alex Bennée
  2025-02-25 14:57   ` Philippe Mathieu-Daudé
@ 2025-02-25 16:00   ` Richard Henderson
  1 sibling, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 16:00 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> Requiring TARGET_PAGE_MASK to be defined gets in the way of building
> this unit once. As tcg_ctx has the value lets use it.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   plugins/api.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/plugins/api.c b/plugins/api.c
> index cf8cdf076a..10b258b08d 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -287,7 +287,7 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn)
>   void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
>   {
>       const DisasContextBase *db = tcg_ctx->plugin_db;
> -    vaddr page0_last = db->pc_first | ~TARGET_PAGE_MASK;
> +    vaddr page0_last = db->pc_first | ~tcg_ctx->page_mask;
>   
>       if (db->fake_insn) {
>           return NULL;

NACK.  While this currently happens to work, it's the wrong api. This value is only live 
during the compilation cycle, and this part of plugins is not that.

For this, qemu_target_page_mask() is your huckleberry.


r~


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

* Re: [PATCH 02/10] plugins/loader: populate target_name with target_name()
  2025-02-25 11:08 ` [PATCH 02/10] plugins/loader: populate target_name with target_name() Alex Bennée
  2025-02-25 14:58   ` Philippe Mathieu-Daudé
@ 2025-02-25 18:04   ` Richard Henderson
  1 sibling, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:04 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> We have a function we can call for this, lets not rely on macros that
> stop us building once.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   plugins/loader.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 99686b5466..827473c8b6 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -297,7 +297,7 @@ int qemu_plugin_load_list(QemuPluginList *head, Error **errp)
>       struct qemu_plugin_desc *desc, *next;
>       g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
>   
> -    info->target_name = TARGET_NAME;
> +    info->target_name = target_name();
>       info->version.min = QEMU_PLUGIN_MIN_VERSION;
>       info->version.cur = QEMU_PLUGIN_VERSION;
>   #ifndef CONFIG_USER_ONLY

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h
  2025-02-25 11:08 ` [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h Alex Bennée
@ 2025-02-25 18:10   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:10 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> hwaddr is a fixed size on all builds.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/qemu/plugin-memory.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/include/qemu/plugin-memory.h b/include/qemu/plugin-memory.h
> index 71c1123308..6065ec7aaf 100644
> --- a/include/qemu/plugin-memory.h
> +++ b/include/qemu/plugin-memory.h
> @@ -9,7 +9,6 @@
>   #ifndef PLUGIN_MEMORY_H
>   #define PLUGIN_MEMORY_H
>   
> -#include "exec/cpu-defs.h"
>   #include "exec/hwaddr.h"
>   
>   struct qemu_plugin_hwaddr {

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 04/10] plugins/api: clean-up the includes
  2025-02-25 11:08 ` [PATCH 04/10] plugins/api: clean-up the includes Alex Bennée
@ 2025-02-25 18:14   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:14 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> Thanks to re-factoring and clean-up work (especially to exec-all) we
> no longer need such broad headers for the api.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   plugins/api.c | 3 ---
>   1 file changed, 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 05/10] plugins/plugin.h: include queue.h
  2025-02-25 11:08 ` [PATCH 05/10] plugins/plugin.h: include queue.h Alex Bennée
@ 2025-02-25 18:15   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:15 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> Headers should bring in what they need so don't rely on getting
> queue.h by side effects. This will help with clean-ups in the
> following patches.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   plugins/plugin.h | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 06/10] plugins/loader: compile loader only once
  2025-02-25 11:08 ` [PATCH 06/10] plugins/loader: compile loader only once Alex Bennée
@ 2025-02-25 18:18   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:18 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> There is very little in loader that is different between builds save
> for a tiny user/system mode difference in the plugin_info structure.
> Create two new files, user and system to hold mode specific helpers
> and move loader into common_ss.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   plugins/plugin.h    |  6 ++++++
>   plugins/loader.c    | 13 ++-----------
>   plugins/system.c    | 24 ++++++++++++++++++++++++
>   plugins/user.c      | 19 +++++++++++++++++++
>   plugins/meson.build |  7 ++++++-
>   5 files changed, 57 insertions(+), 12 deletions(-)
>   create mode 100644 plugins/system.c
>   create mode 100644 plugins/user.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 07/10] plugins/api: split out binary path/start/end/entry code
  2025-02-25 11:08 ` [PATCH 07/10] plugins/api: split out binary path/start/end/entry code Alex Bennée
@ 2025-02-25 18:32   ` Richard Henderson
  2025-02-25 18:57     ` Alex Bennée
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:32 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> To move the main api.c to a single build compilation object we need to
> start splitting out user and system specific code. As we need to grob
> around host headers we move these particular helpers into the *-user
> mode directories.
> 
> The binary/start/end/entry helpers are all NOPs for system mode.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   linux-user/plugin-api.c | 43 +++++++++++++++++++++++++++++++++++++++++
>   plugins/api-system.c    | 39 +++++++++++++++++++++++++++++++++++++
>   plugins/api.c           | 43 -----------------------------------------
>   linux-user/meson.build  |  1 +
>   plugins/meson.build     |  2 +-
>   5 files changed, 84 insertions(+), 44 deletions(-)
>   create mode 100644 linux-user/plugin-api.c
>   create mode 100644 plugins/api-system.c

Surely this breaks bsd-user.

Ideally this would go in common-user, but I think you'd need to move structures out of 
{bsd,linux}-user/qemu.h into include/user/.

In the very short term you could put plugin-api.c.inc in common-user, and


#include "qemu.h"
#include "common-user/plugin-api.c.inc"


in both linux-user and bsd-user.


r~


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

* Re: [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers
  2025-02-25 11:08 ` [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers Alex Bennée
@ 2025-02-25 18:40   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:40 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> These only work for system-mode and are NOPs for user-mode.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   plugins/api-system.c | 58 ++++++++++++++++++++++++++++++++++++
>   plugins/api-user.c   | 40 +++++++++++++++++++++++++
>   plugins/api.c        | 70 --------------------------------------------
>   plugins/meson.build  |  2 +-
>   4 files changed, 99 insertions(+), 71 deletions(-)
>   create mode 100644 plugins/api-user.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 09/10] plugins/api: split out time control helpers
  2025-02-25 11:08 ` [PATCH 09/10] plugins/api: split out time control helpers Alex Bennée
@ 2025-02-25 18:45   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:45 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> These are only usable in system mode where we control the timer. For
> user-mode make them NOPs.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   plugins/api-system.c | 34 ++++++++++++++++++++++++++++++++++
>   plugins/api-user.c   | 17 +++++++++++++++++
>   plugins/api.c        | 41 -----------------------------------------
>   3 files changed, 51 insertions(+), 41 deletions(-)

Code movement, so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

> +static bool has_control;
> +static Error *migration_blocker;
> +
> +const void *qemu_plugin_request_time_control(void)
> +{
> +    if (!has_control) {
> +        has_control = true;
> +        error_setg(&migration_blocker,
> +                   "TCG plugin time control does not support migration");
> +        migrate_add_blocker(&migration_blocker, NULL);
> +        return &has_control;
> +    }
> +    return NULL;
> +}

But I'm really not a fan of either this API, or the duplication between has_control and 
migration_blocker != NULL.


r~


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

* Re: [PATCH 10/10] plugins/api: build only once
  2025-02-25 11:08 ` [PATCH 10/10] plugins/api: build only once Alex Bennée
@ 2025-02-25 18:46   ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2025-02-25 18:46 UTC (permalink / raw)
  To: qemu-devel

On 2/25/25 03:08, Alex Bennée wrote:
> Now all the softmmu/user-mode stuff has been split out we can build
> this compilation unit only once.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   plugins/api.c       | 11 -----------
>   plugins/meson.build |  3 +--
>   2 files changed, 1 insertion(+), 13 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 07/10] plugins/api: split out binary path/start/end/entry code
  2025-02-25 18:32   ` Richard Henderson
@ 2025-02-25 18:57     ` Alex Bennée
  0 siblings, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2025-02-25 18:57 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

Richard Henderson <richard.henderson@linaro.org> writes:

> On 2/25/25 03:08, Alex Bennée wrote:
>> To move the main api.c to a single build compilation object we need to
>> start splitting out user and system specific code. As we need to grob
>> around host headers we move these particular helpers into the *-user
>> mode directories.
>> The binary/start/end/entry helpers are all NOPs for system mode.
>> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
>> ---
>>   linux-user/plugin-api.c | 43 +++++++++++++++++++++++++++++++++++++++++
>>   plugins/api-system.c    | 39 +++++++++++++++++++++++++++++++++++++
>>   plugins/api.c           | 43 -----------------------------------------
>>   linux-user/meson.build  |  1 +
>>   plugins/meson.build     |  2 +-
>>   5 files changed, 84 insertions(+), 44 deletions(-)
>>   create mode 100644 linux-user/plugin-api.c
>>   create mode 100644 plugins/api-system.c
>
> Surely this breaks bsd-user.

It didn't seem to - but my local building isn't working so maybe CI
disables plugins.

>
> Ideally this would go in common-user, but I think you'd need to move
> structures out of {bsd,linux}-user/qemu.h into include/user/.
>
> In the very short term you could put plugin-api.c.inc in common-user, and
>
>
> #include "qemu.h"
> #include "common-user/plugin-api.c.inc"
>
>
> in both linux-user and bsd-user.
>
>
> r~

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


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

* Re: [PATCH 00/10] plugins: reduce total number of build objects
  2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
                   ` (9 preceding siblings ...)
  2025-02-25 11:08 ` [PATCH 10/10] plugins/api: build only once Alex Bennée
@ 2025-02-25 19:21 ` Pierrick Bouvier
  10 siblings, 0 replies; 25+ messages in thread
From: Pierrick Bouvier @ 2025-02-25 19:21 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Laurent Vivier, Alexandre Iooss, Mahmoud Mandour

Hi Alex,

On 2/25/25 03:08, Alex Bennée wrote:
> As we move towards a more modular build this series converts both
> loader and api to build once objects. For both objects the only real
> difference is between user mode and system emulation so those bits
> have been hived off into those source sets.
> 
> The remaining core plugin is more intimately aligned with the TCG
> backend so requires definitions like TCG_TARGET_LONG. Hopefully this
> can been cleaned up once Richards TCG rationalisation code is added.
> 
> Please review.
> 
> Alex.
> 
> Alex Bennée (10):
>    plugins/api: use tcg_ctx to get TARGET_PAGE_MASK
>    plugins/loader: populate target_name with target_name()
>    include/qemu: plugin-memory.h doesn't need cpu-defs.h
>    plugins/api: clean-up the includes
>    plugins/plugin.h: include queue.h
>    plugins/loader: compile loader only once
>    plugins/api: split out binary path/start/end/entry code
>    plugins/api: split out the vaddr/hwaddr helpers
>    plugins/api: split out time control helpers
>    plugins/api: build only once
> 

for all the series:
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Additionnally, I would suggest to go further and merge functions for 
system and user mode, by checking at runtime what is the mode, by 
introducing a helper function for that. This way, all files can be 
really linked together, without having to select a different set of 
sources depending on which binary is built.

Regards,
Pierrick

>   include/qemu/plugin-memory.h |   1 -
>   plugins/plugin.h             |   7 ++
>   linux-user/plugin-api.c      |  43 +++++++++
>   plugins/api-system.c         | 131 +++++++++++++++++++++++++++
>   plugins/api-user.c           |  57 ++++++++++++
>   plugins/api.c                | 170 +----------------------------------
>   plugins/loader.c             |  15 +---
>   plugins/system.c             |  24 +++++
>   plugins/user.c               |  19 ++++
>   linux-user/meson.build       |   1 +
>   plugins/meson.build          |   8 +-
>   11 files changed, 292 insertions(+), 184 deletions(-)
>   create mode 100644 linux-user/plugin-api.c
>   create mode 100644 plugins/api-system.c
>   create mode 100644 plugins/api-user.c
>   create mode 100644 plugins/system.c
>   create mode 100644 plugins/user.c
> 


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

end of thread, other threads:[~2025-02-25 19:21 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-25 11:08 [PATCH 00/10] plugins: reduce total number of build objects Alex Bennée
2025-02-25 11:08 ` [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK Alex Bennée
2025-02-25 14:57   ` Philippe Mathieu-Daudé
2025-02-25 16:00   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 02/10] plugins/loader: populate target_name with target_name() Alex Bennée
2025-02-25 14:58   ` Philippe Mathieu-Daudé
2025-02-25 18:04   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h Alex Bennée
2025-02-25 18:10   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 04/10] plugins/api: clean-up the includes Alex Bennée
2025-02-25 18:14   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 05/10] plugins/plugin.h: include queue.h Alex Bennée
2025-02-25 18:15   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 06/10] plugins/loader: compile loader only once Alex Bennée
2025-02-25 18:18   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 07/10] plugins/api: split out binary path/start/end/entry code Alex Bennée
2025-02-25 18:32   ` Richard Henderson
2025-02-25 18:57     ` Alex Bennée
2025-02-25 11:08 ` [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers Alex Bennée
2025-02-25 18:40   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 09/10] plugins/api: split out time control helpers Alex Bennée
2025-02-25 18:45   ` Richard Henderson
2025-02-25 11:08 ` [PATCH 10/10] plugins/api: build only once Alex Bennée
2025-02-25 18:46   ` Richard Henderson
2025-02-25 19:21 ` [PATCH 00/10] plugins: reduce total number of build objects Pierrick Bouvier

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