On Tue, Aug 07, 2018 at 11:29:48AM +0200, Roman Kapl wrote:
> Add support for DBCR (debug control register) based debugging as used on
> BookE ppc. So far supports only branch and single-step events, but these are
> the important ones. GDB in Linux guest can now do single-stepping.
>
> Signed-off-by: Roman Kapl <r...@sysgo.com>
> ---
> target/ppc/cpu.h | 5 ++
> target/ppc/excp_helper.c | 3 +-
> target/ppc/translate.c | 107
> ++++++++++++++++++++++++++++++----------
> target/ppc/translate_init.inc.c | 17 +++++++
> 4 files changed, 104 insertions(+), 28 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 4edcf62cf7..ec149349e2 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -481,6 +481,11 @@ struct ppc_slb_t {
> #define msr_ts ((env->msr >> MSR_TS1) & 3)
> #define msr_tm ((env->msr >> MSR_TM) & 1)
>
> +#define DBCR0_ICMP (1 << 27)
> +#define DBCR0_BRT (1 << 26)
> +#define DBSR_ICMP (1 << 27)
> +#define DBSR_BRT (1 << 26)
> +
> /* Hypervisor bit is more specific */
> #if defined(TARGET_PPC64)
> #define MSR_HVB (1ULL << MSR_SHV)
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index d6e97a90e0..3463efaf4e 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -359,8 +359,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int
> excp_model, int excp)
> default:
> break;
> }
> - /* XXX: TODO */
> - cpu_abort(cs, "Debug exception is not implemented yet !\n");
> + /* DBSR already modified by caller */
Ths is unconditionally enabling the exception for every platform,
which doesn't seem right. I think it would be better to add the
specific exception models which use the DBCR to the switch statement
above this.
> break;
> case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point unavailable
> */
> env->spr[SPR_BOOKE_ESR] = ESR_SPV;
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 9eaa10b421..69cd45dd81 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -211,6 +211,7 @@ struct DisasContext {
> bool gtse;
> ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
> int singlestep_enabled;
> + uint32_t flags;
> uint64_t insns_flags;
> uint64_t insns_flags2;
> };
> @@ -251,6 +252,17 @@ struct opc_handler_t {
> #endif
> };
>
> +/* SPR load/store helpers */
> +static inline void gen_load_spr(TCGv t, int reg)
> +{
> + tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, spr[reg]));
> +}
> +
> +static inline void gen_store_spr(int reg, TCGv t)
> +{
> + tcg_gen_st_tl(t, cpu_env, offsetof(CPUPPCState, spr[reg]));
> +}
> +
> static inline void gen_set_access_type(DisasContext *ctx, int access_type)
> {
> if (ctx->need_access_type && ctx->access_type != access_type) {
> @@ -313,6 +325,38 @@ static void gen_exception_nip(DisasContext *ctx,
> uint32_t excp,
> ctx->exception = (excp);
> }
>
> +/* Translates the EXCP_TRACE/BRANCH to an appropriate exception depending
> + * on processor version (BookE vs regular)
It's not obvious what "regular" means in this context.
> + */
> +static uint32_t gen_prep_dbgex(DisasContext *ctx, uint32_t excp)
> +{
> + if ((ctx->singlestep_enabled & CPU_SINGLE_STEP)
> + && (excp == POWERPC_EXCP_BRANCH)) {
> + /* Trace excpt. has priority */
> + excp = POWERPC_EXCP_TRACE;
> + }
> + if (ctx->flags & POWERPC_FLAG_DE) {
> + target_ulong dbsr = 0;
> + switch (excp) {
> + case POWERPC_EXCP_TRACE:
> + dbsr = DBCR0_ICMP;
> + break;
> + case POWERPC_EXCP_BRANCH:
> + dbsr = DBCR0_BRT;
> + break;
> + }
> + TCGv t0 = tcg_temp_new();
> + gen_load_spr(t0, SPR_BOOKE_DBSR);
> + tcg_gen_ori_tl(t0, t0, dbsr);
> + gen_store_spr(SPR_BOOKE_DBSR, t0);
> + tcg_temp_free(t0);
> + return POWERPC_EXCP_DEBUG;
> + } else {
> + return excp;
> + }
> + return POWERPC_EXCP_NONE;
> +}
> +
> static void gen_debug_exception(DisasContext *ctx)
> {
> TCGv_i32 t0;
> @@ -575,17 +619,6 @@ typedef struct opcode_t {
> }
> #endif
>
> -/* SPR load/store helpers */
> -static inline void gen_load_spr(TCGv t, int reg)
> -{
> - tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, spr[reg]));
> -}
> -
> -static inline void gen_store_spr(int reg, TCGv t)
> -{
> - tcg_gen_st_tl(t, cpu_env, offsetof(CPUPPCState, spr[reg]));
> -}
> -
You move these around, but the new code doesn't seem to use them, so
I'm not sure why.
> /* Invalid instruction */
> static void gen_invalid(DisasContext *ctx)
> {
> @@ -3602,6 +3635,24 @@ static inline bool use_goto_tb(DisasContext *ctx,
> target_ulong dest)
> #endif
> }
>
> +static void gen_lookup_and_goto_ptr(DisasContext *ctx)
> +{
> + int sse = ctx->singlestep_enabled;
> + if (unlikely(sse)) {
> + if (sse & GDBSTUB_SINGLE_STEP) {
> + gen_debug_exception(ctx);
> + } else if (sse & (CPU_SINGLE_STEP | CPU_BRANCH_STEP)) {
> + uint32_t excp = gen_prep_dbgex(ctx, POWERPC_EXCP_BRANCH);
> + if (excp != POWERPC_EXCP_NONE) {
> + gen_exception(ctx, excp);
> + }
> + }
> + tcg_gen_exit_tb(NULL, 0);
> + } else {
> + tcg_gen_lookup_and_goto_ptr();
> + }
> +}
> +
> /*** Branch
> ***/
> static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
> {
> @@ -3614,18 +3665,7 @@ static void gen_goto_tb(DisasContext *ctx, int n,
> target_ulong dest)
> tcg_gen_exit_tb(ctx->base.tb, n);
> } else {
> tcg_gen_movi_tl(cpu_nip, dest & ~3);
> - if (unlikely(ctx->singlestep_enabled)) {
> - if ((ctx->singlestep_enabled &
> - (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) &&
> - (ctx->exception == POWERPC_EXCP_BRANCH ||
> - ctx->exception == POWERPC_EXCP_TRACE)) {
> - gen_exception_nip(ctx, POWERPC_EXCP_TRACE, dest);
> - }
> - if (ctx->singlestep_enabled & GDBSTUB_SINGLE_STEP) {
> - gen_debug_exception(ctx);
> - }
> - }
> - tcg_gen_lookup_and_goto_ptr();
> + gen_lookup_and_goto_ptr(ctx);
> }
> }
>
> @@ -3668,8 +3708,8 @@ static void gen_bcond(DisasContext *ctx, int type)
> uint32_t bo = BO(ctx->opcode);
> TCGLabel *l1;
> TCGv target;
> -
> ctx->exception = POWERPC_EXCP_BRANCH;
> +
> if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
> target = tcg_temp_local_new();
> if (type == BCOND_CTR)
> @@ -3733,10 +3773,11 @@ static void gen_bcond(DisasContext *ctx, int type)
> } else {
> tcg_gen_andi_tl(cpu_nip, target, ~3);
> }
> - tcg_gen_lookup_and_goto_ptr();
> + gen_lookup_and_goto_ptr(ctx);
> tcg_temp_free(target);
> }
> if ((bo & 0x14) != 0x14) {
> + /* fallthrough case */
> gen_set_label(l1);
> gen_goto_tb(ctx, 1, ctx->base.pc_next);
> }
> @@ -7419,6 +7460,7 @@ static void ppc_tr_init_disas_context(DisasContextBase
> *dcbase, CPUState *cs)
> ctx->need_access_type = !(env->mmu_model & POWERPC_MMU_64B);
> ctx->le_mode = !!(env->hflags & (1 << MSR_LE));
> ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE;
> + ctx->flags = env->flags;
> #if defined(TARGET_PPC64)
> ctx->sf_mode = msr_is_64bit(env, env->msr);
> ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
> @@ -7455,6 +7497,17 @@ static void ppc_tr_init_disas_context(DisasContextBase
> *dcbase, CPUState *cs)
> ctx->singlestep_enabled = 0;
> if ((env->flags & POWERPC_FLAG_BE) && msr_be)
> ctx->singlestep_enabled |= CPU_BRANCH_STEP;
> + if ((env->flags & POWERPC_FLAG_DE) && msr_de) {
> + ctx->singlestep_enabled = 0;
> + target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
> + if (dbcr0 & DBCR0_ICMP) {
> + ctx->singlestep_enabled |= CPU_SINGLE_STEP;
> + }
> + if (dbcr0 & DBCR0_BRT) {
> + ctx->singlestep_enabled |= CPU_BRANCH_STEP;
> + }
> +
> + }
> if (unlikely(ctx->base.singlestep_enabled)) {
> ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
> }
> @@ -7565,7 +7618,9 @@ static void ppc_tr_translate_insn(DisasContextBase
> *dcbase, CPUState *cs)
> ctx->exception != POWERPC_SYSCALL &&
> ctx->exception != POWERPC_EXCP_TRAP &&
> ctx->exception != POWERPC_EXCP_BRANCH)) {
> - gen_exception_nip(ctx, POWERPC_EXCP_TRACE, ctx->base.pc_next);
> + uint32_t excp = gen_prep_dbgex(ctx, POWERPC_EXCP_TRACE);
> + if (excp != POWERPC_EXCP_NONE)
> + gen_exception_nip(ctx, excp, ctx->base.pc_next);
> }
>
> if (tcg_check_temp_count()) {
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 7813b1b004..f1be7b7953 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -498,6 +498,7 @@ static void spr_write_40x_pit(DisasContext *ctx, int
> sprn, int gprn)
>
> static void spr_write_40x_dbcr0(DisasContext *ctx, int sprn, int gprn)
> {
> + gen_store_spr(sprn, cpu_gpr[gprn]);
> gen_helper_store_40x_dbcr0(cpu_env, cpu_gpr[gprn]);
> /* We must stop translation as we may have rebooted */
> gen_stop_exception(ctx);
> @@ -1769,6 +1770,14 @@ static void gen_spr_BookE(CPUPPCState *env, uint64_t
> ivor_mask)
> SPR_NOACCESS, SPR_NOACCESS,
> &spr_read_generic, &spr_write_generic,
> 0x00000000);
> + spr_register(env, SPR_BOOKE_DSRR0, "DSRR0",
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_generic,
> + 0x00000000);
> + spr_register(env, SPR_BOOKE_DSRR1, "DSRR1",
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_generic,
> + 0x00000000);
> /* XXX : not implemented */
> spr_register(env, SPR_BOOKE_DBSR, "DBSR",
> SPR_NOACCESS, SPR_NOACCESS,
> @@ -1841,6 +1850,14 @@ static void gen_spr_BookE(CPUPPCState *env, uint64_t
> ivor_mask)
> SPR_NOACCESS, SPR_NOACCESS,
> &spr_read_generic, &spr_write_generic,
> 0x00000000);
> + spr_register(env, SPR_BOOKE_SPRG8, "SPRG8",
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_generic,
> + 0x00000000);
> + spr_register(env, SPR_BOOKE_SPRG9, "SPRG9",
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_generic,
> + 0x00000000);
> }
>
> static inline uint32_t gen_tlbncfg(uint32_t assoc, uint32_t minsize,
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
