* [PATCH v5 1/9] plugins: prepare introduction of new inline ops
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 2/9] plugins: extract generate ptr for qemu_plugin_u64 Pierrick Bouvier
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Until now, only add_u64 was available, and all functions assumed this or
were named uniquely.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 2 +-
accel/tcg/plugin-gen.c | 6 +++---
plugins/core.c | 14 ++++++++++++--
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 18062528c17..bee1647cfc4 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -69,7 +69,7 @@ union qemu_plugin_cb_sig {
enum plugin_dyn_cb_type {
PLUGIN_CB_REGULAR,
PLUGIN_CB_MEM_REGULAR,
- PLUGIN_CB_INLINE,
+ PLUGIN_CB_INLINE_ADD_U64,
};
/*
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 3db74ae9bfe..8028ae76c3a 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -113,7 +113,7 @@ static void gen_udata_cb(struct qemu_plugin_dyn_cb *cb)
tcg_temp_free_i32(cpu_index);
}
-static void gen_inline_cb(struct qemu_plugin_dyn_cb *cb)
+static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
{
GArray *arr = cb->inline_insn.entry.score->data;
size_t offset = cb->inline_insn.entry.offset;
@@ -158,8 +158,8 @@ static void inject_cb(struct qemu_plugin_dyn_cb *cb)
case PLUGIN_CB_REGULAR:
gen_udata_cb(cb);
break;
- case PLUGIN_CB_INLINE:
- gen_inline_cb(cb);
+ case PLUGIN_CB_INLINE_ADD_U64:
+ gen_inline_add_u64_cb(cb);
break;
default:
g_assert_not_reached();
diff --git a/plugins/core.c b/plugins/core.c
index 0213513ec65..a8557b54ff7 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -316,6 +316,16 @@ static struct qemu_plugin_dyn_cb *plugin_get_dyn_cb(GArray **arr)
return &g_array_index(cbs, struct qemu_plugin_dyn_cb, cbs->len - 1);
}
+static enum plugin_dyn_cb_type op_to_cb_type(enum qemu_plugin_op op)
+{
+ switch (op) {
+ case QEMU_PLUGIN_INLINE_ADD_U64:
+ return PLUGIN_CB_INLINE_ADD_U64;
+ default:
+ g_assert_not_reached();
+ }
+}
+
void plugin_register_inline_op_on_entry(GArray **arr,
enum qemu_plugin_mem_rw rw,
enum qemu_plugin_op op,
@@ -326,7 +336,7 @@ void plugin_register_inline_op_on_entry(GArray **arr,
dyn_cb = plugin_get_dyn_cb(arr);
dyn_cb->userp = NULL;
- dyn_cb->type = PLUGIN_CB_INLINE;
+ dyn_cb->type = op_to_cb_type(op);
dyn_cb->rw = rw;
dyn_cb->inline_insn.entry = entry;
dyn_cb->inline_insn.op = op;
@@ -551,7 +561,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
cb->regular.f.vcpu_mem(cpu->cpu_index, make_plugin_meminfo(oi, rw),
vaddr, cb->userp);
break;
- case PLUGIN_CB_INLINE:
+ case PLUGIN_CB_INLINE_ADD_U64:
exec_inline_op(cb, cpu->cpu_index);
break;
default:
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 2/9] plugins: extract generate ptr for qemu_plugin_u64
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 1/9] plugins: prepare introduction of new inline ops Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 3/9] plugins: add new inline op STORE_U64 Pierrick Bouvier
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
accel/tcg/plugin-gen.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 8028ae76c3a..8e2c3ef94f6 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -113,24 +113,33 @@ static void gen_udata_cb(struct qemu_plugin_dyn_cb *cb)
tcg_temp_free_i32(cpu_index);
}
-static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
+static TCGv_ptr gen_plugin_u64_ptr(qemu_plugin_u64 entry)
{
- GArray *arr = cb->inline_insn.entry.score->data;
- size_t offset = cb->inline_insn.entry.offset;
- TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
- TCGv_i64 val = tcg_temp_ebb_new_i64();
TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+ GArray *arr = entry.score->data;
+ char *base_ptr = arr->data + entry.offset;
+ size_t entry_size = g_array_get_element_size(arr);
+
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
tcg_gen_ld_i32(cpu_index, tcg_env,
-offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
- tcg_gen_muli_i32(cpu_index, cpu_index, g_array_get_element_size(arr));
+ tcg_gen_muli_i32(cpu_index, cpu_index, entry_size);
tcg_gen_ext_i32_ptr(ptr, cpu_index);
tcg_temp_free_i32(cpu_index);
+ tcg_gen_addi_ptr(ptr, ptr, (intptr_t) base_ptr);
- tcg_gen_addi_ptr(ptr, ptr, (intptr_t)arr->data);
- tcg_gen_ld_i64(val, ptr, offset);
+ return ptr;
+}
+
+static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
+{
+ TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
+ TCGv_i64 val = tcg_temp_ebb_new_i64();
+
+ tcg_gen_ld_i64(val, ptr, 0);
tcg_gen_addi_i64(val, val, cb->inline_insn.imm);
- tcg_gen_st_i64(val, ptr, offset);
+ tcg_gen_st_i64(val, ptr, 0);
tcg_temp_free_i64(val);
tcg_temp_free_ptr(ptr);
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 3/9] plugins: add new inline op STORE_U64
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 1/9] plugins: prepare introduction of new inline ops Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 2/9] plugins: extract generate ptr for qemu_plugin_u64 Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 4/9] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 1 +
include/qemu/qemu-plugin.h | 4 ++--
accel/tcg/plugin-gen.c | 13 +++++++++++++
plugins/core.c | 6 ++++++
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index bee1647cfc4..0c5df7fa90a 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -70,6 +70,7 @@ enum plugin_dyn_cb_type {
PLUGIN_CB_REGULAR,
PLUGIN_CB_MEM_REGULAR,
PLUGIN_CB_INLINE_ADD_U64,
+ PLUGIN_CB_INLINE_STORE_U64,
};
/*
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4fc6c3739b2..c5cac897a0b 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -305,12 +305,12 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
* enum qemu_plugin_op - describes an inline op
*
* @QEMU_PLUGIN_INLINE_ADD_U64: add an immediate value uint64_t
- *
- * Note: currently only a single inline op is supported.
+ * @QEMU_PLUGIN_INLINE_STORE_U64: store an immediate value uint64_t
*/
enum qemu_plugin_op {
QEMU_PLUGIN_INLINE_ADD_U64,
+ QEMU_PLUGIN_INLINE_STORE_U64,
};
/**
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 8e2c3ef94f6..a5313cbbb2f 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -145,6 +145,16 @@ static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
tcg_temp_free_ptr(ptr);
}
+static void gen_inline_store_u64_cb(struct qemu_plugin_dyn_cb *cb)
+{
+ TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
+ TCGv_i64 val = tcg_constant_i64(cb->inline_insn.imm);
+
+ tcg_gen_st_i64(val, ptr, 0);
+
+ tcg_temp_free_ptr(ptr);
+}
+
static void gen_mem_cb(struct qemu_plugin_dyn_cb *cb,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
{
@@ -170,6 +180,9 @@ static void inject_cb(struct qemu_plugin_dyn_cb *cb)
case PLUGIN_CB_INLINE_ADD_U64:
gen_inline_add_u64_cb(cb);
break;
+ case PLUGIN_CB_INLINE_STORE_U64:
+ gen_inline_store_u64_cb(cb);
+ break;
default:
g_assert_not_reached();
}
diff --git a/plugins/core.c b/plugins/core.c
index a8557b54ff7..e1bf0dc3717 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -321,6 +321,8 @@ static enum plugin_dyn_cb_type op_to_cb_type(enum qemu_plugin_op op)
switch (op) {
case QEMU_PLUGIN_INLINE_ADD_U64:
return PLUGIN_CB_INLINE_ADD_U64;
+ case QEMU_PLUGIN_INLINE_STORE_U64:
+ return PLUGIN_CB_INLINE_STORE_U64;
default:
g_assert_not_reached();
}
@@ -535,6 +537,9 @@ void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int cpu_index)
case QEMU_PLUGIN_INLINE_ADD_U64:
*val += cb->inline_insn.imm;
break;
+ case QEMU_PLUGIN_INLINE_STORE_U64:
+ *val = cb->inline_insn.imm;
+ break;
default:
g_assert_not_reached();
}
@@ -562,6 +567,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
vaddr, cb->userp);
break;
case PLUGIN_CB_INLINE_ADD_U64:
+ case PLUGIN_CB_INLINE_STORE_U64:
exec_inline_op(cb, cpu->cpu_index);
break;
default:
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 4/9] tests/plugin/inline: add test for STORE_U64 inline op
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (2 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 3/9] plugins: add new inline op STORE_U64 Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 5/9] plugins: conditional callbacks Pierrick Bouvier
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
tests/plugin/inline.c | 41 +++++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 0163e9b51c5..103c3a22f6e 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -22,6 +22,12 @@ typedef struct {
uint64_t count_mem_inline;
} CPUCount;
+typedef struct {
+ uint64_t data_insn;
+ uint64_t data_tb;
+ uint64_t data_mem;
+} CPUData;
+
static struct qemu_plugin_scoreboard *counts;
static qemu_plugin_u64 count_tb;
static qemu_plugin_u64 count_tb_inline;
@@ -29,6 +35,10 @@ static qemu_plugin_u64 count_insn;
static qemu_plugin_u64 count_insn_inline;
static qemu_plugin_u64 count_mem;
static qemu_plugin_u64 count_mem_inline;
+static struct qemu_plugin_scoreboard *data;
+static qemu_plugin_u64 data_insn;
+static qemu_plugin_u64 data_tb;
+static qemu_plugin_u64 data_mem;
static uint64_t global_count_tb;
static uint64_t global_count_insn;
@@ -109,11 +119,13 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
stats_mem();
qemu_plugin_scoreboard_free(counts);
+ qemu_plugin_scoreboard_free(data);
}
static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_tb, cpu_index, 1);
+ g_assert(qemu_plugin_u64_get(data_tb, cpu_index) == (uintptr_t) udata);
g_mutex_lock(&tb_lock);
max_cpu_index = MAX(max_cpu_index, cpu_index);
global_count_tb++;
@@ -123,6 +135,7 @@ static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_insn, cpu_index, 1);
+ g_assert(qemu_plugin_u64_get(data_insn, cpu_index) == (uintptr_t) udata);
g_mutex_lock(&insn_lock);
global_count_insn++;
g_mutex_unlock(&insn_lock);
@@ -131,9 +144,10 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
static void vcpu_mem_access(unsigned int cpu_index,
qemu_plugin_meminfo_t info,
uint64_t vaddr,
- void *userdata)
+ void *udata)
{
qemu_plugin_u64_add(count_mem, cpu_index, 1);
+ g_assert(qemu_plugin_u64_get(data_mem, cpu_index) == (uintptr_t) udata);
g_mutex_lock(&mem_lock);
global_count_mem++;
g_mutex_unlock(&mem_lock);
@@ -141,20 +155,34 @@ static void vcpu_mem_access(unsigned int cpu_index,
static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
{
+ void *tb_store = tb;
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_STORE_U64, data_tb, (uintptr_t) tb_store);
qemu_plugin_register_vcpu_tb_exec_cb(
- tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, 0);
+ tb, vcpu_tb_exec, QEMU_PLUGIN_CB_NO_REGS, tb_store);
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_ADD_U64, count_tb_inline, 1);
for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
+ void *insn_store = insn;
+ void *mem_store = (char *)insn_store + 0xff;
+
+ qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+ insn, QEMU_PLUGIN_INLINE_STORE_U64, data_insn,
+ (uintptr_t) insn_store);
qemu_plugin_register_vcpu_insn_exec_cb(
- insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, 0);
+ insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, insn_store);
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
+
+ qemu_plugin_register_vcpu_mem_inline_per_vcpu(
+ insn, QEMU_PLUGIN_MEM_RW,
+ QEMU_PLUGIN_INLINE_STORE_U64,
+ data_mem, (uintptr_t) mem_store);
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
- QEMU_PLUGIN_MEM_RW, 0);
+ QEMU_PLUGIN_MEM_RW, mem_store);
qemu_plugin_register_vcpu_mem_inline_per_vcpu(
insn, QEMU_PLUGIN_MEM_RW,
QEMU_PLUGIN_INLINE_ADD_U64,
@@ -179,6 +207,11 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
counts, CPUCount, count_insn_inline);
count_mem_inline = qemu_plugin_scoreboard_u64_in_struct(
counts, CPUCount, count_mem_inline);
+ data = qemu_plugin_scoreboard_new(sizeof(CPUData));
+ data_insn = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_insn);
+ data_tb = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_tb);
+ data_mem = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_mem);
+
qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 5/9] plugins: conditional callbacks
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (3 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 4/9] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 6/9] tests/plugin/inline: add test for conditional callback Pierrick Bouvier
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Extend plugins API to support callback called with a given criteria
(evaluated inline).
Added functions:
- qemu_plugin_register_vcpu_tb_exec_cond_cb
- qemu_plugin_register_vcpu_insn_exec_cond_cb
They expect as parameter a condition, a qemu_plugin_u64_t (op1) and an
immediate (op2). Callback is called if op1 |cond| op2 is true.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 8 ++++
include/qemu/qemu-plugin.h | 76 ++++++++++++++++++++++++++++++++++++
plugins/plugin.h | 8 ++++
accel/tcg/plugin-gen.c | 48 +++++++++++++++++++++++
plugins/api.c | 39 ++++++++++++++++++
plugins/core.c | 32 +++++++++++++++
plugins/qemu-plugins.symbols | 2 +
7 files changed, 213 insertions(+)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 0c5df7fa90a..0c0aae09e6f 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -68,6 +68,7 @@ union qemu_plugin_cb_sig {
enum plugin_dyn_cb_type {
PLUGIN_CB_REGULAR,
+ PLUGIN_CB_COND,
PLUGIN_CB_MEM_REGULAR,
PLUGIN_CB_INLINE_ADD_U64,
PLUGIN_CB_INLINE_STORE_U64,
@@ -89,6 +90,13 @@ struct qemu_plugin_dyn_cb {
union qemu_plugin_cb_sig f;
TCGHelperInfo *info;
} regular;
+ struct {
+ union qemu_plugin_cb_sig f;
+ TCGHelperInfo *info;
+ qemu_plugin_u64 entry;
+ enum qemu_plugin_cond cond;
+ uint64_t imm;
+ } cond;
struct {
qemu_plugin_u64 entry;
enum qemu_plugin_op op;
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index c5cac897a0b..337de25ece7 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -262,6 +262,29 @@ enum qemu_plugin_mem_rw {
QEMU_PLUGIN_MEM_RW,
};
+/**
+ * enum qemu_plugin_cond - condition to enable callback
+ *
+ * @QEMU_PLUGIN_COND_NEVER: false
+ * @QEMU_PLUGIN_COND_ALWAYS: true
+ * @QEMU_PLUGIN_COND_EQ: is equal?
+ * @QEMU_PLUGIN_COND_NE: is not equal?
+ * @QEMU_PLUGIN_COND_LT: is less than?
+ * @QEMU_PLUGIN_COND_LE: is less than or equal?
+ * @QEMU_PLUGIN_COND_GT: is greater than?
+ * @QEMU_PLUGIN_COND_GE: is greater than or equal?
+ */
+enum qemu_plugin_cond {
+ QEMU_PLUGIN_COND_NEVER,
+ QEMU_PLUGIN_COND_ALWAYS,
+ QEMU_PLUGIN_COND_EQ,
+ QEMU_PLUGIN_COND_NE,
+ QEMU_PLUGIN_COND_LT,
+ QEMU_PLUGIN_COND_LE,
+ QEMU_PLUGIN_COND_GT,
+ QEMU_PLUGIN_COND_GE,
+};
+
/**
* typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
* @id: unique plugin id
@@ -301,6 +324,32 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
enum qemu_plugin_cb_flags flags,
void *userdata);
+/**
+ * qemu_plugin_register_vcpu_tb_exec_cond_cb() - register conditional callback
+ * @tb: the opaque qemu_plugin_tb handle for the translation
+ * @cb: callback function
+ * @cond: condition to enable callback
+ * @entry: first operand for condition
+ * @imm: second operand for condition
+ * @flags: does the plugin read or write the CPU's registers?
+ * @userdata: any plugin data to pass to the @cb?
+ *
+ * The @cb function is called when a translated unit executes if
+ * entry @cond imm is true.
+ * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
+ * this function is equivalent to qemu_plugin_register_vcpu_tb_exec_cb.
+ * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
+ * callback is never installed.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *userdata);
+
/**
* enum qemu_plugin_op - describes an inline op
*
@@ -344,6 +393,33 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
enum qemu_plugin_cb_flags flags,
void *userdata);
+/**
+ * qemu_plugin_register_vcpu_insn_exec_cond_cb() - conditional insn execution cb
+ * @insn: the opaque qemu_plugin_insn handle for an instruction
+ * @cb: callback function
+ * @flags: does the plugin read or write the CPU's registers?
+ * @cond: condition to enable callback
+ * @entry: first operand for condition
+ * @imm: second operand for condition
+ * @userdata: any plugin data to pass to the @cb?
+ *
+ * The @cb function is called when an instruction executes if
+ * entry @cond imm is true.
+ * If condition is QEMU_PLUGIN_COND_ALWAYS, condition is never interpreted and
+ * this function is equivalent to qemu_plugin_register_vcpu_insn_exec_cb.
+ * If condition QEMU_PLUGIN_COND_NEVER, condition is never interpreted and
+ * callback is never installed.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ struct qemu_plugin_insn *insn,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *userdata);
+
/**
* qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu() - insn exec inline op
* @insn: the opaque qemu_plugin_insn handle for an instruction
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 7c34f23cfcb..7d4b4e21f7c 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -93,6 +93,14 @@ plugin_register_dyn_cb__udata(GArray **arr,
qemu_plugin_vcpu_udata_cb_t cb,
enum qemu_plugin_cb_flags flags, void *udata);
+void
+plugin_register_dyn_cond_cb__udata(GArray **arr,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata);
void plugin_register_vcpu_mem_cb(GArray **arr,
void *cb,
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index a5313cbbb2f..9deddd74c42 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -132,6 +132,51 @@ static TCGv_ptr gen_plugin_u64_ptr(qemu_plugin_u64 entry)
return ptr;
}
+static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
+{
+ switch (cond) {
+ case QEMU_PLUGIN_COND_EQ:
+ return TCG_COND_EQ;
+ case QEMU_PLUGIN_COND_NE:
+ return TCG_COND_NE;
+ case QEMU_PLUGIN_COND_LT:
+ return TCG_COND_LTU;
+ case QEMU_PLUGIN_COND_LE:
+ return TCG_COND_LEU;
+ case QEMU_PLUGIN_COND_GT:
+ return TCG_COND_GTU;
+ case QEMU_PLUGIN_COND_GE:
+ return TCG_COND_GEU;
+ default:
+ /* ALWAYS and NEVER conditions should never reach */
+ g_assert_not_reached();
+ }
+}
+
+static void gen_udata_cond_cb(struct qemu_plugin_dyn_cb *cb)
+{
+ TCGv_ptr ptr = gen_plugin_u64_ptr(cb->cond.entry);
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+ TCGv_i64 val = tcg_temp_ebb_new_i64();
+ TCGLabel *after_cb = gen_new_label();
+
+ /* Condition should be negated, as calling the cb is the "else" path */
+ TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond.cond));
+
+ tcg_gen_ld_i64(val, ptr, 0);
+ tcg_gen_brcondi_i64(cond, val, cb->cond.imm, after_cb);
+ tcg_gen_ld_i32(cpu_index, tcg_env,
+ -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ tcg_gen_call2(cb->cond.f.vcpu_udata, cb->cond.info, NULL,
+ tcgv_i32_temp(cpu_index),
+ tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
+ gen_set_label(after_cb);
+
+ tcg_temp_free_i64(val);
+ tcg_temp_free_i32(cpu_index);
+ tcg_temp_free_ptr(ptr);
+}
+
static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
{
TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
@@ -177,6 +222,9 @@ static void inject_cb(struct qemu_plugin_dyn_cb *cb)
case PLUGIN_CB_REGULAR:
gen_udata_cb(cb);
break;
+ case PLUGIN_CB_COND:
+ gen_udata_cond_cb(cb);
+ break;
case PLUGIN_CB_INLINE_ADD_U64:
gen_inline_add_u64_cb(cb);
break;
diff --git a/plugins/api.c b/plugins/api.c
index 3912c9cc8f6..2242d40bbbc 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -96,6 +96,25 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
}
}
+void qemu_plugin_register_vcpu_tb_exec_cond_cb(struct qemu_plugin_tb *tb,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata)
+{
+ if (cond == QEMU_PLUGIN_COND_NEVER || tb->mem_only) {
+ return;
+ }
+ if (cond == QEMU_PLUGIN_COND_ALWAYS) {
+ qemu_plugin_register_vcpu_tb_exec_cb(tb, cb, flags, udata);
+ return;
+ }
+ plugin_register_dyn_cond_cb__udata(&tb->cbs, cb, flags,
+ cond, entry, imm, udata);
+}
+
void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
struct qemu_plugin_tb *tb,
enum qemu_plugin_op op,
@@ -117,6 +136,26 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
}
}
+void qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ struct qemu_plugin_insn *insn,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata)
+{
+ if (cond == QEMU_PLUGIN_COND_NEVER || insn->mem_only) {
+ return;
+ }
+ if (cond == QEMU_PLUGIN_COND_ALWAYS) {
+ qemu_plugin_register_vcpu_insn_exec_cb(insn, cb, flags, udata);
+ return;
+ }
+ plugin_register_dyn_cond_cb__udata(&insn->insn_cbs, cb, flags,
+ cond, entry, imm, udata);
+}
+
void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
struct qemu_plugin_insn *insn,
enum qemu_plugin_op op,
diff --git a/plugins/core.c b/plugins/core.c
index e1bf0dc3717..b3d0208e022 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -371,6 +371,38 @@ void plugin_register_dyn_cb__udata(GArray **arr,
dyn_cb->regular.info = &info[flags];
}
+void plugin_register_dyn_cond_cb__udata(GArray **arr,
+ qemu_plugin_vcpu_udata_cb_t cb,
+ enum qemu_plugin_cb_flags flags,
+ enum qemu_plugin_cond cond,
+ qemu_plugin_u64 entry,
+ uint64_t imm,
+ void *udata)
+{
+ static TCGHelperInfo info[3] = {
+ [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
+ [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
+ /*
+ * Match qemu_plugin_vcpu_udata_cb_t:
+ * void (*)(uint32_t, void *)
+ */
+ [0 ... 2].typemask = (dh_typemask(void, 0) |
+ dh_typemask(i32, 1) |
+ dh_typemask(ptr, 2))
+ };
+
+ struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
+ dyn_cb->userp = udata;
+ dyn_cb->type = PLUGIN_CB_COND;
+ dyn_cb->cond.f.vcpu_udata = cb;
+ dyn_cb->cond.cond = cond;
+ dyn_cb->cond.entry = entry;
+ dyn_cb->cond.imm = imm;
+
+ assert((unsigned)flags < ARRAY_SIZE(info));
+ dyn_cb->cond.info = &info[flags];
+}
+
void plugin_register_vcpu_mem_cb(GArray **arr,
void *cb,
enum qemu_plugin_cb_flags flags,
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index a9fac056c7f..aa0a77a319f 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -27,6 +27,7 @@
qemu_plugin_register_vcpu_idle_cb;
qemu_plugin_register_vcpu_init_cb;
qemu_plugin_register_vcpu_insn_exec_cb;
+ qemu_plugin_register_vcpu_insn_exec_cond_cb;
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu;
qemu_plugin_register_vcpu_mem_cb;
qemu_plugin_register_vcpu_mem_inline_per_vcpu;
@@ -34,6 +35,7 @@
qemu_plugin_register_vcpu_syscall_cb;
qemu_plugin_register_vcpu_syscall_ret_cb;
qemu_plugin_register_vcpu_tb_exec_cb;
+ qemu_plugin_register_vcpu_tb_exec_cond_cb;
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu;
qemu_plugin_register_vcpu_tb_trans_cb;
qemu_plugin_reset;
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 6/9] tests/plugin/inline: add test for conditional callback
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (4 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 5/9] plugins: conditional callbacks Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 7/9] plugins: distinct types for callbacks Pierrick Bouvier
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Count number of tb and insn executed using a conditional callback. We
ensure the callback has been called expected number of time (per vcpu).
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
tests/plugin/inline.c | 89 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 86 insertions(+), 3 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 103c3a22f6e..cd63827b7d8 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -20,8 +20,14 @@ typedef struct {
uint64_t count_insn_inline;
uint64_t count_mem;
uint64_t count_mem_inline;
+ uint64_t tb_cond_num_trigger;
+ uint64_t tb_cond_track_count;
+ uint64_t insn_cond_num_trigger;
+ uint64_t insn_cond_track_count;
} CPUCount;
+static const uint64_t cond_trigger_limit = 100;
+
typedef struct {
uint64_t data_insn;
uint64_t data_tb;
@@ -35,6 +41,10 @@ static qemu_plugin_u64 count_insn;
static qemu_plugin_u64 count_insn_inline;
static qemu_plugin_u64 count_mem;
static qemu_plugin_u64 count_mem_inline;
+static qemu_plugin_u64 tb_cond_num_trigger;
+static qemu_plugin_u64 tb_cond_track_count;
+static qemu_plugin_u64 insn_cond_num_trigger;
+static qemu_plugin_u64 insn_cond_track_count;
static struct qemu_plugin_scoreboard *data;
static qemu_plugin_u64 data_insn;
static qemu_plugin_u64 data_tb;
@@ -56,12 +66,19 @@ static void stats_insn(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_insn);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_insn_inline);
+ const uint64_t cond_num_trigger =
+ qemu_plugin_u64_sum(insn_cond_num_trigger);
+ const uint64_t cond_track_left = qemu_plugin_u64_sum(insn_cond_track_count);
+ const uint64_t conditional =
+ cond_num_trigger * cond_trigger_limit + cond_track_left;
printf("insn: %" PRIu64 "\n", expected);
printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+ printf("insn: %" PRIu64 " (cond cb)\n", conditional);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
+ g_assert(conditional == expected);
}
static void stats_tb(void)
@@ -70,12 +87,18 @@ static void stats_tb(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_tb);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_tb_inline);
+ const uint64_t cond_num_trigger = qemu_plugin_u64_sum(tb_cond_num_trigger);
+ const uint64_t cond_track_left = qemu_plugin_u64_sum(tb_cond_track_count);
+ const uint64_t conditional =
+ cond_num_trigger * cond_trigger_limit + cond_track_left;
printf("tb: %" PRIu64 "\n", expected);
printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+ printf("tb: %" PRIu64 " (conditional cb)\n", conditional);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
+ g_assert(conditional == expected);
}
static void stats_mem(void)
@@ -104,14 +127,35 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
const uint64_t insn_inline = qemu_plugin_u64_get(count_insn_inline, i);
const uint64_t mem = qemu_plugin_u64_get(count_mem, i);
const uint64_t mem_inline = qemu_plugin_u64_get(count_mem_inline, i);
- printf("cpu %d: tb (%" PRIu64 ", %" PRIu64 ") | "
- "insn (%" PRIu64 ", %" PRIu64 ") | "
+ const uint64_t tb_cond_trigger =
+ qemu_plugin_u64_get(tb_cond_num_trigger, i);
+ const uint64_t tb_cond_left =
+ qemu_plugin_u64_get(tb_cond_track_count, i);
+ const uint64_t insn_cond_trigger =
+ qemu_plugin_u64_get(insn_cond_num_trigger, i);
+ const uint64_t insn_cond_left =
+ qemu_plugin_u64_get(insn_cond_track_count, i);
+ printf("cpu %d: tb (%" PRIu64 ", %" PRIu64
+ ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+ ") | "
+ "insn (%" PRIu64 ", %" PRIu64
+ ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+ ") | "
"mem (%" PRIu64 ", %" PRIu64 ")"
"\n",
- i, tb, tb_inline, insn, insn_inline, mem, mem_inline);
+ i,
+ tb, tb_inline,
+ tb_cond_trigger, cond_trigger_limit, tb_cond_left,
+ insn, insn_inline,
+ insn_cond_trigger, cond_trigger_limit, insn_cond_left,
+ mem, mem_inline);
g_assert(tb == tb_inline);
g_assert(insn == insn_inline);
g_assert(mem == mem_inline);
+ g_assert(tb_cond_trigger == tb / cond_trigger_limit);
+ g_assert(tb_cond_left == tb % cond_trigger_limit);
+ g_assert(insn_cond_trigger == insn / cond_trigger_limit);
+ g_assert(insn_cond_left == insn % cond_trigger_limit);
}
stats_tb();
@@ -132,6 +176,24 @@ static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
g_mutex_unlock(&tb_lock);
}
+static void vcpu_tb_cond_exec(unsigned int cpu_index, void *udata)
+{
+ g_assert(qemu_plugin_u64_get(tb_cond_track_count, cpu_index) ==
+ cond_trigger_limit);
+ g_assert(qemu_plugin_u64_get(data_tb, cpu_index) == (uintptr_t) udata);
+ qemu_plugin_u64_set(tb_cond_track_count, cpu_index, 0);
+ qemu_plugin_u64_add(tb_cond_num_trigger, cpu_index, 1);
+}
+
+static void vcpu_insn_cond_exec(unsigned int cpu_index, void *udata)
+{
+ g_assert(qemu_plugin_u64_get(insn_cond_track_count, cpu_index) ==
+ cond_trigger_limit);
+ g_assert(qemu_plugin_u64_get(data_insn, cpu_index) == (uintptr_t) udata);
+ qemu_plugin_u64_set(insn_cond_track_count, cpu_index, 0);
+ qemu_plugin_u64_add(insn_cond_num_trigger, cpu_index, 1);
+}
+
static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_insn, cpu_index, 1);
@@ -163,6 +225,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_ADD_U64, count_tb_inline, 1);
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_ADD_U64, tb_cond_track_count, 1);
+ qemu_plugin_register_vcpu_tb_exec_cond_cb(
+ tb, vcpu_tb_cond_exec, QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_COND_EQ, tb_cond_track_count, cond_trigger_limit, tb_store);
+
for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
void *insn_store = insn;
@@ -176,6 +244,13 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
+ qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+ insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_cond_track_count, 1);
+ qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ insn, vcpu_insn_cond_exec, QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_COND_EQ, insn_cond_track_count, cond_trigger_limit,
+ insn_store);
+
qemu_plugin_register_vcpu_mem_inline_per_vcpu(
insn, QEMU_PLUGIN_MEM_RW,
QEMU_PLUGIN_INLINE_STORE_U64,
@@ -207,6 +282,14 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
counts, CPUCount, count_insn_inline);
count_mem_inline = qemu_plugin_scoreboard_u64_in_struct(
counts, CPUCount, count_mem_inline);
+ tb_cond_num_trigger = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, tb_cond_num_trigger);
+ tb_cond_track_count = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, tb_cond_track_count);
+ insn_cond_num_trigger = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, insn_cond_num_trigger);
+ insn_cond_track_count = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, insn_cond_track_count);
data = qemu_plugin_scoreboard_new(sizeof(CPUData));
data_insn = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_insn);
data_tb = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_tb);
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 7/9] plugins: distinct types for callbacks
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (5 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 6/9] tests/plugin/inline: add test for conditional callback Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 8/9] plugins: extract cpu_index generate Pierrick Bouvier
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
To prevent errors when writing new types of callbacks or inline
operations, we split callbacks data to distinct types.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 46 ++++++++++++++-----------
plugins/plugin.h | 2 +-
accel/tcg/plugin-gen.c | 58 +++++++++++++++++---------------
plugins/core.c | 76 ++++++++++++++++++++++--------------------
4 files changed, 98 insertions(+), 84 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 0c0aae09e6f..313b7c72684 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -74,34 +74,40 @@ enum plugin_dyn_cb_type {
PLUGIN_CB_INLINE_STORE_U64,
};
+struct qemu_plugin_regular_cb {
+ union qemu_plugin_cb_sig f;
+ TCGHelperInfo *info;
+ void *userp;
+ enum qemu_plugin_mem_rw rw;
+};
+
+struct qemu_plugin_inline_cb {
+ qemu_plugin_u64 entry;
+ enum qemu_plugin_op op;
+ uint64_t imm;
+ enum qemu_plugin_mem_rw rw;
+};
+
+struct qemu_plugin_conditional_cb {
+ union qemu_plugin_cb_sig f;
+ TCGHelperInfo *info;
+ void *userp;
+ qemu_plugin_u64 entry;
+ enum qemu_plugin_cond cond;
+ uint64_t imm;
+};
+
/*
* A dynamic callback has an insertion point that is determined at run-time.
* Usually the insertion point is somewhere in the code cache; think for
* instance of a callback to be called upon the execution of a particular TB.
*/
struct qemu_plugin_dyn_cb {
- void *userp;
enum plugin_dyn_cb_type type;
- /* @rw applies to mem callbacks only (both regular and inline) */
- enum qemu_plugin_mem_rw rw;
- /* fields specific to each dyn_cb type go here */
union {
- struct {
- union qemu_plugin_cb_sig f;
- TCGHelperInfo *info;
- } regular;
- struct {
- union qemu_plugin_cb_sig f;
- TCGHelperInfo *info;
- qemu_plugin_u64 entry;
- enum qemu_plugin_cond cond;
- uint64_t imm;
- } cond;
- struct {
- qemu_plugin_u64 entry;
- enum qemu_plugin_op op;
- uint64_t imm;
- } inline_insn;
+ struct qemu_plugin_regular_cb regular;
+ struct qemu_plugin_conditional_cb cond;
+ struct qemu_plugin_inline_cb inline_insn;
};
};
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 7d4b4e21f7c..80d5daa9171 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -108,7 +108,7 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
enum qemu_plugin_mem_rw rw,
void *udata);
-void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int cpu_index);
+void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index);
int plugin_num_vcpus(void);
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 9deddd74c42..b829a959398 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -101,13 +101,13 @@ static void gen_disable_mem_helper(void)
offsetof(ArchCPU, env));
}
-static void gen_udata_cb(struct qemu_plugin_dyn_cb *cb)
+static void gen_udata_cb(struct qemu_plugin_regular_cb *cb)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
tcg_gen_ld_i32(cpu_index, tcg_env,
-offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
- tcg_gen_call2(cb->regular.f.vcpu_udata, cb->regular.info, NULL,
+ tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
tcg_temp_free_i32(cpu_index);
@@ -153,21 +153,21 @@ static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
}
}
-static void gen_udata_cond_cb(struct qemu_plugin_dyn_cb *cb)
+static void gen_udata_cond_cb(struct qemu_plugin_conditional_cb *cb)
{
- TCGv_ptr ptr = gen_plugin_u64_ptr(cb->cond.entry);
+ TCGv_ptr ptr = gen_plugin_u64_ptr(cb->entry);
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
TCGv_i64 val = tcg_temp_ebb_new_i64();
TCGLabel *after_cb = gen_new_label();
/* Condition should be negated, as calling the cb is the "else" path */
- TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond.cond));
+ TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond));
tcg_gen_ld_i64(val, ptr, 0);
- tcg_gen_brcondi_i64(cond, val, cb->cond.imm, after_cb);
+ tcg_gen_brcondi_i64(cond, val, cb->imm, after_cb);
tcg_gen_ld_i32(cpu_index, tcg_env,
-offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
- tcg_gen_call2(cb->cond.f.vcpu_udata, cb->cond.info, NULL,
+ tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
gen_set_label(after_cb);
@@ -177,37 +177,37 @@ static void gen_udata_cond_cb(struct qemu_plugin_dyn_cb *cb)
tcg_temp_free_ptr(ptr);
}
-static void gen_inline_add_u64_cb(struct qemu_plugin_dyn_cb *cb)
+static void gen_inline_add_u64_cb(struct qemu_plugin_inline_cb *cb)
{
- TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
+ TCGv_ptr ptr = gen_plugin_u64_ptr(cb->entry);
TCGv_i64 val = tcg_temp_ebb_new_i64();
tcg_gen_ld_i64(val, ptr, 0);
- tcg_gen_addi_i64(val, val, cb->inline_insn.imm);
+ tcg_gen_addi_i64(val, val, cb->imm);
tcg_gen_st_i64(val, ptr, 0);
tcg_temp_free_i64(val);
tcg_temp_free_ptr(ptr);
}
-static void gen_inline_store_u64_cb(struct qemu_plugin_dyn_cb *cb)
+static void gen_inline_store_u64_cb(struct qemu_plugin_inline_cb *cb)
{
- TCGv_ptr ptr = gen_plugin_u64_ptr(cb->inline_insn.entry);
- TCGv_i64 val = tcg_constant_i64(cb->inline_insn.imm);
+ TCGv_ptr ptr = gen_plugin_u64_ptr(cb->entry);
+ TCGv_i64 val = tcg_constant_i64(cb->imm);
tcg_gen_st_i64(val, ptr, 0);
tcg_temp_free_ptr(ptr);
}
-static void gen_mem_cb(struct qemu_plugin_dyn_cb *cb,
+static void gen_mem_cb(struct qemu_plugin_regular_cb *cb,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
tcg_gen_ld_i32(cpu_index, tcg_env,
-offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
- tcg_gen_call4(cb->regular.f.vcpu_mem, cb->regular.info, NULL,
+ 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),
@@ -220,16 +220,16 @@ static void inject_cb(struct qemu_plugin_dyn_cb *cb)
{
switch (cb->type) {
case PLUGIN_CB_REGULAR:
- gen_udata_cb(cb);
+ gen_udata_cb(&cb->regular);
break;
case PLUGIN_CB_COND:
- gen_udata_cond_cb(cb);
+ gen_udata_cond_cb(&cb->cond);
break;
case PLUGIN_CB_INLINE_ADD_U64:
- gen_inline_add_u64_cb(cb);
+ gen_inline_add_u64_cb(&cb->inline_insn);
break;
case PLUGIN_CB_INLINE_STORE_U64:
- gen_inline_store_u64_cb(cb);
+ gen_inline_store_u64_cb(&cb->inline_insn);
break;
default:
g_assert_not_reached();
@@ -240,15 +240,21 @@ static void inject_mem_cb(struct qemu_plugin_dyn_cb *cb,
enum qemu_plugin_mem_rw rw,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
{
- if (cb->rw & rw) {
- switch (cb->type) {
- case PLUGIN_CB_MEM_REGULAR:
- gen_mem_cb(cb, meminfo, addr);
- break;
- default:
+ switch (cb->type) {
+ case PLUGIN_CB_MEM_REGULAR:
+ if (rw && cb->regular.rw) {
+ gen_mem_cb(&cb->regular, meminfo, addr);
+ }
+ break;
+ case PLUGIN_CB_INLINE_ADD_U64:
+ case PLUGIN_CB_INLINE_STORE_U64:
+ if (rw && cb->inline_insn.rw) {
inject_cb(cb);
- break;
}
+ break;
+ default:
+ g_assert_not_reached();
+ break;
}
}
diff --git a/plugins/core.c b/plugins/core.c
index b3d0208e022..7ea2ee208db 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -336,13 +336,13 @@ void plugin_register_inline_op_on_entry(GArray **arr,
{
struct qemu_plugin_dyn_cb *dyn_cb;
+ struct qemu_plugin_inline_cb inline_cb = { .rw = rw,
+ .entry = entry,
+ .op = op,
+ .imm = imm };
dyn_cb = plugin_get_dyn_cb(arr);
- dyn_cb->userp = NULL;
dyn_cb->type = op_to_cb_type(op);
- dyn_cb->rw = rw;
- dyn_cb->inline_insn.entry = entry;
- dyn_cb->inline_insn.op = op;
- dyn_cb->inline_insn.imm = imm;
+ dyn_cb->inline_insn = inline_cb;
}
void plugin_register_dyn_cb__udata(GArray **arr,
@@ -361,14 +361,14 @@ void plugin_register_dyn_cb__udata(GArray **arr,
dh_typemask(i32, 1) |
dh_typemask(ptr, 2))
};
+ assert((unsigned)flags < ARRAY_SIZE(info));
struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
- dyn_cb->userp = udata;
+ struct qemu_plugin_regular_cb regular_cb = { .f.vcpu_udata = cb,
+ .userp = udata,
+ .info = &info[flags] };
dyn_cb->type = PLUGIN_CB_REGULAR;
- dyn_cb->regular.f.vcpu_udata = cb;
-
- assert((unsigned)flags < ARRAY_SIZE(info));
- dyn_cb->regular.info = &info[flags];
+ dyn_cb->regular = regular_cb;
}
void plugin_register_dyn_cond_cb__udata(GArray **arr,
@@ -390,17 +390,17 @@ void plugin_register_dyn_cond_cb__udata(GArray **arr,
dh_typemask(i32, 1) |
dh_typemask(ptr, 2))
};
+ assert((unsigned)flags < ARRAY_SIZE(info));
struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
- dyn_cb->userp = udata;
+ struct qemu_plugin_conditional_cb cond_cb = { .userp = udata,
+ .f.vcpu_udata = cb,
+ .cond = cond,
+ .entry = entry,
+ .imm = imm,
+ .info = &info[flags] };
dyn_cb->type = PLUGIN_CB_COND;
- dyn_cb->cond.f.vcpu_udata = cb;
- dyn_cb->cond.cond = cond;
- dyn_cb->cond.entry = entry;
- dyn_cb->cond.imm = imm;
-
- assert((unsigned)flags < ARRAY_SIZE(info));
- dyn_cb->cond.info = &info[flags];
+ dyn_cb->cond = cond_cb;
}
void plugin_register_vcpu_mem_cb(GArray **arr,
@@ -432,15 +432,15 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
dh_typemask(i64, 3) |
dh_typemask(ptr, 4))
};
+ assert((unsigned)flags < ARRAY_SIZE(info));
struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
- dyn_cb->userp = udata;
+ struct qemu_plugin_regular_cb regular_cb = { .userp = udata,
+ .rw = rw,
+ .f.vcpu_mem = cb,
+ .info = &info[flags] };
dyn_cb->type = PLUGIN_CB_MEM_REGULAR;
- dyn_cb->rw = rw;
- dyn_cb->regular.f.vcpu_mem = cb;
-
- assert((unsigned)flags < ARRAY_SIZE(info));
- dyn_cb->regular.info = &info[flags];
+ dyn_cb->regular = regular_cb;
}
/*
@@ -557,20 +557,20 @@ void qemu_plugin_flush_cb(void)
plugin_cb__simple(QEMU_PLUGIN_EV_FLUSH);
}
-void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int cpu_index)
+void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index)
{
- char *ptr = cb->inline_insn.entry.score->data->data;
+ char *ptr = cb->entry.score->data->data;
size_t elem_size = g_array_get_element_size(
- cb->inline_insn.entry.score->data);
- size_t offset = cb->inline_insn.entry.offset;
+ cb->entry.score->data);
+ size_t offset = cb->entry.offset;
uint64_t *val = (uint64_t *)(ptr + offset + cpu_index * elem_size);
- switch (cb->inline_insn.op) {
+ switch (cb->op) {
case QEMU_PLUGIN_INLINE_ADD_U64:
- *val += cb->inline_insn.imm;
+ *val += cb->imm;
break;
case QEMU_PLUGIN_INLINE_STORE_U64:
- *val = cb->inline_insn.imm;
+ *val = cb->imm;
break;
default:
g_assert_not_reached();
@@ -590,17 +590,19 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
struct qemu_plugin_dyn_cb *cb =
&g_array_index(arr, struct qemu_plugin_dyn_cb, i);
- if (!(rw & cb->rw)) {
- break;
- }
switch (cb->type) {
case PLUGIN_CB_MEM_REGULAR:
- cb->regular.f.vcpu_mem(cpu->cpu_index, make_plugin_meminfo(oi, rw),
- vaddr, cb->userp);
+ if (rw && cb->regular.rw) {
+ cb->regular.f.vcpu_mem(cpu->cpu_index,
+ make_plugin_meminfo(oi, rw),
+ vaddr, cb->regular.userp);
+ }
break;
case PLUGIN_CB_INLINE_ADD_U64:
case PLUGIN_CB_INLINE_STORE_U64:
- exec_inline_op(cb, cpu->cpu_index);
+ if (rw && cb->inline_insn.rw) {
+ exec_inline_op(&cb->inline_insn, cpu->cpu_index);
+ }
break;
default:
g_assert_not_reached();
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 8/9] plugins: extract cpu_index generate
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (6 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 7/9] plugins: distinct types for callbacks Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 9/9] plugins: remove op from qemu_plugin_inline_cb Pierrick Bouvier
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
accel/tcg/plugin-gen.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index b829a959398..7b73520e788 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -101,12 +101,17 @@ static void gen_disable_mem_helper(void)
offsetof(ArchCPU, env));
}
+static TCGv_i32 gen_cpu_index(void)
+{
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+ tcg_gen_ld_i32(cpu_index, tcg_env,
+ -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ return cpu_index;
+}
+
static void gen_udata_cb(struct qemu_plugin_regular_cb *cb)
{
- TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
-
- tcg_gen_ld_i32(cpu_index, tcg_env,
- -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ TCGv_i32 cpu_index = gen_cpu_index();
tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
@@ -121,9 +126,7 @@ static TCGv_ptr gen_plugin_u64_ptr(qemu_plugin_u64 entry)
char *base_ptr = arr->data + entry.offset;
size_t entry_size = g_array_get_element_size(arr);
- TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
- tcg_gen_ld_i32(cpu_index, tcg_env,
- -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ TCGv_i32 cpu_index = gen_cpu_index();
tcg_gen_muli_i32(cpu_index, cpu_index, entry_size);
tcg_gen_ext_i32_ptr(ptr, cpu_index);
tcg_temp_free_i32(cpu_index);
@@ -156,7 +159,6 @@ static TCGCond plugin_cond_to_tcgcond(enum qemu_plugin_cond cond)
static void gen_udata_cond_cb(struct qemu_plugin_conditional_cb *cb)
{
TCGv_ptr ptr = gen_plugin_u64_ptr(cb->entry);
- TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
TCGv_i64 val = tcg_temp_ebb_new_i64();
TCGLabel *after_cb = gen_new_label();
@@ -165,15 +167,14 @@ 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);
- tcg_gen_ld_i32(cpu_index, tcg_env,
- -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ TCGv_i32 cpu_index = gen_cpu_index();
tcg_gen_call2(cb->f.vcpu_udata, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_ptr_temp(tcg_constant_ptr(cb->userp)));
+ tcg_temp_free_i32(cpu_index);
gen_set_label(after_cb);
tcg_temp_free_i64(val);
- tcg_temp_free_i32(cpu_index);
tcg_temp_free_ptr(ptr);
}
@@ -203,10 +204,7 @@ static void gen_inline_store_u64_cb(struct qemu_plugin_inline_cb *cb)
static void gen_mem_cb(struct qemu_plugin_regular_cb *cb,
qemu_plugin_meminfo_t meminfo, TCGv_i64 addr)
{
- TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
-
- tcg_gen_ld_i32(cpu_index, tcg_env,
- -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ TCGv_i32 cpu_index = gen_cpu_index();
tcg_gen_call4(cb->f.vcpu_mem, cb->info, NULL,
tcgv_i32_temp(cpu_index),
tcgv_i32_temp(tcg_constant_i32(meminfo)),
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v5 9/9] plugins: remove op from qemu_plugin_inline_cb
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (7 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 8/9] plugins: extract cpu_index generate Pierrick Bouvier
@ 2024-05-02 18:08 ` Pierrick Bouvier
2024-05-02 18:21 ` [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
2024-05-02 19:16 ` Gustavo Romero
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:08 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Pierrick Bouvier, Mahmoud Mandour,
Richard Henderson, Alexandre Iooss, Paolo Bonzini
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 1 -
plugins/plugin.h | 4 +++-
plugins/core.c | 13 +++++++------
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 313b7c72684..1ad8a59209b 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -83,7 +83,6 @@ struct qemu_plugin_regular_cb {
struct qemu_plugin_inline_cb {
qemu_plugin_u64 entry;
- enum qemu_plugin_op op;
uint64_t imm;
enum qemu_plugin_mem_rw rw;
};
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 80d5daa9171..30e2299a54d 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -108,7 +108,9 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
enum qemu_plugin_mem_rw rw,
void *udata);
-void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index);
+void exec_inline_op(enum plugin_dyn_cb_type type,
+ struct qemu_plugin_inline_cb *cb,
+ int cpu_index);
int plugin_num_vcpus(void);
diff --git a/plugins/core.c b/plugins/core.c
index 7ea2ee208db..a9f19e197aa 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -338,7 +338,6 @@ void plugin_register_inline_op_on_entry(GArray **arr,
struct qemu_plugin_inline_cb inline_cb = { .rw = rw,
.entry = entry,
- .op = op,
.imm = imm };
dyn_cb = plugin_get_dyn_cb(arr);
dyn_cb->type = op_to_cb_type(op);
@@ -557,7 +556,9 @@ void qemu_plugin_flush_cb(void)
plugin_cb__simple(QEMU_PLUGIN_EV_FLUSH);
}
-void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index)
+void exec_inline_op(enum plugin_dyn_cb_type type,
+ struct qemu_plugin_inline_cb *cb,
+ int cpu_index)
{
char *ptr = cb->entry.score->data->data;
size_t elem_size = g_array_get_element_size(
@@ -565,11 +566,11 @@ void exec_inline_op(struct qemu_plugin_inline_cb *cb, int cpu_index)
size_t offset = cb->entry.offset;
uint64_t *val = (uint64_t *)(ptr + offset + cpu_index * elem_size);
- switch (cb->op) {
- case QEMU_PLUGIN_INLINE_ADD_U64:
+ switch (type) {
+ case PLUGIN_CB_INLINE_ADD_U64:
*val += cb->imm;
break;
- case QEMU_PLUGIN_INLINE_STORE_U64:
+ case PLUGIN_CB_INLINE_STORE_U64:
*val = cb->imm;
break;
default:
@@ -601,7 +602,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
case PLUGIN_CB_INLINE_ADD_U64:
case PLUGIN_CB_INLINE_STORE_U64:
if (rw && cb->inline_insn.rw) {
- exec_inline_op(&cb->inline_insn, cpu->cpu_index);
+ exec_inline_op(cb->type, &cb->inline_insn, cpu->cpu_index);
}
break;
default:
--
2.39.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 0/9] TCG plugins new inline operations
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (8 preceding siblings ...)
2024-05-02 18:08 ` [PATCH v5 9/9] plugins: remove op from qemu_plugin_inline_cb Pierrick Bouvier
@ 2024-05-02 18:21 ` Pierrick Bouvier
2024-05-02 19:16 ` Gustavo Romero
10 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 18:21 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Mahmoud Mandour, Richard Henderson,
Alexandre Iooss, Paolo Bonzini
Contrary to what the cover letter says, all patches have been reviewed
in the series since v4.
On 5/2/24 11:08, Pierrick Bouvier wrote:
> This series implement two new operations for plugins:
> - Store inline allows to write a specific value to a scoreboard.
> - Conditional callback executes a callback only when a given condition is true.
> The condition is evaluated inline.
>
> It's possible to mix various inline operations (add, store) with conditional
> callbacks, allowing efficient "trap" based counters.
>
> It builds on top of new scoreboard API, introduced in the previous series.
>
> NOTE: Two patches still need review
>
> v2
> --
>
> - fixed issue with udata not being passed to conditional callback
> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>
> v3
> --
>
> - rebased on top of "plugins: Rewrite plugin code generation":
> 20240316015720.3661236-1-richard.henderson@linaro.org
> - single pass code generation
> - small cleanups for code generation
>
> v4
> --
>
> - remove op field from qemu_plugin_inline_cb
> - use tcg_constant_i64 to load immediate value to store
>
> v5
> --
>
> - rebase on top of master now that Richard's series was merged
>
> Pierrick Bouvier (9):
> plugins: prepare introduction of new inline ops
> plugins: extract generate ptr for qemu_plugin_u64
> plugins: add new inline op STORE_U64
> tests/plugin/inline: add test for STORE_U64 inline op
> plugins: conditional callbacks
> tests/plugin/inline: add test for conditional callback
> plugins: distinct types for callbacks
> plugins: extract cpu_index generate
> plugins: remove op from qemu_plugin_inline_cb
>
> include/qemu/plugin.h | 42 +++++++----
> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
> plugins/plugin.h | 12 +++-
> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
> plugins/api.c | 39 ++++++++++
> plugins/core.c | 109 ++++++++++++++++++++--------
> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
> plugins/qemu-plugins.symbols | 2 +
> 8 files changed, 466 insertions(+), 84 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 0/9] TCG plugins new inline operations
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
` (9 preceding siblings ...)
2024-05-02 18:21 ` [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
@ 2024-05-02 19:16 ` Gustavo Romero
2024-05-02 19:45 ` Pierrick Bouvier
10 siblings, 1 reply; 16+ messages in thread
From: Gustavo Romero @ 2024-05-02 19:16 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Hi Pierrick,
On 5/2/24 3:08 PM, Pierrick Bouvier wrote:
> This series implement two new operations for plugins:
> - Store inline allows to write a specific value to a scoreboard.
> - Conditional callback executes a callback only when a given condition is true.
> The condition is evaluated inline.
>
> It's possible to mix various inline operations (add, store) with conditional
> callbacks, allowing efficient "trap" based counters.
>
> It builds on top of new scoreboard API, introduced in the previous series.
>
> NOTE: Two patches still need review
>
> v2
> --
>
> - fixed issue with udata not being passed to conditional callback
> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>
> v3
> --
>
> - rebased on top of "plugins: Rewrite plugin code generation":
> 20240316015720.3661236-1-richard.henderson@linaro.org
> - single pass code generation
> - small cleanups for code generation
>
> v4
> --
>
> - remove op field from qemu_plugin_inline_cb
> - use tcg_constant_i64 to load immediate value to store
>
> v5
> --
>
> - rebase on top of master now that Richard's series was merged
>
> Pierrick Bouvier (9):
> plugins: prepare introduction of new inline ops
> plugins: extract generate ptr for qemu_plugin_u64
> plugins: add new inline op STORE_U64
> tests/plugin/inline: add test for STORE_U64 inline op
> plugins: conditional callbacks
> tests/plugin/inline: add test for conditional callback
> plugins: distinct types for callbacks
> plugins: extract cpu_index generate
> plugins: remove op from qemu_plugin_inline_cb
>
> include/qemu/plugin.h | 42 +++++++----
> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
> plugins/plugin.h | 12 +++-
> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
> plugins/api.c | 39 ++++++++++
> plugins/core.c | 109 ++++++++++++++++++++--------
> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
> plugins/qemu-plugins.symbols | 2 +
> 8 files changed, 466 insertions(+), 84 deletions(-)
The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing.
Is this intentional?
Cheers,
Gustavo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 0/9] TCG plugins new inline operations
2024-05-02 19:16 ` Gustavo Romero
@ 2024-05-02 19:45 ` Pierrick Bouvier
2024-05-02 20:09 ` Gustavo Romero
0 siblings, 1 reply; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 19:45 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel
Hi Gustavo,
On 5/2/24 12:16, Gustavo Romero wrote:
> Hi Pierrick,
>
> On 5/2/24 3:08 PM, Pierrick Bouvier wrote:
>> This series implement two new operations for plugins:
>> - Store inline allows to write a specific value to a scoreboard.
>> - Conditional callback executes a callback only when a given condition is true.
>> The condition is evaluated inline.
>>
>> It's possible to mix various inline operations (add, store) with conditional
>> callbacks, allowing efficient "trap" based counters.
>>
>> It builds on top of new scoreboard API, introduced in the previous series.
>>
>> NOTE: Two patches still need review
>>
>> v2
>> --
>>
>> - fixed issue with udata not being passed to conditional callback
>> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>>
>> v3
>> --
>>
>> - rebased on top of "plugins: Rewrite plugin code generation":
>> 20240316015720.3661236-1-richard.henderson@linaro.org
>> - single pass code generation
>> - small cleanups for code generation
>>
>> v4
>> --
>>
>> - remove op field from qemu_plugin_inline_cb
>> - use tcg_constant_i64 to load immediate value to store
>>
>> v5
>> --
>>
>> - rebase on top of master now that Richard's series was merged
>>
>> Pierrick Bouvier (9):
>> plugins: prepare introduction of new inline ops
>> plugins: extract generate ptr for qemu_plugin_u64
>> plugins: add new inline op STORE_U64
>> tests/plugin/inline: add test for STORE_U64 inline op
>> plugins: conditional callbacks
>> tests/plugin/inline: add test for conditional callback
>> plugins: distinct types for callbacks
>> plugins: extract cpu_index generate
>> plugins: remove op from qemu_plugin_inline_cb
>>
>> include/qemu/plugin.h | 42 +++++++----
>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
>> plugins/plugin.h | 12 +++-
>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
>> plugins/api.c | 39 ++++++++++
>> plugins/core.c | 109 ++++++++++++++++++++--------
>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
>> plugins/qemu-plugins.symbols | 2 +
>> 8 files changed, 466 insertions(+), 84 deletions(-)
>
> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing.
>
> Is this intentional?
>
Do you mean there is no multiline commit message for those changes?
Indeed, for some of those patches, the change is a single line commit
message.
>
> Cheers,
> Gustavo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 0/9] TCG plugins new inline operations
2024-05-02 19:45 ` Pierrick Bouvier
@ 2024-05-02 20:09 ` Gustavo Romero
2024-05-02 20:48 ` Gustavo Romero
0 siblings, 1 reply; 16+ messages in thread
From: Gustavo Romero @ 2024-05-02 20:09 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
On 5/2/24 4:45 PM, Pierrick Bouvier wrote:
> Hi Gustavo,
>
> On 5/2/24 12:16, Gustavo Romero wrote:
>> Hi Pierrick,
>>
>> On 5/2/24 3:08 PM, Pierrick Bouvier wrote:
>>> This series implement two new operations for plugins:
>>> - Store inline allows to write a specific value to a scoreboard.
>>> - Conditional callback executes a callback only when a given condition is true.
>>> The condition is evaluated inline.
>>>
>>> It's possible to mix various inline operations (add, store) with conditional
>>> callbacks, allowing efficient "trap" based counters.
>>>
>>> It builds on top of new scoreboard API, introduced in the previous series.
>>>
>>> NOTE: Two patches still need review
>>>
>>> v2
>>> --
>>>
>>> - fixed issue with udata not being passed to conditional callback
>>> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>>>
>>> v3
>>> --
>>>
>>> - rebased on top of "plugins: Rewrite plugin code generation":
>>> 20240316015720.3661236-1-richard.henderson@linaro.org
>>> - single pass code generation
>>> - small cleanups for code generation
>>>
>>> v4
>>> --
>>>
>>> - remove op field from qemu_plugin_inline_cb
>>> - use tcg_constant_i64 to load immediate value to store
>>>
>>> v5
>>> --
>>>
>>> - rebase on top of master now that Richard's series was merged
>>>
>>> Pierrick Bouvier (9):
>>> plugins: prepare introduction of new inline ops
>>> plugins: extract generate ptr for qemu_plugin_u64
>>> plugins: add new inline op STORE_U64
>>> tests/plugin/inline: add test for STORE_U64 inline op
>>> plugins: conditional callbacks
>>> tests/plugin/inline: add test for conditional callback
>>> plugins: distinct types for callbacks
>>> plugins: extract cpu_index generate
>>> plugins: remove op from qemu_plugin_inline_cb
>>>
>>> include/qemu/plugin.h | 42 +++++++----
>>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
>>> plugins/plugin.h | 12 +++-
>>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
>>> plugins/api.c | 39 ++++++++++
>>> plugins/core.c | 109 ++++++++++++++++++++--------
>>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
>>> plugins/qemu-plugins.symbols | 2 +
>>> 8 files changed, 466 insertions(+), 84 deletions(-)
>>
>> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing.
>>
>> Is this intentional?
>>
>
> Do you mean there is no multiline commit message for those changes?
> Indeed, for some of those patches, the change is a single line commit message.
I just see a commit title and the Signed-off-by. For example, in 8/9
I see the following on git log:
commit f518898aa09b42e317b887237bb75a432b477c6d
Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Date: Thu May 2 11:08:46 2024 -0700
plugins: extract cpu_index generate
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
It has only the title: "plugins: extract cpu_index generate"
and R-b and S-b, so no description about the changes.
Cheers,
Gustavo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 0/9] TCG plugins new inline operations
2024-05-02 20:09 ` Gustavo Romero
@ 2024-05-02 20:48 ` Gustavo Romero
2024-05-02 21:16 ` Pierrick Bouvier
0 siblings, 1 reply; 16+ messages in thread
From: Gustavo Romero @ 2024-05-02 20:48 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Hi Pierrick,
On 5/2/24 5:09 PM, Gustavo Romero wrote:
> On 5/2/24 4:45 PM, Pierrick Bouvier wrote:
>> Hi Gustavo,
>>
>> On 5/2/24 12:16, Gustavo Romero wrote:
>>> Hi Pierrick,
>>>
>>> On 5/2/24 3:08 PM, Pierrick Bouvier wrote:
>>>> This series implement two new operations for plugins:
>>>> - Store inline allows to write a specific value to a scoreboard.
>>>> - Conditional callback executes a callback only when a given condition is true.
>>>> The condition is evaluated inline.
>>>>
>>>> It's possible to mix various inline operations (add, store) with conditional
>>>> callbacks, allowing efficient "trap" based counters.
>>>>
>>>> It builds on top of new scoreboard API, introduced in the previous series.
>>>>
>>>> NOTE: Two patches still need review
>>>>
>>>> v2
>>>> --
>>>>
>>>> - fixed issue with udata not being passed to conditional callback
>>>> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>>>>
>>>> v3
>>>> --
>>>>
>>>> - rebased on top of "plugins: Rewrite plugin code generation":
>>>> 20240316015720.3661236-1-richard.henderson@linaro.org
>>>> - single pass code generation
>>>> - small cleanups for code generation
>>>>
>>>> v4
>>>> --
>>>>
>>>> - remove op field from qemu_plugin_inline_cb
>>>> - use tcg_constant_i64 to load immediate value to store
>>>>
>>>> v5
>>>> --
>>>>
>>>> - rebase on top of master now that Richard's series was merged
>>>>
>>>> Pierrick Bouvier (9):
>>>> plugins: prepare introduction of new inline ops
>>>> plugins: extract generate ptr for qemu_plugin_u64
>>>> plugins: add new inline op STORE_U64
>>>> tests/plugin/inline: add test for STORE_U64 inline op
>>>> plugins: conditional callbacks
>>>> tests/plugin/inline: add test for conditional callback
>>>> plugins: distinct types for callbacks
>>>> plugins: extract cpu_index generate
>>>> plugins: remove op from qemu_plugin_inline_cb
>>>>
>>>> include/qemu/plugin.h | 42 +++++++----
>>>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
>>>> plugins/plugin.h | 12 +++-
>>>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
>>>> plugins/api.c | 39 ++++++++++
>>>> plugins/core.c | 109 ++++++++++++++++++++--------
>>>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
>>>> plugins/qemu-plugins.symbols | 2 +
>>>> 8 files changed, 466 insertions(+), 84 deletions(-)
>>>
>>> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing.
>>>
>>> Is this intentional?
>>>
>>
>> Do you mean there is no multiline commit message for those changes?
>> Indeed, for some of those patches, the change is a single line commit message.
>
> I just see a commit title and the Signed-off-by. For example, in 8/9
> I see the following on git log:
>
> commit f518898aa09b42e317b887237bb75a432b477c6d
> Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Date: Thu May 2 11:08:46 2024 -0700
>
> plugins: extract cpu_index generate
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>
> It has only the title: "plugins: extract cpu_index generate"
> and R-b and S-b, so no description about the changes.
The description (the body of the commit message) is mentioned here:
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message
After quickly looking at the committed logs, I do notice some commits missing it,
but I really believe it's important to have it for the reasons outlined in these
guidelines.
Cheers,
Gustavo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v5 0/9] TCG plugins new inline operations
2024-05-02 20:48 ` Gustavo Romero
@ 2024-05-02 21:16 ` Pierrick Bouvier
0 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-05-02 21:16 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel
On 5/2/24 13:48, Gustavo Romero wrote:
> Hi Pierrick,
>
> On 5/2/24 5:09 PM, Gustavo Romero wrote:
>> On 5/2/24 4:45 PM, Pierrick Bouvier wrote:
>>> Hi Gustavo,
>>>
>>> On 5/2/24 12:16, Gustavo Romero wrote:
>>>> Hi Pierrick,
>>>>
>>>> On 5/2/24 3:08 PM, Pierrick Bouvier wrote:
>>>>> This series implement two new operations for plugins:
>>>>> - Store inline allows to write a specific value to a scoreboard.
>>>>> - Conditional callback executes a callback only when a given condition is true.
>>>>> The condition is evaluated inline.
>>>>>
>>>>> It's possible to mix various inline operations (add, store) with conditional
>>>>> callbacks, allowing efficient "trap" based counters.
>>>>>
>>>>> It builds on top of new scoreboard API, introduced in the previous series.
>>>>>
>>>>> NOTE: Two patches still need review
>>>>>
>>>>> v2
>>>>> --
>>>>>
>>>>> - fixed issue with udata not being passed to conditional callback
>>>>> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>>>>>
>>>>> v3
>>>>> --
>>>>>
>>>>> - rebased on top of "plugins: Rewrite plugin code generation":
>>>>> 20240316015720.3661236-1-richard.henderson@linaro.org
>>>>> - single pass code generation
>>>>> - small cleanups for code generation
>>>>>
>>>>> v4
>>>>> --
>>>>>
>>>>> - remove op field from qemu_plugin_inline_cb
>>>>> - use tcg_constant_i64 to load immediate value to store
>>>>>
>>>>> v5
>>>>> --
>>>>>
>>>>> - rebase on top of master now that Richard's series was merged
>>>>>
>>>>> Pierrick Bouvier (9):
>>>>> plugins: prepare introduction of new inline ops
>>>>> plugins: extract generate ptr for qemu_plugin_u64
>>>>> plugins: add new inline op STORE_U64
>>>>> tests/plugin/inline: add test for STORE_U64 inline op
>>>>> plugins: conditional callbacks
>>>>> tests/plugin/inline: add test for conditional callback
>>>>> plugins: distinct types for callbacks
>>>>> plugins: extract cpu_index generate
>>>>> plugins: remove op from qemu_plugin_inline_cb
>>>>>
>>>>> include/qemu/plugin.h | 42 +++++++----
>>>>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
>>>>> plugins/plugin.h | 12 +++-
>>>>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
>>>>> plugins/api.c | 39 ++++++++++
>>>>> plugins/core.c | 109 ++++++++++++++++++++--------
>>>>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
>>>>> plugins/qemu-plugins.symbols | 2 +
>>>>> 8 files changed, 466 insertions(+), 84 deletions(-)
>>>>
>>>> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing.
>>>>
>>>> Is this intentional?
>>>>
>>>
>>> Do you mean there is no multiline commit message for those changes?
>>> Indeed, for some of those patches, the change is a single line commit message.
>>
>> I just see a commit title and the Signed-off-by. For example, in 8/9
>> I see the following on git log:
>>
>> commit f518898aa09b42e317b887237bb75a432b477c6d
>> Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Date: Thu May 2 11:08:46 2024 -0700
>>
>> plugins: extract cpu_index generate
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> It has only the title: "plugins: extract cpu_index generate"
>> and R-b and S-b, so no description about the changes.
>
> The description (the body of the commit message) is mentioned here:
>
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message
>
> After quickly looking at the committed logs, I do notice some commits missing it,
> but I really believe it's important to have it for the reasons outlined in these
> guidelines.
>
You're right, I just sent a v6 with more elaborated commit messages.
Thanks,
Pierrick
>
> Cheers,
> Gustavo
^ permalink raw reply [flat|nested] 16+ messages in thread