Re: [Qemu-ppc] [PATCH 2/7] Add MSR VSX and Associated Exception

[Richard Henderson]

On 10/22/2013 04:06 AM, Anton Blanchard wrote:
> From: Tom Musta <address@hidden>
> 
> This patch adds support for the VSX bit of the PowerPC Machine
> State Register (MSR) as well as the corresponding VSX Unavailable
> exception.
> 
> The VSX bit is added to the defined bits masks of the Power7 and
> Power8 CPU models.
> 
> Signed-off-by: Tom Musta <address@hidden>
> Signed-off-by: Anton Blanchard <address@hidden>
> ---
> 
> Index: b/target-ppc/cpu.h
> ===================================================================
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -236,6 +236,8 @@ enum {
>      POWERPC_EXCP_NMEXTBR  = 91, /* Non maskable external breakpoint          
> */
>      POWERPC_EXCP_ITLBE    = 92, /* Instruction TLB error                     
> */
>      POWERPC_EXCP_DTLBE    = 93, /* Data TLB error                            
> */
> +    /* VSX Unavailable (Power ISA 2.06 and later)                            
> */
> +    POWERPC_EXCP_VSXU     = 94, /* VSX Unavailable                           
> */
>      /* EOL                                                                   
> */
>      POWERPC_EXCP_NB       = 96,
>      /* QEMU exceptions: used internally during code translation              
> */
> @@ -427,6 +429,7 @@ struct ppc_slb_t {
>  #define MSR_VR   25 /* altivec available                            x hflags 
> */
>  #define MSR_SPE  25 /* SPE enable for BookE                         x hflags 
> */
>  #define MSR_AP   23 /* Access privilege state on 602                  hflags 
> */
> +#define MSR_VSX  23 /* Vector Scalar Extension (ISA 2.06 and later) x hflags 
> */
>  #define MSR_SA   22 /* Supervisor access mode on 602                  hflags 
> */
>  #define MSR_KEY  19 /* key bit on 603e                                       
> */
>  #define MSR_POW  18 /* Power management                                      
> */
> @@ -467,6 +470,7 @@ struct ppc_slb_t {
>  #define msr_vr   ((env->msr >> MSR_VR)   & 1)
>  #define msr_spe  ((env->msr >> MSR_SPE)  & 1)
>  #define msr_ap   ((env->msr >> MSR_AP)   & 1)
> +#define msr_vsx  ((env->msr >> MSR_VSX)  & 1)
>  #define msr_sa   ((env->msr >> MSR_SA)   & 1)
>  #define msr_key  ((env->msr >> MSR_KEY)  & 1)
>  #define msr_pow  ((env->msr >> MSR_POW)  & 1)
> Index: b/target-ppc/excp_helper.c
> ===================================================================
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -390,6 +390,11 @@ static inline void powerpc_excp(PowerPCC
>              new_msr |= (target_ulong)MSR_HVB;
>          }
>          goto store_current;
> +    case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               
> */
> +        if (lpes1 == 0) {
> +            new_msr |= (target_ulong)MSR_HVB;
> +        }
> +        goto store_current;
>      case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    
> */
>          LOG_EXCP("PIT exception\n");
>          goto store_next;
> Index: b/target-ppc/translate.c
> ===================================================================
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -199,6 +199,7 @@ typedef struct DisasContext {
>  #endif
>      int fpu_enabled;
>      int altivec_enabled;
> +    int vsx_enabled;
>      int spe_enabled;
>      ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
>      int singlestep_enabled;
> @@ -9763,6 +9764,11 @@ static inline void gen_intermediate_code
>          ctx.altivec_enabled = msr_vr;
>      else
>          ctx.altivec_enabled = 0;
> +    if ((env->flags & POWERPC_FLAG_VSX) && msr_vsx) {
> +        ctx.vsx_enabled = msr_vsx;
> +    } else {
> +        ctx.vsx_enabled = 0;
> +    }

In order to get correct code generation when the msr changes,
one has to adjust the TB flags.  Which for target-ppc means
adjusting env->hflags.

See hreg_compute_hflags.

Ideally, target-ppc would initialize these ctx bits from tb->flags
instead of env->msr to make this obvious.  But so long as the two
are in sync it's not technically a bug.


r~