* [PATCH v2 1/5] plugins: prepare introduction of new inline ops
2024-03-12 7:54 [PATCH v2 0/5] TCG plugins new inline operations Pierrick Bouvier
@ 2024-03-12 7:54 ` Pierrick Bouvier
2024-03-12 21:01 ` Richard Henderson
2024-03-12 7:54 ` [PATCH v2 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:54 UTC (permalink / raw)
To: qemu-devel
Cc: Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour, Paolo Bonzini,
Alex Bennée, 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 8fa5a600ac3..09ff7c70127 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.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/5] plugins: prepare introduction of new inline ops
2024-03-12 7:54 ` [PATCH v2 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
@ 2024-03-12 21:01 ` Richard Henderson
0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2024-03-12 21:01 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alexandre Iooss, Mahmoud Mandour, Paolo Bonzini, Alex Bennée
On 3/11/24 21:54, Pierrick Bouvier wrote:
> 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(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] plugins: add new inline op STORE_U64
2024-03-12 7:54 [PATCH v2 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-03-12 7:54 ` [PATCH v2 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
@ 2024-03-12 7:54 ` Pierrick Bouvier
2024-03-12 21:15 ` Richard Henderson
2024-03-12 7:54 ` [PATCH v2 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:54 UTC (permalink / raw)
To: qemu-devel
Cc: Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour, Paolo Bonzini,
Alex Bennée, 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 09ff7c70127..b7feed224a8 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.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/5] plugins: add new inline op STORE_U64
2024-03-12 7:54 ` [PATCH v2 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
@ 2024-03-12 21:15 ` Richard Henderson
2024-03-13 7:58 ` Pierrick Bouvier
0 siblings, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2024-03-12 21:15 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alexandre Iooss, Mahmoud Mandour, Paolo Bonzini, Alex Bennée
On 3/11/24 21:54, Pierrick Bouvier wrote:
> +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);
> +}
I was never fond of this full pattern generate...
> @@ -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;
> +}
... followed by pattern match and modify. I think adding more of this is fragile, and a
mistake.
(1) This encodes knowledge of the order of the parts of a mov_i64 for 32-bit host.
(2) You shouldn't use TCG_LOW/HIGH of tcg_constant_i64, but two separate calls to
tcg_constant_i32 with the parts of @v.
But all of this would be easier if we simply generate new code now, instead of copy.
> +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);
You'd also be able to fold offset into the store. This would allow the scoreboard address
to be entered once into the constant pool and have multiple uses.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/5] plugins: add new inline op STORE_U64
2024-03-12 21:15 ` Richard Henderson
@ 2024-03-13 7:58 ` Pierrick Bouvier
0 siblings, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-13 7:58 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Alexandre Iooss, Mahmoud Mandour, Paolo Bonzini, Alex Bennée
On 3/13/24 01:15, Richard Henderson wrote:
> On 3/11/24 21:54, Pierrick Bouvier wrote:
>> +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);
>> +}
>
> I was never fond of this full pattern generate...
>
I agree with you. Didn't want to start this discussion, but yes,
implementing this feels clunky and error prone (especially the replace
part, that depends on architecture bitness for which you execute).
>> @@ -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;
>> +}
>
> ... followed by pattern match and modify. I think adding more of this is fragile, and a
> mistake.
>
> (1) This encodes knowledge of the order of the parts of a mov_i64 for 32-bit host.
> (2) You shouldn't use TCG_LOW/HIGH of tcg_constant_i64, but two separate calls to
> tcg_constant_i32 with the parts of @v.
>
> But all of this would be easier if we simply generate new code now, instead of copy.
I'm open to work on this kind of change, and simply have a single pass
that generate tcg ops, just before optimizing step, and translation to
target arch. I would like to hear what Alex opinion is on doing this.
>
>> +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);
>
> You'd also be able to fold offset into the store. This would allow the scoreboard address
> to be entered once into the constant pool and have multiple uses.
>
The problem is that several callbacks can operate on several scoreboards
(with different entries offset), so I'm not sure what we can precompute
here.
We would need to keep a set of all target scoreboards, pre-compute final
pointer for everyone of them, and emit this before any callback code.
This sounded more complicated than just emitting all this.
>
> r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] tests/plugin/inline: add test for STORE_U64 inline op
2024-03-12 7:54 [PATCH v2 0/5] TCG plugins new inline operations Pierrick Bouvier
2024-03-12 7:54 ` [PATCH v2 1/5] plugins: prepare introduction of new inline ops Pierrick Bouvier
2024-03-12 7:54 ` [PATCH v2 2/5] plugins: add new inline op STORE_U64 Pierrick Bouvier
@ 2024-03-12 7:54 ` Pierrick Bouvier
2024-03-12 7:54 ` [PATCH v2 4/5] plugins: conditional callbacks Pierrick Bouvier
2024-03-12 7:54 ` [PATCH v2 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
4 siblings, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:54 UTC (permalink / raw)
To: qemu-devel
Cc: Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour, Paolo Bonzini,
Alex Bennée, 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.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v2 4/5] plugins: conditional callbacks
2024-03-12 7:54 [PATCH v2 0/5] TCG plugins new inline operations Pierrick Bouvier
` (2 preceding siblings ...)
2024-03-12 7:54 ` [PATCH v2 3/5] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
@ 2024-03-12 7:54 ` Pierrick Bouvier
2024-03-12 21:25 ` Richard Henderson
2024-03-12 7:54 ` [PATCH v2 5/5] tests/plugin/inline: add test for condition callback Pierrick Bouvier
4 siblings, 1 reply; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:54 UTC (permalink / raw)
To: qemu-devel
Cc: Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour, Paolo Bonzini,
Alex Bennée, 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..790d9630dd2 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, cb->userp);
+ 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 b7feed224a8..fdf6167655e 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.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/5] plugins: conditional callbacks
2024-03-12 7:54 ` [PATCH v2 4/5] plugins: conditional callbacks Pierrick Bouvier
@ 2024-03-12 21:25 ` Richard Henderson
2024-03-13 7:53 ` Pierrick Bouvier
0 siblings, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2024-03-12 21:25 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alexandre Iooss, Mahmoud Mandour, Paolo Bonzini, Alex Bennée
On 3/11/24 21:54, Pierrick Bouvier wrote:
> +/**
> + * 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,
> +};
Do you really need to expose ALWAYS/NEVER?
I guess these are all unsigned? Would it be clearer to add "U" suffixes?
r~
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/5] plugins: conditional callbacks
2024-03-12 21:25 ` Richard Henderson
@ 2024-03-13 7:53 ` Pierrick Bouvier
0 siblings, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-13 7:53 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Alexandre Iooss, Mahmoud Mandour, Paolo Bonzini, Alex Bennée
On 3/13/24 01:25, Richard Henderson wrote:
> On 3/11/24 21:54, Pierrick Bouvier wrote:
>> +/**
>> + * 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,
>> +};
>
> Do you really need to expose ALWAYS/NEVER?
Not mandatory, but offers possibility to have a condition (based on a
plugin command line parameter for instance), without using necessarily
conditional callbacks (if option=off,option=on or other value).
> I guess these are all unsigned? Would it be clearer to add "U" suffixes?
>
For now we only have conditional callbacks/inline ops with unsigned type
(qemu_plugin_u64). Found this to be redundant, but we can always add
this suffix if you think it's better.
>
> r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] tests/plugin/inline: add test for condition callback
2024-03-12 7:54 [PATCH v2 0/5] TCG plugins new inline operations Pierrick Bouvier
` (3 preceding siblings ...)
2024-03-12 7:54 ` [PATCH v2 4/5] plugins: conditional callbacks Pierrick Bouvier
@ 2024-03-12 7:54 ` Pierrick Bouvier
4 siblings, 0 replies; 11+ messages in thread
From: Pierrick Bouvier @ 2024-03-12 7:54 UTC (permalink / raw)
To: qemu-devel
Cc: Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour, Paolo Bonzini,
Alex Bennée, 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 | 89 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 86 insertions(+), 3 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 30acc7a1838..771c246094e 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -20,8 +20,14 @@ typedef struct {
uint64_t count_insn_inline;
uint64_t count_mem;
uint64_t count_mem_inline;
+ uint64_t tb_cond_num_trigger;
+ uint64_t tb_cond_track_count;
+ uint64_t insn_cond_num_trigger;
+ uint64_t insn_cond_track_count;
} CPUCount;
+static const uint64_t cond_trigger_limit = 100;
+
typedef struct {
uint64_t data_insn;
uint64_t data_tb;
@@ -35,6 +41,10 @@ static qemu_plugin_u64 count_insn;
static qemu_plugin_u64 count_insn_inline;
static qemu_plugin_u64 count_mem;
static qemu_plugin_u64 count_mem_inline;
+static qemu_plugin_u64 tb_cond_num_trigger;
+static qemu_plugin_u64 tb_cond_track_count;
+static qemu_plugin_u64 insn_cond_num_trigger;
+static qemu_plugin_u64 insn_cond_track_count;
static struct qemu_plugin_scoreboard *data;
static qemu_plugin_u64 data_insn;
static qemu_plugin_u64 data_tb;
@@ -56,12 +66,19 @@ static void stats_insn(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_insn);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_insn_inline);
+ const uint64_t cond_num_trigger =
+ qemu_plugin_u64_sum(insn_cond_num_trigger);
+ const uint64_t cond_track_left = qemu_plugin_u64_sum(insn_cond_track_count);
+ const uint64_t conditional =
+ cond_num_trigger * cond_trigger_limit + cond_track_left;
printf("insn: %" PRIu64 "\n", expected);
printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+ printf("insn: %" PRIu64 " (cond cb)\n", conditional);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
+ g_assert(conditional == expected);
}
static void stats_tb(void)
@@ -70,12 +87,18 @@ static void stats_tb(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_tb);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_tb_inline);
+ const uint64_t cond_num_trigger = qemu_plugin_u64_sum(tb_cond_num_trigger);
+ const uint64_t cond_track_left = qemu_plugin_u64_sum(tb_cond_track_count);
+ const uint64_t conditional =
+ cond_num_trigger * cond_trigger_limit + cond_track_left;
printf("tb: %" PRIu64 "\n", expected);
printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+ printf("tb: %" PRIu64 " (conditional cb)\n", conditional);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
+ g_assert(conditional == expected);
}
static void stats_mem(void)
@@ -104,14 +127,35 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
const uint64_t insn_inline = qemu_plugin_u64_get(count_insn_inline, i);
const uint64_t mem = qemu_plugin_u64_get(count_mem, i);
const uint64_t mem_inline = qemu_plugin_u64_get(count_mem_inline, i);
- printf("cpu %d: tb (%" PRIu64 ", %" PRIu64 ") | "
- "insn (%" PRIu64 ", %" PRIu64 ") | "
+ const uint64_t tb_cond_trigger =
+ qemu_plugin_u64_get(tb_cond_num_trigger, i);
+ const uint64_t tb_cond_left =
+ qemu_plugin_u64_get(tb_cond_track_count, i);
+ const uint64_t insn_cond_trigger =
+ qemu_plugin_u64_get(insn_cond_num_trigger, i);
+ const uint64_t insn_cond_left =
+ qemu_plugin_u64_get(insn_cond_track_count, i);
+ printf("cpu %d: tb (%" PRIu64 ", %" PRIu64
+ ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+ ") | "
+ "insn (%" PRIu64 ", %" PRIu64
+ ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
+ ") | "
"mem (%" PRIu64 ", %" PRIu64 ")"
"\n",
- i, tb, tb_inline, insn, insn_inline, mem, mem_inline);
+ i,
+ tb, tb_inline,
+ tb_cond_trigger, cond_trigger_limit, tb_cond_left,
+ insn, insn_inline,
+ insn_cond_trigger, cond_trigger_limit, insn_cond_left,
+ mem, mem_inline);
g_assert(tb == tb_inline);
g_assert(insn == insn_inline);
g_assert(mem == mem_inline);
+ g_assert(tb_cond_trigger == tb / cond_trigger_limit);
+ g_assert(tb_cond_left == tb % cond_trigger_limit);
+ g_assert(insn_cond_trigger == insn / cond_trigger_limit);
+ g_assert(insn_cond_left == insn % cond_trigger_limit);
}
stats_tb();
@@ -132,6 +176,24 @@ static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
g_mutex_unlock(&tb_lock);
}
+static void vcpu_tb_cond_exec(unsigned int cpu_index, void *udata)
+{
+ g_assert(qemu_plugin_u64_get(tb_cond_track_count, cpu_index) ==
+ cond_trigger_limit);
+ g_assert(qemu_plugin_u64_get(data_tb, cpu_index) == (uintptr_t) udata);
+ qemu_plugin_u64_set(tb_cond_track_count, cpu_index, 0);
+ qemu_plugin_u64_add(tb_cond_num_trigger, cpu_index, 1);
+}
+
+static void vcpu_insn_cond_exec(unsigned int cpu_index, void *udata)
+{
+ g_assert(qemu_plugin_u64_get(insn_cond_track_count, cpu_index) ==
+ cond_trigger_limit);
+ g_assert(qemu_plugin_u64_get(data_insn, cpu_index) == (uintptr_t) udata);
+ qemu_plugin_u64_set(insn_cond_track_count, cpu_index, 0);
+ qemu_plugin_u64_add(insn_cond_num_trigger, cpu_index, 1);
+}
+
static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
{
qemu_plugin_u64_add(count_insn, cpu_index, 1);
@@ -163,6 +225,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_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, tb_store);
+
for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
void *insn_store = insn;
@@ -176,6 +244,13 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
+ qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+ insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_cond_track_count, 1);
+ qemu_plugin_register_vcpu_insn_exec_cond_cb(
+ insn, vcpu_insn_cond_exec, QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_COND_EQ, insn_cond_track_count, cond_trigger_limit,
+ insn_store);
+
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
QEMU_PLUGIN_MEM_RW, mem_store);
@@ -207,6 +282,14 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
counts, CPUCount, count_insn_inline);
count_mem_inline = qemu_plugin_scoreboard_u64_in_struct(
counts, CPUCount, count_mem_inline);
+ tb_cond_num_trigger = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, tb_cond_num_trigger);
+ tb_cond_track_count = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, tb_cond_track_count);
+ insn_cond_num_trigger = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, insn_cond_num_trigger);
+ insn_cond_track_count = qemu_plugin_scoreboard_u64_in_struct(
+ counts, CPUCount, insn_cond_track_count);
data = qemu_plugin_scoreboard_new(sizeof(CPUData));
data_insn = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_insn);
data_tb = qemu_plugin_scoreboard_u64_in_struct(data, CPUData, data_tb);
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread