[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 1/4] target/riscv: Add itrigger support when icount is not
|
From: |
Alistair Francis |
|
Subject: |
Re: [PATCH v1 1/4] target/riscv: Add itrigger support when icount is not enabled |
|
Date: |
Mon, 7 Nov 2022 11:37:10 +1000 |
On Thu, Oct 13, 2022 at 4:32 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
> When icount is not enabled, there is no API in QEMU that can get the
> guest instruction number.
>
> Translate the guest code in a way that each TB only has one instruction.
I don't think this is a great idea.
Why can't we just require icount be enabled if a user wants this? Or singlestep?
Alistair
> After executing the instruction, decrease the count by 1 until it reaches 0
> where the itrigger fires.
>
> Note that only when priviledge matches the itrigger configuration,
> the count will decrease.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
> target/riscv/cpu.h | 2 +
> target/riscv/cpu_helper.c | 6 ++
> target/riscv/debug.c | 71 +++++++++++++++++++
> target/riscv/debug.h | 12 ++++
> target/riscv/helper.h | 2 +
> .../riscv/insn_trans/trans_privileged.c.inc | 4 +-
> target/riscv/insn_trans/trans_rvi.c.inc | 8 +--
> target/riscv/insn_trans/trans_rvv.c.inc | 4 +-
> target/riscv/translate.c | 33 ++++++++-
> 9 files changed, 131 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index b131fa8c8e..24bafda27d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -621,6 +621,8 @@ FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
> FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
> FIELD(TB_FLAGS, VTA, 24, 1)
> FIELD(TB_FLAGS, VMA, 25, 1)
> +/* Native debug itrigger */
> +FIELD(TB_FLAGS, ITRIGGER, 26, 1)
>
> #ifdef TARGET_RISCV32
> #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 278d163803..263282f230 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -27,7 +27,9 @@
> #include "tcg/tcg-op.h"
> #include "trace.h"
> #include "semihosting/common-semi.h"
> +#include "sysemu/cpu-timers.h"
> #include "cpu_bits.h"
> +#include "debug.h"
>
> int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
> {
> @@ -103,6 +105,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env,
> target_ulong *pc,
> flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
> get_field(env->mstatus_hs, MSTATUS_VS));
> }
> + if (riscv_feature(env, RISCV_FEATURE_DEBUG) && !icount_enabled()) {
> + flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER,
> + riscv_itrigger_enabled(env));
> + }
> #endif
>
> flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 26ea764407..45a3537d5c 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -29,6 +29,7 @@
> #include "cpu.h"
> #include "trace.h"
> #include "exec/exec-all.h"
> +#include "exec/helper-proto.h"
>
> /*
> * The following M-mode trigger CSRs are implemented:
> @@ -498,6 +499,76 @@ static void type6_reg_write(CPURISCVState *env,
> target_ulong index,
> return;
> }
>
> +/* icount trigger type */
> +static inline int
> +itrigger_get_count(CPURISCVState *env, int index)
> +{
> + return get_field(env->tdata1[index], ITRIGGER_COUNT);
> +}
> +
> +static inline void
> +itrigger_set_count(CPURISCVState *env, int index, int value)
> +{
> + env->tdata1[index] = set_field(env->tdata1[index],
> + ITRIGGER_COUNT, value);
> +}
> +
> +static bool check_itrigger_priv(CPURISCVState *env, int index)
> +{
> + target_ulong tdata1 = env->tdata1[index];
> + if (riscv_cpu_virt_enabled(env)) {
> + /* check VU/VS bit against current privilege level */
> + return (get_field(tdata1, ITRIGGER_VS) == env->priv) ||
> + (get_field(tdata1, ITRIGGER_VU) == env->priv);
> + } else {
> + /* check U/S/M bit against current privilege level */
> + return (get_field(tdata1, ITRIGGER_M) == env->priv) ||
> + (get_field(tdata1, ITRIGGER_S) == env->priv) ||
> + (get_field(tdata1, ITRIGGER_U) == env->priv);
> + }
> +}
> +
> +bool riscv_itrigger_enabled(CPURISCVState *env)
> +{
> + int count;
> + for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
> + if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> + continue;
> + }
> + if (check_itrigger_priv(env, i)) {
> + continue;
> + }
> + count = itrigger_get_count(env, i);
> + if (!count) {
> + continue;
> + }
> + return true;
> + }
> +
> + return false;
> +}
> +
> +void helper_itrigger_match(CPURISCVState *env)
> +{
> + int count;
> + for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
> + if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> + continue;
> + }
> + if (check_itrigger_priv(env, i)) {
> + continue;
> + }
> + count = itrigger_get_count(env, i);
> + if (!count) {
> + continue;
> + }
> + itrigger_set_count(env, i, count--);
> + if (!count) {
> + do_trigger_action(env, i);
> + }
> + }
> +}
> +
> target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
> {
> switch (tdata_index) {
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index a1226b4d29..cc3358e69b 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -118,6 +118,17 @@ enum {
> SIZE_NUM = 16
> };
>
> +/* itrigger filed masks */
> +#define ITRIGGER_ACTION 0x3f
> +#define ITRIGGER_U BIT(6)
> +#define ITRIGGER_S BIT(7)
> +#define ITRIGGER_PENDING BIT(8)
> +#define ITRIGGER_M BIT(9)
> +#define ITRIGGER_COUNT (0x3fff << 10)
> +#define ITRIGGER_HIT BIT(24)
> +#define ITRIGGER_VU BIT(25)
> +#define ITRIGGER_VS BIT(26)
> +
> bool tdata_available(CPURISCVState *env, int tdata_index);
>
> target_ulong tselect_csr_read(CPURISCVState *env);
> @@ -134,4 +145,5 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs,
> CPUWatchpoint *wp);
>
> void riscv_trigger_init(CPURISCVState *env);
>
> +bool riscv_itrigger_enabled(CPURISCVState *env);
> #endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index a03014fe67..227c7122ef 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -109,6 +109,8 @@ DEF_HELPER_1(sret, tl, env)
> DEF_HELPER_1(mret, tl, env)
> DEF_HELPER_1(wfi, void, env)
> DEF_HELPER_1(tlb_flush, void, env)
> +/* Native Debug */
> +DEF_HELPER_1(itrigger_match, void, env)
> #endif
>
> /* Hypervisor functions */
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc
> b/target/riscv/insn_trans/trans_privileged.c.inc
> index 3281408a87..59501b2780 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -78,7 +78,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
> if (has_ext(ctx, RVS)) {
> decode_save_opc(ctx);
> gen_helper_sret(cpu_pc, cpu_env);
> - tcg_gen_exit_tb(NULL, 0); /* no chaining */
> + exit_tb(ctx); /* no chaining */
> ctx->base.is_jmp = DISAS_NORETURN;
> } else {
> return false;
> @@ -94,7 +94,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
> #ifndef CONFIG_USER_ONLY
> decode_save_opc(ctx);
> gen_helper_mret(cpu_pc, cpu_env);
> - tcg_gen_exit_tb(NULL, 0); /* no chaining */
> + exit_tb(ctx); /* no chaining */
> ctx->base.is_jmp = DISAS_NORETURN;
> return true;
> #else
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index c49dbec0eb..5c69b88d1e 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -66,7 +66,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> }
>
> gen_set_gpri(ctx, a->rd, ctx->pc_succ_insn);
> - tcg_gen_lookup_and_goto_ptr();
> + lookup_and_goto_ptr(ctx);
>
> if (misaligned) {
> gen_set_label(misaligned);
> @@ -803,7 +803,7 @@ static bool trans_pause(DisasContext *ctx, arg_pause *a)
> * end the TB and return to main loop
> */
> gen_set_pc_imm(ctx, ctx->pc_succ_insn);
> - tcg_gen_exit_tb(NULL, 0);
> + exit_tb(ctx);
> ctx->base.is_jmp = DISAS_NORETURN;
>
> return true;
> @@ -827,7 +827,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i
> *a)
> * however we need to end the translation block
> */
> gen_set_pc_imm(ctx, ctx->pc_succ_insn);
> - tcg_gen_exit_tb(NULL, 0);
> + exit_tb(ctx);
> ctx->base.is_jmp = DISAS_NORETURN;
> return true;
> }
> @@ -838,7 +838,7 @@ static bool do_csr_post(DisasContext *ctx)
> decode_save_opc(ctx);
> /* We may have changed important cpu state -- exit to main loop. */
> gen_set_pc_imm(ctx, ctx->pc_succ_insn);
> - tcg_gen_exit_tb(NULL, 0);
> + exit_tb(ctx);
> ctx->base.is_jmp = DISAS_NORETURN;
> return true;
> }
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 4dea4413ae..d455acedbf 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -196,7 +196,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1,
> TCGv s2)
> mark_vs_dirty(s);
>
> gen_set_pc_imm(s, s->pc_succ_insn);
> - tcg_gen_lookup_and_goto_ptr();
> + lookup_and_goto_ptr(s);
> s->base.is_jmp = DISAS_NORETURN;
>
> if (rd == 0 && rs1 == 0) {
> @@ -222,7 +222,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1,
> TCGv s2)
> gen_set_gpr(s, rd, dst);
> mark_vs_dirty(s);
> gen_set_pc_imm(s, s->pc_succ_insn);
> - tcg_gen_lookup_and_goto_ptr();
> + lookup_and_goto_ptr(s);
> s->base.is_jmp = DISAS_NORETURN;
>
> return true;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index db123da5ec..d704265a37 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -111,6 +111,8 @@ typedef struct DisasContext {
> /* PointerMasking extension */
> bool pm_mask_enabled;
> bool pm_base_enabled;
> + /* Use icount trigger for native debug */
> + bool itrigger;
> /* TCG of the current insn_start */
> TCGOp *insn_start;
> } DisasContext;
> @@ -252,15 +254,39 @@ static void gen_exception_inst_addr_mis(DisasContext
> *ctx)
> generate_exception(ctx, RISCV_EXCP_INST_ADDR_MIS);
> }
>
> +static void lookup_and_goto_ptr(DisasContext *ctx)
> +{
> +#ifndef CONFIG_USER_ONLY
> + if (ctx->itrigger) {
> + gen_helper_itrigger_match(cpu_env);
> + }
> +#endif
> + tcg_gen_lookup_and_goto_ptr();
> +}
> +
> +static void exit_tb(DisasContext *ctx)
> +{
> +#ifndef CONFIG_USER_ONLY
> + if (ctx->itrigger) {
> + gen_helper_itrigger_match(cpu_env);
> + }
> +#endif
> + tcg_gen_exit_tb(NULL, 0);
> +}
> +
> static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
> {
> - if (translator_use_goto_tb(&ctx->base, dest)) {
> + /*
> + * Under itrigger, instruction executes one by one like singlestep,
> + * direct block chain benefits will be small.
> + */
> + if (translator_use_goto_tb(&ctx->base, dest) && !ctx->itrigger) {
> tcg_gen_goto_tb(n);
> gen_set_pc_imm(ctx, dest);
> tcg_gen_exit_tb(ctx->base.tb, n);
> } else {
> gen_set_pc_imm(ctx, dest);
> - tcg_gen_lookup_and_goto_ptr();
> + lookup_and_goto_ptr(ctx);
> }
> }
>
> @@ -1136,6 +1162,7 @@ static void
> riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> memset(ctx->ftemp, 0, sizeof(ctx->ftemp));
> ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED);
> ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
> + ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
> ctx->zero = tcg_constant_tl(0);
> }
>
> @@ -1175,7 +1202,7 @@ static void riscv_tr_translate_insn(DisasContextBase
> *dcbase, CPUState *cpu)
>
> /* Only the first insn within a TB is allowed to cross a page boundary.
> */
> if (ctx->base.is_jmp == DISAS_NEXT) {
> - if (!is_same_page(&ctx->base, ctx->base.pc_next)) {
> + if (ctx->itrigger || !is_same_page(&ctx->base, ctx->base.pc_next)) {
> ctx->base.is_jmp = DISAS_TOO_MANY;
> } else {
> unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK;
> --
> 2.17.1
>
>
- Re: [PATCH v1 1/4] target/riscv: Add itrigger support when icount is not enabled,
Alistair Francis <=