Re: [Qemu-devel] [PATCH 1/9] target-mips: Add CP0_Ebase.WG (write gate) support

[Yongbok Kim]

On 06/09/2016 12:03, James Hogan wrote:
> Add support for the CP0_EBase.WG bit, which allows upper bits to be
> written (bits 31:30 on MIPS32, or bits 63:30 on MIPS64), along with the
> CP0_Config5.CV bit to control whether the exception vector for Cache
> Error exceptions is forced into KSeg1.
> 
> This is necessary on MIPS32 to support Segmentation Control and Enhanced
> Virtual Addressing (EVA) extensions (where KSeg1 addresses may not
> represent an unmapped uncached segment).
> 
> It is also useful on MIPS64 to allow the exception base to reside in
> XKPhys, and possibly out of range of KSEG0 and KSEG1.
> 
> Signed-off-by: James Hogan <address@hidden>
> Cc: Leon Alrae <address@hidden>
> Cc: Aurelien Jarno <address@hidden>
> ---
>  target-mips/cpu.h            |  4 +++-
>  target-mips/helper.c         | 13 +++++++------
>  target-mips/machine.c        |  6 +++---
>  target-mips/op_helper.c      | 13 +++++++++++--
>  target-mips/translate.c      |  8 +++++---
>  target-mips/translate_init.c |  1 +
>  6 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 5182dc74ffa3..6ea2bf14c817 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -399,7 +399,9 @@ struct CPUMIPSState {
>  #define CP0Ca_EC    2
>      target_ulong CP0_EPC;
>      int32_t CP0_PRid;
> -    int32_t CP0_EBase;
> +    target_ulong CP0_EBase;
> +    target_ulong CP0_EBase_rw_bitmask;
> +#define CP0EBase_WG 11
>      target_ulong CP0_CMGCRBase;
>      int32_t CP0_Config0;
>  #define CP0C0_M    31
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index c864b15b97a8..29ebf391cb94 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -821,11 +821,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
>          goto set_EPC;
>      case EXCP_CACHE:
>          cause = 30;
> -        if (env->CP0_Status & (1 << CP0St_BEV)) {
> -            offset = 0x100;
> -        } else {
> -            offset = 0x20000100;
> -        }
> +        offset = 0x100;
>   set_EPC:
>          if (!(env->CP0_Status & (1 << CP0St_EXL))) {
>              env->CP0_EPC = exception_resume_pc(env);
> @@ -851,9 +847,14 @@ void mips_cpu_do_interrupt(CPUState *cs)
>          env->hflags &= ~MIPS_HFLAG_BMASK;
>          if (env->CP0_Status & (1 << CP0St_BEV)) {
>              env->active_tc.PC = env->exception_base + 0x200;
> +        } else if (cause == 30 && !(env->CP0_Config5 & (1 << CP0C5_CV))) {

&& !Config3SC? per MIPS32PRA 4.10.1.6.
It is not able to check Config3SC yet as it has not been defined but it can
be added later after the patch #7

> +            /* Force KSeg1 for cache errors */
> +            env->active_tc.PC = (int32_t)KSEG1_BASE |
> +                                (env->CP0_EBase & 0x1FFFF000);
>          } else {
> -            env->active_tc.PC = (int32_t)(env->CP0_EBase & ~0x3ff);
> +            env->active_tc.PC = env->CP0_EBase & ~0xfff;
>          }
> +
>          env->active_tc.PC += offset;
>          set_hflags_for_handler(env);
>          env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << 
> CP0Ca_EC);
> diff --git a/target-mips/machine.c b/target-mips/machine.c
> index a27f2f156da9..e795fecaa0d6 100644
> --- a/target-mips/machine.c
> +++ b/target-mips/machine.c
> @@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = {
>  
>  const VMStateDescription vmstate_mips_cpu = {
>      .name = "cpu",
> -    .version_id = 8,
> -    .minimum_version_id = 8,
> +    .version_id = 9,
> +    .minimum_version_id = 9,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
>          /* Active TC */
> @@ -267,7 +267,7 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_INT32(env.CP0_Cause, MIPSCPU),
>          VMSTATE_UINTTL(env.CP0_EPC, MIPSCPU),
>          VMSTATE_INT32(env.CP0_PRid, MIPSCPU),
> -        VMSTATE_INT32(env.CP0_EBase, MIPSCPU),
> +        VMSTATE_UINTTL(env.CP0_EBase, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config0, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config1, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config2, MIPSCPU),
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index ea2f2abe198c..71ad16e41dd4 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -1526,14 +1526,23 @@ target_ulong helper_mftc0_ebase(CPUMIPSState *env)
>  
>  void helper_mtc0_ebase(CPUMIPSState *env, target_ulong arg1)
>  {
> -    env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF000) | (arg1 & 0x3FFFF000);
> +    if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {

I guess it has to check the WG bit of EBase register rather than the new WG
bit value. "To ensure backward compatibility with MIPS32, the write-gate
bit must be set before the upper bits can be changed."

CP0_EBase_rw_bitmask is now added but it is not really used other than WG
bit. Perhaps rename it into something like CP0_EBaseWG_rw_bitmask would be
better to avoid any confusion. Otherwise assign those hard-coded mask into
the bitmask variable in the cpu_state_reset().

> +        env->CP0_EBase = (env->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
> +    } else {
> +        env->CP0_EBase = (env->CP0_EBase & ~0x3FFFF800) | (arg1 & 
> 0x3FFFF800);
> +    }
>  }
>  
>  void helper_mttc0_ebase(CPUMIPSState *env, target_ulong arg1)
>  {
>      int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
>      CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
> -    other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF000) | (arg1 & 
> 0x3FFFF000);
> +    if (arg1 & (1 << CP0EBase_WG) & env->CP0_EBase_rw_bitmask) {

Same here.

> +        other->CP0_EBase = (other->CP0_EBase & 0x7ff) | (arg1 & ~0x7ff);
> +    } else {
> +        other->CP0_EBase = (other->CP0_EBase & ~0x3FFFF800) |
> +                           (arg1 & 0x3FFFF800);
> +    }
>  }
>  
>  target_ulong helper_mftc0_configx(CPUMIPSState *env, target_ulong idx)
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index bab52cb25498..e224c2f09af4 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -5316,7 +5316,8 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int 
> reg, int sel)
>              break;
>          case 1:
>              check_insn(ctx, ISA_MIPS32R2);
> -            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
> +            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
> +            tcg_gen_ext32s_tl(arg, arg);
>              rn = "EBase";
>              break;
>          case 3:
> @@ -6630,7 +6631,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int 
> reg, int sel)
>              break;
>          case 1:
>              check_insn(ctx, ISA_MIPS32R2);
> -            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_EBase));
> +            tcg_gen_ld_tl(arg, cpu_env, offsetof(CPUMIPSState, CP0_EBase));
>              rn = "EBase";
>              break;
>          case 3:
> @@ -20247,6 +20248,7 @@ void cpu_state_reset(CPUMIPSState *env)
>      env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
>      env->CP0_PageGrain_rw_bitmask = env->cpu_model->CP0_PageGrain_rw_bitmask;
>      env->CP0_PageGrain = env->cpu_model->CP0_PageGrain;
> +    env->CP0_EBase_rw_bitmask = env->cpu_model->CP0_EBase_rw_bitmask;
>      env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
>      env->active_fpu.fcr31_rw_bitmask = env->cpu_model->CP1_fcr31_rw_bitmask;
>      env->active_fpu.fcr31 = env->cpu_model->CP1_fcr31;
> @@ -20297,7 +20299,7 @@ void cpu_state_reset(CPUMIPSState *env)
>      if (kvm_enabled()) {
>          env->CP0_EBase |= 0x40000000;
>      } else {
> -        env->CP0_EBase |= 0x80000000;
> +        env->CP0_EBase |= (int32_t)0x80000000;
>      }
>      if (env->CP0_Config3 & (1 << CP0C3_CMGCR)) {
>          env->CP0_CMGCRBase = 0x1fbf8000 >> 4;
> diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
> index 39ed5c4c1b12..f327e6e7de6c 100644
> --- a/target-mips/translate_init.c
> +++ b/target-mips/translate_init.c
> @@ -101,6 +101,7 @@ struct mips_def_t {
>      int32_t CP0_SRSConf4;
>      int32_t CP0_PageGrain_rw_bitmask;
>      int32_t CP0_PageGrain;
> +    target_ulong CP0_EBase_rw_bitmask;
>      int insn_flags;
>      enum mips_mmu_types mmu_type;
>  };
> 


Regards,
Yongbok