Re: [Qemu-devel] [PATCH v4 12/16] target-or32: Add system instructions

[Blue Swirl]

On Mon, Jun 11, 2012 at 6:32 AM, Jia Liu <address@hidden> wrote:
> Add OpenRISC system instruction helpers and translation.
>
> Signed-off-by: Jia Liu <address@hidden>
> ---
>  target-openrisc/Makefile.objs |    3 +-
>  target-openrisc/helper.h      |    4 +
>  target-openrisc/sys_helper.c  |  235 
> +++++++++++++++++++++++++++++++++++++++++
>  target-openrisc/translate.c   |   18 ++++
>  4 files changed, 259 insertions(+), 1 deletion(-)
>  create mode 100644 target-openrisc/sys_helper.c
>
> diff --git a/target-openrisc/Makefile.objs b/target-openrisc/Makefile.objs
> index 0d72c33..9d13a5d 100644
> --- a/target-openrisc/Makefile.objs
> +++ b/target-openrisc/Makefile.objs
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_SOFTMMU) += machine.o
>  obj-y += cpu.o excp.o intrpt.o mmu.o translate.o
> -obj-y += excp_helper.o fpu_helper.o int_helper.o intrpt_helper.o mmu_helper.o
> +obj-y += excp_helper.o fpu_helper.o int_helper.o intrpt_helper.o \
> +         mmu_helper.o sys_helper.o
> diff --git a/target-openrisc/helper.h b/target-openrisc/helper.h
> index b3c1a17..6e95f9f 100644
> --- a/target-openrisc/helper.h
> +++ b/target-openrisc/helper.h
> @@ -66,4 +66,8 @@ DEF_HELPER_FLAGS_3(mul, 0, tl, env, tl, tl)
>  /* interrupt */
>  DEF_HELPER_FLAGS_1(rfe, 0, void, env)
>
> +/* sys */
> +DEF_HELPER_FLAGS_4(mtspr, 0, void, env, tl, tl, tl)
> +DEF_HELPER_FLAGS_4(mfspr, 0, void, env, tl, tl, tl)
> +
>  #include "def-helper.h"
> diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
> new file mode 100644
> index 0000000..89faab1
> --- /dev/null
> +++ b/target-openrisc/sys_helper.c
> @@ -0,0 +1,235 @@
> +/*
> + *  OpenRISC system instructions helper routines
> + *
> + *  Copyright (c) 2011-2012 Jia Liu <address@hidden>
> + *                          Zhizhou Zhang <address@hidden>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cpu.h"
> +#include "helper.h"
> +
> +#define TO_SPR(group, number) (((group)<<11)+(number))

Please add spaces around operators.

> +void HELPER(mtspr)(CPUOpenRISCState * env,
> +                   target_ulong ra, target_ulong rb, uint32_t offset)
> +{
> +#if !defined(CONFIG_USER_ONLY)
> +    int spr = env->gpr[ra] | offset;
> +    int idx;
> +
> +    switch (spr) {
> +    case TO_SPR(0, 0): /* VR */
> +        env->vr = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(0, 16): /* NPC */
> +        env->npc = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(0, 17): /* SR */
> +        if ((env->sr & (SR_IME | SR_DME | SR_SM)) ^
> +            (env->gpr[rb] & (SR_IME | SR_DME | SR_SM))) {
> +            tlb_flush(env, 1);
> +        }
> +        env->sr = env->gpr[rb];
> +        env->sr |= SR_FO;      /* FO is const equal to 1 */
> +        if (env->sr & SR_DME) {
> +            env->tlb->map_address_data = &get_phys_data;
> +        } else {
> +            env->tlb->map_address_data = &get_phys_nommu;
> +        }
> +
> +        if (env->sr & SR_IME) {
> +            env->tlb->map_address_code = &get_phys_code;
> +        } else {
> +            env->tlb->map_address_code = &get_phys_nommu;
> +        }
> +        break;
> +
> +    case TO_SPR(0, 18): /* PPC */
> +        env->ppc = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(0, 32): /* EPCR */
> +        env->epcr = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(0, 48): /* EEAR */
> +        env->eear = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(0, 64): /* ESR */
> +        env->esr = env->gpr[rb];
> +        break;
> +    case TO_SPR(1, 512) ... TO_SPR(1, 639): /* DTLBW0MR 0-127 */
> +        idx = spr - TO_SPR(1, 512);
> +        if (!(env->gpr[rb] & 1)) {
> +            tlb_flush_page(env, env->tlb->dtlb[0][idx].mr & 
> TARGET_PAGE_MASK);
> +        }
> +        env->tlb->dtlb[0][idx].mr = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(1, 640) ... TO_SPR(1, 767): /* DTLBW0TR 0-127 */
> +        idx = spr - TO_SPR(1, 640);
> +        env->tlb->dtlb[0][idx].tr = env->gpr[rb];
> +        break;
> +    case TO_SPR(1, 768) ... TO_SPR(1, 895):   /* DTLBW1MR 0-127 */
> +    case TO_SPR(1, 896) ... TO_SPR(1, 1023):  /* DTLBW1TR 0-127 */
> +    case TO_SPR(1, 1024) ... TO_SPR(1, 1151): /* DTLBW2MR 0-127 */
> +    case TO_SPR(1, 1152) ... TO_SPR(1, 1279): /* DTLBW2TR 0-127 */
> +    case TO_SPR(1, 1280) ... TO_SPR(1, 1407): /* DTLBW3MR 0-127 */
> +    case TO_SPR(1, 1408) ... TO_SPR(1, 1535): /* DTLBW3TR 0-127 */
> +        break;
> +    case TO_SPR(2, 512) ... TO_SPR(2, 639):   /* ITLBW0MR 0-127 */
> +        idx = spr - TO_SPR(2, 512);
> +        if (!(env->gpr[rb] & 1)) {
> +            tlb_flush_page(env, env->tlb->itlb[0][idx].mr & 
> TARGET_PAGE_MASK);
> +        }
> +        env->tlb->itlb[0][idx].mr = env->gpr[rb];
> +        break;
> +
> +    case TO_SPR(2, 640) ... TO_SPR(2, 767): /* ITLBW0TR 0-127 */
> +        idx = spr - TO_SPR(2, 640);
> +        env->tlb->itlb[0][idx].tr = env->gpr[rb];

Missing 'break' here.

> +    case TO_SPR(2, 768) ... TO_SPR(2, 895):   /* ITLBW1MR 0-127 */
> +    case TO_SPR(2, 896) ... TO_SPR(2, 1023):  /* ITLBW1TR 0-127 */
> +    case TO_SPR(2, 1024) ... TO_SPR(2, 1151): /* ITLBW2MR 0-127 */
> +    case TO_SPR(2, 1152) ... TO_SPR(2, 1279): /* ITLBW2TR 0-127 */
> +    case TO_SPR(2, 1280) ... TO_SPR(2, 1407): /* ITLBW3MR 0-127 */
> +    case TO_SPR(2, 1408) ... TO_SPR(2, 1535): /* ITLBW3TR 0-127 */
> +        break;
> +    case TO_SPR(9, 0):  /* PICMR */
> +        cpu_openrisc_store_picmr(env, env->gpr[rb]);
> +        break;
> +    case TO_SPR(9, 2):  /* PICSR */
> +        cpu_openrisc_store_picsr(env, env->gpr[rb]);
> +        break;
> +    case TO_SPR(10, 0): /* TTMR */
> +        cpu_openrisc_store_compare(env, env->gpr[rb]);
> +        break;
> +    case TO_SPR(10, 1): /* TTCR */
> +        cpu_openrisc_store_count(env, env->gpr[rb]);
> +        break;
> +    default:
> +        break;
> +    }
> +#endif
> +}
> +
> +void HELPER(mfspr)(CPUOpenRISCState * env,
> +                   target_ulong rd, target_ulong ra, uint32_t offset)
> +{
> +#if !defined(CONFIG_USER_ONLY)
> +    int spr = env->gpr[ra] | offset;
> +    int idx;
> +
> +    switch (spr) {
> +    case TO_SPR(0, 0): /* VR */
> +        env->gpr[rd] = (env->vr & SPR_VR);
> +        break;
> +
> +    case TO_SPR(0, 1): /* UPR */
> +        env->gpr[rd] = 0x619;    /* TT, DM, IM, UP present */
> +        env->gpr[rd] = env->upr;

The two stores don't seem correct, maybe you miss '|'?

> +        break;
> +
> +    case TO_SPR(0, 2): /* CPUCFGR */
> +        env->gpr[rd] = 0x000000a0;
> +        env->gpr[rd] = env->cpucfgr;
> +        break;
> +
> +    case TO_SPR(0, 3): /* DMMUCFGR */
> +        env->gpr[rd] = 0x18;    /* 1Way, 64 entries */
> +        env->gpr[rd] = env->dmmucfgr;
> +        break;
> +    case TO_SPR(0, 4): /* IMMUCFGR */
> +        env->gpr[rd] = 0x18;
> +        env->gpr[rd] = env->immucfgr;
> +        break;
> +
> +    case TO_SPR(0, 16): /* NPC */
> +        env->gpr[rd] = env->npc;
> +        break;
> +
> +    case TO_SPR(0, 17): /* SR */
> +        env->gpr[rd] = env->sr;
> +        break;
> +
> +    case TO_SPR(0, 18): /* PPC */
> +        env->gpr[rd] = env->ppc;
> +        break;
> +
> +    case TO_SPR(0, 32): /* EPCR */
> +        env->gpr[rd] = env->epcr;
> +        break;
> +
> +    case TO_SPR(0, 48): /* EEAR */
> +        env->gpr[rd] = env->eear;
> +        break;
> +
> +    case TO_SPR(0, 64): /* ESR */
> +        env->gpr[rd] = env->esr;
> +        break;
> +
> +    case TO_SPR(1, 512) ... TO_SPR(1, 639): /* DTLBW0MR 0-127 */
> +        idx = spr - TO_SPR(1, 512);
> +        env->gpr[rd] = env->tlb->dtlb[0][idx].mr;
> +        break;
> +
> +    case TO_SPR(1, 640) ... TO_SPR(1, 767): /* DTLBW0TR 0-127 */
> +        idx = spr - TO_SPR(1, 640);
> +        env->gpr[rd] = env->tlb->dtlb[0][idx].tr;
> +        break;
> +    case TO_SPR(1, 768) ... TO_SPR(1, 895):   /* DTLBW1MR 0-127 */
> +    case TO_SPR(1, 896) ... TO_SPR(1, 1023):  /* DTLBW1TR 0-127 */
> +    case TO_SPR(1, 1024) ... TO_SPR(1, 1151): /* DTLBW2MR 0-127 */
> +    case TO_SPR(1, 1152) ... TO_SPR(1, 1279): /* DTLBW2TR 0-127 */
> +    case TO_SPR(1, 1280) ... TO_SPR(1, 1407): /* DTLBW3MR 0-127 */
> +    case TO_SPR(1, 1408) ... TO_SPR(1, 1535): /* DTLBW3TR 0-127 */
> +        break;
> +
> +    case TO_SPR(2, 512) ... TO_SPR(2, 639): /* ITLBW0MR 0-127 */
> +        idx = spr - TO_SPR(2, 512);
> +        env->gpr[rd] = env->tlb->itlb[0][idx].mr;
> +        break;
> +
> +    case TO_SPR(2, 640) ... TO_SPR(2, 767): /* ITLBW0TR 0-127 */
> +        idx = spr - TO_SPR(2, 640);
> +        env->gpr[rd] = env->tlb->itlb[0][idx].tr;
> +        break;
> +    case TO_SPR(2, 768) ... TO_SPR(2, 895):   /* ITLBW1MR 0-127 */
> +    case TO_SPR(2, 896) ... TO_SPR(2, 1023):  /* ITLBW1TR 0-127 */
> +    case TO_SPR(2, 1024) ... TO_SPR(2, 1151): /* ITLBW2MR 0-127 */
> +    case TO_SPR(2, 1152) ... TO_SPR(2, 1279): /* ITLBW2TR 0-127 */
> +    case TO_SPR(2, 1280) ... TO_SPR(2, 1407): /* ITLBW3MR 0-127 */
> +    case TO_SPR(2, 1408) ... TO_SPR(2, 1535): /* ITLBW3TR 0-127 */
> +        break;
> +    case TO_SPR(9, 0):  /* PICMR */
> +        env->gpr[rd] = env->picmr;
> +        break;
> +    case TO_SPR(9, 2):  /* PICSR */
> +        env->gpr[rd] = env->picsr;
> +        break;
> +    case TO_SPR(10, 0): /* TTMR */
> +        env->gpr[rd] = env->ttmr;
> +        break;
> +    case TO_SPR(10, 1): /* TTCR */
> +        env->gpr[rd] = cpu_openrisc_get_count(env);
> +        break;
> +    default:
> +        break;
> +    }
> +#endif
> +}
> diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
> index ec004be..f0f46f3 100644
> --- a/target-openrisc/translate.c
> +++ b/target-openrisc/translate.c
> @@ -828,10 +828,28 @@ static void dec_misc(DisasContext *dc, CPUOpenRISCState 
> *env, uint32_t insn)
>
>     case 0x2d:   /*l.mfspr*/
>         LOG_DIS("l.mfspr r%d, r%d, %d\n", rd, ra, I16);
> +        {
> +            TCGv_i32 ti = tcg_const_i32(I16);
> +            TCGv td = tcg_const_tl(rd);
> +            TCGv ta = tcg_const_tl(ra);

The usual way is to pass the contents of the registers (instead of
register numbers like you are doing) to the helpers and then move the
return value back to registers. That way TCG can see that a register
is used or changed and optimize some moves.

> +            gen_helper_mfspr(cpu_env, td, ta, ti);
> +            tcg_temp_free_i32(ti);
> +            tcg_temp_free(td);
> +            tcg_temp_free(ta);
> +        }
>         break;
>
>     case 0x30:  /*l.mtspr*/
>         LOG_DIS("l.mtspr %d, r%d, r%d, %d\n", I5, ra, rb, I11);
> +        {
> +            TCGv_i32 ti = tcg_const_i32(tmp);
> +            TCGv ta = tcg_const_tl(ra);
> +            TCGv tb = tcg_const_tl(rb);
> +            gen_helper_mtspr(cpu_env, ta, tb, ti);
> +            tcg_temp_free_i32(ti);
> +            tcg_temp_free(ta);
> +            tcg_temp_free(tb);
> +        }
>         break;
>
>     case 0x34:   /*l.sd*/
> --
> 1.7.9.5
>
>