Re: [Qemu-devel] [PATCH v4 23/29] target-ppc: Add POWER8's TM SPRs

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 06/04/2014 03:58 AM, Tom Musta wrote:
> On 6/3/2014 4:27 AM, Alexey Kardashevskiy wrote:
>> This adds TM (Transactional Memory) SPRs.
>>
>> This adds generic spr_read_prev_upper32()/spr_write_prev_upper32() to
>> handle upper half SPRs such as TEXASRU which is upper half of TEXASR.
>> Since this is not the only register like that and their numbers go
>> consequently, it makes sense to generalize the helpers.
>>
>> This adds a gen_msr_facility_check() helper which purpose is to generate
>> the Facility Unavailable exception if the facility is disabled.
>> It is a copy of gen_fscr_facility_check() but it checks for enabled
>> facility in MSR rather than FSCR/HFSCR. It still sets the interrupt cause
>> in FSCR/HFSCR (whichever is passed to the helper).
>>
>> This adds spr_read_tm/spr_write_tm/spr_read_tm_upper32/spr_write_tm_upper32
>> which are used for TM SPRs.
>>
>> This adds TM-relates MSR bits definitions. This enables TM in POWER8 CPU class'
>> msr_mask.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v4:
>> * enable MSR_TM in msr_mask
>> * tested compile with --enable-tcg-debug and ppc-softmmu
>> * re-implemented spr_(read|write)_prev_upper32 using TCGv types (not i32 or i64)
>> ---
>>  target-ppc/cpu.h            | 10 ++++++
>>  target-ppc/helper.h         |  1 +
>>  target-ppc/misc_helper.c    | 12 +++++++
>>  target-ppc/translate_init.c | 85 +++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 108 insertions(+)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index cf1ccad..8ea471c 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -427,6 +427,9 @@ struct ppc_slb_t {
>>  #define MSR_TAG  62 /* Tag-active mode (POWERx ?)                            */
>>  #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on 630                  */
>>  #define MSR_SHV  60 /* hypervisor state                               hflags */
>> +#define MSR_TS0  34 /* Transactional state, 2 bits (Book3s)                  */
>> +#define MSR_TS1  33
>> +#define MSR_TM   32 /* Transactional Memory Available (Book3s)               */
>>  #define MSR_CM   31 /* Computation mode for BookE                     hflags */
>>  #define MSR_ICM  30 /* Interrupt computation mode for BookE                  */
>>  #define MSR_THV  29 /* hypervisor state for 32 bits PowerPC           hflags */
>> @@ -503,6 +506,9 @@ struct ppc_slb_t {
>>  #define msr_pmm  ((env->msr >> MSR_PMM)  & 1)
>>  #define msr_ri   ((env->msr >> MSR_RI)   & 1)
>>  #define msr_le   ((env->msr >> MSR_LE)   & 1)
>> +#define msr_ts   ((env->msr >> MSR_TS1)  & 3)
>> +#define msr_tm   ((env->msr >> MSR_TM)   & 1)
>> +
>>  /* Hypervisor bit is more specific */
>>  #if defined(TARGET_PPC64)
>>  #define MSR_HVB (1ULL << MSR_SHV)
>> @@ -1275,6 +1281,10 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>  #define SPR_MPC_EIE           (0x050)
>>  #define SPR_MPC_EID           (0x051)
>>  #define SPR_MPC_NRI           (0x052)
>> +#define SPR_TFHAR             (0x080)
>> +#define SPR_TFIAR             (0x081)
>> +#define SPR_TEXASR            (0x082)
>> +#define SPR_TEXASRU           (0x083)
>>  #define SPR_UCTRL             (0x088)
>>  #define SPR_MPC_CMPA          (0x090)
>>  #define SPR_MPC_CMPB          (0x091)
>> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
>> index 9041ba0..4c211b8 100644
>> --- a/target-ppc/helper.h
>> +++ b/target-ppc/helper.h
>> @@ -580,6 +580,7 @@ DEF_HELPER_3(store_dcr, void, env, tl, tl)
>>  DEF_HELPER_2(load_dump_spr, void, env, i32)
>>  DEF_HELPER_2(store_dump_spr, void, env, i32)
>>  DEF_HELPER_4(fscr_facility_check, void, env, i32, i32, i32)
>> +DEF_HELPER_4(msr_facility_check, void, env, i32, i32, i32)
>>  DEF_HELPER_1(load_tbl, tl, env)
>>  DEF_HELPER_1(load_tbu, tl, env)
>>  DEF_HELPER_1(load_atbl, tl, env)
>> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
>> index 9ef18c4..409deb6 100644
>> --- a/target-ppc/misc_helper.c
>> +++ b/target-ppc/misc_helper.c
>> @@ -61,6 +61,18 @@ void helper_fscr_facility_check(CPUPPCState *env, uint32_t bit,
>>  #endif
>>  }
>>  
>> +void helper_msr_facility_check(CPUPPCState *env, uint32_t bit,
>> +                               uint32_t sprn, uint32_t cause)
>> +{
>> +#ifdef TARGET_PPC64
>> +    if (env->msr & (1ULL << bit)) {
>> +        /* Facility is enabled, continue */
>> +        return;
>> +    }
>> +    raise_fu_exception(env, bit, sprn, cause);
>> +#endif
>> +}
>> +
>>  #if !defined(CONFIG_USER_ONLY)
>>  
>>  void helper_store_sdr1(CPUPPCState *env, target_ulong val)
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 6bb0788..bb4201c 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -7297,6 +7297,45 @@ static void gen_fscr_facility_check(void *opaque, int facility_sprn, int bit,
>>      tcg_temp_free_i32(t1);
>>  }
>>  
>> +static void gen_msr_facility_check(void *opaque, int facility_sprn, int bit,
>> +                                   int sprn, int cause)
>> +{
>> +    TCGv_i32 t1 = tcg_const_i32(bit);
>> +    TCGv_i32 t2 = tcg_const_i32(sprn);
>> +    TCGv_i32 t3 = tcg_const_i32(cause);
>> +
>> +    gen_update_current_nip(opaque);
>> +    gen_helper_msr_facility_check(cpu_env, t1, t2, t3);
>> +
>> +    tcg_temp_free_i32(t3);
>> +    tcg_temp_free_i32(t2);
>> +    tcg_temp_free_i32(t1);
>> +}
>> +
>> +static void spr_read_prev_upper32(void *opaque, int gprn, int sprn)
>> +{
>> +    TCGv spr_up = tcg_temp_new();
>> +    TCGv spr = tcg_temp_new();
>> +
>> +    gen_load_spr(spr, sprn - 1);
>> +    tcg_gen_shri_tl(spr_up, spr, 32);
>> +    tcg_gen_ext32u_tl(cpu_gpr[gprn], spr_up);
>> +
>> +    tcg_temp_free(spr);
>> +    tcg_temp_free(spr_up);
>> +}
>> +
>> +static void spr_write_prev_upper32(void *opaque, int sprn, int gprn)
>> +{
>> +    TCGv spr = tcg_temp_new();
>> +
>> +    gen_load_spr(spr, sprn - 1);
>> +    tcg_gen_deposit_tl(spr, spr, cpu_gpr[gprn], 32, 32);
>> +    gen_store_spr(sprn - 1, spr);
>> +
>> +    tcg_temp_free(spr);
>> +}
>> +
>>  static int check_pow_970 (CPUPPCState *env)
>>  {
>>      if (env->spr[SPR_HID0] & 0x01C00000) {
>> @@ -7630,6 +7669,50 @@ static void gen_spr_power8_tce_address_control(CPUPPCState *env)
>>                   0x00000000);
>>  }
>>  
>> +static void spr_read_tm(void *opaque, int gprn, int sprn)
>> +{
>> +    gen_msr_facility_check(opaque, SPR_FSCR, MSR_TM, sprn, FSCR_IC_TM);
>> +    spr_read_generic(opaque, gprn, sprn);
>> +}
>> +
>> +static void spr_write_tm(void *opaque, int sprn, int gprn)
>> +{
>> +    gen_msr_facility_check(opaque, SPR_FSCR, MSR_TM, sprn, FSCR_IC_TM);
>> +    spr_write_generic(opaque, sprn, gprn);
>> +}
>> +
>> +static void spr_read_tm_upper32(void *opaque, int gprn, int sprn)
>> +{
>> +    gen_msr_facility_check(opaque, SPR_FSCR, MSR_TM, sprn, FSCR_IC_TM);
>> +    spr_read_prev_upper32(opaque, gprn, sprn);
>> +}
>> +
>> +static void spr_write_tm_upper32(void *opaque, int sprn, int gprn)
>> +{
>> +    gen_msr_facility_check(opaque, SPR_FSCR, MSR_TM, sprn, FSCR_IC_TM);
>> +    spr_write_prev_upper32(opaque, sprn, gprn);
>> +}
>> +
>> +static void gen_spr_power8_tm(CPUPPCState *env)
>> +{
>> +    spr_register_kvm(env, SPR_TFHAR, "TFHAR",
>> +                     &spr_read_tm, &spr_write_tm,
>> +                     &spr_read_tm, &spr_write_tm,
>> +                     KVM_REG_PPC_TFHAR, 0x00000000);
>> +    spr_register_kvm(env, SPR_TFIAR, "TFIAR",
>> +                     &spr_read_tm, &spr_write_tm,
>> +                     &spr_read_tm, &spr_write_tm,
>> +                     KVM_REG_PPC_TFIAR, 0x00000000);
>> +    spr_register_kvm(env, SPR_TEXASR, "TEXASR",
>> +                     &spr_read_tm, &spr_write_tm,
>> +                     &spr_read_tm, &spr_write_tm,
>> +                     KVM_REG_PPC_TEXASR, 0x00000000);
>> +    spr_register(env, SPR_TEXASRU, "TEXASRU",
>> +                 &spr_read_tm_upper32, &spr_write_tm_upper32,
>> +                 &spr_read_tm_upper32, &spr_write_tm_upper32,
>> +                 0x00000000);
>> +}
>> +
>>  static void gen_spr_power8_fscr(CPUPPCState *env)
>>  {
>>      spr_register_kvm(env, SPR_FSCR, "FSCR",
>> @@ -7685,6 +7768,7 @@ static void init_proc_book3s_64(CPUPPCState *env, int version)
>>          gen_spr_power8_fscr(env);
>>          gen_spr_power8_pmu_hypv(env);
>>          gen_spr_power8_pmu_user(env);
>> +        gen_spr_power8_tm(env);
>>      }
>>  #if !defined(CONFIG_USER_ONLY)
>>      switch (version) {
>> @@ -8056,6 +8140,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>                          PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>>                          PPC2_ISA205 | PPC2_ISA207S;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>> +                    (1ull << MSR_TM) |
>>                      (1ull << MSR_VR) |
>>                      (1ull << MSR_VSX) |
>>                      (1ull << MSR_EE) |
>>
> 
> There are user-mode impacts here as well .... although I think we are a long way off from doing anything with TM.
> 
> The typical pattern is to default MSR enable bits to 1 ... see translate_init.c/ppc_cpu_reset:



--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9459,19 +9459,19 @@ static void ppc_cpu_reset(CPUState *s)
 #endif
 #if defined(CONFIG_USER_ONLY)
     msr |= (target_ulong)1 << MSR_FP; /* Allow floating point usage */
     msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
     msr |= (target_ulong)1 << MSR_VSX; /* Allow VSX usage */
     msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
     msr |= (target_ulong)1 << MSR_PR;
+    msr |= (target_ulong)1 << MSR_TM; /* Transactional memory */
 #if !defined(TARGET_WORDS_BIGENDIAN)
     msr |= (target_ulong)1 << MSR_LE; /* Little-endian user mode */
 #endif
 #endif


So I'll do this and if MSR_TM is not in msr_mask (CPUs older than POWER8),
the guest won't see it and we are fine. Correct?



>   9490  /* CPUClass::reset() */
>   9491  static void ppc_cpu_reset(CPUState *s)
>   9492  {
>   9493      PowerPCCPU *cpu = POWERPC_CPU(s);
>   9494      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>   9495      CPUPPCState *env = &cpu->env;
>   9496      target_ulong msr;
>   9497      int i;
>   9498
>   9499      pcc->parent_reset(s);
>   9500
>   9501      msr = (target_ulong)0;
>   9502      if (0) {
>   9503          /* XXX: find a suitable condition to enable the hypervisor mode */
>   9504          msr |= (target_ulong)MSR_HVB;
>   9505      }
>   9506      msr |= (target_ulong)0 << MSR_AP; /* TO BE CHECKED */
>   9507      msr |= (target_ulong)0 << MSR_SA; /* TO BE CHECKED */
>   9508      msr |= (target_ulong)1 << MSR_EP;
>   9509  #if defined(DO_SINGLE_STEP) && 0
>   9510      /* Single step trace mode */
>   9511      msr |= (target_ulong)1 << MSR_SE;
>   9512      msr |= (target_ulong)1 << MSR_BE;
>   9513  #endif
>   9514  #if defined(CONFIG_USER_ONLY)
>   9515      msr |= (target_ulong)1 << MSR_FP; /* Allow floating point usage */
>   9516      msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
>   9517      msr |= (target_ulong)1 << MSR_VSX; /* Allow VSX usage */
>   9518      msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
>   9519      msr |= (target_ulong)1 << MSR_PR;
>   9520  #if !defined(TARGET_WORDS_BIGENDIAN)
> 


-- 
Alexey