Re: [Qemu-devel] [PATCH v2 16/35] target-arm: Remove unnecessary code now read/write fns can't fail

[Peter Crosthwaite]

On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <address@hidden> wrote:
> Now that cpreg read and write functions can't fail and throw an
> exception, we can remove the code from the translator that synchronises
> the guest PC in case an exception is thrown.
>

Based on my comment last patch, this may be overly restrictive. You
are setting something of a developer trap for anyone that wants to
throw an exception for CPReg accesses for non-access reasons.

Regards,
Peter

> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target-arm/translate-a64.c | 2 --
>  target-arm/translate.c     | 4 ----
>  2 files changed, 6 deletions(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index d90ffd1..f437359 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1248,7 +1248,6 @@ static void handle_sys(DisasContext *s, uint32_t insn, 
> bool isread,
>              tcg_gen_movi_i64(tcg_rt, ri->resetvalue);
>          } else if (ri->readfn) {
>              TCGv_ptr tmpptr;
> -            gen_a64_set_pc_im(s->pc - 4);
>              tmpptr = tcg_const_ptr(ri);
>              gen_helper_get_cp_reg64(tcg_rt, cpu_env, tmpptr);
>              tcg_temp_free_ptr(tmpptr);
> @@ -1261,7 +1260,6 @@ static void handle_sys(DisasContext *s, uint32_t insn, 
> bool isread,
>              return;
>          } else if (ri->writefn) {
>              TCGv_ptr tmpptr;
> -            gen_a64_set_pc_im(s->pc - 4);
>              tmpptr = tcg_const_ptr(ri);
>              gen_helper_set_cp_reg64(cpu_env, tmpptr, tcg_rt);
>              tcg_temp_free_ptr(tmpptr);
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 2713081..8149a3b 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6837,7 +6837,6 @@ static int disas_coproc_insn(CPUARMState * env, 
> DisasContext *s, uint32_t insn)
>                      tmp64 = tcg_const_i64(ri->resetvalue);
>                  } else if (ri->readfn) {
>                      TCGv_ptr tmpptr;
> -                    gen_set_pc_im(s, s->pc);
>                      tmp64 = tcg_temp_new_i64();
>                      tmpptr = tcg_const_ptr(ri);
>                      gen_helper_get_cp_reg64(tmp64, cpu_env, tmpptr);
> @@ -6860,7 +6859,6 @@ static int disas_coproc_insn(CPUARMState * env, 
> DisasContext *s, uint32_t insn)
>                      tmp = tcg_const_i32(ri->resetvalue);
>                  } else if (ri->readfn) {
>                      TCGv_ptr tmpptr;
> -                    gen_set_pc_im(s, s->pc);
>                      tmp = tcg_temp_new_i32();
>                      tmpptr = tcg_const_ptr(ri);
>                      gen_helper_get_cp_reg(tmp, cpu_env, tmpptr);
> @@ -6895,7 +6893,6 @@ static int disas_coproc_insn(CPUARMState * env, 
> DisasContext *s, uint32_t insn)
>                  tcg_temp_free_i32(tmphi);
>                  if (ri->writefn) {
>                      TCGv_ptr tmpptr = tcg_const_ptr(ri);
> -                    gen_set_pc_im(s, s->pc);
>                      gen_helper_set_cp_reg64(cpu_env, tmpptr, tmp64);
>                      tcg_temp_free_ptr(tmpptr);
>                  } else {
> @@ -6906,7 +6903,6 @@ static int disas_coproc_insn(CPUARMState * env, 
> DisasContext *s, uint32_t insn)
>                  if (ri->writefn) {
>                      TCGv_i32 tmp;
>                      TCGv_ptr tmpptr;
> -                    gen_set_pc_im(s, s->pc);
>                      tmp = load_reg(s, rt);
>                      tmpptr = tcg_const_ptr(ri);
>                      gen_helper_set_cp_reg(cpu_env, tmpptr, tmp);
> --
> 1.8.5
>
>