diff mbox

[21/77] ppc: Rework generation of priv and inval interrupts

Message ID 1447201710-10229-22-git-send-email-benh@kernel.crashing.org
State New
Headers show

Commit Message

Benjamin Herrenschmidt Nov. 11, 2015, 12:27 a.m. UTC 
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>
---
 linux-user/main.c        |   1 +
 target-ppc/excp_helper.c |  19 ++
 target-ppc/translate.c   | 678 ++++++++++++++++++++---------------------------
 3 files changed, 302 insertions(+), 396 deletions(-)

Comments

David Gibson Nov. 20, 2015, 7:45 a.m. UTC | #1 
On Wed, Nov 11, 2015 at 11:27:34AM +1100, Benjamin Herrenschmidt wrote:
> 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>

[snip]
>  static void spr_noaccess(DisasContext *ctx, int gprn, int sprn)
> @@ -4340,7 +4350,7 @@ static inline void gen_op_mfspr(DisasContext *ctx)
>                  printf("Trying to read 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 {
>          /* Not defined */
> @@ -4348,7 +4358,25 @@ static inline void gen_op_mfspr(DisasContext *ctx)
>                   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>          printf("Trying to read invalid spr %d (0x%03x) at "
>                 TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);

So, I'm not 100% following the logic below, but it looks like the
existing code used SPR_NOACCESS to mark things which generated a
privilege exception compared to NULL for things which generated an
invalid instruction exception.  Using that encoding, can you simplify
the logic here?  Alternatively can you use the logic here to avoid the
SPR_NOACESS encoding?

> -        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);
> +            }
> +        }
> +#if !defined(CONFIG_USER_ONLY)
> +        /* HV priv */
> +        if (ctx->spr_cb[sprn].hea_read) {
> +            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +        }

If you're in PR mode, and it's an SPR with an hea_read function and
has the 0x10 bit set, won't this call gen_priv_exception twice?

I also see no path here which will call gen_inval_exception(), is that
right?  If you're in HV mode and it's a truly invalid SPRN, isn't that
what you'd want?

> +#endif
>      }
>  }



>  
> @@ -4395,13 +4423,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();
> @@ -4420,20 +4444,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));
> @@ -4488,7 +4508,7 @@ static void gen_mtspr(DisasContext *ctx)
>                       TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>              printf("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 {
>          /* Not defined */
> @@ -4496,7 +4516,25 @@ static void gen_mtspr(DisasContext *ctx)
>                   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
>          printf("Trying to write invalid spr %d (0x%03x) at "
>                 TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
> -        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);
> +            }
> +        }
> +#if !defined(CONFIG_USER_ONLY)
> +        /* HV priv */
> +        if (ctx->spr_cb[sprn].hea_write) {
> +            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +        }
> +#endif

Same concerns here as for mfspr.

[snip]
>  /* 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 || !ctx->hv)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> -        return;
> -    }
> +    CHK_SV;

You have CHK_SV here, but the original code checks for HV, as does
your new code for tlbia and tlbiel, is that right?

[snip]
>  /* tlbsync */
>  static void gen_tlbsync(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;
> -    }
> +    GEN_PRIV;
> +#else    
> +    CHK_HV;
> +

Old code didn't check for HV, mode, but AFAICT it should have, so this
looks correct.

[snip]
> @@ -5941,18 +5921,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;

Is the same thing as tlbivax, or some ancient instruction?  AFAICT the
ISA says tlbivax is hypervisor privileged.

>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>      gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }

[snip]
>  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;

ISA says tlbivax is hypervisor privileged when the CPU has a
hypervisor mode, which I guess booke206 probably doesn't?

>      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;

And apparently hv vs. sv privilege of tlbilx depends on the EPCR
register.  Again, may not be relevant for 2.06.

>      t0 = tcg_temp_new();
>      gen_addr_reg_index(ctx, t0);
>  
> @@ -6672,7 +6574,7 @@ static void gen_tlbilx_booke206(DisasContext *ctx)
>      }
>  
>      tcg_temp_free(t0);
> -#endif
> +#endif /* defined(CONFIG_USER_ONLY) */
>  }
Benjamin Herrenschmidt Nov. 24, 2015, 12:44 a.m. UTC | #2 
On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote:
> 

> So, I'm not 100% following the logic below, but it looks like the

> existing code used SPR_NOACCESS to mark things which generated a

> privilege exception compared to NULL for things which generated an

> invalid instruction exception.  Using that encoding, can you simplify

> the logic here?  Alternatively can you use the logic here to avoid

> the SPR_NOACESS encoding?


Well, so the SPR_NOACCESS has to do with how you react to a known SPR
who has explicit access permissions. The logic below is described in
the ISA for an unknown SPR number.

I don't know whether the access permission of "known" SPRs always
honor the 0x10 bit trick, and changing that in qemu would be a
fairly large patch. So I'd rather stick to the logic here for
"unknown" SPRs which matches the ISA definition.

I'll update the patch though for arch 2.07 as it defines a few
reserved SPRs as no-ops.

However:

> > -        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);

> > +            }

> > +        }

> > +#if !defined(CONFIG_USER_ONLY)

> > +        /* HV priv */

> > +        if (ctx->spr_cb[sprn].hea_read) {

> > +            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);

> > +        }


That latest bit is bogus.

> If you're in PR mode, and it's an SPR with an hea_read function and

> has the 0x10 bit set, won't this call gen_priv_exception twice?


Yes, I've removed it. It should be handled by the SPR_NOACCESS.

> I also see no path here which will call gen_inval_exception(), is

> that

> right?  If you're in HV mode and it's a truly invalid SPRN, isn't

> that

> what you'd want?


No, the ISA says it's a nop.

Cheers,
Ben.

> > +#endif

> >      }

> >  }

> 

> 

> 

> >  

> > @@ -4395,13 +4423,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();

> > @@ -4420,20 +4444,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));

> > @@ -4488,7 +4508,7 @@ static void gen_mtspr(DisasContext *ctx)

> >                       TARGET_FMT_lx "\n", sprn, sprn, ctx->nip -

> > 4);

> >              printf("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 {

> >          /* Not defined */

> > @@ -4496,7 +4516,25 @@ static void gen_mtspr(DisasContext *ctx)

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

> >          printf("Trying to write invalid spr %d (0x%03x) at "

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

> > -        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);

> > +            }

> > +        }

> > +#if !defined(CONFIG_USER_ONLY)

> > +        /* HV priv */

> > +        if (ctx->spr_cb[sprn].hea_write) {

> > +            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);

> > +        }

> > +#endif

> 

> Same concerns here as for mfspr.

> 

> [snip]

> >  /* 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 || !ctx->hv)) {

> > -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);

> > -        return;

> > -    }

> > +    CHK_SV;

> 

> You have CHK_SV here, but the original code checks for HV, as does

> your new code for tlbia and tlbiel, is that right?

> 

> [snip]

> >  /* tlbsync */

> >  static void gen_tlbsync(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;

> > -    }

> > +    GEN_PRIV;

> > +#else    

> > +    CHK_HV;

> > +

> 

> Old code didn't check for HV, mode, but AFAICT it should have, so

> this

> looks correct.

> 

> [snip]

> > @@ -5941,18 +5921,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;

> 

> Is the same thing as tlbivax, or some ancient instruction?  AFAICT

> the

> ISA says tlbivax is hypervisor privileged.

> 

> >      t0 = tcg_temp_new();

> >      gen_addr_reg_index(ctx, t0);

> >      gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);

> >      tcg_temp_free(t0);

> > -#endif

> > +#endif /* defined(CONFIG_USER_ONLY) */

> >  }

> 

> [snip]

> >  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;

> 

> ISA says tlbivax is hypervisor privileged when the CPU has a

> hypervisor mode, which I guess booke206 probably doesn't?

> 

> >      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;

> 

> And apparently hv vs. sv privilege of tlbilx depends on the EPCR

> register.  Again, may not be relevant for 2.06.

> 

> >      t0 = tcg_temp_new();

> >      gen_addr_reg_index(ctx, t0);

> >  

> > @@ -6672,7 +6574,7 @@ static void gen_tlbilx_booke206(DisasContext

> > *ctx)

> >      }

> >  

> >      tcg_temp_free(t0);

> > -#endif

> > +#endif /* defined(CONFIG_USER_ONLY) */

> >  }

>
Benjamin Herrenschmidt Nov. 24, 2015, 12:51 a.m. UTC | #3 
On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote:
> snip]
> >  /* 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 || !ctx->hv)) {
> > -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> > -        return;
> > -    }
> > +    CHK_SV;
> 
> You have CHK_SV here, but the original code checks for HV, as does
> your new code for tlbia and tlbiel, is that right?

Yes. tlbiel is supervisor accessible (for weird reasons).

> [snip]
> >  /* tlbsync */
> >  static void gen_tlbsync(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;
> > -    }
> > +    GEN_PRIV;
> > +#else    
> > +    CHK_HV;
> > +
> 
> Old code didn't check for HV, mode, but AFAICT it should have, so
> this looks correct.

Yes, this is a hypervisor instruction.

> [snip]
> > @@ -5941,18 +5921,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;
> 
> Is the same thing as tlbivax, or some ancient instruction?  AFAICT
> the ISA says tlbivax is hypervisor privileged.

"tlbiva" is the 4xx variant, there is no hypervisor mode on these
things.

> >      t0 = tcg_temp_new();
> >      gen_addr_reg_index(ctx, t0);
> >      gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> >      tcg_temp_free(t0);
> > -#endif
> > +#endif /* defined(CONFIG_USER_ONLY) */
> >  }
> 
> [snip]
> >  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;
> 
> ISA says tlbivax is hypervisor privileged when the CPU has a
> hypervisor mode, which I guess booke206 probably doesn't?

Right so here, the "problem" is that afaik, TCG doesn't implement
the BookE hypervisor mode. So with my limited BookE testing
ability I prefer sticking to a mechanical replacement that matches
the existing code. It can be fixed later if necessary.

> >      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;
> 
> And apparently hv vs. sv privilege of tlbilx depends on the EPCR
> register.  Again, may not be relevant for 2.06.

Well, here too, I basically preserve existing BookE TCG behaviour,
whether it's correct or not. That can be fixed separately if somebody
cares about BookE HV mode.

> >      t0 = tcg_temp_new();
> >      gen_addr_reg_index(ctx, t0);
> >  
> > @@ -6672,7 +6574,7 @@ static void gen_tlbilx_booke206(DisasContext
> *ctx)
> >      }
> >  
> >      tcg_temp_free(t0);
> > -#endif
> > +#endif /* defined(CONFIG_USER_ONLY) */
> >  }
>
David Gibson Nov. 24, 2015, 2:22 a.m. UTC | #4 
On Tue, Nov 24, 2015 at 11:44:36AM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote:
> > 
> > So, I'm not 100% following the logic below, but it looks like the
> > existing code used SPR_NOACCESS to mark things which generated a
> > privilege exception compared to NULL for things which generated an
> > invalid instruction exception.  Using that encoding, can you simplify
> > the logic here?  Alternatively can you use the logic here to avoid
> > the SPR_NOACESS encoding?
> 
> Well, so the SPR_NOACCESS has to do with how you react to a known SPR
> who has explicit access permissions. The logic below is described in
> the ISA for an unknown SPR number.
> 
> I don't know whether the access permission of "known" SPRs always
> honor the 0x10 bit trick, and changing that in qemu would be a
> fairly large patch. So I'd rather stick to the logic here for
> "unknown" SPRs which matches the ISA definition.
> 
> I'll update the patch though for arch 2.07 as it defines a few
> reserved SPRs as no-ops.

Ok, that makes sense.

> However:
> 
> > > -        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);
> > > +            }
> > > +        }
> > > +#if !defined(CONFIG_USER_ONLY)
> > > +        /* HV priv */
> > > +        if (ctx->spr_cb[sprn].hea_read) {
> > > +            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> > > +        }
> 
> That latest bit is bogus.
> 
> > If you're in PR mode, and it's an SPR with an hea_read function and
> > has the 0x10 bit set, won't this call gen_priv_exception twice?
> 
> Yes, I've removed it. It should be handled by the SPR_NOACCESS.
> 
> > I also see no path here which will call gen_inval_exception(), is
> > that
> > right?  If you're in HV mode and it's a truly invalid SPRN, isn't
> > that
> > what you'd want?
> 
> No, the ISA says it's a nop.

Huh, ok.

Some comments referencing the ISA might be useful here.
David Gibson Nov. 24, 2015, 2:22 a.m. UTC | #5 
On Tue, Nov 24, 2015 at 11:51:44AM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote:
> > snip]
> > >  /* 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 || !ctx->hv)) {
> > > -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> > > -        return;
> > > -    }
> > > +    CHK_SV;
> > 
> > You have CHK_SV here, but the original code checks for HV, as does
> > your new code for tlbia and tlbiel, is that right?
> 
> Yes. tlbiel is supervisor accessible (for weird reasons).
> 
> > [snip]
> > >  /* tlbsync */
> > >  static void gen_tlbsync(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;
> > > -    }
> > > +    GEN_PRIV;
> > > +#else    
> > > +    CHK_HV;
> > > +
> > 
> > Old code didn't check for HV, mode, but AFAICT it should have, so
> > this looks correct.
> 
> Yes, this is a hypervisor instruction.
> 
> > [snip]
> > > @@ -5941,18 +5921,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;
> > 
> > Is the same thing as tlbivax, or some ancient instruction?  AFAICT
> > the ISA says tlbivax is hypervisor privileged.
> 
> "tlbiva" is the 4xx variant, there is no hypervisor mode on these
> things.
> 
> > >      t0 = tcg_temp_new();
> > >      gen_addr_reg_index(ctx, t0);
> > >      gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> > >      tcg_temp_free(t0);
> > > -#endif
> > > +#endif /* defined(CONFIG_USER_ONLY) */
> > >  }
> > 
> > [snip]
> > >  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;
> > 
> > ISA says tlbivax is hypervisor privileged when the CPU has a
> > hypervisor mode, which I guess booke206 probably doesn't?
> 
> Right so here, the "problem" is that afaik, TCG doesn't implement
> the BookE hypervisor mode. So with my limited BookE testing
> ability I prefer sticking to a mechanical replacement that matches
> the existing code. It can be fixed later if necessary.

Fair enough.

> > >      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;
> > 
> > And apparently hv vs. sv privilege of tlbilx depends on the EPCR
> > register.  Again, may not be relevant for 2.06.
> 
> Well, here too, I basically preserve existing BookE TCG behaviour,
> whether it's correct or not. That can be fixed separately if somebody
> cares about BookE HV mode.
> 
> > >      t0 = tcg_temp_new();
> > >      gen_addr_reg_index(ctx, t0);
> > >  
> > > @@ -6672,7 +6574,7 @@ static void gen_tlbilx_booke206(DisasContext
> > *ctx)
> > >      }
> > >  
> > >      tcg_temp_free(t0);
> > > -#endif
> > > +#endif /* defined(CONFIG_USER_ONLY) */
> > >  }
> > 
>
diff mbox

Patch

diff --git a/linux-user/main.c b/linux-user/main.c
index 8acfe0f..beb621f 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1654,6 +1654,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 716b27b..80a70f4 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -127,6 +127,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 e8bbd59..3f657b1 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -321,7 +321,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 */
@@ -362,6 +374,20 @@  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)                                       \
@@ -2950,7 +2976,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;
     }
 
@@ -3073,7 +3099,7 @@  static void gen_std(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;
         }
 
@@ -4086,13 +4112,10 @@  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
     /* 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);
@@ -4103,13 +4126,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);
@@ -4119,13 +4139,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->hv)) {
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-        return;
-    }
+    CHK_HV;
     gen_helper_hrfid(cpu_env);
     gen_sync_exception(ctx);
 #endif
@@ -4288,15 +4305,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)
@@ -4340,7 +4350,7 @@  static inline void gen_op_mfspr(DisasContext *ctx)
                 printf("Trying to read 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 {
         /* Not defined */
@@ -4348,7 +4358,25 @@  static inline void gen_op_mfspr(DisasContext *ctx)
                  TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
         printf("Trying to read invalid spr %d (0x%03x) at "
                TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
-        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);
+            }
+        }
+#if !defined(CONFIG_USER_ONLY)
+        /* HV priv */
+        if (ctx->spr_cb[sprn].hea_read) {
+            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+        }
+#endif
     }
 }
 
@@ -4395,13 +4423,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();
@@ -4420,20 +4444,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));
@@ -4488,7 +4508,7 @@  static void gen_mtspr(DisasContext *ctx)
                      TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
             printf("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 {
         /* Not defined */
@@ -4496,7 +4516,25 @@  static void gen_mtspr(DisasContext *ctx)
                  TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
         printf("Trying to write invalid spr %d (0x%03x) at "
                TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
-        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);
+            }
+        }
+#if !defined(CONFIG_USER_ONLY)
+        /* HV priv */
+        if (ctx->spr_cb[sprn].hea_write) {
+            gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+        }
+#endif
     }
 }
 
@@ -4518,13 +4556,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);
@@ -4534,7 +4570,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 */
@@ -4655,72 +4691,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)
@@ -4730,115 +4758,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) */
 }
 #endif /* defined(TARGET_PPC64) */
 
@@ -4849,40 +4863,34 @@  static void gen_slbmfev(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 || !ctx->hv)) {
-        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)]);
@@ -4891,56 +4899,52 @@  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);
-#else
-    if (unlikely(ctx->pr)) {
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-        return;
-    }
+    GEN_PRIV;
+#else    
+    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)
+
 /* slbia */
 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: */
@@ -5639,14 +5643,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 */
@@ -5655,28 +5656,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 */
@@ -5685,28 +5680,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 */
@@ -5720,15 +5709,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 */
@@ -5740,15 +5726,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);
@@ -5757,38 +5741,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 */
@@ -5941,18 +5921,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_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
     tcg_temp_free(t0);
-#endif
+#endif /* defined(CONFIG_USER_ONLY) */
 }
 
 /* All 405 MAC instructions are translated here */
@@ -6174,38 +6152,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 */
@@ -6213,18 +6187,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 */
@@ -6232,18 +6203,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 */
@@ -6269,28 +6237,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);
@@ -6299,7 +6258,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 */
@@ -6314,60 +6273,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 */
@@ -6376,32 +6315,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 */
@@ -6410,12 +6343,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,
@@ -6429,20 +6359,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);
@@ -6454,19 +6382,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)],
@@ -6480,7 +6406,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 */
@@ -6489,12 +6415,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:
@@ -6510,20 +6434,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);
@@ -6535,19 +6457,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:
@@ -6563,7 +6482,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 */
@@ -6571,30 +6490,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)]);
@@ -6605,54 +6517,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);
 
@@ -6672,7 +6574,7 @@  static void gen_tlbilx_booke206(DisasContext *ctx)
     }
 
     tcg_temp_free(t0);
-#endif
+#endif /* defined(CONFIG_USER_ONLY) */
 }
 
 
@@ -6680,13 +6582,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));
@@ -6696,19 +6596,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 */
@@ -6716,7 +6613,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 */
@@ -6756,29 +6653,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                         ***/
@@ -9780,7 +9669,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
@@ -9788,10 +9677,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;                                                \