* [PATCH 1/5] plugins: prepare introduction of new inline ops
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Until now, only add_u64 was available, and all functions assumed this or
were named uniquely.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 2 +-
plugins/plugin.h | 1 +
accel/tcg/plugin-gen.c | 77 +++++++++++++++++++++---------------------
plugins/api.c | 23 ++++++++++---
plugins/core.c | 5 +--
5 files changed, 61 insertions(+), 47 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 12a96cea2a4..33a7cbe910c 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -74,7 +74,7 @@ enum plugin_dyn_cb_type {
enum plugin_dyn_cb_subtype {
PLUGIN_CB_REGULAR,
PLUGIN_CB_REGULAR_R,
- PLUGIN_CB_INLINE,
+ PLUGIN_CB_INLINE_ADD_U64,
PLUGIN_N_CB_SUBTYPES,
};
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 7c34f23cfcb..696b1fa38b0 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -70,6 +70,7 @@ struct qemu_plugin_ctx *plugin_id_to_ctx_locked(qemu_plugin_id_t id);
void plugin_register_inline_op_on_entry(GArray **arr,
enum qemu_plugin_mem_rw rw,
+ enum plugin_dyn_cb_subtype type,
enum qemu_plugin_op op,
qemu_plugin_u64 entry,
uint64_t imm);
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 8028786c7bb..494467e0833 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -81,7 +81,7 @@ enum plugin_gen_from {
enum plugin_gen_cb {
PLUGIN_GEN_CB_UDATA,
PLUGIN_GEN_CB_UDATA_R,
- PLUGIN_GEN_CB_INLINE,
+ PLUGIN_GEN_CB_INLINE_ADD_U64,
PLUGIN_GEN_CB_MEM,
PLUGIN_GEN_ENABLE_MEM_HELPER,
PLUGIN_GEN_DISABLE_MEM_HELPER,
@@ -127,11 +127,7 @@ static void gen_empty_udata_cb_no_rwg(void)
gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
}
-/*
- * For now we only support addi_i64.
- * When we support more ops, we can generate one empty inline cb for each.
- */
-static void gen_empty_inline_cb(void)
+static void gen_empty_inline_cb_add_u64(void)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
@@ -219,9 +215,11 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
gen_empty_mem_helper);
/* fall through */
case PLUGIN_GEN_FROM_TB:
+ /* emit inline op before any callback */
+ gen_wrapped(from, PLUGIN_GEN_CB_INLINE_ADD_U64,
+ gen_empty_inline_cb_add_u64);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
- gen_wrapped(from, PLUGIN_GEN_CB_INLINE, gen_empty_inline_cb);
break;
default:
g_assert_not_reached();
@@ -232,13 +230,14 @@ void plugin_gen_empty_mem_callback(TCGv_i64 addr, uint32_t info)
{
enum qemu_plugin_mem_rw rw = get_plugin_meminfo_rw(info);
+ /* emit inline op before any callback */
+ gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_INLINE_ADD_U64, rw);
+ gen_empty_inline_cb_add_u64();
+ tcg_gen_plugin_cb_end();
+
gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_MEM, rw);
gen_empty_mem_cb(addr, info);
tcg_gen_plugin_cb_end();
-
- gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_INLINE, rw);
- gen_empty_inline_cb();
- tcg_gen_plugin_cb_end();
}
static TCGOp *find_op(TCGOp *op, TCGOpcode opc)
@@ -436,9 +435,9 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
return op;
}
-static TCGOp *append_inline_cb(const struct qemu_plugin_dyn_cb *cb,
- TCGOp *begin_op, TCGOp *op,
- int *unused)
+static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
+ TCGOp *begin_op, TCGOp *op,
+ int *unused)
{
char *ptr = cb->inline_insn.entry.score->data->data;
size_t elem_size = g_array_get_element_size(
@@ -538,9 +537,9 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
}
static void
-inject_inline_cb(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
+inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
{
- inject_cb_type(cbs, begin_op, append_inline_cb, ok);
+ inject_cb_type(cbs, begin_op, append_inline_cb_add_u64, ok);
}
static void
@@ -588,8 +587,9 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
GArray *arr;
size_t n_cbs, i;
- cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
- cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE];
+ /* emit inline op before any callback */
+ cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
+ cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
n_cbs = 0;
for (i = 0; i < ARRAY_SIZE(cbs); i++) {
@@ -655,10 +655,11 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
}
-static void plugin_gen_tb_inline(const struct qemu_plugin_tb *ptb,
- TCGOp *begin_op)
+static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
{
- inject_inline_cb(ptb->cbs[PLUGIN_CB_INLINE], begin_op, op_ok);
+ inject_inline_cb_add_u64(ptb->cbs[PLUGIN_CB_INLINE_ADD_U64],
+ begin_op, op_ok);
}
static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
@@ -677,12 +678,12 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
}
-static void plugin_gen_insn_inline(const struct qemu_plugin_tb *ptb,
- TCGOp *begin_op, int insn_idx)
+static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
{
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
- inject_inline_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE],
- begin_op, op_ok);
+ inject_inline_cb_add_u64(
+ insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE_ADD_U64], begin_op, op_ok);
}
static void plugin_gen_mem_regular(const struct qemu_plugin_tb *ptb,
@@ -692,14 +693,12 @@ static void plugin_gen_mem_regular(const struct qemu_plugin_tb *ptb,
inject_mem_cb(insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR], begin_op);
}
-static void plugin_gen_mem_inline(const struct qemu_plugin_tb *ptb,
- TCGOp *begin_op, int insn_idx)
+static void plugin_gen_mem_inline_add_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
{
- const GArray *cbs;
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-
- cbs = insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE];
- inject_inline_cb(cbs, begin_op, op_rw);
+ inject_inline_cb_add_u64(insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64],
+ begin_op, op_rw);
}
static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
@@ -748,8 +747,8 @@ static void pr_ops(void)
case PLUGIN_GEN_CB_UDATA:
type = "udata";
break;
- case PLUGIN_GEN_CB_INLINE:
- type = "inline";
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ type = "inline add u64";
break;
case PLUGIN_GEN_CB_MEM:
type = "mem";
@@ -799,8 +798,8 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_tb_udata_r(plugin_tb, op);
break;
- case PLUGIN_GEN_CB_INLINE:
- plugin_gen_tb_inline(plugin_tb, op);
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ plugin_gen_tb_inline_add_u64(plugin_tb, op);
break;
default:
g_assert_not_reached();
@@ -818,8 +817,8 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
break;
- case PLUGIN_GEN_CB_INLINE:
- plugin_gen_insn_inline(plugin_tb, op, insn_idx);
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
break;
case PLUGIN_GEN_ENABLE_MEM_HELPER:
plugin_gen_enable_mem_helper(plugin_tb, op, insn_idx);
@@ -837,8 +836,8 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_MEM:
plugin_gen_mem_regular(plugin_tb, op, insn_idx);
break;
- case PLUGIN_GEN_CB_INLINE:
- plugin_gen_mem_inline(plugin_tb, op, insn_idx);
+ case PLUGIN_GEN_CB_INLINE_ADD_U64:
+ plugin_gen_mem_inline_add_u64(plugin_tb, op, insn_idx);
break;
default:
g_assert_not_reached();
diff --git a/plugins/api.c b/plugins/api.c
index 58e851effdb..e3bcacfa99a 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -55,6 +55,16 @@
#endif
#endif
+static enum plugin_dyn_cb_subtype op_to_cb_subtype(enum qemu_plugin_op op)
+{
+ switch (op) {
+ case QEMU_PLUGIN_INLINE_ADD_U64:
+ return PLUGIN_CB_INLINE_ADD_U64;
+ default:
+ g_assert_not_reached();
+ }
+}
+
/* Uninstall and Reset handlers */
void qemu_plugin_uninstall(qemu_plugin_id_t id, qemu_plugin_simple_cb_t cb)
@@ -108,8 +118,9 @@ void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
uint64_t imm)
{
if (!tb->mem_only) {
- plugin_register_inline_op_on_entry(
- &tb->cbs[PLUGIN_CB_INLINE], 0, op, entry, imm);
+ enum plugin_dyn_cb_subtype type = op_to_cb_subtype(op);
+ plugin_register_inline_op_on_entry(&tb->cbs[type],
+ 0, type, op, entry, imm);
}
}
@@ -135,8 +146,9 @@ void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
uint64_t imm)
{
if (!insn->mem_only) {
- plugin_register_inline_op_on_entry(
- &insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE], 0, op, entry, imm);
+ enum plugin_dyn_cb_subtype type = op_to_cb_subtype(op);
+ plugin_register_inline_op_on_entry(&insn->cbs[PLUGIN_CB_INSN][type],
+ 0, type, op, entry, imm);
}
}
@@ -162,8 +174,9 @@ void qemu_plugin_register_vcpu_mem_inline_per_vcpu(
qemu_plugin_u64 entry,
uint64_t imm)
{
+ enum plugin_dyn_cb_subtype type = op_to_cb_subtype(op);
plugin_register_inline_op_on_entry(
- &insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE], rw, op, entry, imm);
+ &insn->cbs[PLUGIN_CB_MEM][type], rw, type, op, entry, imm);
}
void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
diff --git a/plugins/core.c b/plugins/core.c
index 11ca20e6267..a641a366ef9 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -318,6 +318,7 @@ static struct qemu_plugin_dyn_cb *plugin_get_dyn_cb(GArray **arr)
void plugin_register_inline_op_on_entry(GArray **arr,
enum qemu_plugin_mem_rw rw,
+ enum plugin_dyn_cb_subtype type,
enum qemu_plugin_op op,
qemu_plugin_u64 entry,
uint64_t imm)
@@ -326,7 +327,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 = type;
dyn_cb->rw = rw;
dyn_cb->inline_insn.entry = entry;
dyn_cb->inline_insn.op = op;
@@ -514,7 +515,7 @@ void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
cb->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.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/5] plugins: add new inline op STORE_U64
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
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 | 114 ++++++++++++++++++++++++++++++++++++-
plugins/api.c | 2 +
plugins/core.c | 4 ++
5 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 33a7cbe910c..d92d64744e6 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -75,6 +75,7 @@ enum plugin_dyn_cb_subtype {
PLUGIN_CB_REGULAR,
PLUGIN_CB_REGULAR_R,
PLUGIN_CB_INLINE_ADD_U64,
+ PLUGIN_CB_INLINE_STORE_U64,
PLUGIN_N_CB_SUBTYPES,
};
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 494467e0833..02c894106e2 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -46,8 +46,9 @@
#include "qemu/plugin.h"
#include "cpu.h"
#include "tcg/tcg.h"
-#include "tcg/tcg-temp-internal.h"
+#include "tcg/tcg-internal.h"
#include "tcg/tcg-op.h"
+#include "tcg/tcg-temp-internal.h"
#include "exec/exec-all.h"
#include "exec/plugin-gen.h"
#include "exec/translator.h"
@@ -82,6 +83,7 @@ enum plugin_gen_cb {
PLUGIN_GEN_CB_UDATA,
PLUGIN_GEN_CB_UDATA_R,
PLUGIN_GEN_CB_INLINE_ADD_U64,
+ PLUGIN_GEN_CB_INLINE_STORE_U64,
PLUGIN_GEN_CB_MEM,
PLUGIN_GEN_ENABLE_MEM_HELPER,
PLUGIN_GEN_DISABLE_MEM_HELPER,
@@ -153,6 +155,30 @@ static void gen_empty_inline_cb_add_u64(void)
tcg_temp_free_i32(cpu_index);
}
+static void gen_empty_inline_cb_store_u64(void)
+{
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+ TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
+ TCGv_i64 val = tcg_temp_ebb_new_i64();
+ TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+
+ tcg_gen_ld_i32(cpu_index, tcg_env,
+ -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ /* second operand will be replaced by immediate value */
+ tcg_gen_mul_i32(cpu_index, cpu_index, cpu_index);
+ tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_index);
+ tcg_gen_movi_ptr(ptr, 0);
+ tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
+
+ tcg_gen_movi_i64(val, 0);
+ tcg_gen_st_i64(val, ptr, 0);
+
+ tcg_temp_free_ptr(ptr);
+ tcg_temp_free_i64(val);
+ tcg_temp_free_ptr(cpu_index_as_ptr);
+ tcg_temp_free_i32(cpu_index);
+}
+
static void gen_empty_mem_cb(TCGv_i64 addr, uint32_t info)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
@@ -218,6 +244,8 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
/* emit inline op before any callback */
gen_wrapped(from, PLUGIN_GEN_CB_INLINE_ADD_U64,
gen_empty_inline_cb_add_u64);
+ gen_wrapped(from, PLUGIN_GEN_CB_INLINE_STORE_U64,
+ gen_empty_inline_cb_store_u64);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
break;
@@ -235,6 +263,11 @@ void plugin_gen_empty_mem_callback(TCGv_i64 addr, uint32_t info)
gen_empty_inline_cb_add_u64();
tcg_gen_plugin_cb_end();
+ gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM,
+ PLUGIN_GEN_CB_INLINE_STORE_U64, rw);
+ gen_empty_inline_cb_store_u64();
+ tcg_gen_plugin_cb_end();
+
gen_plugin_cb_start(PLUGIN_GEN_FROM_MEM, PLUGIN_GEN_CB_MEM, rw);
gen_empty_mem_cb(addr, info);
tcg_gen_plugin_cb_end();
@@ -352,6 +385,20 @@ static TCGOp *copy_st_i64(TCGOp **begin_op, TCGOp *op)
return op;
}
+static TCGOp *copy_mov_i64(TCGOp **begin_op, TCGOp *op, uint64_t v)
+{
+ if (TCG_TARGET_REG_BITS == 32) {
+ op = copy_op(begin_op, op, INDEX_op_mov_i32);
+ op->args[1] = tcgv_i32_arg(TCGV_LOW(tcg_constant_i64(v)));
+ op = copy_op(begin_op, op, INDEX_op_mov_i32);
+ op->args[1] = tcgv_i32_arg(TCGV_HIGH(tcg_constant_i64(v)));
+ } else {
+ op = copy_op(begin_op, op, INDEX_op_mov_i64);
+ op->args[1] = tcgv_i64_arg(tcg_constant_i64(v));
+ }
+ return op;
+}
+
static TCGOp *copy_add_i64(TCGOp **begin_op, TCGOp *op, uint64_t v)
{
if (TCG_TARGET_REG_BITS == 32) {
@@ -455,6 +502,24 @@ static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
return op;
}
+static TCGOp *append_inline_cb_store_u64(const struct qemu_plugin_dyn_cb *cb,
+ TCGOp *begin_op, TCGOp *op,
+ int *unused)
+{
+ char *ptr = cb->inline_insn.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;
+ op = copy_ld_i32(&begin_op, op);
+ op = copy_mul_i32(&begin_op, op, elem_size);
+ op = copy_ext_i32_ptr(&begin_op, op);
+ op = copy_const_ptr(&begin_op, op, ptr + offset);
+ op = copy_add_ptr(&begin_op, op);
+ op = copy_mov_i64(&begin_op, op, cb->inline_insn.imm);
+ op = copy_st_i64(&begin_op, op);
+ return op;
+}
+
static TCGOp *append_mem_cb(const struct qemu_plugin_dyn_cb *cb,
TCGOp *begin_op, TCGOp *op, int *cb_idx)
{
@@ -542,6 +607,12 @@ inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
inject_cb_type(cbs, begin_op, append_inline_cb_add_u64, ok);
}
+static void
+inject_inline_cb_store_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
+{
+ inject_cb_type(cbs, begin_op, append_inline_cb_store_u64, ok);
+}
+
static void
inject_mem_cb(const GArray *cbs, TCGOp *begin_op)
{
@@ -583,13 +654,14 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
struct qemu_plugin_insn *plugin_insn,
TCGOp *begin_op)
{
- GArray *cbs[2];
+ GArray *cbs[3];
GArray *arr;
size_t n_cbs, i;
/* emit inline op before any callback */
cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
- cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
+ cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
+ cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
n_cbs = 0;
for (i = 0; i < ARRAY_SIZE(cbs); i++) {
@@ -662,6 +734,13 @@ static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
begin_op, op_ok);
}
+static void plugin_gen_tb_inline_store_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
+{
+ inject_inline_cb_store_u64(ptb->cbs[PLUGIN_CB_INLINE_STORE_U64],
+ begin_op, op_ok);
+}
+
static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -686,6 +765,14 @@ static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE_ADD_U64], begin_op, op_ok);
}
+static void plugin_gen_insn_inline_store_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_inline_cb_store_u64(
+ insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE_STORE_U64], begin_op, op_ok);
+}
+
static void plugin_gen_mem_regular(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -701,6 +788,15 @@ static void plugin_gen_mem_inline_add_u64(const struct qemu_plugin_tb *ptb,
begin_op, op_rw);
}
+static void plugin_gen_mem_inline_store_u64(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_inline_cb_store_u64(
+ insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64],
+ begin_op, op_rw);
+}
+
static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -750,6 +846,9 @@ static void pr_ops(void)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
type = "inline add u64";
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ type = "inline store u64";
+ break;
case PLUGIN_GEN_CB_MEM:
type = "mem";
break;
@@ -801,6 +900,9 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_tb_inline_add_u64(plugin_tb, op);
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ plugin_gen_tb_inline_store_u64(plugin_tb, op);
+ break;
default:
g_assert_not_reached();
}
@@ -820,6 +922,9 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ plugin_gen_insn_inline_store_u64(plugin_tb, op, insn_idx);
+ break;
case PLUGIN_GEN_ENABLE_MEM_HELPER:
plugin_gen_enable_mem_helper(plugin_tb, op, insn_idx);
break;
@@ -839,6 +944,9 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_mem_inline_add_u64(plugin_tb, op, insn_idx);
break;
+ case PLUGIN_GEN_CB_INLINE_STORE_U64:
+ plugin_gen_mem_inline_store_u64(plugin_tb, op, insn_idx);
+ break;
default:
g_assert_not_reached();
}
diff --git a/plugins/api.c b/plugins/api.c
index e3bcacfa99a..4a033c16f83 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -60,6 +60,8 @@ static enum plugin_dyn_cb_subtype op_to_cb_subtype(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();
}
diff --git a/plugins/core.c b/plugins/core.c
index a641a366ef9..11f72594229 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -489,6 +489,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();
}
@@ -516,6 +519,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.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
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..30acc7a1838 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_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);
+ qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+ tb, QEMU_PLUGIN_INLINE_STORE_U64, data_tb, (uintptr_t) 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;
+ void *mem_store = (char *)insn_store + 0xff;
+
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_STORE_U64, data_insn,
+ (uintptr_t) 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_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_STORE_U64,
+ data_mem, (uintptr_t) 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.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] plugins: conditional callbacks
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
` (2 preceding siblings ...)
2024-02-29 5:53 ` [PATCH 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-03-11 10:08 ` Alex Bennée
2024-03-11 15:43 ` Alex Bennée
2024-02-29 5:53 ` [PATCH 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
2024-03-08 10:41 ` [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
5 siblings, 2 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
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.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
include/qemu/plugin.h | 7 ++
include/qemu/qemu-plugin.h | 76 +++++++++++++++
plugins/plugin.h | 8 ++
accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
plugins/api.c | 51 ++++++++++
plugins/core.c | 19 ++++
plugins/qemu-plugins.symbols | 2 +
7 files changed, 334 insertions(+), 3 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index d92d64744e6..056102b2361 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
enum plugin_dyn_cb_subtype {
PLUGIN_CB_REGULAR,
PLUGIN_CB_REGULAR_R,
+ PLUGIN_CB_COND,
+ PLUGIN_CB_COND_R,
PLUGIN_CB_INLINE_ADD_U64,
PLUGIN_CB_INLINE_STORE_U64,
PLUGIN_N_CB_SUBTYPES,
@@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
enum qemu_plugin_op op;
uint64_t imm;
} inline_insn;
+ struct {
+ qemu_plugin_u64 entry;
+ enum qemu_plugin_cond cond;
+ uint64_t imm;
+ } cond_cb;
};
};
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 696b1fa38b0..118cc99bb2a 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -94,6 +94,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 02c894106e2..15762a26439 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -46,6 +46,7 @@
#include "qemu/plugin.h"
#include "cpu.h"
#include "tcg/tcg.h"
+#include "tcg/tcg-cond.h"
#include "tcg/tcg-internal.h"
#include "tcg/tcg-op.h"
#include "tcg/tcg-temp-internal.h"
@@ -82,6 +83,8 @@ enum plugin_gen_from {
enum plugin_gen_cb {
PLUGIN_GEN_CB_UDATA,
PLUGIN_GEN_CB_UDATA_R,
+ PLUGIN_GEN_CB_COND_UDATA,
+ PLUGIN_GEN_CB_COND_UDATA_R,
PLUGIN_GEN_CB_INLINE_ADD_U64,
PLUGIN_GEN_CB_INLINE_STORE_U64,
PLUGIN_GEN_CB_MEM,
@@ -119,16 +122,58 @@ static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
tcg_temp_free_i32(cpu_index);
}
+static void gen_empty_cond_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
+{
+ TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
+ TCGv_i32 cpu_offset = tcg_temp_ebb_new_i32();
+ TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
+ TCGv_i64 val = tcg_temp_ebb_new_i64();
+ TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+ TCGv_ptr udata = tcg_temp_ebb_new_ptr();
+ TCGLabel *after_cb = gen_new_label();
+
+ tcg_gen_movi_ptr(udata, 0);
+ tcg_gen_ld_i32(cpu_index, tcg_env,
+ -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
+ /* second operand will be replaced by immediate value */
+ tcg_gen_mul_i32(cpu_offset, cpu_index, cpu_index);
+ tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_offset);
+ tcg_gen_movi_ptr(ptr, 0);
+ tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
+ tcg_gen_ld_i64(val, ptr, 0);
+
+ tcg_gen_brcondi_i64(TCG_COND_EQ, val, 0, after_cb);
+ gen_helper(cpu_index, udata);
+ gen_set_label(after_cb);
+
+ tcg_temp_free_ptr(udata);
+ tcg_temp_free_ptr(ptr);
+ tcg_temp_free_i64(val);
+ tcg_temp_free_ptr(cpu_index_as_ptr);
+ tcg_temp_free_i32(cpu_offset);
+ tcg_temp_free_i32(cpu_index);
+}
+
static void gen_empty_udata_cb_no_wg(void)
{
gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
}
+static void gen_empty_cond_udata_cb_no_wg(void)
+{
+ gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
+}
+
static void gen_empty_udata_cb_no_rwg(void)
{
gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
}
+static void gen_empty_cond_udata_cb_no_rwg(void)
+{
+ gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
+}
+
static void gen_empty_inline_cb_add_u64(void)
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
@@ -248,6 +293,10 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
gen_empty_inline_cb_store_u64);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
+ gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA,
+ gen_empty_cond_udata_cb_no_rwg);
+ gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA_R,
+ gen_empty_cond_udata_cb_no_wg);
break;
default:
g_assert_not_reached();
@@ -456,6 +505,29 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
return op;
}
+static TCGOp *copy_brcondi_i64(TCGOp **begin_op, TCGOp *op, TCGCond cond,
+ uint64_t imm)
+{
+ if (TCG_TARGET_REG_BITS == 32) {
+ op = copy_op(begin_op, op, INDEX_op_brcond2_i32);
+ /* low(arg1), high(arg1), 32(arg2), 32(arg2 >> 32), cond, label */
+ op->args[2] = tcgv_i32_arg(tcg_constant_i32(imm));
+ op->args[3] = tcgv_i32_arg(tcg_constant_i32(imm >> 32));
+ op->args[4] = cond;
+ } else {
+ op = copy_op(begin_op, op, INDEX_op_brcond_i64);
+ /* arg1, arg2, cond, label */
+ op->args[1] = tcgv_i64_arg(tcg_constant_i64(imm));
+ op->args[2] = cond;
+ }
+ return op;
+}
+
+static TCGOp *copy_set_label(TCGOp **begin_op, TCGOp *op)
+{
+ return copy_op(begin_op, op, INDEX_op_set_label);
+}
+
/*
* When we append/replace ops here we are sensitive to changing patterns of
* TCGOps generated by the tcg_gen_FOO calls when we generated the
@@ -482,6 +554,50 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
return op;
}
+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 TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
+ TCGOp *begin_op, TCGOp *op, int *cb_idx)
+{
+ char *ptr = cb->cond_cb.entry.score->data->data;
+ size_t elem_size = g_array_get_element_size(
+ cb->cond_cb.entry.score->data);
+ size_t offset = cb->cond_cb.entry.offset;
+ /* Condition should be negated, as calling the cb is the "else" path */
+ TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
+
+ op = copy_const_ptr(&begin_op, op, ptr);
+ op = copy_ld_i32(&begin_op, op);
+ op = copy_mul_i32(&begin_op, op, elem_size);
+ op = copy_ext_i32_ptr(&begin_op, op);
+ op = copy_const_ptr(&begin_op, op, ptr + offset);
+ op = copy_add_ptr(&begin_op, op);
+ op = copy_ld_i64(&begin_op, op);
+ op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
+ op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
+ op = copy_set_label(&begin_op, op);
+ return op;
+}
+
static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
TCGOp *begin_op, TCGOp *op,
int *unused)
@@ -601,6 +717,12 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
inject_cb_type(cbs, begin_op, append_udata_cb, op_ok);
}
+static void
+inject_cond_udata_cb(const GArray *cbs, TCGOp *begin_op)
+{
+ inject_cb_type(cbs, begin_op, append_cond_udata_cb, op_ok);
+}
+
static void
inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
{
@@ -654,7 +776,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
struct qemu_plugin_insn *plugin_insn,
TCGOp *begin_op)
{
- GArray *cbs[3];
+ GArray *cbs[4];
GArray *arr;
size_t n_cbs, i;
@@ -662,6 +784,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
+ cbs[3] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_COND];
n_cbs = 0;
for (i = 0; i < ARRAY_SIZE(cbs); i++) {
@@ -727,6 +850,18 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
}
+static void plugin_gen_tb_cond_udata(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
+{
+ inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND], begin_op);
+}
+
+static void plugin_gen_tb_cond_udata_r(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op)
+{
+ inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND_R], begin_op);
+}
+
static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op)
{
@@ -745,7 +880,6 @@ static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-
inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR], begin_op);
}
@@ -753,10 +887,23 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
-
inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
}
+static void plugin_gen_insn_cond_udata(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND], begin_op);
+}
+
+static void plugin_gen_insn_cond_udata_r(const struct qemu_plugin_tb *ptb,
+ TCGOp *begin_op, int insn_idx)
+{
+ struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
+ inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND_R], begin_op);
+}
+
static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
TCGOp *begin_op, int insn_idx)
{
@@ -843,6 +990,15 @@ static void pr_ops(void)
case PLUGIN_GEN_CB_UDATA:
type = "udata";
break;
+ case PLUGIN_GEN_CB_UDATA_R:
+ type = "udata (read)";
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA:
+ type = "cond udata";
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA_R:
+ type = "cond udata (read)";
+ break;
case PLUGIN_GEN_CB_INLINE_ADD_U64:
type = "inline add u64";
break;
@@ -897,6 +1053,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_tb_udata_r(plugin_tb, op);
break;
+ case PLUGIN_GEN_CB_COND_UDATA:
+ plugin_gen_tb_cond_udata(plugin_tb, op);
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA_R:
+ plugin_gen_tb_cond_udata_r(plugin_tb, op);
+ break;
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_tb_inline_add_u64(plugin_tb, op);
break;
@@ -919,6 +1081,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
case PLUGIN_GEN_CB_UDATA_R:
plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
break;
+ case PLUGIN_GEN_CB_COND_UDATA:
+ plugin_gen_insn_cond_udata(plugin_tb, op, insn_idx);
+ break;
+ case PLUGIN_GEN_CB_COND_UDATA_R:
+ plugin_gen_insn_cond_udata_r(plugin_tb, op, insn_idx);
+ break;
case PLUGIN_GEN_CB_INLINE_ADD_U64:
plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
break;
diff --git a/plugins/api.c b/plugins/api.c
index 4a033c16f83..f24d68d7a68 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -113,6 +113,31 @@ 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;
+ }
+ int index = flags == QEMU_PLUGIN_CB_R_REGS ||
+ flags == QEMU_PLUGIN_CB_RW_REGS ?
+ PLUGIN_CB_COND_R : PLUGIN_CB_COND;
+
+ plugin_register_dyn_cond_cb__udata(&tb->cbs[index],
+ 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,
@@ -141,6 +166,32 @@ 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;
+ }
+ int index = flags == QEMU_PLUGIN_CB_R_REGS ||
+ flags == QEMU_PLUGIN_CB_RW_REGS ?
+ PLUGIN_CB_COND_R : PLUGIN_CB_COND;
+
+ plugin_register_dyn_cond_cb__udata(&insn->cbs[PLUGIN_CB_INSN][index],
+ 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 11f72594229..82d5f17438e 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -347,6 +347,25 @@ void plugin_register_dyn_cb__udata(GArray **arr,
dyn_cb->type = PLUGIN_CB_REGULAR;
}
+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)
+{
+ struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
+
+ dyn_cb->userp = udata;
+ /* Note flags are discarded as unused. */
+ dyn_cb->f.vcpu_udata = cb;
+ dyn_cb->type = PLUGIN_CB_COND;
+ dyn_cb->cond_cb.cond = cond;
+ dyn_cb->cond_cb.entry = entry;
+ dyn_cb->cond_cb.imm = imm;
+}
+
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.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
@ 2024-03-11 10:08 ` Alex Bennée
2024-03-12 6:03 ` Pierrick Bouvier
2024-03-11 15:43 ` Alex Bennée
1 sibling, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2024-03-11 10:08 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 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.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
> include/qemu/plugin.h | 7 ++
> include/qemu/qemu-plugin.h | 76 +++++++++++++++
> plugins/plugin.h | 8 ++
> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
> plugins/api.c | 51 ++++++++++
> plugins/core.c | 19 ++++
> plugins/qemu-plugins.symbols | 2 +
> 7 files changed, 334 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index d92d64744e6..056102b2361 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
> enum plugin_dyn_cb_subtype {
> PLUGIN_CB_REGULAR,
> PLUGIN_CB_REGULAR_R,
> + PLUGIN_CB_COND,
> + PLUGIN_CB_COND_R,
> PLUGIN_CB_INLINE_ADD_U64,
> PLUGIN_CB_INLINE_STORE_U64,
> PLUGIN_N_CB_SUBTYPES,
> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
> enum qemu_plugin_op op;
> uint64_t imm;
> } inline_insn;
> + struct {
> + qemu_plugin_u64 entry;
> + enum qemu_plugin_cond cond;
> + uint64_t imm;
> + } cond_cb;
> };
> };
>
> 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,
Is this a fixed entry or part of a scoreboard?
I'm trying to write a control flow plugin with a structure like:
/* We use this to track the current execution state */
typedef struct {
/* address of start of block */
uint64_t block_start;
/* address of end of block */
uint64_t block_end;
/* address of last executed PC */
uint64_t last_pc;
} VCPUScoreBoard;
And I want to check to see if last_pc (set by STORE_U64 for each insn) == block_end
(set at start of TB with what we know).
Is this something I need to get with:
last_pc = qemu_plugin_scoreboard_u64_in_struct(state, VCPUScoreBoard, last_pc);
?
> + 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 696b1fa38b0..118cc99bb2a 100644
> --- a/plugins/plugin.h
> +++ b/plugins/plugin.h
> @@ -94,6 +94,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 02c894106e2..15762a26439 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -46,6 +46,7 @@
> #include "qemu/plugin.h"
> #include "cpu.h"
> #include "tcg/tcg.h"
> +#include "tcg/tcg-cond.h"
> #include "tcg/tcg-internal.h"
> #include "tcg/tcg-op.h"
> #include "tcg/tcg-temp-internal.h"
> @@ -82,6 +83,8 @@ enum plugin_gen_from {
> enum plugin_gen_cb {
> PLUGIN_GEN_CB_UDATA,
> PLUGIN_GEN_CB_UDATA_R,
> + PLUGIN_GEN_CB_COND_UDATA,
> + PLUGIN_GEN_CB_COND_UDATA_R,
> PLUGIN_GEN_CB_INLINE_ADD_U64,
> PLUGIN_GEN_CB_INLINE_STORE_U64,
> PLUGIN_GEN_CB_MEM,
> @@ -119,16 +122,58 @@ static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
> tcg_temp_free_i32(cpu_index);
> }
>
> +static void gen_empty_cond_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
> +{
> + TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
> + TCGv_i32 cpu_offset = tcg_temp_ebb_new_i32();
> + TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
> + TCGv_i64 val = tcg_temp_ebb_new_i64();
> + TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
> + TCGv_ptr udata = tcg_temp_ebb_new_ptr();
> + TCGLabel *after_cb = gen_new_label();
> +
> + tcg_gen_movi_ptr(udata, 0);
> + tcg_gen_ld_i32(cpu_index, tcg_env,
> + -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
> + /* second operand will be replaced by immediate value */
> + tcg_gen_mul_i32(cpu_offset, cpu_index, cpu_index);
> + tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_offset);
> + tcg_gen_movi_ptr(ptr, 0);
> + tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
> + tcg_gen_ld_i64(val, ptr, 0);
> +
> + tcg_gen_brcondi_i64(TCG_COND_EQ, val, 0, after_cb);
> + gen_helper(cpu_index, udata);
> + gen_set_label(after_cb);
> +
> + tcg_temp_free_ptr(udata);
> + tcg_temp_free_ptr(ptr);
> + tcg_temp_free_i64(val);
> + tcg_temp_free_ptr(cpu_index_as_ptr);
> + tcg_temp_free_i32(cpu_offset);
> + tcg_temp_free_i32(cpu_index);
> +}
> +
> static void gen_empty_udata_cb_no_wg(void)
> {
> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
> }
>
> +static void gen_empty_cond_udata_cb_no_wg(void)
> +{
> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
> +}
> +
> static void gen_empty_udata_cb_no_rwg(void)
> {
> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
> }
>
> +static void gen_empty_cond_udata_cb_no_rwg(void)
> +{
> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
> +}
> +
> static void gen_empty_inline_cb_add_u64(void)
> {
> TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
> @@ -248,6 +293,10 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
> gen_empty_inline_cb_store_u64);
> gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
> gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA,
> + gen_empty_cond_udata_cb_no_rwg);
> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA_R,
> + gen_empty_cond_udata_cb_no_wg);
> break;
> default:
> g_assert_not_reached();
> @@ -456,6 +505,29 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
> return op;
> }
>
> +static TCGOp *copy_brcondi_i64(TCGOp **begin_op, TCGOp *op, TCGCond cond,
> + uint64_t imm)
> +{
> + if (TCG_TARGET_REG_BITS == 32) {
> + op = copy_op(begin_op, op, INDEX_op_brcond2_i32);
> + /* low(arg1), high(arg1), 32(arg2), 32(arg2 >> 32), cond, label */
> + op->args[2] = tcgv_i32_arg(tcg_constant_i32(imm));
> + op->args[3] = tcgv_i32_arg(tcg_constant_i32(imm >> 32));
> + op->args[4] = cond;
> + } else {
> + op = copy_op(begin_op, op, INDEX_op_brcond_i64);
> + /* arg1, arg2, cond, label */
> + op->args[1] = tcgv_i64_arg(tcg_constant_i64(imm));
> + op->args[2] = cond;
> + }
> + return op;
> +}
> +
> +static TCGOp *copy_set_label(TCGOp **begin_op, TCGOp *op)
> +{
> + return copy_op(begin_op, op, INDEX_op_set_label);
> +}
> +
> /*
> * When we append/replace ops here we are sensitive to changing patterns of
> * TCGOps generated by the tcg_gen_FOO calls when we generated the
> @@ -482,6 +554,50 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
> return op;
> }
>
> +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 TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
> +{
> + char *ptr = cb->cond_cb.entry.score->data->data;
> + size_t elem_size = g_array_get_element_size(
> + cb->cond_cb.entry.score->data);
> + size_t offset = cb->cond_cb.entry.offset;
> + /* Condition should be negated, as calling the cb is the "else" path */
> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
> +
> + op = copy_const_ptr(&begin_op, op, ptr);
> + op = copy_ld_i32(&begin_op, op);
> + op = copy_mul_i32(&begin_op, op, elem_size);
> + op = copy_ext_i32_ptr(&begin_op, op);
> + op = copy_const_ptr(&begin_op, op, ptr + offset);
> + op = copy_add_ptr(&begin_op, op);
> + op = copy_ld_i64(&begin_op, op);
> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
> + op = copy_set_label(&begin_op, op);
> + return op;
> +}
> +
> static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
> TCGOp *begin_op, TCGOp *op,
> int *unused)
> @@ -601,6 +717,12 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
> inject_cb_type(cbs, begin_op, append_udata_cb, op_ok);
> }
>
> +static void
> +inject_cond_udata_cb(const GArray *cbs, TCGOp *begin_op)
> +{
> + inject_cb_type(cbs, begin_op, append_cond_udata_cb, op_ok);
> +}
> +
> static void
> inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
> {
> @@ -654,7 +776,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
> struct qemu_plugin_insn *plugin_insn,
> TCGOp *begin_op)
> {
> - GArray *cbs[3];
> + GArray *cbs[4];
> GArray *arr;
> size_t n_cbs, i;
>
> @@ -662,6 +784,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
> cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
> cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
> cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
> + cbs[3] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_COND];
>
> n_cbs = 0;
> for (i = 0; i < ARRAY_SIZE(cbs); i++) {
> @@ -727,6 +850,18 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
> inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
> }
>
> +static void plugin_gen_tb_cond_udata(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op)
> +{
> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND], begin_op);
> +}
> +
> +static void plugin_gen_tb_cond_udata_r(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op)
> +{
> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND_R], begin_op);
> +}
> +
> static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op)
> {
> @@ -745,7 +880,6 @@ static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op, int insn_idx)
> {
> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> -
> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR], begin_op);
> }
>
> @@ -753,10 +887,23 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op, int insn_idx)
> {
> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> -
> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
> }
>
> +static void plugin_gen_insn_cond_udata(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op, int insn_idx)
> +{
> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND], begin_op);
> +}
> +
> +static void plugin_gen_insn_cond_udata_r(const struct qemu_plugin_tb *ptb,
> + TCGOp *begin_op, int insn_idx)
> +{
> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND_R], begin_op);
> +}
> +
> static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
> TCGOp *begin_op, int insn_idx)
> {
> @@ -843,6 +990,15 @@ static void pr_ops(void)
> case PLUGIN_GEN_CB_UDATA:
> type = "udata";
> break;
> + case PLUGIN_GEN_CB_UDATA_R:
> + type = "udata (read)";
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA:
> + type = "cond udata";
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA_R:
> + type = "cond udata (read)";
> + break;
> case PLUGIN_GEN_CB_INLINE_ADD_U64:
> type = "inline add u64";
> break;
> @@ -897,6 +1053,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
> case PLUGIN_GEN_CB_UDATA_R:
> plugin_gen_tb_udata_r(plugin_tb, op);
> break;
> + case PLUGIN_GEN_CB_COND_UDATA:
> + plugin_gen_tb_cond_udata(plugin_tb, op);
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA_R:
> + plugin_gen_tb_cond_udata_r(plugin_tb, op);
> + break;
> case PLUGIN_GEN_CB_INLINE_ADD_U64:
> plugin_gen_tb_inline_add_u64(plugin_tb, op);
> break;
> @@ -919,6 +1081,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
> case PLUGIN_GEN_CB_UDATA_R:
> plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
> break;
> + case PLUGIN_GEN_CB_COND_UDATA:
> + plugin_gen_insn_cond_udata(plugin_tb, op, insn_idx);
> + break;
> + case PLUGIN_GEN_CB_COND_UDATA_R:
> + plugin_gen_insn_cond_udata_r(plugin_tb, op, insn_idx);
> + break;
> case PLUGIN_GEN_CB_INLINE_ADD_U64:
> plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
> break;
> diff --git a/plugins/api.c b/plugins/api.c
> index 4a033c16f83..f24d68d7a68 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -113,6 +113,31 @@ 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;
> + }
> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
> + flags == QEMU_PLUGIN_CB_RW_REGS ?
> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
> +
> + plugin_register_dyn_cond_cb__udata(&tb->cbs[index],
> + 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,
> @@ -141,6 +166,32 @@ 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;
> + }
> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
> + flags == QEMU_PLUGIN_CB_RW_REGS ?
> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
> +
> + plugin_register_dyn_cond_cb__udata(&insn->cbs[PLUGIN_CB_INSN][index],
> + 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 11f72594229..82d5f17438e 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -347,6 +347,25 @@ void plugin_register_dyn_cb__udata(GArray **arr,
> dyn_cb->type = PLUGIN_CB_REGULAR;
> }
>
> +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)
> +{
> + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
> +
> + dyn_cb->userp = udata;
> + /* Note flags are discarded as unused. */
> + dyn_cb->f.vcpu_udata = cb;
> + dyn_cb->type = PLUGIN_CB_COND;
> + dyn_cb->cond_cb.cond = cond;
> + dyn_cb->cond_cb.entry = entry;
> + dyn_cb->cond_cb.imm = imm;
> +}
> +
> 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;
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-11 10:08 ` Alex Bennée
@ 2024-03-12 6:03 ` Pierrick Bouvier
2024-03-12 15:04 ` Alex Bennée
0 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 6:03 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/11/24 14:08, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> 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.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>> include/qemu/plugin.h | 7 ++
>> include/qemu/qemu-plugin.h | 76 +++++++++++++++
>> plugins/plugin.h | 8 ++
>> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
>> plugins/api.c | 51 ++++++++++
>> plugins/core.c | 19 ++++
>> plugins/qemu-plugins.symbols | 2 +
>> 7 files changed, 334 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>> index d92d64744e6..056102b2361 100644
>> --- a/include/qemu/plugin.h
>> +++ b/include/qemu/plugin.h
>> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
>> enum plugin_dyn_cb_subtype {
>> PLUGIN_CB_REGULAR,
>> PLUGIN_CB_REGULAR_R,
>> + PLUGIN_CB_COND,
>> + PLUGIN_CB_COND_R,
>> PLUGIN_CB_INLINE_ADD_U64,
>> PLUGIN_CB_INLINE_STORE_U64,
>> PLUGIN_N_CB_SUBTYPES,
>> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
>> enum qemu_plugin_op op;
>> uint64_t imm;
>> } inline_insn;
>> + struct {
>> + qemu_plugin_u64 entry;
>> + enum qemu_plugin_cond cond;
>> + uint64_t imm;
>> + } cond_cb;
>> };
>> };
>>
>> 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,
>
> Is this a fixed entry or part of a scoreboard?
>
entry is an entry of scoreboard (automatically associated to each vcpu
using vcpu_index) and can be modified by any other inline op, or
callback. @imm (next parameter) is fixed yes.
callback will be called only if entry <cond> imm true.
tests/plugin/inline.c has tests for this, and store operation.
> I'm trying to write a control flow plugin with a structure like:
>
> /* We use this to track the current execution state */
> typedef struct {
> /* address of start of block */
> uint64_t block_start;
> /* address of end of block */
> uint64_t block_end;
> /* address of last executed PC */
> uint64_t last_pc;
> } VCPUScoreBoard;
>
Seems ok.
> And I want to check to see if last_pc (set by STORE_U64 for each insn) == block_end
> (set at start of TB with what we know).
>
> Is this something I need to get with:
>
> last_pc = qemu_plugin_scoreboard_u64_in_struct(state, VCPUScoreBoard, last_pc);
>
Yes.
> ?
>
>> + 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 696b1fa38b0..118cc99bb2a 100644
>> --- a/plugins/plugin.h
>> +++ b/plugins/plugin.h
>> @@ -94,6 +94,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 02c894106e2..15762a26439 100644
>> --- a/accel/tcg/plugin-gen.c
>> +++ b/accel/tcg/plugin-gen.c
>> @@ -46,6 +46,7 @@
>> #include "qemu/plugin.h"
>> #include "cpu.h"
>> #include "tcg/tcg.h"
>> +#include "tcg/tcg-cond.h"
>> #include "tcg/tcg-internal.h"
>> #include "tcg/tcg-op.h"
>> #include "tcg/tcg-temp-internal.h"
>> @@ -82,6 +83,8 @@ enum plugin_gen_from {
>> enum plugin_gen_cb {
>> PLUGIN_GEN_CB_UDATA,
>> PLUGIN_GEN_CB_UDATA_R,
>> + PLUGIN_GEN_CB_COND_UDATA,
>> + PLUGIN_GEN_CB_COND_UDATA_R,
>> PLUGIN_GEN_CB_INLINE_ADD_U64,
>> PLUGIN_GEN_CB_INLINE_STORE_U64,
>> PLUGIN_GEN_CB_MEM,
>> @@ -119,16 +122,58 @@ static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
>> tcg_temp_free_i32(cpu_index);
>> }
>>
>> +static void gen_empty_cond_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
>> +{
>> + TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
>> + TCGv_i32 cpu_offset = tcg_temp_ebb_new_i32();
>> + TCGv_ptr cpu_index_as_ptr = tcg_temp_ebb_new_ptr();
>> + TCGv_i64 val = tcg_temp_ebb_new_i64();
>> + TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
>> + TCGv_ptr udata = tcg_temp_ebb_new_ptr();
>> + TCGLabel *after_cb = gen_new_label();
>> +
>> + tcg_gen_movi_ptr(udata, 0);
>> + tcg_gen_ld_i32(cpu_index, tcg_env,
>> + -offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
>> + /* second operand will be replaced by immediate value */
>> + tcg_gen_mul_i32(cpu_offset, cpu_index, cpu_index);
>> + tcg_gen_ext_i32_ptr(cpu_index_as_ptr, cpu_offset);
>> + tcg_gen_movi_ptr(ptr, 0);
>> + tcg_gen_add_ptr(ptr, ptr, cpu_index_as_ptr);
>> + tcg_gen_ld_i64(val, ptr, 0);
>> +
>> + tcg_gen_brcondi_i64(TCG_COND_EQ, val, 0, after_cb);
>> + gen_helper(cpu_index, udata);
>> + gen_set_label(after_cb);
>> +
>> + tcg_temp_free_ptr(udata);
>> + tcg_temp_free_ptr(ptr);
>> + tcg_temp_free_i64(val);
>> + tcg_temp_free_ptr(cpu_index_as_ptr);
>> + tcg_temp_free_i32(cpu_offset);
>> + tcg_temp_free_i32(cpu_index);
>> +}
>> +
>> static void gen_empty_udata_cb_no_wg(void)
>> {
>> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
>> }
>>
>> +static void gen_empty_cond_udata_cb_no_wg(void)
>> +{
>> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
>> +}
>> +
>> static void gen_empty_udata_cb_no_rwg(void)
>> {
>> gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
>> }
>>
>> +static void gen_empty_cond_udata_cb_no_rwg(void)
>> +{
>> + gen_empty_cond_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
>> +}
>> +
>> static void gen_empty_inline_cb_add_u64(void)
>> {
>> TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
>> @@ -248,6 +293,10 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
>> gen_empty_inline_cb_store_u64);
>> gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb_no_rwg);
>> gen_wrapped(from, PLUGIN_GEN_CB_UDATA_R, gen_empty_udata_cb_no_wg);
>> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA,
>> + gen_empty_cond_udata_cb_no_rwg);
>> + gen_wrapped(from, PLUGIN_GEN_CB_COND_UDATA_R,
>> + gen_empty_cond_udata_cb_no_wg);
>> break;
>> default:
>> g_assert_not_reached();
>> @@ -456,6 +505,29 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
>> return op;
>> }
>>
>> +static TCGOp *copy_brcondi_i64(TCGOp **begin_op, TCGOp *op, TCGCond cond,
>> + uint64_t imm)
>> +{
>> + if (TCG_TARGET_REG_BITS == 32) {
>> + op = copy_op(begin_op, op, INDEX_op_brcond2_i32);
>> + /* low(arg1), high(arg1), 32(arg2), 32(arg2 >> 32), cond, label */
>> + op->args[2] = tcgv_i32_arg(tcg_constant_i32(imm));
>> + op->args[3] = tcgv_i32_arg(tcg_constant_i32(imm >> 32));
>> + op->args[4] = cond;
>> + } else {
>> + op = copy_op(begin_op, op, INDEX_op_brcond_i64);
>> + /* arg1, arg2, cond, label */
>> + op->args[1] = tcgv_i64_arg(tcg_constant_i64(imm));
>> + op->args[2] = cond;
>> + }
>> + return op;
>> +}
>> +
>> +static TCGOp *copy_set_label(TCGOp **begin_op, TCGOp *op)
>> +{
>> + return copy_op(begin_op, op, INDEX_op_set_label);
>> +}
>> +
>> /*
>> * When we append/replace ops here we are sensitive to changing patterns of
>> * TCGOps generated by the tcg_gen_FOO calls when we generated the
>> @@ -482,6 +554,50 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>> return op;
>> }
>>
>> +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 TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
>> +{
>> + char *ptr = cb->cond_cb.entry.score->data->data;
>> + size_t elem_size = g_array_get_element_size(
>> + cb->cond_cb.entry.score->data);
>> + size_t offset = cb->cond_cb.entry.offset;
>> + /* Condition should be negated, as calling the cb is the "else" path */
>> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
>> +
>> + op = copy_const_ptr(&begin_op, op, ptr);
>> + op = copy_ld_i32(&begin_op, op);
>> + op = copy_mul_i32(&begin_op, op, elem_size);
>> + op = copy_ext_i32_ptr(&begin_op, op);
>> + op = copy_const_ptr(&begin_op, op, ptr + offset);
>> + op = copy_add_ptr(&begin_op, op);
>> + op = copy_ld_i64(&begin_op, op);
>> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
>> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
>> + op = copy_set_label(&begin_op, op);
>> + return op;
>> +}
>> +
>> static TCGOp *append_inline_cb_add_u64(const struct qemu_plugin_dyn_cb *cb,
>> TCGOp *begin_op, TCGOp *op,
>> int *unused)
>> @@ -601,6 +717,12 @@ inject_udata_cb(const GArray *cbs, TCGOp *begin_op)
>> inject_cb_type(cbs, begin_op, append_udata_cb, op_ok);
>> }
>>
>> +static void
>> +inject_cond_udata_cb(const GArray *cbs, TCGOp *begin_op)
>> +{
>> + inject_cb_type(cbs, begin_op, append_cond_udata_cb, op_ok);
>> +}
>> +
>> static void
>> inject_inline_cb_add_u64(const GArray *cbs, TCGOp *begin_op, op_ok_fn ok)
>> {
>> @@ -654,7 +776,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
>> struct qemu_plugin_insn *plugin_insn,
>> TCGOp *begin_op)
>> {
>> - GArray *cbs[3];
>> + GArray *cbs[4];
>> GArray *arr;
>> size_t n_cbs, i;
>>
>> @@ -662,6 +784,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
>> cbs[0] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_ADD_U64];
>> cbs[1] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE_STORE_U64];
>> cbs[2] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR];
>> + cbs[3] = plugin_insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_COND];
>>
>> n_cbs = 0;
>> for (i = 0; i < ARRAY_SIZE(cbs); i++) {
>> @@ -727,6 +850,18 @@ static void plugin_gen_tb_udata_r(const struct qemu_plugin_tb *ptb,
>> inject_udata_cb(ptb->cbs[PLUGIN_CB_REGULAR_R], begin_op);
>> }
>>
>> +static void plugin_gen_tb_cond_udata(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op)
>> +{
>> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND], begin_op);
>> +}
>> +
>> +static void plugin_gen_tb_cond_udata_r(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op)
>> +{
>> + inject_cond_udata_cb(ptb->cbs[PLUGIN_CB_COND_R], begin_op);
>> +}
>> +
>> static void plugin_gen_tb_inline_add_u64(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op)
>> {
>> @@ -745,7 +880,6 @@ static void plugin_gen_insn_udata(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op, int insn_idx)
>> {
>> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> -
>> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR], begin_op);
>> }
>>
>> @@ -753,10 +887,23 @@ static void plugin_gen_insn_udata_r(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op, int insn_idx)
>> {
>> struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> -
>> inject_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR_R], begin_op);
>> }
>>
>> +static void plugin_gen_insn_cond_udata(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op, int insn_idx)
>> +{
>> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND], begin_op);
>> +}
>> +
>> +static void plugin_gen_insn_cond_udata_r(const struct qemu_plugin_tb *ptb,
>> + TCGOp *begin_op, int insn_idx)
>> +{
>> + struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
>> + inject_cond_udata_cb(insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_COND_R], begin_op);
>> +}
>> +
>> static void plugin_gen_insn_inline_add_u64(const struct qemu_plugin_tb *ptb,
>> TCGOp *begin_op, int insn_idx)
>> {
>> @@ -843,6 +990,15 @@ static void pr_ops(void)
>> case PLUGIN_GEN_CB_UDATA:
>> type = "udata";
>> break;
>> + case PLUGIN_GEN_CB_UDATA_R:
>> + type = "udata (read)";
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA:
>> + type = "cond udata";
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA_R:
>> + type = "cond udata (read)";
>> + break;
>> case PLUGIN_GEN_CB_INLINE_ADD_U64:
>> type = "inline add u64";
>> break;
>> @@ -897,6 +1053,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>> case PLUGIN_GEN_CB_UDATA_R:
>> plugin_gen_tb_udata_r(plugin_tb, op);
>> break;
>> + case PLUGIN_GEN_CB_COND_UDATA:
>> + plugin_gen_tb_cond_udata(plugin_tb, op);
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA_R:
>> + plugin_gen_tb_cond_udata_r(plugin_tb, op);
>> + break;
>> case PLUGIN_GEN_CB_INLINE_ADD_U64:
>> plugin_gen_tb_inline_add_u64(plugin_tb, op);
>> break;
>> @@ -919,6 +1081,12 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
>> case PLUGIN_GEN_CB_UDATA_R:
>> plugin_gen_insn_udata_r(plugin_tb, op, insn_idx);
>> break;
>> + case PLUGIN_GEN_CB_COND_UDATA:
>> + plugin_gen_insn_cond_udata(plugin_tb, op, insn_idx);
>> + break;
>> + case PLUGIN_GEN_CB_COND_UDATA_R:
>> + plugin_gen_insn_cond_udata_r(plugin_tb, op, insn_idx);
>> + break;
>> case PLUGIN_GEN_CB_INLINE_ADD_U64:
>> plugin_gen_insn_inline_add_u64(plugin_tb, op, insn_idx);
>> break;
>> diff --git a/plugins/api.c b/plugins/api.c
>> index 4a033c16f83..f24d68d7a68 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -113,6 +113,31 @@ 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;
>> + }
>> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
>> + flags == QEMU_PLUGIN_CB_RW_REGS ?
>> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
>> +
>> + plugin_register_dyn_cond_cb__udata(&tb->cbs[index],
>> + 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,
>> @@ -141,6 +166,32 @@ 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;
>> + }
>> + int index = flags == QEMU_PLUGIN_CB_R_REGS ||
>> + flags == QEMU_PLUGIN_CB_RW_REGS ?
>> + PLUGIN_CB_COND_R : PLUGIN_CB_COND;
>> +
>> + plugin_register_dyn_cond_cb__udata(&insn->cbs[PLUGIN_CB_INSN][index],
>> + 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 11f72594229..82d5f17438e 100644
>> --- a/plugins/core.c
>> +++ b/plugins/core.c
>> @@ -347,6 +347,25 @@ void plugin_register_dyn_cb__udata(GArray **arr,
>> dyn_cb->type = PLUGIN_CB_REGULAR;
>> }
>>
>> +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)
>> +{
>> + struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
>> +
>> + dyn_cb->userp = udata;
>> + /* Note flags are discarded as unused. */
>> + dyn_cb->f.vcpu_udata = cb;
>> + dyn_cb->type = PLUGIN_CB_COND;
>> + dyn_cb->cond_cb.cond = cond;
>> + dyn_cb->cond_cb.entry = entry;
>> + dyn_cb->cond_cb.imm = imm;
>> +}
>> +
>> 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;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-12 6:03 ` Pierrick Bouvier
@ 2024-03-12 15:04 ` Alex Bennée
2024-03-12 16:04 ` Pierrick Bouvier
0 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2024-03-12 15:04 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> On 3/11/24 14:08, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> 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.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>> include/qemu/plugin.h | 7 ++
>>> include/qemu/qemu-plugin.h | 76 +++++++++++++++
>>> plugins/plugin.h | 8 ++
>>> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
>>> plugins/api.c | 51 ++++++++++
>>> plugins/core.c | 19 ++++
>>> plugins/qemu-plugins.symbols | 2 +
>>> 7 files changed, 334 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>>> index d92d64744e6..056102b2361 100644
>>> --- a/include/qemu/plugin.h
>>> +++ b/include/qemu/plugin.h
>>> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
>>> enum plugin_dyn_cb_subtype {
>>> PLUGIN_CB_REGULAR,
>>> PLUGIN_CB_REGULAR_R,
>>> + PLUGIN_CB_COND,
>>> + PLUGIN_CB_COND_R,
>>> PLUGIN_CB_INLINE_ADD_U64,
>>> PLUGIN_CB_INLINE_STORE_U64,
>>> PLUGIN_N_CB_SUBTYPES,
>>> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
>>> enum qemu_plugin_op op;
>>> uint64_t imm;
>>> } inline_insn;
>>> + struct {
>>> + qemu_plugin_u64 entry;
>>> + enum qemu_plugin_cond cond;
>>> + uint64_t imm;
>>> + } cond_cb;
>>> };
>>> };
>>> 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,
>> Is this a fixed entry or part of a scoreboard?
>>
>
> entry is an entry of scoreboard (automatically associated to each vcpu
> using vcpu_index) and can be modified by any other inline op, or
> callback. @imm (next parameter) is fixed yes.
>
> callback will be called only if entry <cond> imm true.
I wonder if having an alternate form for comparing two scoreboard
entries would be useful?
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-12 15:04 ` Alex Bennée
@ 2024-03-12 16:04 ` Pierrick Bouvier
0 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 16:04 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/12/24 19:04, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> On 3/11/24 14:08, Alex Bennée wrote:
>>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>> include/qemu/plugin.h | 7 ++
>>>> include/qemu/qemu-plugin.h | 76 +++++++++++++++
>>>> plugins/plugin.h | 8 ++
>>>> accel/tcg/plugin-gen.c | 174 ++++++++++++++++++++++++++++++++++-
>>>> plugins/api.c | 51 ++++++++++
>>>> plugins/core.c | 19 ++++
>>>> plugins/qemu-plugins.symbols | 2 +
>>>> 7 files changed, 334 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>>>> index d92d64744e6..056102b2361 100644
>>>> --- a/include/qemu/plugin.h
>>>> +++ b/include/qemu/plugin.h
>>>> @@ -74,6 +74,8 @@ enum plugin_dyn_cb_type {
>>>> enum plugin_dyn_cb_subtype {
>>>> PLUGIN_CB_REGULAR,
>>>> PLUGIN_CB_REGULAR_R,
>>>> + PLUGIN_CB_COND,
>>>> + PLUGIN_CB_COND_R,
>>>> PLUGIN_CB_INLINE_ADD_U64,
>>>> PLUGIN_CB_INLINE_STORE_U64,
>>>> PLUGIN_N_CB_SUBTYPES,
>>>> @@ -97,6 +99,11 @@ struct qemu_plugin_dyn_cb {
>>>> enum qemu_plugin_op op;
>>>> uint64_t imm;
>>>> } inline_insn;
>>>> + struct {
>>>> + qemu_plugin_u64 entry;
>>>> + enum qemu_plugin_cond cond;
>>>> + uint64_t imm;
>>>> + } cond_cb;
>>>> };
>>>> };
>>>> 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,
>>> Is this a fixed entry or part of a scoreboard?
>>>
>>
>> entry is an entry of scoreboard (automatically associated to each vcpu
>> using vcpu_index) and can be modified by any other inline op, or
>> callback. @imm (next parameter) is fixed yes.
>>
>> callback will be called only if entry <cond> imm true.
>
> I wonder if having an alternate form for comparing two scoreboard
> entries would be useful?
>
We can always add a new API for that in the future if a specific need is
identified. In our current use cases, this need was not revealed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
2024-03-11 10:08 ` Alex Bennée
@ 2024-03-11 15:43 ` Alex Bennée
2024-03-12 6:03 ` Pierrick Bouvier
1 sibling, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2024-03-11 15:43 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 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.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
<snip>
>
> +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 TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
> +{
> + char *ptr = cb->cond_cb.entry.score->data->data;
> + size_t elem_size = g_array_get_element_size(
> + cb->cond_cb.entry.score->data);
> + size_t offset = cb->cond_cb.entry.offset;
> + /* Condition should be negated, as calling the cb is the "else" path */
> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
> +
> + op = copy_const_ptr(&begin_op, op, ptr);
> + op = copy_ld_i32(&begin_op, op);
> + op = copy_mul_i32(&begin_op, op, elem_size);
> + op = copy_ext_i32_ptr(&begin_op, op);
> + op = copy_const_ptr(&begin_op, op, ptr + offset);
> + op = copy_add_ptr(&begin_op, op);
> + op = copy_ld_i64(&begin_op, op);
> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
> + op = copy_set_label(&begin_op, op);
> + return op;
I think we are missing something here to ensure that udata is set
correctly for the callback, see my RFC:
Subject: [RFC PATCH] contrib/plugins: control flow plugin (WIP!)
Date: Mon, 11 Mar 2024 15:34:32 +0000
Message-Id: <20240311153432.1395190-1-alex.bennee@linaro.org>
which is seeing the same value every time in the callback.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-11 15:43 ` Alex Bennée
@ 2024-03-12 6:03 ` Pierrick Bouvier
2024-03-12 7:37 ` Pierrick Bouvier
0 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 6:03 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/11/24 19:43, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> 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.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> <snip>
>>
>> +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 TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
>> +{
>> + char *ptr = cb->cond_cb.entry.score->data->data;
>> + size_t elem_size = g_array_get_element_size(
>> + cb->cond_cb.entry.score->data);
>> + size_t offset = cb->cond_cb.entry.offset;
>> + /* Condition should be negated, as calling the cb is the "else" path */
>> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
>> +
>> + op = copy_const_ptr(&begin_op, op, ptr);
>> + op = copy_ld_i32(&begin_op, op);
>> + op = copy_mul_i32(&begin_op, op, elem_size);
>> + op = copy_ext_i32_ptr(&begin_op, op);
>> + op = copy_const_ptr(&begin_op, op, ptr + offset);
>> + op = copy_add_ptr(&begin_op, op);
>> + op = copy_ld_i64(&begin_op, op);
>> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
>> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
>> + op = copy_set_label(&begin_op, op);
>> + return op;
>
> I think we are missing something here to ensure that udata is set
> correctly for the callback, see my RFC:
>
> Subject: [RFC PATCH] contrib/plugins: control flow plugin (WIP!)
> Date: Mon, 11 Mar 2024 15:34:32 +0000
> Message-Id: <20240311153432.1395190-1-alex.bennee@linaro.org>
>
> which is seeing the same value every time in the callback.
>
I'm trying to reproduce and will answer on this thread.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] plugins: conditional callbacks
2024-03-12 6:03 ` Pierrick Bouvier
@ 2024-03-12 7:37 ` Pierrick Bouvier
0 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:37 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss,
Richard Henderson
On 3/12/24 10:03, Pierrick Bouvier wrote:
> On 3/11/24 19:43, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> 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.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> <snip>
>>>
>>> +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 TCGOp *append_cond_udata_cb(const struct qemu_plugin_dyn_cb *cb,
>>> + TCGOp *begin_op, TCGOp *op, int *cb_idx)
>>> +{
>>> + char *ptr = cb->cond_cb.entry.score->data->data;
>>> + size_t elem_size = g_array_get_element_size(
>>> + cb->cond_cb.entry.score->data);
>>> + size_t offset = cb->cond_cb.entry.offset;
>>> + /* Condition should be negated, as calling the cb is the "else" path */
>>> + TCGCond cond = tcg_invert_cond(plugin_cond_to_tcgcond(cb->cond_cb.cond));
>>> +
>>> + op = copy_const_ptr(&begin_op, op, ptr);
This line was wrong, and cb->userp should be copied instead.
I'll fix this, add a test specifically checking udata for conditional
callback and resend the series.
>>> + op = copy_ld_i32(&begin_op, op);
>>> + op = copy_mul_i32(&begin_op, op, elem_size);
>>> + op = copy_ext_i32_ptr(&begin_op, op);
>>> + op = copy_const_ptr(&begin_op, op, ptr + offset);
>>> + op = copy_add_ptr(&begin_op, op);
>>> + op = copy_ld_i64(&begin_op, op);
>>> + op = copy_brcondi_i64(&begin_op, op, cond, cb->cond_cb.imm);
>>> + op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
>>> + op = copy_set_label(&begin_op, op);
>>> + return op;
>>
>> I think we are missing something here to ensure that udata is set
>> correctly for the callback, see my RFC:
>>
>> Subject: [RFC PATCH] contrib/plugins: control flow plugin (WIP!)
>> Date: Mon, 11 Mar 2024 15:34:32 +0000
>> Message-Id: <20240311153432.1395190-1-alex.bennee@linaro.org>
>>
>> which is seeing the same value every time in the callback.
>>
>
> I'm trying to reproduce and will answer on this thread.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] tests/plugin/inline: add test for condition callback
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
` (3 preceding siblings ...)
2024-02-29 5:53 ` [PATCH 4/5] plugins: conditional callbacks Pierrick Bouvier
@ 2024-02-29 5:53 ` Pierrick Bouvier
2024-03-08 10:41 ` [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-02-29 5:53 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Pierrick Bouvier, Richard Henderson
Count number of tb and insn executed using a conditional callback. We
ensure the callback has been called expected number of time (per vcpu).
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
tests/plugin/inline.c | 87 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 84 insertions(+), 3 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 30acc7a1838..8720ecc358a 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,22 @@ 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);
+ 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);
+ 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 +223,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_STORE_U64, data_tb, (uintptr_t) tb_store);
+ 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, NULL);
+
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 +242,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,
+ NULL);
+
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
QEMU_PLUGIN_MEM_RW, mem_store);
@@ -207,6 +280,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.43.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] TCG plugins new inline operations
2024-02-29 5:53 [PATCH 0/5] TCG plugins new inline operations Pierrick Bouvier
` (4 preceding siblings ...)
2024-02-29 5:53 ` [PATCH 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
@ 2024-03-08 10:41 ` Pierrick Bouvier
5 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2024-03-08 10:41 UTC (permalink / raw)
To: qemu-devel
Cc: Mahmoud Mandour, Paolo Bonzini, Alexandre Iooss, Alex Bennée,
Richard Henderson
Hi everyone,
polite ping for this series, that might have a chance to be included for
9.0 soft-freeze.
Regards,
Pierrick
On 2/29/24 09:53, 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.
>
> Based-on: 20240229052506.933222-1-pierrick.bouvier@linaro.org
>
> Pierrick Bouvier (5):
> plugins: prepare introduction of new inline ops
> 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 condition callback
>
> include/qemu/plugin.h | 10 +-
> include/qemu/qemu-plugin.h | 80 +++++++-
> plugins/plugin.h | 9 +
> accel/tcg/plugin-gen.c | 359 +++++++++++++++++++++++++++++++----
> plugins/api.c | 76 +++++++-
> plugins/core.c | 28 ++-
> tests/plugin/inline.c | 128 ++++++++++++-
> plugins/qemu-plugins.symbols | 2 +
> 8 files changed, 633 insertions(+), 59 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread