Re: [Qemu-devel] [PATCH 24/32] ppc: Make alignment exceptions suck less

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]

On Wed, Jul 27, 2016 at 08:21:18AM +1000, Benjamin Herrenschmidt wrote:
> The current alignment exception generation tries to load the opcode
> to put in DSISR from a context where a cpu_ldl_code() is really not
> a good idea. It might fault and longjmp out and that's not something
> we want happening here.
> 
> Instead, pass the releavant opcode bits via the error_code.
> 
> There are a couple of cases of alignment interrupts that won't set
> anything, the ones coming from access to direct store segments, but
> that doesn't happen in practice, nobody used direct store segments
> and they are gone from newer chips.

Do I understand correctly that this isn't actually new?  This was
already wrong for direct store segments, you've just noted it?

> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  target-ppc/excp_helper.c | 9 +++++----
>  target-ppc/translate.c   | 2 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index c31bbad..9a26578 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -260,11 +260,12 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          }
>          break;
>      case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
> -        /* XXX: this is false */
>          /* Get rS/rD and rA from faulting opcode */
> -        /* Broken for LE mode */
> -        env->spr[SPR_DSISR] |= (cpu_ldl_code(env, env->nip)
> -                                & 0x03FF0000) >> 16;
> +        /* Note: the opcode fields will not be set properly for a direct
> +         * store load/store, but nobody cares as nobody actually uses
> +         * direct store segments.
> +         */
> +        env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
>          break;
>      case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
>          switch (env->error_code & ~0xF) {
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index ddfec33..9af3f5f 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -2202,7 +2202,7 @@ static inline void gen_check_align(DisasContext *ctx, TCGv EA, int mask)
>      tcg_gen_andi_tl(t0, EA, mask);
>      tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);
>      t1 = tcg_const_i32(POWERPC_EXCP_ALIGN);
> -    t2 = tcg_const_i32(0);
> +    t2 = tcg_const_i32(ctx->opcode & 0x03FF0000);
>      gen_update_nip(ctx, ctx->nip - 4);
>      gen_helper_raise_exception_err(cpu_env, t1, t2);
>      tcg_temp_free_i32(t1);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]