| Message ID | 20210323184340.619757-11-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/ppc: Fix truncation of env->hflags | expand |
On Tue, Mar 23, 2021 at 12:43:40PM -0600, Richard Henderson wrote: > Verify that hflags was updated correctly whenever we change > cpu state that is used by hflags. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Applied to ppc-for-6.0, thanks. > --- > target/ppc/cpu.h | 5 +++++ > target/ppc/helper_regs.c | 29 +++++++++++++++++++++++++++-- > 2 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 3d021f61f3..69fc9a2831 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -2425,6 +2425,10 @@ void cpu_write_xer(CPUPPCState *env, target_ulong xer); > */ > #define is_book3s_arch2x(ctx) (!!((ctx)->insns_flags & PPC_SEGMENT_64B)) > > +#ifdef CONFIG_DEBUG_TCG > +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > + target_ulong *cs_base, uint32_t *flags); > +#else > static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > target_ulong *cs_base, uint32_t *flags) > { > @@ -2432,6 +2436,7 @@ static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > *cs_base = 0; > *flags = env->hflags; > } > +#endif > > void QEMU_NORETURN raise_exception(CPUPPCState *env, uint32_t exception); > void QEMU_NORETURN raise_exception_ra(CPUPPCState *env, uint32_t exception, > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c > index 5411a67e9a..3723872aa6 100644 > --- a/target/ppc/helper_regs.c > +++ b/target/ppc/helper_regs.c > @@ -43,7 +43,7 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env) > env->tgpr[3] = tmp; > } > > -void hreg_compute_hflags(CPUPPCState *env) > +static uint32_t hreg_compute_hflags_value(CPUPPCState *env) > { > target_ulong msr = env->msr; > uint32_t ppc_flags = env->flags; > @@ -155,9 +155,34 @@ void hreg_compute_hflags(CPUPPCState *env) > hflags |= dmmu_idx << HFLAGS_DMMU_IDX; > #endif > > - env->hflags = hflags | (msr & msr_mask); > + return hflags | (msr & msr_mask); > } > > +void hreg_compute_hflags(CPUPPCState *env) > +{ > + env->hflags = hreg_compute_hflags_value(env); > +} > + > +#ifdef CONFIG_DEBUG_TCG > +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > + target_ulong *cs_base, uint32_t *flags) > +{ > + uint32_t hflags_current = env->hflags; > + uint32_t hflags_rebuilt; > + > + *pc = env->nip; > + *cs_base = 0; > + *flags = hflags_current; > + > + hflags_rebuilt = hreg_compute_hflags_value(env); > + if (unlikely(hflags_current != hflags_rebuilt)) { > + cpu_abort(env_cpu(env), > + "TCG hflags mismatch (current:0x%08x rebuilt:0x%08x)\n", > + hflags_current, hflags_rebuilt); > + } > +} > +#endif > + > void cpu_interrupt_exittb(CPUState *cs) > { > if (!kvm_enabled()) { -- 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
On Wed, Mar 24, 2021 at 11:12:20AM +1100, David Gibson wrote: > On Tue, Mar 23, 2021 at 12:43:40PM -0600, Richard Henderson wrote: > > Verify that hflags was updated correctly whenever we change > > cpu state that is used by hflags. > > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > Applied to ppc-for-6.0, thanks. Alas, this appears to cause a failure in the 'build-user' test on gitlab CI :(. I haven't managed to reproduce it locally, so I'm not sure what's going on. > > > --- > > target/ppc/cpu.h | 5 +++++ > > target/ppc/helper_regs.c | 29 +++++++++++++++++++++++++++-- > > 2 files changed, 32 insertions(+), 2 deletions(-) > > > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > > index 3d021f61f3..69fc9a2831 100644 > > --- a/target/ppc/cpu.h > > +++ b/target/ppc/cpu.h > > @@ -2425,6 +2425,10 @@ void cpu_write_xer(CPUPPCState *env, target_ulong xer); > > */ > > #define is_book3s_arch2x(ctx) (!!((ctx)->insns_flags & PPC_SEGMENT_64B)) > > > > +#ifdef CONFIG_DEBUG_TCG > > +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > > + target_ulong *cs_base, uint32_t *flags); > > +#else > > static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > > target_ulong *cs_base, uint32_t *flags) > > { > > @@ -2432,6 +2436,7 @@ static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > > *cs_base = 0; > > *flags = env->hflags; > > } > > +#endif > > > > void QEMU_NORETURN raise_exception(CPUPPCState *env, uint32_t exception); > > void QEMU_NORETURN raise_exception_ra(CPUPPCState *env, uint32_t exception, > > diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c > > index 5411a67e9a..3723872aa6 100644 > > --- a/target/ppc/helper_regs.c > > +++ b/target/ppc/helper_regs.c > > @@ -43,7 +43,7 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env) > > env->tgpr[3] = tmp; > > } > > > > -void hreg_compute_hflags(CPUPPCState *env) > > +static uint32_t hreg_compute_hflags_value(CPUPPCState *env) > > { > > target_ulong msr = env->msr; > > uint32_t ppc_flags = env->flags; > > @@ -155,9 +155,34 @@ void hreg_compute_hflags(CPUPPCState *env) > > hflags |= dmmu_idx << HFLAGS_DMMU_IDX; > > #endif > > > > - env->hflags = hflags | (msr & msr_mask); > > + return hflags | (msr & msr_mask); > > } > > > > +void hreg_compute_hflags(CPUPPCState *env) > > +{ > > + env->hflags = hreg_compute_hflags_value(env); > > +} > > + > > +#ifdef CONFIG_DEBUG_TCG > > +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, > > + target_ulong *cs_base, uint32_t *flags) > > +{ > > + uint32_t hflags_current = env->hflags; > > + uint32_t hflags_rebuilt; > > + > > + *pc = env->nip; > > + *cs_base = 0; > > + *flags = hflags_current; > > + > > + hflags_rebuilt = hreg_compute_hflags_value(env); > > + if (unlikely(hflags_current != hflags_rebuilt)) { > > + cpu_abort(env_cpu(env), > > + "TCG hflags mismatch (current:0x%08x rebuilt:0x%08x)\n", > > + hflags_current, hflags_rebuilt); > > + } > > +} > > +#endif > > + > > void cpu_interrupt_exittb(CPUState *cs) > > { > > if (!kvm_enabled()) { > -- 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
On 3/25/21 2:47 AM, David Gibson wrote: > On Wed, Mar 24, 2021 at 11:12:20AM +1100, David Gibson wrote: >> On Tue, Mar 23, 2021 at 12:43:40PM -0600, Richard Henderson wrote: >>> Verify that hflags was updated correctly whenever we change >>> cpu state that is used by hflags. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> >> Applied to ppc-for-6.0, thanks. > > Alas, this appears to cause a failure in the 'build-user' test on > gitlab CI :(. I haven't managed to reproduce it locally, so I'm not > sure what's going on. I guess you mean e.g. https://gitlab.com/dgibson/qemu/-/jobs/1126364147 ? I'll have a look. r~
On 3/26/21 6:41 AM, Richard Henderson wrote: > On 3/25/21 2:47 AM, David Gibson wrote: >> On Wed, Mar 24, 2021 at 11:12:20AM +1100, David Gibson wrote: >>> On Tue, Mar 23, 2021 at 12:43:40PM -0600, Richard Henderson wrote: >>>> Verify that hflags was updated correctly whenever we change >>>> cpu state that is used by hflags. >>>> >>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> >>> Applied to ppc-for-6.0, thanks. >> >> Alas, this appears to cause a failure in the 'build-user' test on >> gitlab CI :(. I haven't managed to reproduce it locally, so I'm not >> sure what's going on. > > I guess you mean e.g. > > https://gitlab.com/dgibson/qemu/-/jobs/1126364147 > > ? I'll have a look. I haven't been able to repo locally, or on gitlab: https://gitlab.com/rth7680/qemu/-/pipelines/277073704 Have you tried simply re-running that job? r~
On Sat, Mar 27, 2021 at 06:46:15AM -0600, Richard Henderson wrote: > On 3/26/21 6:41 AM, Richard Henderson wrote: > > On 3/25/21 2:47 AM, David Gibson wrote: > > > On Wed, Mar 24, 2021 at 11:12:20AM +1100, David Gibson wrote: > > > > On Tue, Mar 23, 2021 at 12:43:40PM -0600, Richard Henderson wrote: > > > > > Verify that hflags was updated correctly whenever we change > > > > > cpu state that is used by hflags. > > > > > > > > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > > > > > > > Applied to ppc-for-6.0, thanks. > > > > > > Alas, this appears to cause a failure in the 'build-user' test on > > > gitlab CI :(. I haven't managed to reproduce it locally, so I'm not > > > sure what's going on. > > > > I guess you mean e.g. > > > > https://gitlab.com/dgibson/qemu/-/jobs/1126364147 Yes, sorry I meant to give you a link. > > > > ? I'll have a look. > > I haven't been able to repo locally, or on gitlab: > > https://gitlab.com/rth7680/qemu/-/pipelines/277073704 Huh.. > Have you tried simply re-running that job? Several times :/ -- 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/cpu.h b/target/ppc/cpu.h index 3d021f61f3..69fc9a2831 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -2425,6 +2425,10 @@ void cpu_write_xer(CPUPPCState *env, target_ulong xer); */ #define is_book3s_arch2x(ctx) (!!((ctx)->insns_flags & PPC_SEGMENT_64B)) +#ifdef CONFIG_DEBUG_TCG +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, + target_ulong *cs_base, uint32_t *flags); +#else static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, target_ulong *cs_base, uint32_t *flags) { @@ -2432,6 +2436,7 @@ static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, *cs_base = 0; *flags = env->hflags; } +#endif void QEMU_NORETURN raise_exception(CPUPPCState *env, uint32_t exception); void QEMU_NORETURN raise_exception_ra(CPUPPCState *env, uint32_t exception, diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c index 5411a67e9a..3723872aa6 100644 --- a/target/ppc/helper_regs.c +++ b/target/ppc/helper_regs.c @@ -43,7 +43,7 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env) env->tgpr[3] = tmp; } -void hreg_compute_hflags(CPUPPCState *env) +static uint32_t hreg_compute_hflags_value(CPUPPCState *env) { target_ulong msr = env->msr; uint32_t ppc_flags = env->flags; @@ -155,9 +155,34 @@ void hreg_compute_hflags(CPUPPCState *env) hflags |= dmmu_idx << HFLAGS_DMMU_IDX; #endif - env->hflags = hflags | (msr & msr_mask); + return hflags | (msr & msr_mask); } +void hreg_compute_hflags(CPUPPCState *env) +{ + env->hflags = hreg_compute_hflags_value(env); +} + +#ifdef CONFIG_DEBUG_TCG +void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc, + target_ulong *cs_base, uint32_t *flags) +{ + uint32_t hflags_current = env->hflags; + uint32_t hflags_rebuilt; + + *pc = env->nip; + *cs_base = 0; + *flags = hflags_current; + + hflags_rebuilt = hreg_compute_hflags_value(env); + if (unlikely(hflags_current != hflags_rebuilt)) { + cpu_abort(env_cpu(env), + "TCG hflags mismatch (current:0x%08x rebuilt:0x%08x)\n", + hflags_current, hflags_rebuilt); + } +} +#endif + void cpu_interrupt_exittb(CPUState *cs) { if (!kvm_enabled()) {
Verify that hflags was updated correctly whenever we change cpu state that is used by hflags. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/cpu.h | 5 +++++ target/ppc/helper_regs.c | 29 +++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) -- 2.25.1