| Message ID | 1441216660-8717-2-git-send-email-rth@twiddle.net |
|---|---|
| State | New |
| Headers | show |
On 2 September 2015 at 18:57, Richard Henderson <rth@twiddle.net> wrote: > This is a bug fix for aarch64. At present, we have branches using > the 32-bit (translate.c) versions of cpu_[NZCV]F, but we set the flags > using the 64-bit (translate-a64.c) versions of cpu_[NZCV]F. From > the view of the TCG code generator, these are unrelated variables. > > The bug is hard to see because we currently only read these variables > from branches, and upon reaching a branch TCG will first spill live > variables and then reload the arguments of the branch. Since the > 32-bit versions were never live until reaching the branch, we'd re-read > the data that had just been spilled from the 64-bit versions. > > There is currently no such problem with the cpu_exclusive_* variables, > but there's no point in tempting fate. > > Signed-off-by: Richard Henderson <rth@twiddle.net> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Should this be cc:qemu-stable@nongnu.org ? thanks -- PMM
On 09/07/2015 09:57 AM, Peter Maydell wrote: > On 2 September 2015 at 18:57, Richard Henderson <rth@twiddle.net> wrote: >> This is a bug fix for aarch64. At present, we have branches using >> the 32-bit (translate.c) versions of cpu_[NZCV]F, but we set the flags >> using the 64-bit (translate-a64.c) versions of cpu_[NZCV]F. From >> the view of the TCG code generator, these are unrelated variables. >> >> The bug is hard to see because we currently only read these variables >> from branches, and upon reaching a branch TCG will first spill live >> variables and then reload the arguments of the branch. Since the >> 32-bit versions were never live until reaching the branch, we'd re-read >> the data that had just been spilled from the 64-bit versions. >> >> There is currently no such problem with the cpu_exclusive_* variables, >> but there's no point in tempting fate. >> >> Signed-off-by: Richard Henderson <rth@twiddle.net> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > Should this be cc:qemu-stable@nongnu.org ? Possibly. It's certainly low risk. r~
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index 5c13e15..1587ab5 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -39,16 +39,9 @@ static TCGv_i64 cpu_X[32]; static TCGv_i64 cpu_pc; -static TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF; /* Load/store exclusive handling */ -static TCGv_i64 cpu_exclusive_addr; -static TCGv_i64 cpu_exclusive_val; static TCGv_i64 cpu_exclusive_high; -#ifdef CONFIG_USER_ONLY -static TCGv_i64 cpu_exclusive_test; -static TCGv_i32 cpu_exclusive_info; -#endif static const char *regnames[] = { "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", @@ -104,23 +97,8 @@ void a64_translate_init(void) regnames[i]); } - cpu_NF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, NF), "NF"); - cpu_ZF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, ZF), "ZF"); - cpu_CF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, CF), "CF"); - cpu_VF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, VF), "VF"); - - cpu_exclusive_addr = tcg_global_mem_new_i64(TCG_AREG0, - offsetof(CPUARMState, exclusive_addr), "exclusive_addr"); - cpu_exclusive_val = tcg_global_mem_new_i64(TCG_AREG0, - offsetof(CPUARMState, exclusive_val), "exclusive_val"); cpu_exclusive_high = tcg_global_mem_new_i64(TCG_AREG0, offsetof(CPUARMState, exclusive_high), "exclusive_high"); -#ifdef CONFIG_USER_ONLY - cpu_exclusive_test = tcg_global_mem_new_i64(TCG_AREG0, - offsetof(CPUARMState, exclusive_test), "exclusive_test"); - cpu_exclusive_info = tcg_global_mem_new_i32(TCG_AREG0, - offsetof(CPUARMState, exclusive_info), "exclusive_info"); -#endif } static inline ARMMMUIdx get_a64_user_mem_index(DisasContext *s) diff --git a/target-arm/translate.c b/target-arm/translate.c index e27634f..3826a02 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -64,12 +64,12 @@ TCGv_ptr cpu_env; /* We reuse the same 64-bit temporaries for efficiency. */ static TCGv_i64 cpu_V0, cpu_V1, cpu_M0; static TCGv_i32 cpu_R[16]; -static TCGv_i32 cpu_CF, cpu_NF, cpu_VF, cpu_ZF; -static TCGv_i64 cpu_exclusive_addr; -static TCGv_i64 cpu_exclusive_val; +TCGv_i32 cpu_CF, cpu_NF, cpu_VF, cpu_ZF; +TCGv_i64 cpu_exclusive_addr; +TCGv_i64 cpu_exclusive_val; #ifdef CONFIG_USER_ONLY -static TCGv_i64 cpu_exclusive_test; -static TCGv_i32 cpu_exclusive_info; +TCGv_i64 cpu_exclusive_test; +TCGv_i32 cpu_exclusive_info; #endif /* FIXME: These should be removed. */ diff --git a/target-arm/translate.h b/target-arm/translate.h index 9ab978f..679bdbc 100644 --- a/target-arm/translate.h +++ b/target-arm/translate.h @@ -62,7 +62,15 @@ typedef struct DisasContext { TCGv_i64 tmp_a64[TMP_A64_MAX]; } DisasContext; +/* Share the TCG temporaries common between 32 and 64 bit modes. */ extern TCGv_ptr cpu_env; +extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF; +extern TCGv_i64 cpu_exclusive_addr; +extern TCGv_i64 cpu_exclusive_val; +#ifdef CONFIG_USER_ONLY +extern TCGv_i64 cpu_exclusive_test; +extern TCGv_i32 cpu_exclusive_info; +#endif static inline int arm_dc_feature(DisasContext *dc, int feature) {
This is a bug fix for aarch64. At present, we have branches using the 32-bit (translate.c) versions of cpu_[NZCV]F, but we set the flags using the 64-bit (translate-a64.c) versions of cpu_[NZCV]F. From the view of the TCG code generator, these are unrelated variables. The bug is hard to see because we currently only read these variables from branches, and upon reaching a branch TCG will first spill live variables and then reload the arguments of the branch. Since the 32-bit versions were never live until reaching the branch, we'd re-read the data that had just been spilled from the 64-bit versions. There is currently no such problem with the cpu_exclusive_* variables, but there's no point in tempting fate. Signed-off-by: Richard Henderson <rth@twiddle.net> --- target-arm/translate-a64.c | 22 ---------------------- target-arm/translate.c | 10 +++++----- target-arm/translate.h | 8 ++++++++ 3 files changed, 13 insertions(+), 27 deletions(-)