qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] target/ppc: Fix SPE unavailable exception triggering


From: David Gibson
Subject: Re: [PATCH v2] target/ppc: Fix SPE unavailable exception triggering
Date: Mon, 3 Aug 2020 16:24:29 +1000

On Mon, Jul 27, 2020 at 10:55:53AM -0700, Matthieu Bucchianeri wrote:
> When emulating certain floating point instructions or vector instructions on
> PowerPC machines, QEMU did not properly generate the SPE/Embedded Floating-
> Point Unavailable interrupt. See the buglink further below for references to
> the relevant NXP documentation.
> 
> This patch fixes the behavior of some evfs* instructions that were
> incorrectly emitting the interrupt.
> 
> More importantly, this patch fixes the behavior of several efd* and ev*
> instructions that were not generating the interrupt. Triggering the
> interrupt for these instructions fixes lazy FPU/vector context switching on
> some operating systems like Linux.
> 
> Without this patch, the result of some double-precision arithmetic could be
> corrupted due to the lack of proper saving and restoring of the upper
> 32-bit part of the general-purpose registers.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1888918
> Buglink: https://bugs.launchpad.net/qemu/+bug/1611394
> Signed-off-by: Matthieu Bucchianeri
> <matthieu.bucchianeri@leostella.com>

Applied to ppc-for-5.2.

> ---
> v2:
>   Split out fix for TCG leak in gen_evmwsmiaa().
> 
> Based-on: <20200727172114.31415-1-matthieu.bucchianeri@leostella.com>
> ([PATCH] target/ppc: Fix TCG leak with the evmwsmiaa instruction)
> 
> target/ppc/translate/spe-impl.inc.c | 97 +++++++++++++++++++----------
>  1 file changed, 64 insertions(+), 33 deletions(-)
> 
> diff --git a/target/ppc/translate/spe-impl.inc.c 
> b/target/ppc/translate/spe-impl.inc.c
> index 42a0d1cffb..2e6e799a25 100644
> --- a/target/ppc/translate/spe-impl.inc.c
> +++ b/target/ppc/translate/spe-impl.inc.c
> @@ -349,14 +349,24 @@ static inline void gen_evmergelohi(DisasContext *ctx)
>  }
>  static inline void gen_evsplati(DisasContext *ctx)
>  {
> -    uint64_t imm = ((int32_t)(rA(ctx->opcode) << 27)) >> 27;
> +    uint64_t imm;
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
> +    imm = ((int32_t)(rA(ctx->opcode) << 27)) >> 27;
> 
>      tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], imm);
>      tcg_gen_movi_tl(cpu_gprh[rD(ctx->opcode)], imm);
>  }
>  static inline void gen_evsplatfi(DisasContext *ctx)
>  {
> -    uint64_t imm = rA(ctx->opcode) << 27;
> +    uint64_t imm;
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
> +    imm = rA(ctx->opcode) << 27;
> 
>      tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], imm);
>      tcg_gen_movi_tl(cpu_gprh[rD(ctx->opcode)], imm);
> @@ -389,21 +399,37 @@ static inline void gen_evsel(DisasContext *ctx)
> 
>  static void gen_evsel0(DisasContext *ctx)
>  {
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
>      gen_evsel(ctx);
>  }
> 
>  static void gen_evsel1(DisasContext *ctx)
>  {
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
>      gen_evsel(ctx);
>  }
> 
>  static void gen_evsel2(DisasContext *ctx)
>  {
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
>      gen_evsel(ctx);
>  }
> 
>  static void gen_evsel3(DisasContext *ctx)
>  {
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
>      gen_evsel(ctx);
>  }
> 
> @@ -518,6 +544,11 @@ static inline void gen_evmwsmia(DisasContext *ctx)
>  {
>      TCGv_i64 tmp;
> 
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
> +
>      gen_evmwsmi(ctx);            /* rD := rA * rB */
> 
>      tmp = tcg_temp_new_i64();
> @@ -534,6 +565,11 @@ static inline void gen_evmwsmiaa(DisasContext *ctx)
>      TCGv_i64 acc;
>      TCGv_i64 tmp;
> 
> +    if (unlikely(!ctx->spe_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);
> +        return;
> +    }
> +
>      gen_evmwsmi(ctx);           /* rD := rA * rB */
> 
>      acc = tcg_temp_new_i64();
> @@ -892,8 +928,14 @@ static inline void gen_##name(DisasContext *ctx)         
>                      \
>  #define GEN_SPEFPUOP_CONV_32_64(name)                                        
>  \
>  static inline void gen_##name(DisasContext *ctx)                             
>  \
>  {                                                                            
>  \
> -    TCGv_i64 t0 = tcg_temp_new_i64();                                        
>  \
> -    TCGv_i32 t1 = tcg_temp_new_i32();                                        
>  \
> +    TCGv_i64 t0;                                                             
>  \
> +    TCGv_i32 t1;                                                             
>  \
> +    if (unlikely(!ctx->spe_enabled)) {                                       
>  \
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);                               
>  \
> +        return;                                                              
>  \
> +    }                                                                        
>  \
> +    t0 = tcg_temp_new_i64();                                                 
>  \
> +    t1 = tcg_temp_new_i32();                                                 
>  \
>      gen_load_gpr64(t0, rB(ctx->opcode));                                     
>  \
>      gen_helper_##name(t1, cpu_env, t0);                                      
>  \
>      tcg_gen_extu_i32_tl(cpu_gpr[rD(ctx->opcode)], t1);                       
>  \
> @@ -903,8 +945,14 @@ static inline void gen_##name(DisasContext *ctx)         
>                      \
>  #define GEN_SPEFPUOP_CONV_64_32(name)                                        
>  \
>  static inline void gen_##name(DisasContext *ctx)                             
>  \
>  {                                                                            
>  \
> -    TCGv_i64 t0 = tcg_temp_new_i64();                                        
>  \
> -    TCGv_i32 t1 = tcg_temp_new_i32();                                        
>  \
> +    TCGv_i64 t0;                                                             
>  \
> +    TCGv_i32 t1;                                                             
>  \
> +    if (unlikely(!ctx->spe_enabled)) {                                       
>  \
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);                               
>  \
> +        return;                                                              
>  \
> +    }                                                                        
>  \
> +    t0 = tcg_temp_new_i64();                                                 
>  \
> +    t1 = tcg_temp_new_i32();                                                 
>  \
>      tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                      
>  \
>      gen_helper_##name(t0, cpu_env, t1);                                      
>  \
>      gen_store_gpr64(rD(ctx->opcode), t0);                                    
>  \
> @@ -914,7 +962,12 @@ static inline void gen_##name(DisasContext *ctx)         
>                      \
>  #define GEN_SPEFPUOP_CONV_64_64(name)                                        
>  \
>  static inline void gen_##name(DisasContext *ctx)                             
>  \
>  {                                                                            
>  \
> -    TCGv_i64 t0 = tcg_temp_new_i64();                                        
>  \
> +    TCGv_i64 t0;                                                             
>  \
> +    if (unlikely(!ctx->spe_enabled)) {                                       
>  \
> +        gen_exception(ctx, POWERPC_EXCP_SPEU);                               
>  \
> +        return;                                                              
>  \
> +    }                                                                        
>  \
> +    t0 = tcg_temp_new_i64();                                                 
>  \
>      gen_load_gpr64(t0, rB(ctx->opcode));                                     
>  \
>      gen_helper_##name(t0, cpu_env, t0);                                      
>  \
>      gen_store_gpr64(rD(ctx->opcode), t0);                                    
>  \
> @@ -923,13 +976,8 @@ static inline void gen_##name(DisasContext *ctx)         
>                      \
>  #define GEN_SPEFPUOP_ARITH2_32_32(name)                                      
>  \
>  static inline void gen_##name(DisasContext *ctx)                             
>  \
>  {                                                                            
>  \
> -    TCGv_i32 t0, t1;                                                         
>  \
> -    if (unlikely(!ctx->spe_enabled)) {                                       
>  \
> -        gen_exception(ctx, POWERPC_EXCP_SPEU);                               
>  \
> -        return;                                                              
>  \
> -    }                                                                        
>  \
> -    t0 = tcg_temp_new_i32();                                                 
>  \
> -    t1 = tcg_temp_new_i32();                                                 
>  \
> +    TCGv_i32 t0 = tcg_temp_new_i32();                                        
>  \
> +    TCGv_i32 t1 = tcg_temp_new_i32();                                        
>  \
>      tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);                      
>  \
>      tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                      
>  \
>      gen_helper_##name(t0, cpu_env, t0, t1);                                  
>  \
> @@ -958,13 +1006,8 @@ static inline void gen_##name(DisasContext *ctx)        
>                       \
>  #define GEN_SPEFPUOP_COMP_32(name)                                           
>  \
>  static inline void gen_##name(DisasContext *ctx)                             
>  \
>  {                                                                            
>  \
> -    TCGv_i32 t0, t1;                                                         
>  \
> -    if (unlikely(!ctx->spe_enabled)) {                                       
>  \
> -        gen_exception(ctx, POWERPC_EXCP_SPEU);                               
>  \
> -        return;                                                              
>  \
> -    }                                                                        
>  \
> -    t0 = tcg_temp_new_i32();                                                 
>  \
> -    t1 = tcg_temp_new_i32();                                                 
>  \
> +    TCGv_i32 t0 = tcg_temp_new_i32();                                        
>  \
> +    TCGv_i32 t1 = tcg_temp_new_i32();                                        
>  \
>                                                                               
>  \
>      tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);                      
>  \
>      tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);                      
>  \
> @@ -1074,28 +1117,16 @@ GEN_SPEFPUOP_ARITH2_32_32(efsmul);
>  GEN_SPEFPUOP_ARITH2_32_32(efsdiv);
>  static inline void gen_efsabs(DisasContext *ctx)
>  {
> -    if (unlikely(!ctx->spe_enabled)) {
> -        gen_exception(ctx, POWERPC_EXCP_SPEU);
> -        return;
> -    }
>      tcg_gen_andi_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
>                      (target_long)~0x80000000LL);
>  }
>  static inline void gen_efsnabs(DisasContext *ctx)
>  {
> -    if (unlikely(!ctx->spe_enabled)) {
> -        gen_exception(ctx, POWERPC_EXCP_SPEU);
> -        return;
> -    }
>      tcg_gen_ori_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
>                     0x80000000);
>  }
>  static inline void gen_efsneg(DisasContext *ctx)
>  {
> -    if (unlikely(!ctx->spe_enabled)) {
> -        gen_exception(ctx, POWERPC_EXCP_SPEU);
> -        return;
> -    }
>      tcg_gen_xori_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
>                      0x80000000);
>  }

-- 
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: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]