Re: [Qemu-devel] [PATCH 18/32] ppc: Don't update NIP in DCR access routines

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Wed, 2016-07-27 at 12:21 +1000, David Gibson wrote:
> On Wed, Jul 27, 2016 at 08:21:12AM +1000, Benjamin Herrenschmidt
> wrote:
> > 
> > This is no longer necessary as the helpers will properly retrieve
> > the return address when needed
> 
> Well, the helpers are only fixed in this patch IIUC.

Yes, all these "Don't update NIP" patches do both the helper and the
removal of the NIP fixup. I can reword the messages if you have a
better wording.
> > 
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >  target-ppc/timebase_helper.c | 23 +++++++++++++----------
> >  target-ppc/translate.c       | 12 ------------
> >  2 files changed, 13 insertions(+), 22 deletions(-)
> > 
> > diff --git a/target-ppc/timebase_helper.c b/target-
> > ppc/timebase_helper.c
> > index a07faa4..73363e0 100644
> > --- a/target-ppc/timebase_helper.c
> > +++ b/target-ppc/timebase_helper.c
> > @@ -19,6 +19,7 @@
> >  #include "qemu/osdep.h"
> >  #include "cpu.h"
> >  #include "exec/helper-proto.h"
> > +#include "exec/exec-all.h"
> >  #include "qemu/log.h"
> >  
> >  /*****************************************************************
> > ************/
> > @@ -143,15 +144,16 @@ target_ulong helper_load_dcr(CPUPPCState
> > *env, target_ulong dcrn)
> >  
> >      if (unlikely(env->dcr_env == NULL)) {
> >          qemu_log_mask(LOG_GUEST_ERROR, "No DCR environment\n");
> > -        helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> > -                                   POWERPC_EXCP_INVAL |
> > -                                   POWERPC_EXCP_INVAL_INVAL);
> > +        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
> > +                               POWERPC_EXCP_INVAL |
> > +                               POWERPC_EXCP_INVAL_INVAL, GETPC());
> >      } else if (unlikely(ppc_dcr_read(env->dcr_env,
> >                                       (uint32_t)dcrn, &val) != 0))
> > {
> >          qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n",
> >                        (uint32_t)dcrn, (uint32_t)dcrn);
> > -        helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> > -                                   POWERPC_EXCP_INVAL |
> > POWERPC_EXCP_PRIV_REG);
> > +        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
> > +                               POWERPC_EXCP_INVAL |
> > +                               POWERPC_EXCP_PRIV_REG, GETPC());
> >      }
> >      return val;
> >  }
> > @@ -160,14 +162,15 @@ void helper_store_dcr(CPUPPCState *env,
> > target_ulong dcrn, target_ulong val)
> >  {
> >      if (unlikely(env->dcr_env == NULL)) {
> >          qemu_log_mask(LOG_GUEST_ERROR, "No DCR environment\n");
> > -        helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> > -                                   POWERPC_EXCP_INVAL |
> > -                                   POWERPC_EXCP_INVAL_INVAL);
> > +        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
> > +                               POWERPC_EXCP_INVAL |
> > +                               POWERPC_EXCP_INVAL_INVAL, GETPC());
> >      } else if (unlikely(ppc_dcr_write(env->dcr_env,
> > (uint32_t)dcrn,
> >                                        (uint32_t)val) != 0)) {
> >          qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d
> > %03x\n",
> >                        (uint32_t)dcrn, (uint32_t)dcrn);
> > -        helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM,
> > -                                   POWERPC_EXCP_INVAL |
> > POWERPC_EXCP_PRIV_REG);
> > +        raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
> > +                               POWERPC_EXCP_INVAL |
> > +                               POWERPC_EXCP_PRIV_REG, GETPC());
> >      }
> >  }
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index 84bcb09..f0e0ec6 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -5262,8 +5262,6 @@ static void gen_mfdcr(DisasContext *ctx)
> >      TCGv dcrn;
> >  
> >      CHK_SV;
> > -    /* NIP cannot be restored if the memory exception comes from
> > an helper */
> > -    gen_update_nip(ctx, ctx->nip - 4);
> >      dcrn = tcg_const_tl(SPR(ctx->opcode));
> >      gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_env, dcrn);
> >      tcg_temp_free(dcrn);
> > @@ -5279,8 +5277,6 @@ static void gen_mtdcr(DisasContext *ctx)
> >      TCGv dcrn;
> >  
> >      CHK_SV;
> > -    /* NIP cannot be restored if the memory exception comes from
> > an helper */
> > -    gen_update_nip(ctx, ctx->nip - 4);
> >      dcrn = tcg_const_tl(SPR(ctx->opcode));
> >      gen_helper_store_dcr(cpu_env, dcrn, cpu_gpr[rS(ctx->opcode)]);
> >      tcg_temp_free(dcrn);
> > @@ -5295,8 +5291,6 @@ static void gen_mfdcrx(DisasContext *ctx)
> >      GEN_PRIV;
> >  #else
> >      CHK_SV;
> > -    /* NIP cannot be restored if the memory exception comes from
> > an helper */
> > -    gen_update_nip(ctx, ctx->nip - 4);
> >      gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_env,
> >                          cpu_gpr[rA(ctx->opcode)]);
> >      /* Note: Rc update flag set leads to undefined state of Rc0 */
> > @@ -5311,8 +5305,6 @@ static void gen_mtdcrx(DisasContext *ctx)
> >      GEN_PRIV;
> >  #else
> >      CHK_SV;
> > -    /* NIP cannot be restored if the memory exception comes from
> > an helper */
> > -    gen_update_nip(ctx, ctx->nip - 4);
> >      gen_helper_store_dcr(cpu_env, cpu_gpr[rA(ctx->opcode)],
> >                           cpu_gpr[rS(ctx->opcode)]);
> >      /* Note: Rc update flag set leads to undefined state of Rc0 */
> > @@ -5322,8 +5314,6 @@ static void gen_mtdcrx(DisasContext *ctx)
> >  /* mfdcrux (PPC 460) : user-mode access to DCR */
> >  static void gen_mfdcrux(DisasContext *ctx)
> >  {
> > -    /* NIP cannot be restored if the memory exception comes from
> > an helper */
> > -    gen_update_nip(ctx, ctx->nip - 4);
> >      gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_env,
> >                          cpu_gpr[rA(ctx->opcode)]);
> >      /* Note: Rc update flag set leads to undefined state of Rc0 */
> > @@ -5332,8 +5322,6 @@ static void gen_mfdcrux(DisasContext *ctx)
> >  /* mtdcrux (PPC 460) : user-mode access to DCR */
> >  static void gen_mtdcrux(DisasContext *ctx)
> >  {
> > -    /* NIP cannot be restored if the memory exception comes from
> > an helper */
> > -    gen_update_nip(ctx, ctx->nip - 4);
> >      gen_helper_store_dcr(cpu_env, cpu_gpr[rA(ctx->opcode)],
> >                           cpu_gpr[rS(ctx->opcode)]);
> >      /* Note: Rc update flag set leads to undefined state of Rc0 */
>