* [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers
@ 2025-06-11 23:24 Rowan Hart
2025-06-11 23:24 ` [PATCH v12 1/7] gdbstub: Expose gdb_write_register function to consumers of gdbstub Rowan Hart
` (9 more replies)
0 siblings, 10 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, Rowan Hart
This patch series adds several new API functions focused on enabling use
cases around reading and writing guest memory from QEMU plugins. To support
these new APIs, some utility functionality around retrieving information about
address spaces is added as well.
The new qemu_plugin_write_register utilizes gdb_write_register, which is now
declared in gdbstub.h for this purpose instead of being static.
qemu_plugin_write_memory_vaddr utilizes cpu_memory_rw_debug much the same as
the existing read_memory_vaddr function does.
The read and write_hwaddr functions are the most different. These functions
use address_space_rw, which works well in most cases. There is an important
caveat that for writes, the page being written will be set dirty by the
write operation. This dirty setting requires locking the page range,
which can contend with an already held lock in page_collection_lock
when called in a tb translate callback with a write to the instruction
memory in the tb. The doc comments warn against doing this, and it's unlikely
anyone would want to do this.
I've also added two test plugins: one that implements a simple hypercall
interface that guest code can use to communicate with the plugin in a
structured way with a test to ensure that this hypercall works and writing
virtual memory works. And one that implements a simple patch utility to patch
memory at runtime. The test for the second plugin ensures the patch applies
successfully to instruction memory, and can use both hw and vaddr methods.
For v3, I've had a few comments from the last submission that I've addressed,
and some that I haven't for one reason or another:
- Enforce QEMU_PLUGIN_CB_ flags in register read/write operations: done!
- Fix my commit messages and add long messages describing commits: done!
- Un-expose AS internals: done! Functions operate on current vCPU, current AS.
- Clean up use of current_cpu: done!
- Make functions take a vcpu_idx: not done. May revisit but it allows footguns.
Even for translation, seems best to not do this now. We can easily add _vcpu
versions of these functions in the future if we change our minds!
For v5, I've just updated the enforcement of the QEMU_PLUGIN_CB_ flags to just
use immediate stores, which simplifies the implementation quite a lot and
should be more efficient too. Thanks Pierrick for the suggestion!
v6 is a formatting pass, I left some whitespace that needed removal, some
license text was wrong, and so forth.
v8 reverts a mistake I made extending the size of arrays of TCGHelperInfo
structs, as I misunderstood their sizes. It preserves adding an explicit
zero as the last entry for clarity, however.
v9 fixes qemu_plugin_read_register to return -1 on parameter or flag state
error instead of 0.
In v10, I relaxed the restriction on when the register r/w functions can be
called, allowing all them to be used from any callback where the CPU is not
currently executing, with additional notes in the documentation for exceptions
(atexit and flush, which do not operate on a specific CPU and in which
current_cpu is not set).
v11 makes the cb flags functions inline and fixes a typo where cpu was asserted
but current_cpu was actually accessed.
v12 removes the hypercalls plugin because the functions it tested are also
tested by the patcher plugin, making it redundant. We'll circle back on a
hypercalls API in the future as a part of the plugin API, not as a plugin
itself.
Rowan Hart (1):
plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W
callbacks
novafacing (6):
gdbstub: Expose gdb_write_register function to consumers of gdbstub
plugins: Add register write API
plugins: Add memory virtual address write API
plugins: Add memory hardware address read/write API
plugins: Add patcher plugin and test
plugins: Update plugin version and add notes
accel/tcg/plugin-gen.c | 30 +++
gdbstub/gdbstub.c | 2 +-
include/exec/gdbstub.h | 14 ++
include/hw/core/cpu.h | 1 +
include/qemu/plugin.h | 15 ++
include/qemu/qemu-plugin.h | 176 ++++++++++++++--
plugins/api.c | 135 +++++++++++-
plugins/core.c | 33 +++
tests/tcg/Makefile.target | 1 +
tests/tcg/plugins/meson.build | 2 +-
tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
tests/tcg/x86_64/system/patch-target.c | 27 +++
tests/tcg/x86_64/system/validate-patch.py | 39 ++++
14 files changed, 725 insertions(+), 23 deletions(-)
create mode 100644 tests/tcg/plugins/patch.c
create mode 100644 tests/tcg/x86_64/system/patch-target.c
create mode 100755 tests/tcg/x86_64/system/validate-patch.py
--
2.49.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 1/7] gdbstub: Expose gdb_write_register function to consumers of gdbstub
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-11 23:24 ` [PATCH v12 2/7] plugins: Add register write API Rowan Hart
` (8 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, novafacing, Julian Ganz
From: novafacing <rowanbhart@gmail.com>
This patch exposes the gdb_write_register function from
gdbstub/gdbstub.c via the exec/gdbstub.h header file to support use in
plugins to write register contents.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Julian Ganz <neither@nut.email>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
gdbstub/gdbstub.c | 2 +-
include/exec/gdbstub.h | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index def0b7e877..dd5fb5667c 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -535,7 +535,7 @@ int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
return 0;
}
-static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
+int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
{
GDBRegisterState *r;
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 0675b0b646..a16c0051ce 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -124,6 +124,20 @@ const GDBFeature *gdb_find_static_feature(const char *xmlname);
*/
int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
+/**
+ * gdb_write_register() - Write a register associated with a CPU.
+ * @cpu: The CPU associated with the register.
+ * @buf: The buffer that the register contents will be set to.
+ * @reg: The register's number returned by gdb_find_feature_register().
+ *
+ * The size of @buf must be at least the size of the register being
+ * written.
+ *
+ * Return: The number of written bytes, or 0 if an error occurred (for
+ * example, an unknown register was provided).
+ */
+int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg);
+
/**
* typedef GDBRegDesc - a register description from gdbstub
*/
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 2/7] plugins: Add register write API
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
2025-06-11 23:24 ` [PATCH v12 1/7] gdbstub: Expose gdb_write_register function to consumers of gdbstub Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-11 23:24 ` [PATCH v12 3/7] plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W callbacks Rowan Hart
` (7 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, novafacing
From: novafacing <rowanbhart@gmail.com>
This patch adds a function to the plugins API to allow plugins to write
register contents. It also moves the qemu_plugin_read_register function
so all the register-related functions are grouped together in the file.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
include/qemu/qemu-plugin.h | 54 ++++++++++++++++++++++++++------------
plugins/api.c | 26 +++++++++++++-----
2 files changed, 56 insertions(+), 24 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 3a850aa216..cfe1692ecb 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -871,7 +871,8 @@ struct qemu_plugin_register;
/**
* typedef qemu_plugin_reg_descriptor - register descriptions
*
- * @handle: opaque handle for retrieving value with qemu_plugin_read_register
+ * @handle: opaque handle for retrieving value with qemu_plugin_read_register or
+ * writing value with qemu_plugin_write_register
* @name: register name
* @feature: optional feature descriptor, can be NULL
*/
@@ -893,6 +894,41 @@ typedef struct {
QEMU_PLUGIN_API
GArray *qemu_plugin_get_registers(void);
+/**
+ * qemu_plugin_read_register() - read register for current vCPU
+ *
+ * @handle: a @qemu_plugin_reg_handle handle
+ * @buf: A GByteArray for the data owned by the plugin
+ *
+ * This function is only available in a context that register read access is
+ * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag.
+ *
+ * Returns the size of the read register. The content of @buf is in target byte
+ * order. On failure returns -1.
+ */
+QEMU_PLUGIN_API
+int qemu_plugin_read_register(struct qemu_plugin_register *handle,
+ GByteArray *buf);
+
+/**
+ * qemu_plugin_write_register() - write register for current vCPU
+ *
+ * @handle: a @qemu_plugin_reg_handle handle
+ * @buf: A GByteArray for the data owned by the plugin
+ *
+ * This function is only available in a context that register write access is
+ * explicitly requested via the QEMU_PLUGIN_CB_RW_REGS flag.
+ *
+ * The size of @buf must be at least the size of the requested register.
+ * Attempting to write a register with @buf smaller than the register size
+ * will result in a crash or other undesired behavior.
+ *
+ * Returns the number of bytes written. On failure returns 0.
+ */
+QEMU_PLUGIN_API
+int qemu_plugin_write_register(struct qemu_plugin_register *handle,
+ GByteArray *buf);
+
/**
* qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
*
@@ -915,22 +951,6 @@ QEMU_PLUGIN_API
bool qemu_plugin_read_memory_vaddr(uint64_t addr,
GByteArray *data, size_t len);
-/**
- * qemu_plugin_read_register() - read register for current vCPU
- *
- * @handle: a @qemu_plugin_reg_handle handle
- * @buf: A GByteArray for the data owned by the plugin
- *
- * This function is only available in a context that register read access is
- * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag.
- *
- * Returns the size of the read register. The content of @buf is in target byte
- * order. On failure returns -1.
- */
-QEMU_PLUGIN_API
-int qemu_plugin_read_register(struct qemu_plugin_register *handle,
- GByteArray *buf);
-
/**
* qemu_plugin_scoreboard_new() - alloc a new scoreboard
*
diff --git a/plugins/api.c b/plugins/api.c
index 3c9d4832e9..6514f2c76a 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -433,6 +433,25 @@ GArray *qemu_plugin_get_registers(void)
return create_register_handles(regs);
}
+int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
+{
+ g_assert(current_cpu);
+
+ return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
+}
+
+int qemu_plugin_write_register(struct qemu_plugin_register *reg,
+ GByteArray *buf)
+{
+ g_assert(current_cpu);
+
+ if (buf->len == 0 || qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS) {
+ return -1;
+ }
+
+ return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1);
+}
+
bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
{
g_assert(current_cpu);
@@ -453,13 +472,6 @@ bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
return true;
}
-int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
-{
- g_assert(current_cpu);
-
- return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
-}
-
struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
{
return plugin_scoreboard_new(element_size);
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 3/7] plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W callbacks
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
2025-06-11 23:24 ` [PATCH v12 1/7] gdbstub: Expose gdb_write_register function to consumers of gdbstub Rowan Hart
2025-06-11 23:24 ` [PATCH v12 2/7] plugins: Add register write API Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-11 23:24 ` [PATCH v12 4/7] plugins: Add memory virtual address write API Rowan Hart
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, Rowan Hart
This patch adds functionality to enforce the requested QEMU_PLUGIN_CB_
flags level passed when registering a callback function using the
plugins API. Each time a callback is about to be invoked, a thread-local
variable will be updated with the level that callback requested. Then,
called API functions (in particular, the register read and write API)
will call qemu_plugin_get_cb_flags() to check the level is at least the
level they require.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
accel/tcg/plugin-gen.c | 30 ++++++++++++++++++++++++++++++
include/hw/core/cpu.h | 1 +
include/qemu/plugin.h | 15 +++++++++++++++
include/qemu/qemu-plugin.h | 19 +++++++++++++------
plugins/api.c | 4 ++++
plugins/core.c | 33 +++++++++++++++++++++++++++++++++
6 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index c1da753894..9920381a84 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -117,10 +117,20 @@ static TCGv_i32 gen_cpu_index(void)
static void gen_udata_cb(struct qemu_plugin_regular_cb *cb)
{
TCGv_i32 cpu_index = gen_cpu_index();
+ enum qemu_plugin_cb_flags cb_flags =
+ tcg_call_to_qemu_plugin_cb_flags(cb->info->flags);
+ TCGv_i32 flags = tcg_constant_i32(cb_flags);
+ TCGv_i32 clear_flags = tcg_constant_i32(QEMU_PLUGIN_CB_NO_REGS);
+ tcg_gen_st_i32(flags, tcg_env,
+ offsetof(CPUState, neg.plugin_cb_flags) - sizeof(CPUState));
tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
+ tcg_gen_st_i32(clear_flags, tcg_env,
+ offsetof(CPUState, neg.plugin_cb_flags) - sizeof(CPUState));
tcg_temp_free_i32(cpu_index);
+ tcg_temp_free_i32(flags);
+ tcg_temp_free_i32(clear_flags);
}
static TCGv_ptr gen_plugin_u64_ptr(qemu_plugin_u64 entry)
@@ -173,10 +183,20 @@ static void gen_udata_cond_cb(struct qemu_plugin_conditional_cb *cb)
tcg_gen_ld_i64(val, ptr, 0);
tcg_gen_brcondi_i64(cond, val, cb->imm, after_cb);
TCGv_i32 cpu_index = gen_cpu_index();
+ enum qemu_plugin_cb_flags cb_flags =
+ tcg_call_to_qemu_plugin_cb_flags(cb->info->flags);
+ TCGv_i32 flags = tcg_constant_i32(cb_flags);
+ TCGv_i32 clear_flags = tcg_constant_i32(QEMU_PLUGIN_CB_NO_REGS);
+ tcg_gen_st_i32(flags, tcg_env,
+ offsetof(CPUState, neg.plugin_cb_flags) - sizeof(CPUState));
tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
+ tcg_gen_st_i32(clear_flags, tcg_env,
+ offsetof(CPUState, neg.plugin_cb_flags) - sizeof(CPUState));
tcg_temp_free_i32(cpu_index);
+ tcg_temp_free_i32(flags);
+ tcg_temp_free_i32(clear_flags);
gen_set_label(after_cb);
tcg_temp_free_i64(val);
@@ -210,12 +230,22 @@ static void gen_mem_cb(struct qemu_plugin_regular_cb *cb,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
{
TCGv_i32 cpu_index = gen_cpu_index();
+ enum qemu_plugin_cb_flags cb_flags =
+ tcg_call_to_qemu_plugin_cb_flags(cb->info->flags);
+ TCGv_i32 flags = tcg_constant_i32(cb_flags);
+ TCGv_i32 clear_flags = tcg_constant_i32(QEMU_PLUGIN_CB_NO_REGS);
+ tcg_gen_st_i32(flags, tcg_env,
+ offsetof(CPUState, neg.plugin_cb_flags) - sizeof(CPUState));
tcg_gen_call4(cb->f.vcpu_mem, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_i32_temp(tcg_constant_i32(meminfo)),
tcgv_i64_temp(addr),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
+ tcg_gen_st_i32(clear_flags, tcg_env,
+ offsetof(CPUState, neg.plugin_cb_flags) - sizeof(CPUState));
tcg_temp_free_i32(cpu_index);
+ tcg_temp_free_i32(flags);
+ tcg_temp_free_i32(clear_flags);
}
static void inject_cb(struct qemu_plugin_dyn_cb *cb)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 33296a1c08..162a56a5da 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -368,6 +368,7 @@ typedef struct CPUNegativeOffsetState {
GArray *plugin_mem_cbs;
uint64_t plugin_mem_value_low;
uint64_t plugin_mem_value_high;
+ int32_t plugin_cb_flags;
#endif
IcountDecr icount_decr;
bool can_do_io;
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 9726a9ebf3..f355c7cb8a 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -209,6 +209,21 @@ void qemu_plugin_user_prefork_lock(void);
*/
void qemu_plugin_user_postfork(bool is_child);
+enum qemu_plugin_cb_flags tcg_call_to_qemu_plugin_cb_flags(int flags);
+
+static inline void qemu_plugin_set_cb_flags(CPUState *cpu,
+ enum qemu_plugin_cb_flags flags)
+{
+ assert(cpu);
+ cpu->neg.plugin_cb_flags = flags;
+}
+
+static inline enum qemu_plugin_cb_flags qemu_plugin_get_cb_flags(void)
+{
+ assert(current_cpu);
+ return current_cpu->neg.plugin_cb_flags;
+}
+
#else /* !CONFIG_PLUGIN */
static inline void qemu_plugin_add_opts(void)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index cfe1692ecb..9c9ebf6ce0 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -254,9 +254,6 @@ typedef struct {
* @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
* @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
* @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
- *
- * Note: currently QEMU_PLUGIN_CB_RW_REGS is unused, plugins cannot change
- * system register state.
*/
enum qemu_plugin_cb_flags {
QEMU_PLUGIN_CB_NO_REGS,
@@ -901,7 +898,12 @@ GArray *qemu_plugin_get_registers(void);
* @buf: A GByteArray for the data owned by the plugin
*
* This function is only available in a context that register read access is
- * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag.
+ * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag, if called inside a
+ * callback that can be registered with a qemu_plugin_cb_flags argument. This
+ * function can also be used in any callback context that does not use a flags
+ * argument, such as in a callback registered with
+ * qemu_plugin_register_vcpu_init_cb(), except for callbacks registered with
+ * qemu_plugin_register_atexit_cb() and qemu_plugin_register_flush_cb().
*
* Returns the size of the read register. The content of @buf is in target byte
* order. On failure returns -1.
@@ -916,8 +918,13 @@ int qemu_plugin_read_register(struct qemu_plugin_register *handle,
* @handle: a @qemu_plugin_reg_handle handle
* @buf: A GByteArray for the data owned by the plugin
*
- * This function is only available in a context that register write access is
- * explicitly requested via the QEMU_PLUGIN_CB_RW_REGS flag.
+ * This function is only available in a context that register read access is
+ * explicitly requested via the QEMU_PLUGIN_CB_RW_REGS flag, if called inside a
+ * callback that can be registered with a qemu_plugin_cb_flags argument. This
+ * function can also be used in any callback context that does not use a flags
+ * argument, such as in a callback registered with
+ * qemu_plugin_register_vcpu_init_cb(), except for callbacks registered with
+ * qemu_plugin_register_atexit_cb() and qemu_plugin_register_flush_cb().
*
* The size of @buf must be at least the size of the requested register.
* Attempting to write a register with @buf smaller than the register size
diff --git a/plugins/api.c b/plugins/api.c
index 6514f2c76a..3f04399c26 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -437,6 +437,10 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
{
g_assert(current_cpu);
+ if (qemu_plugin_get_cb_flags() == QEMU_PLUGIN_CB_NO_REGS) {
+ return -1;
+ }
+
return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
}
diff --git a/plugins/core.c b/plugins/core.c
index eb9281fe54..6ae5bbc0ef 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -15,6 +15,7 @@
#include "qemu/lockable.h"
#include "qemu/option.h"
#include "qemu/plugin.h"
+#include "qemu/qemu-plugin.h"
#include "qemu/queue.h"
#include "qemu/rcu_queue.h"
#include "qemu/rcu.h"
@@ -266,7 +267,9 @@ static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data unused)
plugin_grow_scoreboards__locked(cpu);
qemu_rec_mutex_unlock(&plugin.lock);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_INIT);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
void qemu_plugin_vcpu_init_hook(CPUState *cpu)
@@ -279,7 +282,9 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
{
bool success;
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_EXIT);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
qemu_rec_mutex_lock(&plugin.lock);
@@ -367,6 +372,7 @@ void plugin_register_dyn_cb__udata(GArray **arr,
static TCGHelperInfo info[3] = {
[QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
[QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
+ [QEMU_PLUGIN_CB_RW_REGS].flags = 0,
/*
* Match qemu_plugin_vcpu_udata_cb_t:
* void (*)(uint32_t, void *)
@@ -396,6 +402,7 @@ void plugin_register_dyn_cond_cb__udata(GArray **arr,
static TCGHelperInfo info[3] = {
[QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
[QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
+ [QEMU_PLUGIN_CB_RW_REGS].flags = 0,
/*
* Match qemu_plugin_vcpu_udata_cb_t:
* void (*)(uint32_t, void *)
@@ -434,6 +441,7 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
static TCGHelperInfo info[3] = {
[QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
[QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
+ [QEMU_PLUGIN_CB_RW_REGS].flags = 0,
/*
* Match qemu_plugin_vcpu_mem_cb_t:
* void (*)(uint32_t, qemu_plugin_meminfo_t, uint64_t, void *)
@@ -473,7 +481,9 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_tb_trans_cb_t func = cb->f.vcpu_tb_trans;
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
func(cb->ctx->id, tb);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
}
@@ -498,7 +508,9 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_syscall_cb_t func = cb->f.vcpu_syscall;
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
func(cb->ctx->id, cpu->cpu_index, num, a1, a2, a3, a4, a5, a6, a7, a8);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
}
@@ -520,7 +532,9 @@ void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_syscall_ret_cb_t func = cb->f.vcpu_syscall_ret;
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
func(cb->ctx->id, cpu->cpu_index, num, ret);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
}
@@ -528,14 +542,18 @@ void qemu_plugin_vcpu_idle_cb(CPUState *cpu)
{
/* idle and resume cb may be called before init, ignore in this case */
if (cpu->cpu_index < plugin.num_vcpus) {
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_IDLE);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
}
void qemu_plugin_vcpu_resume_cb(CPUState *cpu)
{
if (cpu->cpu_index < plugin.num_vcpus) {
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_RESUME);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
}
@@ -615,9 +633,13 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
switch (cb->type) {
case PLUGIN_CB_MEM_REGULAR:
if (rw & cb->regular.rw) {
+ qemu_plugin_set_cb_flags(cpu,
+ tcg_call_to_qemu_plugin_cb_flags(cb->regular.info->flags));
+
cb->regular.f.vcpu_mem(cpu->cpu_index,
make_plugin_meminfo(oi, rw),
vaddr, cb->regular.userp);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
break;
case PLUGIN_CB_INLINE_ADD_U64:
@@ -760,3 +782,14 @@ void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score)
g_array_free(score->data, TRUE);
g_free(score);
}
+
+enum qemu_plugin_cb_flags tcg_call_to_qemu_plugin_cb_flags(int flags)
+{
+ if (flags & TCG_CALL_NO_RWG) {
+ return QEMU_PLUGIN_CB_NO_REGS;
+ } else if (flags & TCG_CALL_NO_WG) {
+ return QEMU_PLUGIN_CB_R_REGS;
+ } else {
+ return QEMU_PLUGIN_CB_RW_REGS;
+ }
+}
\ No newline at end of file
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 4/7] plugins: Add memory virtual address write API
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (2 preceding siblings ...)
2025-06-11 23:24 ` [PATCH v12 3/7] plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W callbacks Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-11 23:24 ` [PATCH v12 5/7] plugins: Add memory hardware address read/write API Rowan Hart
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, novafacing
From: novafacing <rowanbhart@gmail.com>
This patch adds functions to the plugins API to allow reading and
writing memory via virtual addresses. These functions only permit doing
so on the current CPU, because there is no way to ensure consistency if
plugins are allowed to read or write to other CPUs that aren't currently
in the context of the plugin.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
include/qemu/qemu-plugin.h | 21 +++++++++++++++++++++
plugins/api.c | 18 ++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 9c9ebf6ce0..4167c46c2a 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -958,6 +958,27 @@ QEMU_PLUGIN_API
bool qemu_plugin_read_memory_vaddr(uint64_t addr,
GByteArray *data, size_t len);
+/**
+ * qemu_plugin_write_memory_vaddr() - write to memory using a virtual address
+ *
+ * @addr: A virtual address to write to
+ * @data: A byte array containing the data to write
+ *
+ * The contents of @data will be written to memory starting at the virtual
+ * address @addr.
+ *
+ * This function does not guarantee consistency of writes, nor does it ensure
+ * that pending writes are flushed either before or after the write takes place,
+ * so callers should take care to only call this function in vCPU context (i.e.
+ * in callbacks) and avoid depending on the existence of data written using this
+ * function which may be overwritten afterward.
+ *
+ * Returns true on success and false on failure.
+ */
+QEMU_PLUGIN_API
+bool qemu_plugin_write_memory_vaddr(uint64_t addr,
+ GByteArray *data);
+
/**
* qemu_plugin_scoreboard_new() - alloc a new scoreboard
*
diff --git a/plugins/api.c b/plugins/api.c
index 3f04399c26..1f64a9ea64 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -476,6 +476,24 @@ bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
return true;
}
+bool qemu_plugin_write_memory_vaddr(uint64_t addr, GByteArray *data)
+{
+ g_assert(current_cpu);
+
+ if (data->len == 0) {
+ return false;
+ }
+
+ int result = cpu_memory_rw_debug(current_cpu, addr, data->data,
+ data->len, true);
+
+ if (result < 0) {
+ return false;
+ }
+
+ return true;
+}
+
struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
{
return plugin_scoreboard_new(element_size);
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 5/7] plugins: Add memory hardware address read/write API
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (3 preceding siblings ...)
2025-06-11 23:24 ` [PATCH v12 4/7] plugins: Add memory virtual address write API Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-17 10:24 ` Alex Bennée
2025-06-11 23:24 ` [PATCH v12 6/7] plugins: Add patcher plugin and test Rowan Hart
` (4 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, novafacing
From: novafacing <rowanbhart@gmail.com>
This patch adds functions to the plugins API to allow plugins to read
and write memory via hardware addresses. The functions use the current
address space of the current CPU in order to avoid exposing address
space information to users. A later patch may want to add a function to
permit a specified address space, for example to facilitate
architecture-specific plugins that want to operate on them, for example
reading ARM secure memory.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
include/qemu/qemu-plugin.h | 93 ++++++++++++++++++++++++++++++++++++
plugins/api.c | 97 ++++++++++++++++++++++++++++++++++++++
2 files changed, 190 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4167c46c2a..5eecdccc67 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -979,6 +979,99 @@ QEMU_PLUGIN_API
bool qemu_plugin_write_memory_vaddr(uint64_t addr,
GByteArray *data);
+/**
+ * enum qemu_plugin_hwaddr_operation_result - result of a memory operation
+ *
+ * @QEMU_PLUGIN_HWADDR_OPERATION_OK: hwaddr operation succeeded
+ * @QEMU_PLUGIN_HWADDR_OPERATION_ERROR: unexpected error occurred
+ * @QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR: error in memory device
+ * @QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED: permission error
+ * @QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS: address was invalid
+ * @QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE: invalid address space
+ */
+enum qemu_plugin_hwaddr_operation_result {
+ QEMU_PLUGIN_HWADDR_OPERATION_OK,
+ QEMU_PLUGIN_HWADDR_OPERATION_ERROR,
+ QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR,
+ QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED,
+ QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS,
+ QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE,
+};
+
+/**
+ * qemu_plugin_read_memory_hwaddr() - read from memory using a hardware address
+ *
+ * @addr: The physical address to read from
+ * @data: A byte array to store data into
+ * @len: The number of bytes to read, starting from @addr
+ *
+ * @len bytes of data is read from the current memory space for the current
+ * vCPU starting at @addr and stored into @data. If @data is not large enough to
+ * hold @len bytes, it will be expanded to the necessary size, reallocating if
+ * necessary. @len must be greater than 0.
+ *
+ * This function does not ensure writes are flushed prior to reading, so
+ * callers should take care when calling this function in plugin callbacks to
+ * avoid attempting to read data which may not yet be written and should use
+ * the memory callback API instead.
+ *
+ * This function is only valid for softmmu targets.
+ *
+ * Returns a qemu_plugin_hwaddr_operation_result indicating the result of the
+ * operation.
+ */
+QEMU_PLUGIN_API
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_read_memory_hwaddr(uint64_t addr, GByteArray *data, size_t len);
+
+/**
+ * qemu_plugin_write_memory_hwaddr() - write to memory using a hardware address
+ *
+ * @addr: A physical address to write to
+ * @data: A byte array containing the data to write
+ *
+ * The contents of @data will be written to memory starting at the hardware
+ * address @addr in the current address space for the current vCPU.
+ *
+ * This function does not guarantee consistency of writes, nor does it ensure
+ * that pending writes are flushed either before or after the write takes place,
+ * so callers should take care when calling this function in plugin callbacks to
+ * avoid depending on the existence of data written using this function which
+ * may be overwritten afterward. In addition, this function requires that the
+ * pages containing the address are not locked. Practically, this means that you
+ * should not write instruction memory in a current translation block inside a
+ * callback registered with qemu_plugin_register_vcpu_tb_trans_cb.
+ *
+ * You can, for example, write instruction memory in a current translation block
+ * in a callback registered with qemu_plugin_register_vcpu_tb_exec_cb, although
+ * be aware that the write will not be flushed until after the translation block
+ * has finished executing. In general, this function should be used to write
+ * data memory or to patch code at a known address, not in a current translation
+ * block.
+ *
+ * This function is only valid for softmmu targets.
+ *
+ * Returns a qemu_plugin_hwaddr_operation_result indicating the result of the
+ * operation.
+ */
+QEMU_PLUGIN_API
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_write_memory_hwaddr(uint64_t addr, GByteArray *data);
+
+/**
+ * qemu_plugin_translate_vaddr() - translate virtual address for current vCPU
+ *
+ * @vaddr: virtual address to translate
+ * @hwaddr: pointer to store the physical address
+ *
+ * This function is only valid in vCPU context (i.e. in callbacks) and is only
+ * valid for softmmu targets.
+ *
+ * Returns true on success and false on failure.
+ */
+QEMU_PLUGIN_API
+bool qemu_plugin_translate_vaddr(uint64_t vaddr, uint64_t *hwaddr);
+
/**
* qemu_plugin_scoreboard_new() - alloc a new scoreboard
*
diff --git a/plugins/api.c b/plugins/api.c
index 1f64a9ea64..eac04cc1f6 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -39,6 +39,7 @@
#include "qemu/main-loop.h"
#include "qemu/plugin.h"
#include "qemu/log.h"
+#include "system/memory.h"
#include "tcg/tcg.h"
#include "exec/gdbstub.h"
#include "exec/target_page.h"
@@ -494,6 +495,102 @@ bool qemu_plugin_write_memory_vaddr(uint64_t addr, GByteArray *data)
return true;
}
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_read_memory_hwaddr(hwaddr addr, GByteArray *data, size_t len)
+{
+#ifdef CONFIG_SOFTMMU
+ if (len == 0) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+ }
+
+ g_assert(current_cpu);
+
+
+ int as_idx = cpu_asidx_from_attrs(current_cpu, MEMTXATTRS_UNSPECIFIED);
+ AddressSpace *as = cpu_get_address_space(current_cpu, as_idx);
+
+ if (as == NULL) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE;
+ }
+
+ g_byte_array_set_size(data, len);
+ MemTxResult res = address_space_rw(as, addr,
+ MEMTXATTRS_UNSPECIFIED, data->data,
+ data->len, false);
+
+ switch (res) {
+ case MEMTX_OK:
+ return QEMU_PLUGIN_HWADDR_OPERATION_OK;
+ case MEMTX_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR;
+ case MEMTX_DECODE_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS;
+ case MEMTX_ACCESS_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED;
+ default:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+ }
+#else
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+#endif
+}
+
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_write_memory_hwaddr(hwaddr addr, GByteArray *data)
+{
+#ifdef CONFIG_SOFTMMU
+ if (data->len == 0) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+ }
+
+ g_assert(current_cpu);
+
+ int as_idx = cpu_asidx_from_attrs(current_cpu, MEMTXATTRS_UNSPECIFIED);
+ AddressSpace *as = cpu_get_address_space(current_cpu, as_idx);
+
+ if (as == NULL) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE;
+ }
+
+ MemTxResult res = address_space_rw(as, addr,
+ MEMTXATTRS_UNSPECIFIED, data->data,
+ data->len, true);
+ switch (res) {
+ case MEMTX_OK:
+ return QEMU_PLUGIN_HWADDR_OPERATION_OK;
+ case MEMTX_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR;
+ case MEMTX_DECODE_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS;
+ case MEMTX_ACCESS_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED;
+ default:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+ }
+#else
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+#endif
+}
+
+bool qemu_plugin_translate_vaddr(uint64_t vaddr, uint64_t *hwaddr)
+{
+#ifdef CONFIG_SOFTMMU
+ g_assert(current_cpu);
+
+ uint64_t res = cpu_get_phys_page_debug(current_cpu, vaddr);
+
+ if (res == (uint64_t)-1) {
+ return false;
+ }
+
+ *hwaddr = res | (vaddr & ~TARGET_PAGE_MASK);
+
+ return true;
+#else
+ return false;
+#endif
+}
+
struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
{
return plugin_scoreboard_new(element_size);
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 6/7] plugins: Add patcher plugin and test
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (4 preceding siblings ...)
2025-06-11 23:24 ` [PATCH v12 5/7] plugins: Add memory hardware address read/write API Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-13 15:19 ` Pierrick Bouvier
2025-06-17 10:35 ` Alex Bennée
2025-06-11 23:24 ` [PATCH v12 7/7] plugins: Update plugin version and add notes Rowan Hart
` (3 subsequent siblings)
9 siblings, 2 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, novafacing
From: novafacing <rowanbhart@gmail.com>
This patch adds a plugin that exercises the virtual and hardware memory
read-write API functions added in a previous patch. The plugin takes a
target and patch byte sequence, and will overwrite any instruction
matching the target byte sequence with the patch.
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
tests/tcg/Makefile.target | 1 +
tests/tcg/plugins/meson.build | 2 +-
tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
tests/tcg/x86_64/system/patch-target.c | 27 +++
tests/tcg/x86_64/system/validate-patch.py | 39 ++++
6 files changed, 336 insertions(+), 6 deletions(-)
create mode 100644 tests/tcg/plugins/patch.c
create mode 100644 tests/tcg/x86_64/system/patch-target.c
create mode 100755 tests/tcg/x86_64/system/validate-patch.py
diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 95ff76ea44..4b709a9d18 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -176,6 +176,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
# Some plugins need additional arguments above the default to fully
# exercise things. We can define them on a per-test basis here.
run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true
+run-plugin-%-with-libpatch.so: PLUGIN_ARGS=$(COMMA)target=ffffffff$(COMMA)patch=00000000
ifeq ($(filter %-softmmu, $(TARGET)),)
run-%: %
diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
index 41f02f2c7f..163042e601 100644
--- a/tests/tcg/plugins/meson.build
+++ b/tests/tcg/plugins/meson.build
@@ -1,6 +1,6 @@
t = []
if get_option('plugins')
- foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall']
+ foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall', 'patch']
if host_os == 'windows'
t += shared_module(i, files(i + '.c') + '../../../contrib/plugins/win32_linker.c',
include_directories: '../../../include/qemu',
diff --git a/tests/tcg/plugins/patch.c b/tests/tcg/plugins/patch.c
new file mode 100644
index 0000000000..450fc51c88
--- /dev/null
+++ b/tests/tcg/plugins/patch.c
@@ -0,0 +1,241 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This plugin patches instructions matching a pattern to a different
+ * instruction as they execute
+ *
+ */
+
+#include "glib.h"
+#include "glibconfig.h"
+
+#include <qemu-plugin.h>
+#include <string.h>
+#include <stdio.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+static bool use_hwaddr;
+static GByteArray *target_data;
+static GByteArray *patch_data;
+
+/**
+ * Parse a string of hexadecimal digits into a GByteArray. The string must be
+ * even length
+ */
+static GByteArray *str_to_bytes(const char *str)
+{
+ size_t len = strlen(str);
+
+ if (len == 0 || len % 2 != 0) {
+ return NULL;
+ }
+
+ GByteArray *bytes = g_byte_array_new();
+ char byte[3] = {0};
+ guint8 value = 0;
+
+ for (size_t i = 0; i < len; i += 2) {
+ byte[0] = str[i];
+ byte[1] = str[i + 1];
+ value = (guint8)g_ascii_strtoull(byte, NULL, 16);
+ g_byte_array_append(bytes, &value, 1);
+ }
+
+ return bytes;
+}
+
+static void patch_hwaddr(unsigned int vcpu_index, void *userdata)
+{
+ uint64_t addr = (uint64_t)userdata;
+ g_autoptr(GString) str = g_string_new(NULL);
+ g_string_printf(str, "patching: @0x%"
+ PRIx64 "\n",
+ addr);
+ qemu_plugin_outs(str->str);
+
+ enum qemu_plugin_hwaddr_operation_result result =
+ qemu_plugin_write_memory_hwaddr(addr, patch_data);
+
+
+ if (result != QEMU_PLUGIN_HWADDR_OPERATION_OK) {
+ g_autoptr(GString) errmsg = g_string_new(NULL);
+ g_string_printf(errmsg, "Failed to write memory: %d\n", result);
+ qemu_plugin_outs(errmsg->str);
+ return;
+ }
+
+ GByteArray *read_data = g_byte_array_new();
+
+ result = qemu_plugin_read_memory_hwaddr(addr, read_data,
+ patch_data->len);
+
+ qemu_plugin_outs("Reading memory...\n");
+
+ if (result != QEMU_PLUGIN_HWADDR_OPERATION_OK) {
+ g_autoptr(GString) errmsg = g_string_new(NULL);
+ g_string_printf(errmsg, "Failed to read memory: %d\n", result);
+ qemu_plugin_outs(errmsg->str);
+ return;
+ }
+
+ if (memcmp(patch_data->data, read_data->data, patch_data->len) != 0) {
+ qemu_plugin_outs("Failed to read back written data\n");
+ }
+
+ qemu_plugin_outs("Success!\n");
+
+ return;
+}
+
+static void patch_vaddr(unsigned int vcpu_index, void *userdata)
+{
+ uint64_t addr = (uint64_t)userdata;
+ uint64_t hwaddr = 0;
+ if (!qemu_plugin_translate_vaddr(addr, &hwaddr)) {
+ qemu_plugin_outs("Failed to translate vaddr\n");
+ return;
+ }
+ g_autoptr(GString) str = g_string_new(NULL);
+ g_string_printf(str, "patching: @0x%"
+ PRIx64 " hw: @0x%" PRIx64 "\n",
+ addr, hwaddr);
+ qemu_plugin_outs(str->str);
+
+ qemu_plugin_outs("Writing memory (vaddr)...\n");
+
+ if (!qemu_plugin_write_memory_vaddr(addr, patch_data)) {
+ qemu_plugin_outs("Failed to write memory\n");
+ return;
+ }
+
+ qemu_plugin_outs("Reading memory (vaddr)...\n");
+
+ g_autoptr(GByteArray) read_data = g_byte_array_new();
+
+ if (!qemu_plugin_read_memory_vaddr(addr, read_data, patch_data->len)) {
+ qemu_plugin_outs("Failed to read memory\n");
+ return;
+ }
+
+ if (memcmp(patch_data->data, read_data->data, patch_data->len) != 0) {
+ qemu_plugin_outs("Failed to read back written data\n");
+ }
+
+ qemu_plugin_outs("Success!\n");
+
+ return;
+}
+
+/*
+ * Callback on translation of a translation block.
+ */
+static void vcpu_tb_trans_cb(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+ uint64_t addr = 0;
+ g_autoptr(GByteArray) insn_data = g_byte_array_new();
+ for (size_t i = 0; i < qemu_plugin_tb_n_insns(tb); i++) {
+ struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
+
+ if (use_hwaddr) {
+ uint64_t vaddr = qemu_plugin_insn_vaddr(insn);
+ if (!qemu_plugin_translate_vaddr(vaddr, &addr)) {
+ qemu_plugin_outs("Failed to translate vaddr\n");
+ continue;
+ }
+ } else {
+ addr = qemu_plugin_insn_vaddr(insn);
+ }
+
+ g_byte_array_set_size(insn_data, qemu_plugin_insn_size(insn));
+ qemu_plugin_insn_data(insn, insn_data->data, insn_data->len);
+
+ if (insn_data->len >= target_data->len &&
+ !memcmp(insn_data->data, target_data->data,
+ MIN(target_data->len, insn_data->len))) {
+ if (use_hwaddr) {
+ qemu_plugin_register_vcpu_tb_exec_cb(tb, patch_hwaddr,
+ QEMU_PLUGIN_CB_NO_REGS,
+ (void *)addr);
+ } else {
+ qemu_plugin_register_vcpu_tb_exec_cb(tb, patch_vaddr,
+ QEMU_PLUGIN_CB_NO_REGS,
+ (void *)addr);
+ }
+ }
+ }
+}
+
+static void usage(void)
+{
+ fprintf(stderr, "Usage: <lib>,target=<bytes>,patch=<new_bytes>"
+ "[,use_hwaddr=true|false]");
+}
+
+/*
+ * Called when the plugin is installed
+ */
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+ const qemu_info_t *info, int argc,
+ char **argv)
+{
+
+ use_hwaddr = true;
+ target_data = NULL;
+ patch_data = NULL;
+
+ if (argc > 4) {
+ usage();
+ return -1;
+ }
+
+ for (size_t i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
+ if (g_strcmp0(tokens[0], "use_hwaddr") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &use_hwaddr)) {
+ fprintf(stderr,
+ "Failed to parse boolean argument use_hwaddr\n");
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "target") == 0) {
+ target_data = str_to_bytes(tokens[1]);
+ if (!target_data) {
+ fprintf(stderr,
+ "Failed to parse target bytes.\n");
+ return -1;
+ }
+ } else if (g_strcmp0(tokens[0], "patch") == 0) {
+ patch_data = str_to_bytes(tokens[1]);
+ if (!patch_data) {
+ fprintf(stderr, "Failed to parse patch bytes.\n");
+ return -1;
+ }
+ } else {
+ fprintf(stderr, "Unknown argument: %s\n", tokens[0]);
+ usage();
+ return -1;
+ }
+ }
+
+ if (!target_data) {
+ fprintf(stderr, "target argument is required\n");
+ usage();
+ return -1;
+ }
+
+ if (!patch_data) {
+ fprintf(stderr, "patch argument is required\n");
+ usage();
+ return -1;
+ }
+
+ if (target_data->len != patch_data->len) {
+ fprintf(stderr, "Target and patch data must be the same length\n");
+ return -1;
+ }
+
+ qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans_cb);
+
+ return 0;
+}
diff --git a/tests/tcg/x86_64/Makefile.softmmu-target b/tests/tcg/x86_64/Makefile.softmmu-target
index ef6bcb4dc7..154910ab72 100644
--- a/tests/tcg/x86_64/Makefile.softmmu-target
+++ b/tests/tcg/x86_64/Makefile.softmmu-target
@@ -7,18 +7,27 @@
#
I386_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/i386/system
-X64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
+X86_64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
# These objects provide the basic boot code and helper functions for all tests
CRT_OBJS=boot.o
-CRT_PATH=$(X64_SYSTEM_SRC)
-LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld
+X86_64_TEST_C_SRCS=$(wildcard $(X86_64_SYSTEM_SRC)/*.c)
+X86_64_TEST_S_SRCS=
+
+X86_64_C_TESTS = $(patsubst $(X86_64_SYSTEM_SRC)/%.c, %, $(X86_64_TEST_C_SRCS))
+X86_64_S_TESTS = $(patsubst $(X86_64_SYSTEM_SRC)/%.S, %, $(X86_64_TEST_S_SRCS))
+
+X86_64_TESTS = $(X86_64_C_TESTS)
+X86_64_TESTS += $(X86_64_S_TESTS)
+
+CRT_PATH=$(X86_64_SYSTEM_SRC)
+LINK_SCRIPT=$(X86_64_SYSTEM_SRC)/kernel.ld
LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_x86_64
CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
-TESTS+=$(MULTIARCH_TESTS)
+TESTS+=$(X86_64_TESTS) $(MULTIARCH_TESTS)
EXTRA_RUNS+=$(MULTIARCH_RUNS)
# building head blobs
@@ -27,11 +36,24 @@ EXTRA_RUNS+=$(MULTIARCH_RUNS)
%.o: $(CRT_PATH)/%.S
$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -Wa,--noexecstack -c $< -o $@
-# Build and link the tests
+# Build and link the multiarch tests
%: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+# Build and link the arch tests
+%: $(X86_64_SYSTEM_SRC)/%.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
+ $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+
memory: CFLAGS+=-DCHECK_UNALIGNED=1
+patch-target: CFLAGS+=-O0
# Running
QEMU_OPTS+=-device isa-debugcon,chardev=output -device isa-debug-exit,iobase=0xf4,iosize=0x4 -kernel
+
+# Add patch-target to ADDITIONAL_PLUGINS_TESTS
+ADDITIONAL_PLUGINS_TESTS += patch-target
+
+run-plugin-patch-target-with-libpatch.so: \
+ PLUGIN_ARGS=$(COMMA)target=ffc0$(COMMA)patch=9090$(COMMA)use_hwaddr=true
+run-plugin-patch-target-with-libpatch.so: \
+ CHECK_PLUGIN_OUTPUT_COMMAND=$(X86_64_SYSTEM_SRC)/validate-patch.py $@.out
\ No newline at end of file
diff --git a/tests/tcg/x86_64/system/patch-target.c b/tests/tcg/x86_64/system/patch-target.c
new file mode 100644
index 0000000000..8a7c0a0ae8
--- /dev/null
+++ b/tests/tcg/x86_64/system/patch-target.c
@@ -0,0 +1,27 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This test target increments a value 100 times. The patcher converts the
+ * inc instruction to a nop, so it only increments the value once.
+ *
+ */
+#include <minilib.h>
+
+int main(void)
+{
+ ml_printf("Running test...\n");
+#if defined(__x86_64__)
+ ml_printf("Testing insn memory read/write...\n");
+ unsigned int x = 0;
+ for (int i = 0; i < 100; i++) {
+ asm volatile (
+ "inc %[x]"
+ : [x] "+a" (x)
+ );
+ }
+ ml_printf("Value: %d\n", x);
+#else
+ #error "This test is only valid for x86_64 architecture."
+#endif
+ return 0;
+}
diff --git a/tests/tcg/x86_64/system/validate-patch.py b/tests/tcg/x86_64/system/validate-patch.py
new file mode 100755
index 0000000000..700950eae5
--- /dev/null
+++ b/tests/tcg/x86_64/system/validate-patch.py
@@ -0,0 +1,39 @@
+#!/usr/bin/env python3
+#
+# validate-patch.py: check the patch applies
+#
+# This program takes two inputs:
+# - the plugin output
+# - the binary output
+#
+# Copyright (C) 2024
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import sys
+from argparse import ArgumentParser
+
+def main() -> None:
+ """
+ Process the arguments, injest the program and plugin out and
+ verify they match up and report if they do not.
+ """
+ parser = ArgumentParser(description="Validate patch")
+ parser.add_argument('test_output',
+ help="The output from the test itself")
+ parser.add_argument('plugin_output',
+ help="The output from plugin")
+ args = parser.parse_args()
+
+ with open(args.test_output, 'r') as f:
+ test_data = f.read()
+ with open(args.plugin_output, 'r') as f:
+ plugin_data = f.read()
+ if "Value: 1" in test_data:
+ sys.exit(0)
+ else:
+ sys.exit(1)
+
+if __name__ == "__main__":
+ main()
+
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v12 7/7] plugins: Update plugin version and add notes
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (5 preceding siblings ...)
2025-06-11 23:24 ` [PATCH v12 6/7] plugins: Add patcher plugin and test Rowan Hart
@ 2025-06-11 23:24 ` Rowan Hart
2025-06-12 3:41 ` [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-11 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost, novafacing
From: novafacing <rowanbhart@gmail.com>
This patch updates the plugin version to gate new APIs and adds notes
describing what has been added.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
include/qemu/qemu-plugin.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 5eecdccc67..c450106af1 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -65,11 +65,18 @@ typedef uint64_t qemu_plugin_id_t;
*
* version 4:
* - added qemu_plugin_read_memory_vaddr
+ *
+ * version 5:
+ * - added qemu_plugin_write_memory_vaddr
+ * - added qemu_plugin_read_memory_hwaddr
+ * - added qemu_plugin_write_memory_hwaddr
+ * - added qemu_plugin_write_register
+ * - added qemu_plugin_translate_vaddr
*/
extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
-#define QEMU_PLUGIN_VERSION 4
+#define QEMU_PLUGIN_VERSION 5
/**
* struct qemu_info_t - system information for plugins
--
2.49.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (6 preceding siblings ...)
2025-06-11 23:24 ` [PATCH v12 7/7] plugins: Update plugin version and add notes Rowan Hart
@ 2025-06-12 3:41 ` Rowan Hart
2025-06-13 15:19 ` Pierrick Bouvier
2025-06-19 16:20 ` Rowan Hart
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-12 3:41 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 142 bytes --]
I forgot to mention it in the cover letter but v12 also addresses
Pierrick's comments to clean up the patch plug-in. Thanks for the feedback!
[-- Attachment #2: Type: text/html, Size: 168 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 6/7] plugins: Add patcher plugin and test
2025-06-11 23:24 ` [PATCH v12 6/7] plugins: Add patcher plugin and test Rowan Hart
@ 2025-06-13 15:19 ` Pierrick Bouvier
2025-06-17 10:35 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Pierrick Bouvier @ 2025-06-13 15:19 UTC (permalink / raw)
To: Rowan Hart, qemu-devel
Cc: Alex Bennée, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
On 6/11/25 4:24 PM, Rowan Hart wrote:
> From: novafacing <rowanbhart@gmail.com>
>
> This patch adds a plugin that exercises the virtual and hardware memory
> read-write API functions added in a previous patch. The plugin takes a
> target and patch byte sequence, and will overwrite any instruction
> matching the target byte sequence with the patch.
>
> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
> ---
> tests/tcg/Makefile.target | 1 +
> tests/tcg/plugins/meson.build | 2 +-
> tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
> tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
> tests/tcg/x86_64/system/patch-target.c | 27 +++
> tests/tcg/x86_64/system/validate-patch.py | 39 ++++
> 6 files changed, 336 insertions(+), 6 deletions(-)
> create mode 100644 tests/tcg/plugins/patch.c
> create mode 100644 tests/tcg/x86_64/system/patch-target.c
> create mode 100755 tests/tcg/x86_64/system/validate-patch.py
>
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 95ff76ea44..4b709a9d18 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -176,6 +176,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
> # Some plugins need additional arguments above the default to fully
> # exercise things. We can define them on a per-test basis here.
> run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true
> +run-plugin-%-with-libpatch.so: PLUGIN_ARGS=$(COMMA)target=ffffffff$(COMMA)patch=00000000
>
> ifeq ($(filter %-softmmu, $(TARGET)),)
> run-%: %
> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> index 41f02f2c7f..163042e601 100644
> --- a/tests/tcg/plugins/meson.build
> +++ b/tests/tcg/plugins/meson.build
> @@ -1,6 +1,6 @@
> t = []
> if get_option('plugins')
> - foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall']
> + foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall', 'patch']
> if host_os == 'windows'
> t += shared_module(i, files(i + '.c') + '../../../contrib/plugins/win32_linker.c',
> include_directories: '../../../include/qemu',
> diff --git a/tests/tcg/plugins/patch.c b/tests/tcg/plugins/patch.c
> new file mode 100644
> index 0000000000..450fc51c88
> --- /dev/null
> +++ b/tests/tcg/plugins/patch.c
> @@ -0,0 +1,241 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This plugin patches instructions matching a pattern to a different
> + * instruction as they execute
> + *
> + */
> +
> +#include "glib.h"
> +#include "glibconfig.h"
> +
> +#include <qemu-plugin.h>
> +#include <string.h>
> +#include <stdio.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +static bool use_hwaddr;
> +static GByteArray *target_data;
> +static GByteArray *patch_data;
> +
> +/**
> + * Parse a string of hexadecimal digits into a GByteArray. The string must be
> + * even length
> + */
> +static GByteArray *str_to_bytes(const char *str)
> +{
> + size_t len = strlen(str);
> +
> + if (len == 0 || len % 2 != 0) {
> + return NULL;
> + }
> +
> + GByteArray *bytes = g_byte_array_new();
> + char byte[3] = {0};
> + guint8 value = 0;
> +
> + for (size_t i = 0; i < len; i += 2) {
> + byte[0] = str[i];
> + byte[1] = str[i + 1];
> + value = (guint8)g_ascii_strtoull(byte, NULL, 16);
> + g_byte_array_append(bytes, &value, 1);
> + }
> +
> + return bytes;
> +}
> +
> +static void patch_hwaddr(unsigned int vcpu_index, void *userdata)
> +{
> + uint64_t addr = (uint64_t)userdata;
> + g_autoptr(GString) str = g_string_new(NULL);
> + g_string_printf(str, "patching: @0x%"
> + PRIx64 "\n",
> + addr);
> + qemu_plugin_outs(str->str);
> +
> + enum qemu_plugin_hwaddr_operation_result result =
> + qemu_plugin_write_memory_hwaddr(addr, patch_data);
> +
> +
> + if (result != QEMU_PLUGIN_HWADDR_OPERATION_OK) {
> + g_autoptr(GString) errmsg = g_string_new(NULL);
> + g_string_printf(errmsg, "Failed to write memory: %d\n", result);
> + qemu_plugin_outs(errmsg->str);
> + return;
> + }
> +
> + GByteArray *read_data = g_byte_array_new();
> +
> + result = qemu_plugin_read_memory_hwaddr(addr, read_data,
> + patch_data->len);
> +
> + qemu_plugin_outs("Reading memory...\n");
> +
> + if (result != QEMU_PLUGIN_HWADDR_OPERATION_OK) {
> + g_autoptr(GString) errmsg = g_string_new(NULL);
> + g_string_printf(errmsg, "Failed to read memory: %d\n", result);
> + qemu_plugin_outs(errmsg->str);
> + return;
> + }
> +
> + if (memcmp(patch_data->data, read_data->data, patch_data->len) != 0) {
> + qemu_plugin_outs("Failed to read back written data\n");
> + }
> +
> + qemu_plugin_outs("Success!\n");
> +
> + return;
> +}
> +
> +static void patch_vaddr(unsigned int vcpu_index, void *userdata)
> +{
> + uint64_t addr = (uint64_t)userdata;
> + uint64_t hwaddr = 0;
> + if (!qemu_plugin_translate_vaddr(addr, &hwaddr)) {
> + qemu_plugin_outs("Failed to translate vaddr\n");
> + return;
> + }
> + g_autoptr(GString) str = g_string_new(NULL);
> + g_string_printf(str, "patching: @0x%"
> + PRIx64 " hw: @0x%" PRIx64 "\n",
> + addr, hwaddr);
> + qemu_plugin_outs(str->str);
> +
> + qemu_plugin_outs("Writing memory (vaddr)...\n");
> +
> + if (!qemu_plugin_write_memory_vaddr(addr, patch_data)) {
> + qemu_plugin_outs("Failed to write memory\n");
> + return;
> + }
> +
> + qemu_plugin_outs("Reading memory (vaddr)...\n");
> +
> + g_autoptr(GByteArray) read_data = g_byte_array_new();
> +
> + if (!qemu_plugin_read_memory_vaddr(addr, read_data, patch_data->len)) {
> + qemu_plugin_outs("Failed to read memory\n");
> + return;
> + }
> +
> + if (memcmp(patch_data->data, read_data->data, patch_data->len) != 0) {
> + qemu_plugin_outs("Failed to read back written data\n");
> + }
> +
> + qemu_plugin_outs("Success!\n");
> +
> + return;
> +}
> +
> +/*
> + * Callback on translation of a translation block.
> + */
> +static void vcpu_tb_trans_cb(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> + uint64_t addr = 0;
> + g_autoptr(GByteArray) insn_data = g_byte_array_new();
> + for (size_t i = 0; i < qemu_plugin_tb_n_insns(tb); i++) {
> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> +
> + if (use_hwaddr) {
> + uint64_t vaddr = qemu_plugin_insn_vaddr(insn);
> + if (!qemu_plugin_translate_vaddr(vaddr, &addr)) {
> + qemu_plugin_outs("Failed to translate vaddr\n");
> + continue;
> + }
> + } else {
> + addr = qemu_plugin_insn_vaddr(insn);
> + }
> +
> + g_byte_array_set_size(insn_data, qemu_plugin_insn_size(insn));
> + qemu_plugin_insn_data(insn, insn_data->data, insn_data->len);
> +
> + if (insn_data->len >= target_data->len &&
> + !memcmp(insn_data->data, target_data->data,
> + MIN(target_data->len, insn_data->len))) {
> + if (use_hwaddr) {
> + qemu_plugin_register_vcpu_tb_exec_cb(tb, patch_hwaddr,
> + QEMU_PLUGIN_CB_NO_REGS,
> + (void *)addr);
> + } else {
> + qemu_plugin_register_vcpu_tb_exec_cb(tb, patch_vaddr,
> + QEMU_PLUGIN_CB_NO_REGS,
> + (void *)addr);
> + }
> + }
> + }
> +}
> +
> +static void usage(void)
> +{
> + fprintf(stderr, "Usage: <lib>,target=<bytes>,patch=<new_bytes>"
> + "[,use_hwaddr=true|false]");
> +}
> +
> +/*
> + * Called when the plugin is installed
> + */
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> + const qemu_info_t *info, int argc,
> + char **argv)
> +{
> +
> + use_hwaddr = true;
> + target_data = NULL;
> + patch_data = NULL;
> +
> + if (argc > 4) {
> + usage();
> + return -1;
> + }
> +
> + for (size_t i = 0; i < argc; i++) {
> + char *opt = argv[i];
> + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> + if (g_strcmp0(tokens[0], "use_hwaddr") == 0) {
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &use_hwaddr)) {
> + fprintf(stderr,
> + "Failed to parse boolean argument use_hwaddr\n");
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "target") == 0) {
> + target_data = str_to_bytes(tokens[1]);
> + if (!target_data) {
> + fprintf(stderr,
> + "Failed to parse target bytes.\n");
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "patch") == 0) {
> + patch_data = str_to_bytes(tokens[1]);
> + if (!patch_data) {
> + fprintf(stderr, "Failed to parse patch bytes.\n");
> + return -1;
> + }
> + } else {
> + fprintf(stderr, "Unknown argument: %s\n", tokens[0]);
> + usage();
> + return -1;
> + }
> + }
> +
> + if (!target_data) {
> + fprintf(stderr, "target argument is required\n");
> + usage();
> + return -1;
> + }
> +
> + if (!patch_data) {
> + fprintf(stderr, "patch argument is required\n");
> + usage();
> + return -1;
> + }
> +
> + if (target_data->len != patch_data->len) {
> + fprintf(stderr, "Target and patch data must be the same length\n");
> + return -1;
> + }
> +
> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans_cb);
> +
> + return 0;
> +}
> diff --git a/tests/tcg/x86_64/Makefile.softmmu-target b/tests/tcg/x86_64/Makefile.softmmu-target
> index ef6bcb4dc7..154910ab72 100644
> --- a/tests/tcg/x86_64/Makefile.softmmu-target
> +++ b/tests/tcg/x86_64/Makefile.softmmu-target
> @@ -7,18 +7,27 @@
> #
>
> I386_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/i386/system
> -X64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
> +X86_64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
>
> # These objects provide the basic boot code and helper functions for all tests
> CRT_OBJS=boot.o
>
> -CRT_PATH=$(X64_SYSTEM_SRC)
> -LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld
> +X86_64_TEST_C_SRCS=$(wildcard $(X86_64_SYSTEM_SRC)/*.c)
> +X86_64_TEST_S_SRCS=
> +
> +X86_64_C_TESTS = $(patsubst $(X86_64_SYSTEM_SRC)/%.c, %, $(X86_64_TEST_C_SRCS))
> +X86_64_S_TESTS = $(patsubst $(X86_64_SYSTEM_SRC)/%.S, %, $(X86_64_TEST_S_SRCS))
> +
> +X86_64_TESTS = $(X86_64_C_TESTS)
> +X86_64_TESTS += $(X86_64_S_TESTS)
> +
> +CRT_PATH=$(X86_64_SYSTEM_SRC)
> +LINK_SCRIPT=$(X86_64_SYSTEM_SRC)/kernel.ld
> LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_x86_64
> CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
> LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
>
> -TESTS+=$(MULTIARCH_TESTS)
> +TESTS+=$(X86_64_TESTS) $(MULTIARCH_TESTS)
> EXTRA_RUNS+=$(MULTIARCH_RUNS)
>
> # building head blobs
> @@ -27,11 +36,24 @@ EXTRA_RUNS+=$(MULTIARCH_RUNS)
> %.o: $(CRT_PATH)/%.S
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) -Wa,--noexecstack -c $< -o $@
>
> -# Build and link the tests
> +# Build and link the multiarch tests
> %: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
>
> +# Build and link the arch tests
> +%: $(X86_64_SYSTEM_SRC)/%.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +
> memory: CFLAGS+=-DCHECK_UNALIGNED=1
> +patch-target: CFLAGS+=-O0
>
> # Running
> QEMU_OPTS+=-device isa-debugcon,chardev=output -device isa-debug-exit,iobase=0xf4,iosize=0x4 -kernel
> +
> +# Add patch-target to ADDITIONAL_PLUGINS_TESTS
> +ADDITIONAL_PLUGINS_TESTS += patch-target
> +
> +run-plugin-patch-target-with-libpatch.so: \
> + PLUGIN_ARGS=$(COMMA)target=ffc0$(COMMA)patch=9090$(COMMA)use_hwaddr=true
> +run-plugin-patch-target-with-libpatch.so: \
> + CHECK_PLUGIN_OUTPUT_COMMAND=$(X86_64_SYSTEM_SRC)/validate-patch.py $@.out
> \ No newline at end of file
> diff --git a/tests/tcg/x86_64/system/patch-target.c b/tests/tcg/x86_64/system/patch-target.c
> new file mode 100644
> index 0000000000..8a7c0a0ae8
> --- /dev/null
> +++ b/tests/tcg/x86_64/system/patch-target.c
> @@ -0,0 +1,27 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This test target increments a value 100 times. The patcher converts the
> + * inc instruction to a nop, so it only increments the value once.
> + *
> + */
> +#include <minilib.h>
> +
> +int main(void)
> +{
> + ml_printf("Running test...\n");
> +#if defined(__x86_64__)
> + ml_printf("Testing insn memory read/write...\n");
> + unsigned int x = 0;
> + for (int i = 0; i < 100; i++) {
> + asm volatile (
> + "inc %[x]"
> + : [x] "+a" (x)
> + );
> + }
> + ml_printf("Value: %d\n", x);
> +#else
> + #error "This test is only valid for x86_64 architecture."
> +#endif
> + return 0;
> +}
> diff --git a/tests/tcg/x86_64/system/validate-patch.py b/tests/tcg/x86_64/system/validate-patch.py
> new file mode 100755
> index 0000000000..700950eae5
> --- /dev/null
> +++ b/tests/tcg/x86_64/system/validate-patch.py
> @@ -0,0 +1,39 @@
> +#!/usr/bin/env python3
> +#
> +# validate-patch.py: check the patch applies
> +#
> +# This program takes two inputs:
> +# - the plugin output
> +# - the binary output
> +#
> +# Copyright (C) 2024
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import sys
> +from argparse import ArgumentParser
> +
> +def main() -> None:
> + """
> + Process the arguments, injest the program and plugin out and
> + verify they match up and report if they do not.
> + """
> + parser = ArgumentParser(description="Validate patch")
> + parser.add_argument('test_output',
> + help="The output from the test itself")
> + parser.add_argument('plugin_output',
> + help="The output from plugin")
> + args = parser.parse_args()
> +
> + with open(args.test_output, 'r') as f:
> + test_data = f.read()
> + with open(args.plugin_output, 'r') as f:
> + plugin_data = f.read()
> + if "Value: 1" in test_data:
> + sys.exit(0)
> + else:
> + sys.exit(1)
> +
> +if __name__ == "__main__":
> + main()
> +
Thanks for the update Rowan.
Looks good to me,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (7 preceding siblings ...)
2025-06-12 3:41 ` [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
@ 2025-06-13 15:19 ` Pierrick Bouvier
2025-06-13 15:57 ` Alex Bennée
2025-06-19 16:20 ` Rowan Hart
9 siblings, 1 reply; 18+ messages in thread
From: Pierrick Bouvier @ 2025-06-13 15:19 UTC (permalink / raw)
To: Rowan Hart, qemu-devel
Cc: Alex Bennée, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
On 6/11/25 4:24 PM, Rowan Hart wrote:
> This patch series adds several new API functions focused on enabling use
> cases around reading and writing guest memory from QEMU plugins. To support
> these new APIs, some utility functionality around retrieving information about
> address spaces is added as well.
>
> The new qemu_plugin_write_register utilizes gdb_write_register, which is now
> declared in gdbstub.h for this purpose instead of being static.
>
> qemu_plugin_write_memory_vaddr utilizes cpu_memory_rw_debug much the same as
> the existing read_memory_vaddr function does.
>
> The read and write_hwaddr functions are the most different. These functions
> use address_space_rw, which works well in most cases. There is an important
> caveat that for writes, the page being written will be set dirty by the
> write operation. This dirty setting requires locking the page range,
> which can contend with an already held lock in page_collection_lock
> when called in a tb translate callback with a write to the instruction
> memory in the tb. The doc comments warn against doing this, and it's unlikely
> anyone would want to do this.
>
> I've also added two test plugins: one that implements a simple hypercall
> interface that guest code can use to communicate with the plugin in a
> structured way with a test to ensure that this hypercall works and writing
> virtual memory works. And one that implements a simple patch utility to patch
> memory at runtime. The test for the second plugin ensures the patch applies
> successfully to instruction memory, and can use both hw and vaddr methods.
>
> For v3, I've had a few comments from the last submission that I've addressed,
> and some that I haven't for one reason or another:
>
> - Enforce QEMU_PLUGIN_CB_ flags in register read/write operations: done!
> - Fix my commit messages and add long messages describing commits: done!
> - Un-expose AS internals: done! Functions operate on current vCPU, current AS.
> - Clean up use of current_cpu: done!
> - Make functions take a vcpu_idx: not done. May revisit but it allows footguns.
> Even for translation, seems best to not do this now. We can easily add _vcpu
> versions of these functions in the future if we change our minds!
>
> For v5, I've just updated the enforcement of the QEMU_PLUGIN_CB_ flags to just
> use immediate stores, which simplifies the implementation quite a lot and
> should be more efficient too. Thanks Pierrick for the suggestion!
>
> v6 is a formatting pass, I left some whitespace that needed removal, some
> license text was wrong, and so forth.
>
> v8 reverts a mistake I made extending the size of arrays of TCGHelperInfo
> structs, as I misunderstood their sizes. It preserves adding an explicit
> zero as the last entry for clarity, however.
>
> v9 fixes qemu_plugin_read_register to return -1 on parameter or flag state
> error instead of 0.
>
> In v10, I relaxed the restriction on when the register r/w functions can be
> called, allowing all them to be used from any callback where the CPU is not
> currently executing, with additional notes in the documentation for exceptions
> (atexit and flush, which do not operate on a specific CPU and in which
> current_cpu is not set).
>
> v11 makes the cb flags functions inline and fixes a typo where cpu was asserted
> but current_cpu was actually accessed.
>
> v12 removes the hypercalls plugin because the functions it tested are also
> tested by the patcher plugin, making it redundant. We'll circle back on a
> hypercalls API in the future as a part of the plugin API, not as a plugin
> itself.
>
> Rowan Hart (1):
> plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W
> callbacks
>
> novafacing (6):
> gdbstub: Expose gdb_write_register function to consumers of gdbstub
> plugins: Add register write API
> plugins: Add memory virtual address write API
> plugins: Add memory hardware address read/write API
> plugins: Add patcher plugin and test
> plugins: Update plugin version and add notes
>
> accel/tcg/plugin-gen.c | 30 +++
> gdbstub/gdbstub.c | 2 +-
> include/exec/gdbstub.h | 14 ++
> include/hw/core/cpu.h | 1 +
> include/qemu/plugin.h | 15 ++
> include/qemu/qemu-plugin.h | 176 ++++++++++++++--
> plugins/api.c | 135 +++++++++++-
> plugins/core.c | 33 +++
> tests/tcg/Makefile.target | 1 +
> tests/tcg/plugins/meson.build | 2 +-
> tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
> tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
> tests/tcg/x86_64/system/patch-target.c | 27 +++
> tests/tcg/x86_64/system/validate-patch.py | 39 ++++
> 14 files changed, 725 insertions(+), 23 deletions(-)
> create mode 100644 tests/tcg/plugins/patch.c
> create mode 100644 tests/tcg/x86_64/system/patch-target.c
> create mode 100755 tests/tcg/x86_64/system/validate-patch.py
>
@Alex,
series looks good to me now.
Would you like to add comments, or is it good for you also?
Thanks,
Pierrick
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers
2025-06-13 15:19 ` Pierrick Bouvier
@ 2025-06-13 15:57 ` Alex Bennée
0 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2025-06-13 15:57 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: Rowan Hart, qemu-devel, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> On 6/11/25 4:24 PM, Rowan Hart wrote:
>> This patch series adds several new API functions focused on enabling use
>> cases around reading and writing guest memory from QEMU plugins. To support
>> these new APIs, some utility functionality around retrieving information about
>> address spaces is added as well.
>> The new qemu_plugin_write_register utilizes gdb_write_register,
>> which is now
>> declared in gdbstub.h for this purpose instead of being static.
>> qemu_plugin_write_memory_vaddr utilizes cpu_memory_rw_debug much the
>> same as
>> the existing read_memory_vaddr function does.
>> The read and write_hwaddr functions are the most different. These
>> functions
>> use address_space_rw, which works well in most cases. There is an important
>> caveat that for writes, the page being written will be set dirty by the
>> write operation. This dirty setting requires locking the page range,
>> which can contend with an already held lock in page_collection_lock
>> when called in a tb translate callback with a write to the instruction
>> memory in the tb. The doc comments warn against doing this, and it's unlikely
>> anyone would want to do this.
>> I've also added two test plugins: one that implements a simple
>> hypercall
>> interface that guest code can use to communicate with the plugin in a
>> structured way with a test to ensure that this hypercall works and writing
>> virtual memory works. And one that implements a simple patch utility to patch
>> memory at runtime. The test for the second plugin ensures the patch applies
>> successfully to instruction memory, and can use both hw and vaddr methods.
>> For v3, I've had a few comments from the last submission that I've
>> addressed,
>> and some that I haven't for one reason or another:
>> - Enforce QEMU_PLUGIN_CB_ flags in register read/write operations:
>> done!
>> - Fix my commit messages and add long messages describing commits: done!
>> - Un-expose AS internals: done! Functions operate on current vCPU, current AS.
>> - Clean up use of current_cpu: done!
>> - Make functions take a vcpu_idx: not done. May revisit but it allows footguns.
>> Even for translation, seems best to not do this now. We can easily add _vcpu
>> versions of these functions in the future if we change our minds!
>> For v5, I've just updated the enforcement of the QEMU_PLUGIN_CB_
>> flags to just
>> use immediate stores, which simplifies the implementation quite a lot and
>> should be more efficient too. Thanks Pierrick for the suggestion!
>> v6 is a formatting pass, I left some whitespace that needed removal,
>> some
>> license text was wrong, and so forth.
>> v8 reverts a mistake I made extending the size of arrays of
>> TCGHelperInfo
>> structs, as I misunderstood their sizes. It preserves adding an explicit
>> zero as the last entry for clarity, however.
>> v9 fixes qemu_plugin_read_register to return -1 on parameter or flag
>> state
>> error instead of 0.
>> In v10, I relaxed the restriction on when the register r/w functions
>> can be
>> called, allowing all them to be used from any callback where the CPU is not
>> currently executing, with additional notes in the documentation for exceptions
>> (atexit and flush, which do not operate on a specific CPU and in which
>> current_cpu is not set).
>> v11 makes the cb flags functions inline and fixes a typo where cpu
>> was asserted
>> but current_cpu was actually accessed.
>> v12 removes the hypercalls plugin because the functions it tested
>> are also
>> tested by the patcher plugin, making it redundant. We'll circle back on a
>> hypercalls API in the future as a part of the plugin API, not as a plugin
>> itself.
>> Rowan Hart (1):
>> plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W
>> callbacks
>> novafacing (6):
>> gdbstub: Expose gdb_write_register function to consumers of gdbstub
>> plugins: Add register write API
>> plugins: Add memory virtual address write API
>> plugins: Add memory hardware address read/write API
>> plugins: Add patcher plugin and test
>> plugins: Update plugin version and add notes
>> accel/tcg/plugin-gen.c | 30 +++
>> gdbstub/gdbstub.c | 2 +-
>> include/exec/gdbstub.h | 14 ++
>> include/hw/core/cpu.h | 1 +
>> include/qemu/plugin.h | 15 ++
>> include/qemu/qemu-plugin.h | 176 ++++++++++++++--
>> plugins/api.c | 135 +++++++++++-
>> plugins/core.c | 33 +++
>> tests/tcg/Makefile.target | 1 +
>> tests/tcg/plugins/meson.build | 2 +-
>> tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
>> tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
>> tests/tcg/x86_64/system/patch-target.c | 27 +++
>> tests/tcg/x86_64/system/validate-patch.py | 39 ++++
>> 14 files changed, 725 insertions(+), 23 deletions(-)
>> create mode 100644 tests/tcg/plugins/patch.c
>> create mode 100644 tests/tcg/x86_64/system/patch-target.c
>> create mode 100755 tests/tcg/x86_64/system/validate-patch.py
>>
>
> @Alex,
> series looks good to me now.
>
> Would you like to add comments, or is it good for you also?
I'll do a pass through next week but I think we are in good shape.
>
> Thanks,
> Pierrick
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 5/7] plugins: Add memory hardware address read/write API
2025-06-11 23:24 ` [PATCH v12 5/7] plugins: Add memory hardware address read/write API Rowan Hart
@ 2025-06-17 10:24 ` Alex Bennée
2025-06-17 15:46 ` Rowan Hart
2025-06-17 17:38 ` Pierrick Bouvier
0 siblings, 2 replies; 18+ messages in thread
From: Alex Bennée @ 2025-06-17 10:24 UTC (permalink / raw)
To: Rowan Hart
Cc: qemu-devel, Pierrick Bouvier, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
Rowan Hart <rowanbhart@gmail.com> writes:
> From: novafacing <rowanbhart@gmail.com>
>
> This patch adds functions to the plugins API to allow plugins to read
> and write memory via hardware addresses. The functions use the current
> address space of the current CPU in order to avoid exposing address
> space information to users. A later patch may want to add a function to
> permit a specified address space, for example to facilitate
> architecture-specific plugins that want to operate on them, for example
> reading ARM secure memory.
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
<snip>
> +/**
> + * qemu_plugin_write_memory_hwaddr() - write to memory using a hardware address
> + *
> + * @addr: A physical address to write to
> + * @data: A byte array containing the data to write
> + *
> + * The contents of @data will be written to memory starting at the hardware
> + * address @addr in the current address space for the current vCPU.
> + *
> + * This function does not guarantee consistency of writes, nor does it ensure
> + * that pending writes are flushed either before or after the write takes place,
> + * so callers should take care when calling this function in plugin callbacks to
> + * avoid depending on the existence of data written using this function which
> + * may be overwritten afterward. In addition, this function requires that the
> + * pages containing the address are not locked. Practically, this means that you
> + * should not write instruction memory in a current translation block inside a
> + * callback registered with qemu_plugin_register_vcpu_tb_trans_cb.
> + *
> + * You can, for example, write instruction memory in a current translation block
> + * in a callback registered with qemu_plugin_register_vcpu_tb_exec_cb, although
> + * be aware that the write will not be flushed until after the translation block
> + * has finished executing. In general, this function should be used to write
> + * data memory or to patch code at a known address, not in a current translation
> + * block.
My main concern about the long list of caveats for writing memory is the
user will almost certainly cause weird things to happen which will then
be hard to debug. I can see the patcher example however it would be
useful to know what other practical uses this interface provides.
<snip>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 6/7] plugins: Add patcher plugin and test
2025-06-11 23:24 ` [PATCH v12 6/7] plugins: Add patcher plugin and test Rowan Hart
2025-06-13 15:19 ` Pierrick Bouvier
@ 2025-06-17 10:35 ` Alex Bennée
1 sibling, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2025-06-17 10:35 UTC (permalink / raw)
To: Rowan Hart
Cc: qemu-devel, Pierrick Bouvier, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
Rowan Hart <rowanbhart@gmail.com> writes:
> From: novafacing <rowanbhart@gmail.com>
>
> This patch adds a plugin that exercises the virtual and hardware memory
> read-write API functions added in a previous patch. The plugin takes a
> target and patch byte sequence, and will overwrite any instruction
> matching the target byte sequence with the patch.
>
> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
> ---
> tests/tcg/Makefile.target | 1 +
> tests/tcg/plugins/meson.build | 2 +-
> tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
> tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
> tests/tcg/x86_64/system/patch-target.c | 27 +++
> tests/tcg/x86_64/system/validate-patch.py | 39 ++++
> 6 files changed, 336 insertions(+), 6 deletions(-)
> create mode 100644 tests/tcg/plugins/patch.c
> create mode 100644 tests/tcg/x86_64/system/patch-target.c
> create mode 100755 tests/tcg/x86_64/system/validate-patch.py
>
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 95ff76ea44..4b709a9d18 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -176,6 +176,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
> # Some plugins need additional arguments above the default to fully
> # exercise things. We can define them on a per-test basis here.
> run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true
> +run-plugin-%-with-libpatch.so: PLUGIN_ARGS=$(COMMA)target=ffffffff$(COMMA)patch=00000000
>
I think we need to manually add this to the x86_64 specific tests because...
> ifeq ($(filter %-softmmu, $(TARGET)),)
> run-%: %
> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> index 41f02f2c7f..163042e601 100644
> --- a/tests/tcg/plugins/meson.build
> +++ b/tests/tcg/plugins/meson.build
> @@ -1,6 +1,6 @@
> t = []
> if get_option('plugins')
> - foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall']
> + foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall', 'patch']
> if host_os == 'windows'
> t += shared_module(i, files(i + '.c') + '../../../contrib/plugins/win32_linker.c',
> include_directories: '../../../include/qemu',
... the problem with adding test patches into tests/tcg is we balloon the
number of test cases. Whats worse we are running on linux-user tests
where we don't exercise anything.
So I think we should filter out the test from the general testing by
fixing up:
PLUGINS=$(patsubst %.c, lib%.so, $(notdir $(wildcard $(PLUGIN_SRC)/*.c)))
<snip>
> +
> +static void usage(void)
> +{
> + fprintf(stderr, "Usage: <lib>,target=<bytes>,patch=<new_bytes>"
> + "[,use_hwaddr=true|false]");
> +}
> +
> +/*
> + * Called when the plugin is installed
> + */
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> + const qemu_info_t *info, int argc,
> + char **argv)
> +{
> +
> + use_hwaddr = true;
> + target_data = NULL;
> + patch_data = NULL;
> +
> + if (argc > 4) {
> + usage();
> + return -1;
> + }
> +
> + for (size_t i = 0; i < argc; i++) {
> + char *opt = argv[i];
> + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> + if (g_strcmp0(tokens[0], "use_hwaddr") == 0) {
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &use_hwaddr)) {
> + fprintf(stderr,
> + "Failed to parse boolean argument use_hwaddr\n");
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "target") == 0) {
> + target_data = str_to_bytes(tokens[1]);
> + if (!target_data) {
> + fprintf(stderr,
> + "Failed to parse target bytes.\n");
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "patch") == 0) {
> + patch_data = str_to_bytes(tokens[1]);
> + if (!patch_data) {
> + fprintf(stderr, "Failed to parse patch bytes.\n");
> + return -1;
> + }
> + } else {
> + fprintf(stderr, "Unknown argument: %s\n", tokens[0]);
> + usage();
> + return -1;
> + }
> + }
> +
> + if (!target_data) {
> + fprintf(stderr, "target argument is required\n");
> + usage();
> + return -1;
> + }
> +
> + if (!patch_data) {
> + fprintf(stderr, "patch argument is required\n");
> + usage();
> + return -1;
> + }
> +
> + if (target_data->len != patch_data->len) {
> + fprintf(stderr, "Target and patch data must be the same length\n");
> + return -1;
> + }
> +
> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans_cb);
> +
> + return 0;
> +}
> diff --git a/tests/tcg/x86_64/Makefile.softmmu-target b/tests/tcg/x86_64/Makefile.softmmu-target
> index ef6bcb4dc7..154910ab72 100644
> --- a/tests/tcg/x86_64/Makefile.softmmu-target
> +++ b/tests/tcg/x86_64/Makefile.softmmu-target
> @@ -7,18 +7,27 @@
> #
>
> I386_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/i386/system
> -X64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
> +X86_64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/x86_64/system
Can we have symbol renaming in a separate patch as it makes diffs messy
to follow otherwise.
>
> # These objects provide the basic boot code and helper functions for all tests
> CRT_OBJS=boot.o
>
> -CRT_PATH=$(X64_SYSTEM_SRC)
> -LINK_SCRIPT=$(X64_SYSTEM_SRC)/kernel.ld
> +X86_64_TEST_C_SRCS=$(wildcard $(X86_64_SYSTEM_SRC)/*.c)
> +X86_64_TEST_S_SRCS=
> +
> +X86_64_C_TESTS = $(patsubst $(X86_64_SYSTEM_SRC)/%.c, %, $(X86_64_TEST_C_SRCS))
> +X86_64_S_TESTS = $(patsubst $(X86_64_SYSTEM_SRC)/%.S, %, $(X86_64_TEST_S_SRCS))
> +
> +X86_64_TESTS = $(X86_64_C_TESTS)
> +X86_64_TESTS += $(X86_64_S_TESTS)
> +
> +CRT_PATH=$(X86_64_SYSTEM_SRC)
> +LINK_SCRIPT=$(X86_64_SYSTEM_SRC)/kernel.ld
> LDFLAGS=-Wl,-T$(LINK_SCRIPT) -Wl,-melf_x86_64
> CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC)
> LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
>
> -TESTS+=$(MULTIARCH_TESTS)
> +TESTS+=$(X86_64_TESTS) $(MULTIARCH_TESTS)
> EXTRA_RUNS+=$(MULTIARCH_RUNS)
>
> # building head blobs
> @@ -27,11 +36,24 @@ EXTRA_RUNS+=$(MULTIARCH_RUNS)
> %.o: $(CRT_PATH)/%.S
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) -Wa,--noexecstack -c $< -o $@
>
> -# Build and link the tests
> +# Build and link the multiarch tests
> %: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
>
> +# Build and link the arch tests
> +%: $(X86_64_SYSTEM_SRC)/%.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +
Is this needed? The aarch64 vtimer system test didn't need a new build stanza.
> memory: CFLAGS+=-DCHECK_UNALIGNED=1
> +patch-target: CFLAGS+=-O0
>
> # Running
> QEMU_OPTS+=-device isa-debugcon,chardev=output -device isa-debug-exit,iobase=0xf4,iosize=0x4 -kernel
> +
> +# Add patch-target to ADDITIONAL_PLUGINS_TESTS
> +ADDITIONAL_PLUGINS_TESTS += patch-target
> +
> +run-plugin-patch-target-with-libpatch.so: \
> + PLUGIN_ARGS=$(COMMA)target=ffc0$(COMMA)patch=9090$(COMMA)use_hwaddr=true
> +run-plugin-patch-target-with-libpatch.so: \
> + CHECK_PLUGIN_OUTPUT_COMMAND=$(X86_64_SYSTEM_SRC)/validate-patch.py $@.out
> \ No newline at end of file
> diff --git a/tests/tcg/x86_64/system/patch-target.c b/tests/tcg/x86_64/system/patch-target.c
> new file mode 100644
> index 0000000000..8a7c0a0ae8
> --- /dev/null
> +++ b/tests/tcg/x86_64/system/patch-target.c
> @@ -0,0 +1,27 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This test target increments a value 100 times. The patcher converts the
> + * inc instruction to a nop, so it only increments the value once.
> + *
> + */
> +#include <minilib.h>
> +
> +int main(void)
> +{
> + ml_printf("Running test...\n");
> +#if defined(__x86_64__)
> + ml_printf("Testing insn memory read/write...\n");
> + unsigned int x = 0;
> + for (int i = 0; i < 100; i++) {
> + asm volatile (
> + "inc %[x]"
> + : [x] "+a" (x)
> + );
> + }
> + ml_printf("Value: %d\n", x);
> +#else
> + #error "This test is only valid for x86_64 architecture."
> +#endif
This is a bit redundant given the test is in tests/tcg/x86_64/system.
> + return 0;
> +}
> diff --git a/tests/tcg/x86_64/system/validate-patch.py b/tests/tcg/x86_64/system/validate-patch.py
> new file mode 100755
> index 0000000000..700950eae5
> --- /dev/null
> +++ b/tests/tcg/x86_64/system/validate-patch.py
> @@ -0,0 +1,39 @@
> +#!/usr/bin/env python3
> +#
> +# validate-patch.py: check the patch applies
> +#
> +# This program takes two inputs:
> +# - the plugin output
> +# - the binary output
> +#
> +# Copyright (C) 2024
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import sys
> +from argparse import ArgumentParser
> +
> +def main() -> None:
> + """
> + Process the arguments, injest the program and plugin out and
> + verify they match up and report if they do not.
> + """
> + parser = ArgumentParser(description="Validate patch")
> + parser.add_argument('test_output',
> + help="The output from the test itself")
> + parser.add_argument('plugin_output',
> + help="The output from plugin")
> + args = parser.parse_args()
> +
> + with open(args.test_output, 'r') as f:
> + test_data = f.read()
> + with open(args.plugin_output, 'r') as f:
> + plugin_data = f.read()
> + if "Value: 1" in test_data:
> + sys.exit(0)
> + else:
> + sys.exit(1)
> +
> +if __name__ == "__main__":
> + main()
> +
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 5/7] plugins: Add memory hardware address read/write API
2025-06-17 10:24 ` Alex Bennée
@ 2025-06-17 15:46 ` Rowan Hart
2025-06-20 13:36 ` Alex Bennée
2025-06-17 17:38 ` Pierrick Bouvier
1 sibling, 1 reply; 18+ messages in thread
From: Rowan Hart @ 2025-06-17 15:46 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Pierrick Bouvier, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
> My main concern about the long list of caveats for writing memory is the
> user will almost certainly cause weird things to happen which will then
> be hard to debug. I can see the patcher example however it would be
> useful to know what other practical uses this interface provides.
>
Of course! My main personal intent here is to facilitate introspection
and manipulation of guest state for security analysis. Some examples of
why the memory/register R/W primitives are necessary here include:
Fuzzing:
- Read registers and memory for tracing control flow, comparison
operands, and profiled values (e.g. memcmp arguments)
- Write memory to inject test cases into the guest (for me and other
fuzzer developers, this is the biggest reason!)
- Write registers to reset execution or skip over complex checks like
checksums
- Read and write memory, and read and write registers, to do basic
snapshot/restore by tracking dirty pages and resetting them
Virtual Machine Introspection (for malware analysis and reverse
engineering):
- Read memory and registers to find kernel, analyze kernel structures,
and retrieve info like process lists, memory mappings
- Read memory and registers to quickly trace malware execution in VM
- Write memory and registers to test behavior under various conditions,
like skipping into checks (motivating example: what happens if you skip
into the kill switch statement for WannaCry)
Runtime patching (as in the example):
- Writing memory to patch critical legacy code in production often can
no longer be built or patched via means other than by applying binary
patches (this is a real problem for e.g. the government, to the point
where DARPA ran a program
https://www.darpa.mil/research/programs/assured-micropatching to work on
it!)
- Writing registers to skip over broken code, redirect to patch code, etc.
Ultimately, the caveats boil down to "don't modify stuff that's touched
by currently executing code". I personally don't think that's
unreasonable (as long as it's in the doc-strings) because for any plugin
that needs to write memory, ensuring the write consistency is probably
the easiest problem to solve and people working in this space are used
to having way worse and jankier workarounds. These plugin functions make
life way easier for them. I have been in touch with 20+ people from
various companies and projects (including Microsoft, where I work, as
well as other big and small tech) all working on plugins that could be
better if this feature existed, so there is definitely a user-base and
appetite for it!
The last cool use-case is that this moves us a long way towards cleaning
up the large number of QEMU forks out there designed for RE and security
testing like QEMU-Nyx, qemuafl, symqemu, and many more. Instead of
maintaining forks of QEMU (many of these are based on 4.2.0 or older!)
folks can just maintain a plugin, which lets them take advantage of
updates and fixes without giant rebases. My goal is to kill these forks
and have these projects write small, maintainable plugins instead, and
the authors are on board :)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 5/7] plugins: Add memory hardware address read/write API
2025-06-17 10:24 ` Alex Bennée
2025-06-17 15:46 ` Rowan Hart
@ 2025-06-17 17:38 ` Pierrick Bouvier
1 sibling, 0 replies; 18+ messages in thread
From: Pierrick Bouvier @ 2025-06-17 17:38 UTC (permalink / raw)
To: Alex Bennée, Rowan Hart
Cc: qemu-devel, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
On 6/17/25 3:24 AM, Alex Bennée wrote:
> Rowan Hart <rowanbhart@gmail.com> writes:
>
>> From: novafacing <rowanbhart@gmail.com>
>>
>> This patch adds functions to the plugins API to allow plugins to read
>> and write memory via hardware addresses. The functions use the current
>> address space of the current CPU in order to avoid exposing address
>> space information to users. A later patch may want to add a function to
>> permit a specified address space, for example to facilitate
>> architecture-specific plugins that want to operate on them, for example
>> reading ARM secure memory.
>>
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
> <snip>
>> +/**
>> + * qemu_plugin_write_memory_hwaddr() - write to memory using a hardware address
>> + *
>> + * @addr: A physical address to write to
>> + * @data: A byte array containing the data to write
>> + *
>> + * The contents of @data will be written to memory starting at the hardware
>> + * address @addr in the current address space for the current vCPU.
>> + *
>> + * This function does not guarantee consistency of writes, nor does it ensure
>> + * that pending writes are flushed either before or after the write takes place,
>> + * so callers should take care when calling this function in plugin callbacks to
>> + * avoid depending on the existence of data written using this function which
>> + * may be overwritten afterward. In addition, this function requires that the
>> + * pages containing the address are not locked. Practically, this means that you
>> + * should not write instruction memory in a current translation block inside a
>> + * callback registered with qemu_plugin_register_vcpu_tb_trans_cb.
>> + *
>> + * You can, for example, write instruction memory in a current translation block
>> + * in a callback registered with qemu_plugin_register_vcpu_tb_exec_cb, although
>> + * be aware that the write will not be flushed until after the translation block
>> + * has finished executing. In general, this function should be used to write
>> + * data memory or to patch code at a known address, not in a current translation
>> + * block.
>
> My main concern about the long list of caveats for writing memory is the
> user will almost certainly cause weird things to happen which will then
> be hard to debug. I can see the patcher example however it would be
> useful to know what other practical uses this interface provides.
>
I understand the concern that allowing modification of execution state
through plugins opens the path for possible bugs. However, it
significantly augment what is possible to do with them, especially for
security researchers, as Rowan listed in his answer.
For once, we have someone motivated to contribute upstream instead of
reinventing another downstream fork, so it should be encouraged.
As well, in case "weird things" happen and people file a bug report,
they will be free to share their plugin, so we can reproduce and solve
the problem. It should concern only users trying to modify state of
execution though, so definitely not the majority of plugins users.
Pierrick
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
` (8 preceding siblings ...)
2025-06-13 15:19 ` Pierrick Bouvier
@ 2025-06-19 16:20 ` Rowan Hart
9 siblings, 0 replies; 18+ messages in thread
From: Rowan Hart @ 2025-06-19 16:20 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Alex Bennée, Paolo Bonzini,
Richard Henderson, Philippe Mathieu-Daudé, Yanan Wang,
Mahmoud Mandour, Marcel Apfelbaum, Alexandre Iooss, Zhao Liu,
Eduardo Habkost
I've updated this patch to address some notes about the build/test
configuration for the patch plugin. Please check
https://lore.kernel.org/qemu-devel/20250619161547.1401448-1-rowanbhart@gmail.com/T/#t
instead.
On 6/11/25 4:24 PM, Rowan Hart wrote:
> This patch series adds several new API functions focused on enabling use
> cases around reading and writing guest memory from QEMU plugins. To support
> these new APIs, some utility functionality around retrieving information about
> address spaces is added as well.
>
> The new qemu_plugin_write_register utilizes gdb_write_register, which is now
> declared in gdbstub.h for this purpose instead of being static.
>
> qemu_plugin_write_memory_vaddr utilizes cpu_memory_rw_debug much the same as
> the existing read_memory_vaddr function does.
>
> The read and write_hwaddr functions are the most different. These functions
> use address_space_rw, which works well in most cases. There is an important
> caveat that for writes, the page being written will be set dirty by the
> write operation. This dirty setting requires locking the page range,
> which can contend with an already held lock in page_collection_lock
> when called in a tb translate callback with a write to the instruction
> memory in the tb. The doc comments warn against doing this, and it's unlikely
> anyone would want to do this.
>
> I've also added two test plugins: one that implements a simple hypercall
> interface that guest code can use to communicate with the plugin in a
> structured way with a test to ensure that this hypercall works and writing
> virtual memory works. And one that implements a simple patch utility to patch
> memory at runtime. The test for the second plugin ensures the patch applies
> successfully to instruction memory, and can use both hw and vaddr methods.
>
> For v3, I've had a few comments from the last submission that I've addressed,
> and some that I haven't for one reason or another:
>
> - Enforce QEMU_PLUGIN_CB_ flags in register read/write operations: done!
> - Fix my commit messages and add long messages describing commits: done!
> - Un-expose AS internals: done! Functions operate on current vCPU, current AS.
> - Clean up use of current_cpu: done!
> - Make functions take a vcpu_idx: not done. May revisit but it allows footguns.
> Even for translation, seems best to not do this now. We can easily add _vcpu
> versions of these functions in the future if we change our minds!
>
> For v5, I've just updated the enforcement of the QEMU_PLUGIN_CB_ flags to just
> use immediate stores, which simplifies the implementation quite a lot and
> should be more efficient too. Thanks Pierrick for the suggestion!
>
> v6 is a formatting pass, I left some whitespace that needed removal, some
> license text was wrong, and so forth.
>
> v8 reverts a mistake I made extending the size of arrays of TCGHelperInfo
> structs, as I misunderstood their sizes. It preserves adding an explicit
> zero as the last entry for clarity, however.
>
> v9 fixes qemu_plugin_read_register to return -1 on parameter or flag state
> error instead of 0.
>
> In v10, I relaxed the restriction on when the register r/w functions can be
> called, allowing all them to be used from any callback where the CPU is not
> currently executing, with additional notes in the documentation for exceptions
> (atexit and flush, which do not operate on a specific CPU and in which
> current_cpu is not set).
>
> v11 makes the cb flags functions inline and fixes a typo where cpu was asserted
> but current_cpu was actually accessed.
>
> v12 removes the hypercalls plugin because the functions it tested are also
> tested by the patcher plugin, making it redundant. We'll circle back on a
> hypercalls API in the future as a part of the plugin API, not as a plugin
> itself.
>
> Rowan Hart (1):
> plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W
> callbacks
>
> novafacing (6):
> gdbstub: Expose gdb_write_register function to consumers of gdbstub
> plugins: Add register write API
> plugins: Add memory virtual address write API
> plugins: Add memory hardware address read/write API
> plugins: Add patcher plugin and test
> plugins: Update plugin version and add notes
>
> accel/tcg/plugin-gen.c | 30 +++
> gdbstub/gdbstub.c | 2 +-
> include/exec/gdbstub.h | 14 ++
> include/hw/core/cpu.h | 1 +
> include/qemu/plugin.h | 15 ++
> include/qemu/qemu-plugin.h | 176 ++++++++++++++--
> plugins/api.c | 135 +++++++++++-
> plugins/core.c | 33 +++
> tests/tcg/Makefile.target | 1 +
> tests/tcg/plugins/meson.build | 2 +-
> tests/tcg/plugins/patch.c | 241 ++++++++++++++++++++++
> tests/tcg/x86_64/Makefile.softmmu-target | 32 ++-
> tests/tcg/x86_64/system/patch-target.c | 27 +++
> tests/tcg/x86_64/system/validate-patch.py | 39 ++++
> 14 files changed, 725 insertions(+), 23 deletions(-)
> create mode 100644 tests/tcg/plugins/patch.c
> create mode 100644 tests/tcg/x86_64/system/patch-target.c
> create mode 100755 tests/tcg/x86_64/system/validate-patch.py
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 5/7] plugins: Add memory hardware address read/write API
2025-06-17 15:46 ` Rowan Hart
@ 2025-06-20 13:36 ` Alex Bennée
0 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2025-06-20 13:36 UTC (permalink / raw)
To: Rowan Hart
Cc: qemu-devel, Pierrick Bouvier, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé, Yanan Wang, Mahmoud Mandour,
Marcel Apfelbaum, Alexandre Iooss, Zhao Liu, Eduardo Habkost
Rowan Hart <rowanbhart@gmail.com> writes:
>> My main concern about the long list of caveats for writing memory is the
>> user will almost certainly cause weird things to happen which will then
>> be hard to debug. I can see the patcher example however it would be
>> useful to know what other practical uses this interface provides.
>>
> Of course! My main personal intent here is to facilitate introspection
> and manipulation of guest state for security analysis. Some examples
> of why the memory/register R/W primitives are necessary here include:
>
> Fuzzing:
> - Read registers and memory for tracing control flow, comparison
> operands, and profiled values (e.g. memcmp arguments)
> - Write memory to inject test cases into the guest (for me and other
> fuzzer developers, this is the biggest reason!)
> - Write registers to reset execution or skip over complex checks like
> checksums
> - Read and write memory, and read and write registers, to do basic
> snapshot/restore by tracking dirty pages and resetting them
>
> Virtual Machine Introspection (for malware analysis and reverse
> engineering):
> - Read memory and registers to find kernel, analyze kernel structures,
> and retrieve info like process lists, memory mappings
> - Read memory and registers to quickly trace malware execution in VM
> - Write memory and registers to test behavior under various
> conditions, like skipping into checks (motivating example: what
> happens if you skip into the kill switch statement for WannaCry)
>
> Runtime patching (as in the example):
> - Writing memory to patch critical legacy code in production often can
> no longer be built or patched via means other than by applying
> binary patches (this is a real problem for e.g. the government, to
> the point where DARPA ran a program
> https://www.darpa.mil/research/programs/assured-micropatching to
> work on it!)
> - Writing registers to skip over broken code, redirect to patch code, etc.
>
> Ultimately, the caveats boil down to "don't modify stuff that's
> touched by currently executing code". I personally don't think that's
> unreasonable (as long as it's in the doc-strings) because for any
> plugin that needs to write memory, ensuring the write consistency is
> probably the easiest problem to solve and people working in this space
> are used to having way worse and jankier workarounds.
I dread to think what jankier workarounds are!
However I accept that a doc string warning will do for now. I think if
we can strengthen the guarantee at a later date to make the feature more
bullet proof we should. For example we could use start/end_exclusive to
halt the other threads while patching is taking place and then trigger a
full tb-flush to be safe. It depends how often we expect to be patching
things out?
Richard,
Do you have any view about this?
> These plugin
> functions make life way easier for them. I have been in touch with 20+
> people from various companies and projects (including Microsoft, where
> I work, as well as other big and small tech) all working on plugins
> that could be better if this feature existed, so there is definitely a
> user-base and appetite for it!
>
> The last cool use-case is that this moves us a long way towards
> cleaning up the large number of QEMU forks out there designed for RE
> and security testing like QEMU-Nyx, qemuafl, symqemu, and many more.
> Instead of maintaining forks of QEMU (many of these are based on 4.2.0
> or older!) folks can just maintain a plugin, which lets them take
> advantage of updates and fixes without giant rebases. My goal is to
> kill these forks and have these projects write small, maintainable
> plugins instead, and the authors are on board :)
Absolutely - I would like to see that too. The main reason those forks
haven't been up-streamable is because they have to make fairly invasive
changes to the frontends to do their instrumentation. I want to grow the
API to the point we can support these more advanced use cases. I am
however being conservative in adding new APIs so we take the time to get
each one right and minimise:
- leaking internal details and constricting future evolution of the emulator
- giving the users too many foot guns in the API
I'll have a look at the next version and see how we are doing.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-06-20 13:37 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 23:24 [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
2025-06-11 23:24 ` [PATCH v12 1/7] gdbstub: Expose gdb_write_register function to consumers of gdbstub Rowan Hart
2025-06-11 23:24 ` [PATCH v12 2/7] plugins: Add register write API Rowan Hart
2025-06-11 23:24 ` [PATCH v12 3/7] plugins: Add enforcement of QEMU_PLUGIN_CB flags in register R/W callbacks Rowan Hart
2025-06-11 23:24 ` [PATCH v12 4/7] plugins: Add memory virtual address write API Rowan Hart
2025-06-11 23:24 ` [PATCH v12 5/7] plugins: Add memory hardware address read/write API Rowan Hart
2025-06-17 10:24 ` Alex Bennée
2025-06-17 15:46 ` Rowan Hart
2025-06-20 13:36 ` Alex Bennée
2025-06-17 17:38 ` Pierrick Bouvier
2025-06-11 23:24 ` [PATCH v12 6/7] plugins: Add patcher plugin and test Rowan Hart
2025-06-13 15:19 ` Pierrick Bouvier
2025-06-17 10:35 ` Alex Bennée
2025-06-11 23:24 ` [PATCH v12 7/7] plugins: Update plugin version and add notes Rowan Hart
2025-06-12 3:41 ` [PATCH v12 0/7] Add additional plugin API functions to read and write memory and registers Rowan Hart
2025-06-13 15:19 ` Pierrick Bouvier
2025-06-13 15:57 ` Alex Bennée
2025-06-19 16:20 ` Rowan Hart
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).