All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Alrae <leon.alrae@imgtec.com>
To: Yongbok Kim <yongbok.kim@imgtec.com>, qemu-devel@nongnu.org
Cc: aurelien@aurel32.net
Subject: Re: [Qemu-devel] [PATCH] target-mips: add MAAR, MAARI register
Date: Wed, 23 Mar 2016 13:27:55 +0000	[thread overview]
Message-ID: <56F299DB.3030306@imgtec.com> (raw)
In-Reply-To: <1456323631-6732-1-git-send-email-yongbok.kim@imgtec.com>

On 24/02/16 14:20, Yongbok Kim wrote:
> The MAAR register is a read/write register included in Release 5
> of the architecture that defines the accessibility attributes of
> physical address regions. In particular, MAAR defines whether an
> instruction fetch or data load can speculatively access a memory
> region within the physical address bounds specified by MAAR.
> 
> As QEMU doesn't do speculative access, hence this patch only
> provides ability to access the registers.
> 
> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
> ---
>  target-mips/cpu.h       |    4 +++
>  target-mips/helper.h    |    6 +++++
>  target-mips/machine.c   |    3 ++
>  target-mips/op_helper.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++
>  target-mips/translate.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 125 insertions(+), 0 deletions(-)

Is there any reason for not enabling this feature anywhere? I remember
that P5600 supports MAAR.

> 
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index bd23c2a..39e2129 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -162,6 +162,7 @@ typedef struct mips_def_t mips_def_t;
>  #define MIPS_FPU_MAX 1
>  #define MIPS_DSP_ACC 4
>  #define MIPS_KSCRATCH_NUM 6
> +#define MIPS_MAAR_MAX 16 /* Must be an even number. */
>  
>  typedef struct TCState TCState;
>  struct TCState {
> @@ -474,10 +475,13 @@ struct CPUMIPSState {
>  #define CP0C5_SBRI       6
>  #define CP0C5_MVH        5
>  #define CP0C5_LLB        4
> +#define CP0C5_MRP        3
>  #define CP0C5_UFR        2
>  #define CP0C5_NFExists   0
>      int32_t CP0_Config6;
>      int32_t CP0_Config7;
> +    uint64_t CP0_MAAR[MIPS_MAAR_MAX];
> +    int32_t CP0_MAARI;
>      /* XXX: Maybe make LLAddr per-TC? */
>      uint64_t lladdr;
>      target_ulong llval;
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index 95b9149..8bd39c2 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -77,6 +77,8 @@ DEF_HELPER_1(mftc0_epc, tl, env)
>  DEF_HELPER_1(mftc0_ebase, tl, env)
>  DEF_HELPER_2(mftc0_configx, tl, env, tl)
>  DEF_HELPER_1(mfc0_lladdr, tl, env)
> +DEF_HELPER_1(mfc0_maar, tl, env)
> +DEF_HELPER_1(mfhc0_maar, tl, env)
>  DEF_HELPER_2(mfc0_watchlo, tl, env, i32)
>  DEF_HELPER_2(mfc0_watchhi, tl, env, i32)
>  DEF_HELPER_1(mfc0_debug, tl, env)
> @@ -88,6 +90,7 @@ DEF_HELPER_1(dmfc0_tccontext, tl, env)
>  DEF_HELPER_1(dmfc0_tcschedule, tl, env)
>  DEF_HELPER_1(dmfc0_tcschefback, tl, env)
>  DEF_HELPER_1(dmfc0_lladdr, tl, env)
> +DEF_HELPER_1(dmfc0_maar, tl, env)
>  DEF_HELPER_2(dmfc0_watchlo, tl, env, i32)
>  #endif /* TARGET_MIPS64 */
>  
> @@ -144,6 +147,9 @@ DEF_HELPER_2(mtc0_config3, void, env, tl)
>  DEF_HELPER_2(mtc0_config4, void, env, tl)
>  DEF_HELPER_2(mtc0_config5, void, env, tl)
>  DEF_HELPER_2(mtc0_lladdr, void, env, tl)
> +DEF_HELPER_2(mtc0_maar, void, env, tl)
> +DEF_HELPER_2(mthc0_maar, void, env, tl)
> +DEF_HELPER_2(mtc0_maari, void, env, tl)
>  DEF_HELPER_3(mtc0_watchlo, void, env, tl, i32)
>  DEF_HELPER_3(mtc0_watchhi, void, env, tl, i32)
>  DEF_HELPER_2(mtc0_xcontext, void, env, tl)
> diff --git a/target-mips/machine.c b/target-mips/machine.c
> index 737f3c2..667bde4 100644
> --- a/target-mips/machine.c
> +++ b/target-mips/machine.c
> @@ -272,6 +272,8 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_INT32(env.CP0_Config3, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config6, MIPSCPU),
>          VMSTATE_INT32(env.CP0_Config7, MIPSCPU),
> +        VMSTATE_UINT64_ARRAY(env.CP0_MAAR, MIPSCPU, MIPS_MAAR_MAX),
> +        VMSTATE_INT32(env.CP0_MAARI, MIPSCPU),

You need to bump version_id.

>          VMSTATE_UINT64(env.lladdr, MIPSCPU),
>          VMSTATE_UINTTL_ARRAY(env.CP0_WatchLo, MIPSCPU, 8),
>          VMSTATE_INT32_ARRAY(env.CP0_WatchHi, MIPSCPU, 8),
> @@ -297,3 +299,4 @@ const VMStateDescription vmstate_mips_cpu = {
>          VMSTATE_END_OF_LIST()
>      },
>  };
> +

New blank line at EOF.

> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 684ec92..13aac92 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -881,6 +881,22 @@ target_ulong helper_mfc0_lladdr(CPUMIPSState *env)
>      return (int32_t)(env->lladdr >> env->CP0_LLAddr_shift);
>  }
>  
> +target_ulong helper_mfc0_maar(CPUMIPSState *env)
> +{
> +    if (env->CP0_MAARI < MIPS_MAAR_MAX) {

I don't think this test is needed. This is already checked when guest
tries to change the CP0_MAARI value

> +        return (int32_t) env->CP0_MAAR[env->CP0_MAARI];
> +    }
> +    return 0;
> +}
> +
> +target_ulong helper_mfhc0_maar(CPUMIPSState *env)
> +{
> +    if (env->CP0_MAARI < MIPS_MAAR_MAX) {
> +        return env->CP0_MAAR[env->CP0_MAARI] >> 32;
> +    }
> +    return 0;
> +}
> +
>  target_ulong helper_mfc0_watchlo(CPUMIPSState *env, uint32_t sel)
>  {
>      return (int32_t)env->CP0_WatchLo[sel];
> @@ -947,6 +963,14 @@ target_ulong helper_dmfc0_lladdr(CPUMIPSState *env)
>      return env->lladdr >> env->CP0_LLAddr_shift;
>  }
>  
> +target_ulong helper_dmfc0_maar(CPUMIPSState *env)
> +{
> +    if (env->CP0_MAARI < MIPS_MAAR_MAX) {
> +        return env->CP0_MAAR[env->CP0_MAARI];
> +    }
> +    return 0;
> +}
> +
>  target_ulong helper_dmfc0_watchlo(CPUMIPSState *env, uint32_t sel)
>  {
>      return env->CP0_WatchLo[sel];
> @@ -1570,6 +1594,38 @@ void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1)
>      env->lladdr = (env->lladdr & ~mask) | (arg1 & mask);
>  }
>  
> +void helper_mtc0_maar(CPUMIPSState *env, target_ulong arg1)
> +{
> +    uint64_t mask = ((env->PAMask >> 4) & ~0xFFFull) | 0x3;
> +
> +    if (env->CP0_MAARI < MIPS_MAAR_MAX) {
> +        env->CP0_MAAR[env->CP0_MAARI] = arg1 & mask;
> +    }
> +}
> +
> +void helper_mthc0_maar(CPUMIPSState *env, target_ulong arg1)
> +{
> +    if (env->CP0_MAARI < MIPS_MAAR_MAX) {
> +        env->CP0_MAAR[env->CP0_MAARI] = ((uint64_t) arg1 << 32) |
> +                (env->CP0_MAAR[env->CP0_MAARI] & 0x00000000ffffffffULL);
> +    }
> +}
> +
> +void helper_mtc0_maari(CPUMIPSState *env, target_ulong arg1)
> +{
> +    int index = arg1 & 0x3f;
> +    if (index == 0x3f) {
> +        /* Software may write all ones to INDEX to determine the
> +           maximum value supported. */
> +        env->CP0_MAARI = MIPS_MAAR_MAX - 1;
> +    } else if (index < MIPS_MAAR_MAX) {
> +        env->CP0_MAARI = index;
> +    }
> +    /* Other than the all ones, if the
> +       value written is not supported, then INDEX is unchanged
> +       from its previous value. */
> +}
> +
>  void helper_mtc0_watchlo(CPUMIPSState *env, target_ulong arg1, uint32_t sel)
>  {
>      /* Watch exceptions for instructions, data loads, data stores
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 658926d..70990d4 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -1429,6 +1429,7 @@ typedef struct DisasContext {
>      bool mvh;
>      int CP0_LLAddr_shift;
>      bool ps;
> +    bool mrp;
>  } DisasContext;
>  
>  enum {
> @@ -4807,6 +4808,13 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>                               ctx->CP0_LLAddr_shift);
>              rn = "LLAddr";
>              break;
> +        case 1:
> +            if (!ctx->mrp) {
> +                goto mfhc0_read_zero;
> +            }

We should use CP0_CHECK just like in gen_mfc0 etc.

> +            gen_helper_mfhc0_maar(arg, cpu_env);
> +            rn = "MAAR";
> +            break;
>          default:
>              goto mfhc0_read_zero;
>          }
> @@ -4878,6 +4886,13 @@ static void gen_mthc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>                 treating MTHC0 to LLAddr as NOP. */
>              rn = "LLAddr";
>              break;
> +        case 1:
> +            if (!ctx->mrp) {
> +                goto mthc0_nop;
> +            }

Same.

Thanks,
Leon

> +            gen_helper_mthc0_maar(cpu_env, arg);
> +            rn = "MAAR";
> +            break;
>          default:
>              goto mthc0_nop;
>          }
> @@ -5334,6 +5349,16 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_helper_mfc0_lladdr(arg, cpu_env);
>              rn = "LLAddr";
>              break;
> +        case 1:
> +            CP0_CHECK(ctx->mrp);
> +            gen_helper_mfc0_maar(arg, cpu_env);
> +            rn = "MAAR";
> +            break;
> +        case 2:
> +            CP0_CHECK(ctx->mrp);
> +            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_MAARI));
> +            rn = "MAARI";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -5963,6 +5988,16 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_helper_mtc0_lladdr(cpu_env, arg);
>              rn = "LLAddr";
>              break;
> +        case 1:
> +            CP0_CHECK(ctx->mrp);
> +            gen_helper_mtc0_maar(cpu_env, arg);
> +            rn = "MAAR";
> +            break;
> +        case 2:
> +            CP0_CHECK(ctx->mrp);
> +            gen_helper_mtc0_maari(cpu_env, arg);
> +            rn = "MAARI";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -6588,6 +6623,16 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_helper_dmfc0_lladdr(arg, cpu_env);
>              rn = "LLAddr";
>              break;
> +        case 1:
> +            CP0_CHECK(ctx->mrp);
> +            gen_helper_dmfc0_maar(arg, cpu_env);
> +            rn = "MAAR";
> +            break;
> +        case 2:
> +            CP0_CHECK(ctx->mrp);
> +            gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_MAARI));
> +            rn = "MAARI";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -7209,6 +7254,16 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
>              gen_helper_mtc0_lladdr(cpu_env, arg);
>              rn = "LLAddr";
>              break;
> +        case 1:
> +            CP0_CHECK(ctx->mrp);
> +            gen_helper_mtc0_maar(cpu_env, arg);
> +            rn = "MAAR";
> +            break;
> +        case 2:
> +            CP0_CHECK(ctx->mrp);
> +            gen_helper_mtc0_maari(cpu_env, arg);
> +            rn = "MAARI";
> +            break;
>          default:
>              goto cp0_unimplemented;
>          }
> @@ -19611,6 +19666,7 @@ void gen_intermediate_code(CPUMIPSState *env, struct TranslationBlock *tb)
>      ctx.ulri = (env->CP0_Config3 >> CP0C3_ULRI) & 1;
>      ctx.ps = ((env->active_fpu.fcr0 >> FCR0_PS) & 1) ||
>               (env->insn_flags & (INSN_LOONGSON2E | INSN_LOONGSON2F));
> +    ctx.mrp = (env->CP0_Config5 >> CP0C5_MRP) & 1;
>      restore_cpu_state(env, &ctx);
>  #ifdef CONFIG_USER_ONLY
>          ctx.mem_idx = MIPS_HFLAG_UM;
> 

      reply	other threads:[~2016-03-23 13:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-24 14:20 [Qemu-devel] [PATCH] target-mips: add MAAR, MAARI register Yongbok Kim
2016-03-23 13:27 ` Leon Alrae [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56F299DB.3030306@imgtec.com \
    --to=leon.alrae@imgtec.com \
    --cc=aurelien@aurel32.net \
    --cc=qemu-devel@nongnu.org \
    --cc=yongbok.kim@imgtec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.