To prevent errors when writing new types of callbacks or inline
operations, we split callbacks data to distinct types.
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 bb224b8e4c7..a078229942f 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -73,34 +73,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 7ecaf670d93..16618adf1bc 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -108,13 +108,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);
@@ -160,21 +160,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);
@@ -184,39 +184,39 @@ 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_ptr ptr = gen_plugin_u64_ptr(cb->entry);
TCGv_i64 val = tcg_temp_ebb_new_i64();
- tcg_gen_movi_i64(val, cb->inline_insn.imm);
+ tcg_gen_movi_i64(val, cb->imm);
tcg_gen_st_i64(val, ptr, 0);
tcg_temp_free_i64(val);
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),
@@ -229,16 +229,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();
@@ -249,15 +249,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.2On 3/25/24 02:41, Pierrick Bouvier wrote:
> To prevent errors when writing new types of callbacks or inline
> operations, we split callbacks data to distinct types.
>
> 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 bb224b8e4c7..a078229942f 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -73,34 +73,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;
> +};
Do you still need 'op' anymore here?
It seems redundant with 'type'.
Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
On 3/25/24 23:23, Richard Henderson wrote:
> On 3/25/24 02:41, Pierrick Bouvier wrote:
>> To prevent errors when writing new types of callbacks or inline
>> operations, we split callbacks data to distinct types.
>>
>> 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 bb224b8e4c7..a078229942f 100644
>> --- a/include/qemu/plugin.h
>> +++ b/include/qemu/plugin.h
>> @@ -73,34 +73,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;
>> +};
>
> Do you still need 'op' anymore here?
> It seems redundant with 'type'.
>
You're right, removed it in a new commit, will post new series.
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~
© 2016 - 2026 Red Hat, Inc.