[PATCH 00/10] plugins: reduce total number of build objects

[PATCH 00/10] plugins: reduce total number of build objects

[Alex Bennée]

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

[PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK

[Alex Bennée]

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

Re: [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK

[Philippe Mathieu-Daudé]

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>

Re: [PATCH 01/10] plugins/api: use tcg_ctx to get TARGET_PAGE_MASK

[Richard Henderson]

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~

[PATCH 02/10] plugins/loader: populate target_name with target_name()

[Alex Bennée]

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

Re: [PATCH 02/10] plugins/loader: populate target_name with target_name()

[Philippe Mathieu-Daudé]

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>

Re: [PATCH 02/10] plugins/loader: populate target_name with target_name()

[Richard Henderson]

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~

[PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h

[Alex Bennée]

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

Re: [PATCH 03/10] include/qemu: plugin-memory.h doesn't need cpu-defs.h

[Richard Henderson]

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~

[PATCH 04/10] plugins/api: clean-up the includes

[Alex Bennée]

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

Re: [PATCH 04/10] plugins/api: clean-up the includes

[Richard Henderson]

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~

[PATCH 05/10] plugins/plugin.h: include queue.h

[Alex Bennée]

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

Re: [PATCH 05/10] plugins/plugin.h: include queue.h

[Richard Henderson]

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~

[PATCH 06/10] plugins/loader: compile loader only once

[Alex Bennée]

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

Re: [PATCH 06/10] plugins/loader: compile loader only once

[Richard Henderson]

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~

[PATCH 07/10] plugins/api: split out binary path/start/end/entry code

[Alex Bennée]

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

Re: [PATCH 07/10] plugins/api: split out binary path/start/end/entry code

[Richard Henderson]

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~

Re: [PATCH 07/10] plugins/api: split out binary path/start/end/entry code

[Alex Bennée]

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

[PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers

[Alex Bennée]

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

Re: [PATCH 08/10] plugins/api: split out the vaddr/hwaddr helpers

[Richard Henderson]

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~

[PATCH 09/10] plugins/api: split out time control helpers

[Alex Bennée]

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

Re: [PATCH 09/10] plugins/api: split out time control helpers

[Richard Henderson]

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~

[PATCH 10/10] plugins/api: build only once

[Alex Bennée]

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

Re: [PATCH 10/10] plugins/api: build only once

[Richard Henderson]

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~

Re: [PATCH 00/10] plugins: reduce total number of build objects

[Pierrick Bouvier]

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
>