| Message ID | 20210315184615.1985590-10-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Fix truncation of env->hflags | expand |
On Mon, Mar 15, 2021 at 12:46:07PM -0600, Richard Henderson wrote: > Because these bits were not in hflags, the code generated > for single-stepping on BookE was essentially random. > Recompute hflags when storing to dbcr0. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > target/ppc/helper_regs.c | 20 +++++++++++++++----- > target/ppc/misc_helper.c | 3 +++ > target/ppc/translate.c | 11 ----------- > 3 files changed, 18 insertions(+), 16 deletions(-) > > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c > index 0a746bffd7..c735540333 100644 > --- a/target/ppc/helper_regs.c > +++ b/target/ppc/helper_regs.c > @@ -107,11 +107,21 @@ void hreg_compute_hflags(CPUPPCState *env) > hflags |= le << MSR_LE; > } > > - if (ppc_flags & POWERPC_FLAG_BE) { > - msr_mask |= 1 << MSR_BE; > - } > - if (ppc_flags & POWERPC_FLAG_SE) { > - msr_mask |= 1 << MSR_SE; > + if (ppc_flags & POWERPC_FLAG_DE) { > + target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0]; > + if (dbcr0 & DBCR0_ICMP) { > + hflags |= 1 << HFLAGS_SE; > + } > + if (dbcr0 & DBCR0_BRT) { > + hflags |= 1 << HFLAGS_BE; > + } > + } else { > + if (ppc_flags & POWERPC_FLAG_BE) { > + msr_mask |= 1 << MSR_BE; > + } > + if (ppc_flags & POWERPC_FLAG_SE) { > + msr_mask |= 1 << MSR_SE; > + } > } > > if (msr_is_64bit(env, msr)) { > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > index b04b4d7c6e..a5ee1fd63c 100644 > --- a/target/ppc/misc_helper.c > +++ b/target/ppc/misc_helper.c > @@ -215,6 +215,9 @@ void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value) > > void helper_store_40x_dbcr0(CPUPPCState *env, target_ulong val) > { > + /* Bits 26 & 27 affect single-stepping */ > + hreg_compute_hflags(env); > + /* Bits 28 & 29 affect reset or shutdown. */ > store_40x_dbcr0(env, val); > } > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index a85b890bb0..7912495f28 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -7923,17 +7923,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > if ((hflags >> HFLAGS_BE) & 1) { > 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; > } -- 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
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c index 0a746bffd7..c735540333 100644 --- a/target/ppc/helper_regs.c +++ b/target/ppc/helper_regs.c @@ -107,11 +107,21 @@ void hreg_compute_hflags(CPUPPCState *env) hflags |= le << MSR_LE; } - if (ppc_flags & POWERPC_FLAG_BE) { - msr_mask |= 1 << MSR_BE; - } - if (ppc_flags & POWERPC_FLAG_SE) { - msr_mask |= 1 << MSR_SE; + if (ppc_flags & POWERPC_FLAG_DE) { + target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0]; + if (dbcr0 & DBCR0_ICMP) { + hflags |= 1 << HFLAGS_SE; + } + if (dbcr0 & DBCR0_BRT) { + hflags |= 1 << HFLAGS_BE; + } + } else { + if (ppc_flags & POWERPC_FLAG_BE) { + msr_mask |= 1 << MSR_BE; + } + if (ppc_flags & POWERPC_FLAG_SE) { + msr_mask |= 1 << MSR_SE; + } } if (msr_is_64bit(env, msr)) { diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c index b04b4d7c6e..a5ee1fd63c 100644 --- a/target/ppc/misc_helper.c +++ b/target/ppc/misc_helper.c @@ -215,6 +215,9 @@ void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value) void helper_store_40x_dbcr0(CPUPPCState *env, target_ulong val) { + /* Bits 26 & 27 affect single-stepping */ + hreg_compute_hflags(env); + /* Bits 28 & 29 affect reset or shutdown. */ store_40x_dbcr0(env, val); } diff --git a/target/ppc/translate.c b/target/ppc/translate.c index a85b890bb0..7912495f28 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -7923,17 +7923,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) if ((hflags >> HFLAGS_BE) & 1) { 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; }
Because these bits were not in hflags, the code generated for single-stepping on BookE was essentially random. Recompute hflags when storing to dbcr0. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/helper_regs.c | 20 +++++++++++++++----- target/ppc/misc_helper.c | 3 +++ target/ppc/translate.c | 11 ----------- 3 files changed, 18 insertions(+), 16 deletions(-) -- 2.25.1