[06/10] ppc: Rework generation of priv and inval interrupts

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Recent server processors use the Hypervisor Emulation Assistance
interrupt for illegal instructions and *some* type of SPR accesses.

Also the code was always generating inval instructions even for priv
violations due to setting the wrong flags

Finally, the checking for PR/HV was open coded everywhere.

This reworks it all, using little helper macros for checking, and
adding the HV interrupt (which gets converted back to program check
in the slow path of excp_helper.c on CPUs that don't want it).

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: fixed checkpatch.pl errors ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 linux-user/main.c        |   1 +
 target-ppc/excp_helper.c |  19 ++
 target-ppc/translate.c   | 690 ++++++++++++++++++++---------------------------
 3 files changed, 311 insertions(+), 399 deletions(-)

On Mon, Jun 13, 2016 at 07:24:52AM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Recent server processors use the Hypervisor Emulation Assistance
> interrupt for illegal instructions and *some* type of SPR accesses.
> 
> Also the code was always generating inval instructions even for priv
> violations due to setting the wrong flags
> 
> Finally, the checking for PR/HV was open coded everywhere.
> 
> This reworks it all, using little helper macros for checking, and
> adding the HV interrupt (which gets converted back to program check
> in the slow path of excp_helper.c on CPUs that don't want it).
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [clg: fixed checkpatch.pl errors ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  linux-user/main.c        |   1 +
>  target-ppc/excp_helper.c |  19 ++
>  target-ppc/translate.c   | 690 ++++++++++++++++++++---------------------------
>  3 files changed, 311 insertions(+), 399 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index f8a8764ae97a..9e9b88b458c4 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -1721,6 +1721,7 @@ void cpu_loop(CPUPPCState *env)
>              queue_signal(env, info.si_signo, &info);
>              break;
>          case POWERPC_EXCP_PROGRAM:  /* Program exception                     */
> +        case POWERPC_EXCP_HV_EMU:   /* HV emulation                          */
>              /* XXX: check this */
>              switch (env->error_code & ~0xF) {
>              case POWERPC_EXCP_FP:
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index 7c44c102db39..054c12de3bff 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -128,6 +128,19 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          ail = 0;
>      }
>  
> +    /* Hypervisor emulation assistance interrupt only exists on server
> +     * arch 2.05 server or later. We also don't want to generate it if
> +     * we don't have HVB in msr_mask (PAPR mode).
> +     */
> +    if (excp == POWERPC_EXCP_HV_EMU
> +#if defined(TARGET_PPC64)
> +        && !((env->mmu_model & POWERPC_MMU_64) && (env->msr_mask & MSR_HVB))
> +#endif /* defined(TARGET_PPC64) */
> +
> +    ) {
> +        excp = POWERPC_EXCP_PROGRAM;
> +    }
> +
>      switch (excp) {
>      case POWERPC_EXCP_NONE:
>          /* Should never happen */
> @@ -249,6 +262,12 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>              break;
>          }
>          goto store_current;
> +    case POWERPC_EXCP_HV_EMU:
> +        srr0 = SPR_HSRR0;
> +        srr1 = SPR_HSRR1;
> +        new_msr |= (target_ulong)MSR_HVB;
> +        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +        goto store_current;
>      case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>          goto store_current;
>      case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a02ddf52bfe6..2ec858063ecc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -325,7 +325,19 @@ static inline void gen_debug_exception(DisasContext *ctx)
>  
>  static inline void gen_inval_exception(DisasContext *ctx, uint32_t error)
>  {
> -    gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL | error);
> +    /* Will be converted to program check if needed */
> +    gen_exception_err(ctx, POWERPC_EXCP_HV_EMU, POWERPC_EXCP_INVAL | error);
> +}
> +
> +static inline void gen_priv_exception(DisasContext *ctx, uint32_t error)
> +{
> +    gen_exception_err(ctx, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_PRIV | error);
> +}
> +
> +static inline void gen_hvpriv_exception(DisasContext *ctx, uint32_t error)
> +{
> +    /* Will be converted to program check if needed */
> +    gen_exception_err(ctx, POWERPC_EXCP_HV_EMU, POWERPC_EXCP_PRIV | error);
>  }
>  
>  /* Stop translation */
> @@ -366,6 +378,33 @@ typedef struct opcode_t {
>      const char *oname;
>  } opcode_t;
>  
> +/* Helpers for priv. check */
> +#define GEN_PRIV                                                \
> +    do {                                                        \
> +        gen_priv_exception(ctx, POWERPC_EXCP_PRIV_OPC); return; \
> +    } while (0)
> +
> +#if defined(CONFIG_USER_ONLY)
> +#define CHK_HV GEN_PRIV
> +#define CHK_SV GEN_PRIV
> +#else
> +#define CHK_HV                                                          \
> +    do {                                                                \
> +        if (unlikely(ctx->pr || !ctx->hv)) {                            \
> +            GEN_PRIV;                                                   \
> +        }                                                               \
> +    } while (0)
> +#define CHK_SV                   \
> +    do {                         \
> +        if (unlikely(ctx->pr)) { \
> +            GEN_PRIV;            \
> +        }                        \
> +    } while (0)
> +#endif
> +
> +#define CHK_NONE
> +
> +
>  /*****************************************************************************/
>  /***                           Instruction decoding                        ***/
>  #define EXTRACT_HELPER(name, shift, nb)                                       \
> @@ -2929,7 +2968,7 @@ static void gen_lq(DisasContext *ctx)
>      bool le_is_supported = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
>  
>      if (!legal_in_user_mode && ctx->pr) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +        gen_priv_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>          return;
>      }
>  
> @@ -3055,7 +3094,7 @@ static void gen_std(DisasContext *ctx)
>          }
>  
>          if (!legal_in_user_mode && ctx->pr) {
> -            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +            gen_priv_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>              return;
>          }
>  
> @@ -4085,7 +4124,7 @@ static void gen_mcrf(DisasContext *ctx)
>  static void gen_rfi(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      /* This instruction doesn't exist anymore on 64-bit server
>       * processors compliant with arch 2.x
> @@ -4095,10 +4134,7 @@ static void gen_rfi(DisasContext *ctx)
>          return;
>      }
>      /* Restore CPU state */
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_update_cfar(ctx, ctx->nip);
>      gen_helper_rfi(cpu_env);
>      gen_sync_exception(ctx);
> @@ -4109,13 +4145,10 @@ static void gen_rfi(DisasContext *ctx)
>  static void gen_rfid(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      /* Restore CPU state */
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_update_cfar(ctx, ctx->nip);
>      gen_helper_rfid(cpu_env);
>      gen_sync_exception(ctx);
> @@ -4125,13 +4158,10 @@ static void gen_rfid(DisasContext *ctx)
>  static void gen_hrfid(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      /* Restore CPU state */
> -    if (unlikely(ctx->pr || !ctx->hv)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_HV;
>      gen_helper_hrfid(cpu_env);
>      gen_sync_exception(ctx);
>  #endif
> @@ -4294,15 +4324,8 @@ static void gen_mfcr(DisasContext *ctx)
>  /* mfmsr */
>  static void gen_mfmsr(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -#else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    CHK_SV;
>      tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_msr);
> -#endif
>  }
>  
>  static void spr_noaccess(DisasContext *ctx, int gprn, int sprn)
> @@ -4348,9 +4371,15 @@ static inline void gen_op_mfspr(DisasContext *ctx)
>                               TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>                  }
>              }
> -            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +            gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>          }
>      } else {
> +        /* ISA 2.07 defines these as no-ops */
> +        if ((ctx->insns_flags2 & PPC2_ISA207S) &&
> +            (sprn >= 808 && sprn <= 811)) {
> +            /* This is a nop */
> +            return;
> +        }
>          /* Not defined */
>          fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at "
>                  TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
> @@ -4358,9 +4387,18 @@ static inline void gen_op_mfspr(DisasContext *ctx)
>              qemu_log("Trying to read invalid spr %d (0x%03x) at "
>                       TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>          }
> -        /* Only generate an exception in user space, otherwise this is a nop */
> -        if (ctx->pr) {
> -            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +
> +        /* The behaviour depends on MSR:PR and SPR# bit 0x10,
> +         * it can generate a priv, a hv emu or a no-op
> +         */
> +        if (sprn & 0x10) {
> +            if (ctx->pr) {
> +                gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +            }
> +        } else {
> +            if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {
> +                gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);

Just double checking this logic.  So in this case we get an exception
to the hypervisor if executed in guest user mode, but a no-op if
executed in guest supervisor mode.  That seems.. odd.

> +            }
>          }
>      }
>  }
> @@ -4408,13 +4446,9 @@ static void gen_mtcrf(DisasContext *ctx)
>  #if defined(TARGET_PPC64)
>  static void gen_mtmsrd(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -#else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    CHK_SV;
> +
> +#if !defined(CONFIG_USER_ONLY)
>      if (ctx->opcode & 0x00010000) {
>          /* Special form that does not need any synchronisation */
>          TCGv t0 = tcg_temp_new();
> @@ -4433,20 +4467,16 @@ static void gen_mtmsrd(DisasContext *ctx)
>          /* Note that mtmsr is not always defined as context-synchronizing */
>          gen_stop_exception(ctx);
>      }
> -#endif
> +#endif /* !defined(CONFIG_USER_ONLY) */
>  }
> -#endif
> +#endif /* defined(TARGET_PPC64) */
>  
>  static void gen_mtmsr(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -#else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> -    if (ctx->opcode & 0x00010000) {
> +    CHK_SV;
> +
> +#if !defined(CONFIG_USER_ONLY)
> +   if (ctx->opcode & 0x00010000) {
>          /* Special form that does not need any synchronisation */
>          TCGv t0 = tcg_temp_new();
>          tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
> @@ -4503,9 +4533,16 @@ static void gen_mtspr(DisasContext *ctx)
>                  qemu_log("Trying to write privileged spr %d (0x%03x) at "
>                           TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>              }
> -            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +            gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>          }
>      } else {
> +        /* ISA 2.07 defines these as no-ops */
> +        if ((ctx->insns_flags2 & PPC2_ISA207S) &&
> +            (sprn >= 808 && sprn <= 811)) {
> +            /* This is a nop */
> +            return;
> +        }
> +
>          /* Not defined */
>          if (qemu_log_separate()) {
>              qemu_log("Trying to write invalid spr %d (0x%03x) at "
> @@ -4514,9 +4551,18 @@ static void gen_mtspr(DisasContext *ctx)
>          fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at "
>                  TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>  
> -        /* Only generate an exception in user space, otherwise this is a nop */
> -        if (ctx->pr) {
> -            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +
> +        /* The behaviour depends on MSR:PR and SPR# bit 0x10,
> +         * it can generate a priv, a hv emu or a no-op
> +         */
> +        if (sprn & 0x10) {
> +            if (ctx->pr) {
> +                gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +            }
> +        } else {
> +            if (ctx->pr || sprn == 0) {
> +                gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +            }
>          }
>      }
>  }
> @@ -4539,13 +4585,11 @@ static void gen_dcbf(DisasContext *ctx)
>  static void gen_dcbi(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv EA, val;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      EA = tcg_temp_new();
>      gen_set_access_type(ctx, ACCESS_CACHE);
>      gen_addr_reg_index(ctx, EA);
> @@ -4555,7 +4599,7 @@ static void gen_dcbi(DisasContext *ctx)
>      gen_qemu_st8(ctx, val, EA);
>      tcg_temp_free(val);
>      tcg_temp_free(EA);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* dcdst */
> @@ -4676,72 +4720,64 @@ static void gen_dcba(DisasContext *ctx)
>  static void gen_mfsr(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_const_tl(SR(ctx->opcode));
>      gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mfsrin */
>  static void gen_mfsrin(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
>      tcg_gen_andi_tl(t0, t0, 0xF);
>      gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mtsr */
>  static void gen_mtsr(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_const_tl(SR(ctx->opcode));
>      gen_helper_store_sr(cpu_env, t0, cpu_gpr[rS(ctx->opcode)]);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mtsrin */
>  static void gen_mtsrin(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    CHK_SV;
> +
>      t0 = tcg_temp_new();
>      tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
>      tcg_gen_andi_tl(t0, t0, 0xF);
>      gen_helper_store_sr(cpu_env, t0, cpu_gpr[rD(ctx->opcode)]);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  #if defined(TARGET_PPC64)
> @@ -4751,115 +4787,101 @@ static void gen_mtsrin(DisasContext *ctx)
>  static void gen_mfsr_64b(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_const_tl(SR(ctx->opcode));
>      gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mfsrin */
>  static void gen_mfsrin_64b(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
>      tcg_gen_andi_tl(t0, t0, 0xF);
>      gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mtsr */
>  static void gen_mtsr_64b(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_const_tl(SR(ctx->opcode));
>      gen_helper_store_sr(cpu_env, t0, cpu_gpr[rS(ctx->opcode)]);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mtsrin */
>  static void gen_mtsrin_64b(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
>      tcg_gen_andi_tl(t0, t0, 0xF);
>      gen_helper_store_sr(cpu_env, t0, cpu_gpr[rS(ctx->opcode)]);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* slbmte */
>  static void gen_slbmte(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_store_slb(cpu_env, cpu_gpr[rB(ctx->opcode)],
>                           cpu_gpr[rS(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_slbmfee(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_load_slb_esid(cpu_gpr[rS(ctx->opcode)], cpu_env,
>                               cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_slbmfev(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_load_slb_vsid(cpu_gpr[rS(ctx->opcode)], cpu_env,
>                               cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_slbfee_(DisasContext *ctx)
> @@ -4895,40 +4917,34 @@ static void gen_slbfee_(DisasContext *ctx)
>  static void gen_tlbia(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr || !ctx->hv)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_HV;
> +
>      gen_helper_tlbia(cpu_env);
> -#endif
> +#endif  /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbiel */
>  static void gen_tlbiel(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbie */
>  static void gen_tlbie(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr || !ctx->hv)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_HV;
> +
>      if (NARROW_MODE(ctx)) {
>          TCGv t0 = tcg_temp_new();
>          tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
> @@ -4937,25 +4953,23 @@ static void gen_tlbie(DisasContext *ctx)
>      } else {
>          gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbsync */
>  static void gen_tlbsync(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr || !ctx->hv)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_HV;
> +
>      /* tlbsync is a nop for server, ptesync handles delayed tlb flush,
>       * embedded however needs to deal with tlbsync. We don't try to be
>       * fancy and swallow the overhead of checking for both.
>       */
>      gen_check_tlb_flush(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  #if defined(TARGET_PPC64)
> @@ -4963,30 +4977,26 @@ static void gen_tlbsync(DisasContext *ctx)
>  static void gen_slbia(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_slbia(cpu_env);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* slbie */
>  static void gen_slbie(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_slbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
> -#endif
> +#endif  /* defined(TARGET_PPC64) */
>  
>  /***                              External control                         ***/
>  /* Optional: */
> @@ -5685,14 +5695,11 @@ static void gen_esa(DisasContext *ctx)
>  static void gen_mfrom(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_helper_602_mfrom(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* 602 - 603 - G2 TLB management */
> @@ -5701,28 +5708,22 @@ static void gen_mfrom(DisasContext *ctx)
>  static void gen_tlbld_6xx(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_helper_6xx_tlbd(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbli */
>  static void gen_tlbli_6xx(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_helper_6xx_tlbi(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* 74xx TLB management */
> @@ -5731,28 +5732,22 @@ static void gen_tlbli_6xx(DisasContext *ctx)
>  static void gen_tlbld_74xx(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_helper_74xx_tlbd(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbli */
>  static void gen_tlbli_74xx(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_helper_74xx_tlbi(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* POWER instructions not in PowerPC 601 */
> @@ -5766,15 +5761,12 @@ static void gen_clf(DisasContext *ctx)
>  /* cli */
>  static void gen_cli(DisasContext *ctx)
>  {
> -    /* Cache line invalidate: privileged and treated as no-op */
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> -#endif
> +    /* Cache line invalidate: privileged and treated as no-op */
> +    CHK_SV;
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* dclst */
> @@ -5786,15 +5778,13 @@ static void gen_dclst(DisasContext *ctx)
>  static void gen_mfsri(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      int ra = rA(ctx->opcode);
>      int rd = rD(ctx->opcode);
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>      tcg_gen_shri_tl(t0, t0, 28);
> @@ -5803,38 +5793,34 @@ static void gen_mfsri(DisasContext *ctx)
>      tcg_temp_free(t0);
>      if (ra != 0 && ra != rd)
>          tcg_gen_mov_tl(cpu_gpr[ra], cpu_gpr[rd]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_rac(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>      gen_helper_rac(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_rfsvc(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
> +
>      gen_helper_rfsvc(cpu_env);
>      gen_sync_exception(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* svc is not implemented for now */
> @@ -5987,18 +5973,16 @@ static void gen_mfapidi(DisasContext *ctx)
>  static void gen_tlbiva(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>      gen_helper_tlbiva(cpu_env, cpu_gpr[rB(ctx->opcode)]);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* All 405 MAC instructions are translated here */
> @@ -6220,38 +6204,34 @@ GEN_MAC_HANDLER(mullhwu, 0x08, 0x0C);
>  static void gen_mfdcr(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv dcrn;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    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);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mtdcr */
>  static void gen_mtdcr(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
>      TCGv dcrn;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +
> +    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);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mfdcrx */
> @@ -6259,18 +6239,15 @@ static void gen_mtdcr(DisasContext *ctx)
>  static void gen_mfdcrx(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    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 */
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mtdcrx */
> @@ -6278,18 +6255,15 @@ static void gen_mfdcrx(DisasContext *ctx)
>  static void gen_mtdcrx(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> -        return;
> -    }
> +    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 */
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* mfdcrux (PPC 460) : user-mode access to DCR */
> @@ -6315,28 +6289,19 @@ static void gen_mtdcrux(DisasContext *ctx)
>  /* dccci */
>  static void gen_dccci(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -#else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* interpreted as no-op */
> -#endif
>  }
>  
>  /* dcread */
>  static void gen_dcread(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv EA, val;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      gen_set_access_type(ctx, ACCESS_CACHE);
>      EA = tcg_temp_new();
>      gen_addr_reg_index(ctx, EA);
> @@ -6345,7 +6310,7 @@ static void gen_dcread(DisasContext *ctx)
>      tcg_temp_free(val);
>      tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], EA);
>      tcg_temp_free(EA);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* icbt */
> @@ -6360,60 +6325,40 @@ static void gen_icbt_40x(DisasContext *ctx)
>  /* iccci */
>  static void gen_iccci(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -#else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* interpreted as no-op */
> -#endif
>  }
>  
>  /* icread */
>  static void gen_icread(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -#else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* interpreted as no-op */
> -#endif
>  }
>  
>  /* rfci (supervisor only) */
>  static void gen_rfci_40x(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* Restore CPU state */
>      gen_helper_40x_rfci(cpu_env);
>      gen_sync_exception(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_rfci(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* Restore CPU state */
>      gen_helper_rfci(cpu_env);
>      gen_sync_exception(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* BookE specific */
> @@ -6422,32 +6367,26 @@ static void gen_rfci(DisasContext *ctx)
>  static void gen_rfdi(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* Restore CPU state */
>      gen_helper_rfdi(cpu_env);
>      gen_sync_exception(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* XXX: not implemented on 440 ? */
>  static void gen_rfmci(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      /* Restore CPU state */
>      gen_helper_rfmci(cpu_env);
>      gen_sync_exception(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* TLB management - PowerPC 405 implementation */
> @@ -6456,12 +6395,9 @@ static void gen_rfmci(DisasContext *ctx)
>  static void gen_tlbre_40x(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      switch (rB(ctx->opcode)) {
>      case 0:
>          gen_helper_4xx_tlbre_hi(cpu_gpr[rD(ctx->opcode)], cpu_env,
> @@ -6475,20 +6411,18 @@ static void gen_tlbre_40x(DisasContext *ctx)
>          gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>          break;
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbsx - tlbsx. */
>  static void gen_tlbsx_40x(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>      gen_helper_4xx_tlbsx(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
> @@ -6500,19 +6434,17 @@ static void gen_tlbsx_40x(DisasContext *ctx)
>          tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 0x02);
>          gen_set_label(l1);
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbwe */
>  static void gen_tlbwe_40x(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
> +
>      switch (rB(ctx->opcode)) {
>      case 0:
>          gen_helper_4xx_tlbwe_hi(cpu_env, cpu_gpr[rA(ctx->opcode)],
> @@ -6526,7 +6458,7 @@ static void gen_tlbwe_40x(DisasContext *ctx)
>          gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>          break;
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* TLB management - PowerPC 440 implementation */
> @@ -6535,12 +6467,10 @@ static void gen_tlbwe_40x(DisasContext *ctx)
>  static void gen_tlbre_440(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
> +
>      switch (rB(ctx->opcode)) {
>      case 0:
>      case 1:
> @@ -6556,20 +6486,18 @@ static void gen_tlbre_440(DisasContext *ctx)
>          gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>          break;
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbsx - tlbsx. */
>  static void gen_tlbsx_440(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>      gen_helper_440_tlbsx(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
> @@ -6581,19 +6509,16 @@ static void gen_tlbsx_440(DisasContext *ctx)
>          tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 0x02);
>          gen_set_label(l1);
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbwe */
>  static void gen_tlbwe_440(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      switch (rB(ctx->opcode)) {
>      case 0:
>      case 1:
> @@ -6609,7 +6534,7 @@ static void gen_tlbwe_440(DisasContext *ctx)
>          gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>          break;
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* TLB management - PowerPC BookE 2.06 implementation */
> @@ -6617,30 +6542,23 @@ static void gen_tlbwe_440(DisasContext *ctx)
>  /* tlbre */
>  static void gen_tlbre_booke206(DisasContext *ctx)
>  {
> -#if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> + #if defined(CONFIG_USER_ONLY)
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> -
> +   CHK_SV;
>      gen_helper_booke206_tlbre(cpu_env);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbsx - tlbsx. */
>  static void gen_tlbsx_booke206(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
>  
> +    CHK_SV;
>      if (rA(ctx->opcode)) {
>          t0 = tcg_temp_new();
>          tcg_gen_mov_tl(t0, cpu_gpr[rD(ctx->opcode)]);
> @@ -6651,54 +6569,44 @@ static void gen_tlbsx_booke206(DisasContext *ctx)
>      tcg_gen_add_tl(t0, t0, cpu_gpr[rB(ctx->opcode)]);
>      gen_helper_booke206_tlbsx(cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* tlbwe */
>  static void gen_tlbwe_booke206(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      gen_update_nip(ctx, ctx->nip - 4);
>      gen_helper_booke206_tlbwe(cpu_env);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_tlbivax_booke206(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
>  
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
> -
>      gen_helper_booke206_tlbivax(cpu_env, t0);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_tlbilx_booke206(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
>  
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>  
> @@ -6718,7 +6626,7 @@ static void gen_tlbilx_booke206(DisasContext *ctx)
>      }
>  
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  
> @@ -6726,13 +6634,11 @@ static void gen_tlbilx_booke206(DisasContext *ctx)
>  static void gen_wrtee(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
>      TCGv t0;
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +
> +    CHK_SV;
>      t0 = tcg_temp_new();
>      tcg_gen_andi_tl(t0, cpu_gpr[rD(ctx->opcode)], (1 << MSR_EE));
>      tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(1 << MSR_EE));
> @@ -6742,19 +6648,16 @@ static void gen_wrtee(DisasContext *ctx)
>       * if we just set msr_ee to 1
>       */
>      gen_stop_exception(ctx);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* wrteei */
>  static void gen_wrteei(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;
>      if (ctx->opcode & 0x00008000) {
>          tcg_gen_ori_tl(cpu_msr, cpu_msr, (1 << MSR_EE));
>          /* Stop translation to have a chance to raise an exception */
> @@ -6762,7 +6665,7 @@ static void gen_wrteei(DisasContext *ctx)
>      } else {
>          tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(1 << MSR_EE));
>      }
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /* PowerPC 440 specific instructions */
> @@ -6802,29 +6705,21 @@ static void gen_icbt_440(DisasContext *ctx)
>  static void gen_msgclr(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> -
> +    CHK_SV;
>      gen_helper_msgclr(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  static void gen_msgsnd(DisasContext *ctx)
>  {
>  #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> +    GEN_PRIV;
>  #else
> -    if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> -
> +    CHK_SV;
>      gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
>  /***                      Altivec vector extension                         ***/
> @@ -9826,7 +9721,7 @@ static void gen_tcheck(DisasContext *ctx)
>  #define GEN_TM_PRIV_NOOP(name)                                 \
>  static inline void gen_##name(DisasContext *ctx)               \
>  {                                                              \
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);           \
> +    gen_priv_exception(ctx, POWERPC_EXCP_PRIV_OPC);           \
>  }
>  
>  #else
> @@ -9834,10 +9729,7 @@ static inline void gen_##name(DisasContext *ctx)               \
>  #define GEN_TM_PRIV_NOOP(name)                                 \
>  static inline void gen_##name(DisasContext *ctx)               \
>  {                                                              \
> -    if (unlikely(ctx->pr)) {                                   \
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);       \
> -        return;                                                \
> -    }                                                          \
> +    CHK_SV;                                                    \
>      if (unlikely(!ctx->tm_enabled)) {                          \
>          gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_TM);   \
>          return;                                                \

On Wed, 2016-06-15 at 11:19 +1000, David Gibson wrote:
> 

> >  static void spr_noaccess(DisasContext *ctx, int gprn, int sprn)

> > @@ -4348,9 +4371,15 @@ static inline void gen_op_mfspr(DisasContext *ctx)

> >                               TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);

> >                  }

> >              }

> > -            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);

> > +            gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);

> >          }

> >      } else {

> > +        /* ISA 2.07 defines these as no-ops */

> > +        if ((ctx->insns_flags2 & PPC2_ISA207S) &&

> > +            (sprn >= 808 && sprn <= 811)) {

> > +            /* This is a nop */

> > +            return;

> > +        }

> >          /* Not defined */

> >          fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at "

> >                  TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);

> > @@ -4358,9 +4387,18 @@ static inline void gen_op_mfspr(DisasContext *ctx)

> >              qemu_log("Trying to read invalid spr %d (0x%03x) at "

> >                       TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);

> >          }

> > -        /* Only generate an exception in user space, otherwise this is a nop */

> > -        if (ctx->pr) {

> > -            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_SPR);

> > +

> > +        /* The behaviour depends on MSR:PR and SPR# bit 0x10,

> > +         * it can generate a priv, a hv emu or a no-op

> > +         */

> > +        if (sprn & 0x10) {

> > +            if (ctx->pr) {

> > +                gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);

> > +            }

> > +        } else {

> > +            if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {

> > +                gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);

> 

> Just double checking this logic.  So in this case we get an exception

> to the hypervisor if executed in guest user mode, but a no-op if

> 

> executed in guest supervisor mode.  That seems.. odd.


From the architecture:

* if spr 0 =0:
  - if MSR PR =1: Hypervisor Emulation Assistance
    interrupt
  - if MSR PR =0: Hypervisor Emulation Assistance
    interrupt for SPRs 0, 4, 5, and 6 and no opera-
    tion (i.e. the instruction is treated as a no-op)
    for all other SPRs
„
* if spr 0 =1:
  - if MSR PR =1: Privileged Instruction type Pro-
    gram interrupt
  - if MSR PR =0: no operation (i.e. the instruction
    is treated as a no-op)

IE. SPRs with 0x10 are supervisor priv, so PR access will trap to
the OS, whether they are implemented or not.

Otherwise, you get the "system illegal isntruction" handler which
is turned into an HVPRIV on all recent processors (the exception code
will turn that back into a 0x700 if the processor doesn't support
HVPRIV).

It was done this way so that an OS (guest) can context switch a bunch
of supervisor SPRs without having to test if they individually exist
on a given processor.

Cheers,
Ben.

>

On Wed, Jun 15, 2016 at 02:31:56PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2016-06-15 at 11:19 +1000, David Gibson wrote:
> > 
> > >  static void spr_noaccess(DisasContext *ctx, int gprn, int sprn)
> > > @@ -4348,9 +4371,15 @@ static inline void gen_op_mfspr(DisasContext *ctx)
> > >                               TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
> > >                  }
> > >              }
> > > -            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> > > +            gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> > >          }
> > >      } else {
> > > +        /* ISA 2.07 defines these as no-ops */
> > > +        if ((ctx->insns_flags2 & PPC2_ISA207S) &&
> > > +            (sprn >= 808 && sprn <= 811)) {
> > > +            /* This is a nop */
> > > +            return;
> > > +        }
> > >          /* Not defined */
> > >          fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at "
> > >                  TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
> > > @@ -4358,9 +4387,18 @@ static inline void gen_op_mfspr(DisasContext *ctx)
> > >              qemu_log("Trying to read invalid spr %d (0x%03x) at "
> > >                       TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
> > >          }
> > > -        /* Only generate an exception in user space, otherwise this is a nop */
> > > -        if (ctx->pr) {
> > > -            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> > > +
> > > +        /* The behaviour depends on MSR:PR and SPR# bit 0x10,
> > > +         * it can generate a priv, a hv emu or a no-op
> > > +         */
> > > +        if (sprn & 0x10) {
> > > +            if (ctx->pr) {
> > > +                gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> > > +            }
> > > +        } else {
> > > +            if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {
> > > +                gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> > 
> > Just double checking this logic.  So in this case we get an exception
> > to the hypervisor if executed in guest user mode, but a no-op if
> > 
> > executed in guest supervisor mode.  That seems.. odd.
> 
> >From the architecture:
> 
> * if spr 0 =0:
>   - if MSR PR =1: Hypervisor Emulation Assistance
>     interrupt
>   - if MSR PR =0: Hypervisor Emulation Assistance
>     interrupt for SPRs 0, 4, 5, and 6 and no opera-
>     tion (i.e. the instruction is treated as a no-op)
>     for all other SPRs
> „
> * if spr 0 =1:
>   - if MSR PR =1: Privileged Instruction type Pro-
>     gram interrupt
>   - if MSR PR =0: no operation (i.e. the instruction
>     is treated as a no-op)
> 
> IE. SPRs with 0x10 are supervisor priv, so PR access will trap to
> the OS, whether they are implemented or not.
> 
> Otherwise, you get the "system illegal isntruction" handler which
> is turned into an HVPRIV on all recent processors (the exception code
> will turn that back into a 0x700 if the processor doesn't support
> HVPRIV).
> 
> It was done this way so that an OS (guest) can context switch a bunch
> of supervisor SPRs without having to test if they individually exist
> on a given processor.

Huh.  Alright then.