Re: [Qemu-devel] [PATCH 1/3] arm: basic support for ARMv4/ARMv4T emulation

[Peter Maydell]

I've just gone through this distinguishing v5 sublevels.
I've also gone back and looked up an older ARM ARM for any v5 vs
v5T differences, and it looks like the only difference really is
whether Thumb mode works: the ARM instruction set is exactly the
same including the existence of BX/BLX.

So I'm going to go back on what I suggested earlier, and say
that I think leaving it as ARCH(5) is better than ARCH(5T).
Sorry for the flip-flopping here.

I've marked up all the ARCH() uses in this patch, even the bits
which are correct as they stand, just for clarity. The rough
summary is that five lines need to change to 5TE.

On "v5TExP" -- yes, that's another one in the v7 ARM ARM's
list of "obsolete" variants.


On 24 March 2011 22:07, Dmitry Eremin-Solenikov <address@hidden> wrote:

> @@ -6129,6 +6131,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>                 }
>             }
>             /* Otherwise PLD; v5TE+ */
> +            ARCH(5);

5TE.

>             return;
>         }
>         if (((insn & 0x0f70f000) == 0x0450f000) ||
> @@ -6255,6 +6258,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>             /* branch link and change to thumb (blx <offset>) */
>             int32_t offset;
>
> +            ARCH(5);

5, so delete as covered by the top level ARCH(5) for any unconditional insn.

>             val = (uint32_t)s->pc;
>             tmp = tcg_temp_new_i32();
>             tcg_gen_movi_i32(tmp, val);
> @@ -6268,6 +6272,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>             gen_bx_im(s, val);
>             return;
>         } else if ((insn & 0x0e000f00) == 0x0c000100) {
> +            ARCH(5);

Can remove, IWMMXT implies 5 anyway.

>             if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
>                 /* iWMMXt register transfer.  */
>                 if (env->cp15.c15_cpar & (1 << 1))
> @@ -6276,8 +6281,10 @@ static void disas_arm_insn(CPUState * env, 
> DisasContext *s)
>             }
>         } else if ((insn & 0x0fe00000) == 0x0c400000) {
>             /* Coprocessor double register transfer.  */
> +            ARCH(5);

5TE.

>         } else if ((insn & 0x0f000010) == 0x0e000010) {
>             /* Additional coprocessor register transfer.  */
> +            ARCH(5);

5 (so deletable).

>         } else if ((insn & 0x0ff10020) == 0x01000000) {
>             uint32_t mask;
>             uint32_t val;


> @@ -6376,10 +6383,12 @@ static void disas_arm_insn(CPUState * env, 
> DisasContext *s)
>         case 0x1:
>             if (op1 == 1) {
>                 /* branch/exchange thumb (bx).  */
> +                ARCH(4T);

4T.

>                 tmp = load_reg(s, rm);
>                 gen_bx(s, tmp);
>             } else if (op1 == 3) {
>                 /* clz */
> +                ARCH(5);

5.

>                 rd = (insn >> 12) & 0xf;
>                 tmp = load_reg(s, rm);
>                 gen_helper_clz(tmp, tmp);
> @@ -6402,6 +6411,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>             if (op1 != 1)
>               goto illegal_op;
>
> +            ARCH(5);

5. (the v5 ARM ARM says BLX works on a non-T v5, it just means
you go into a state where everything undefs).

>             /* branch link/exchange thumb (blx) */
>             tmp = load_reg(s, rm);
>             tmp2 = tcg_temp_new_i32();
> @@ -6410,6 +6420,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>             gen_bx(s, tmp);
>             break;
>         case 0x5: /* saturating add/subtract */
> +            ARCH(5);

5TE.

>             rd = (insn >> 12) & 0xf;
>             rn = (insn >> 16) & 0xf;
>             tmp = load_reg(s, rm);
> @@ -6431,12 +6442,14 @@ static void disas_arm_insn(CPUState * env, 
> DisasContext *s)
>                 goto illegal_op;
>             }
>             /* bkpt */
> +            ARCH(5);

5.

>             gen_exception_insn(s, 4, EXCP_BKPT);
>             break;
>         case 0x8: /* signed multiply */
>         case 0xa:
>         case 0xc:
>         case 0xe:
> +            ARCH(5);

5TE.

>             rs = (insn >> 8) & 0xf;
>             rn = (insn >> 12) & 0xf;
>             rd = (insn >> 16) & 0xf;
> @@ -6832,6 +6845,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>                     }
>                     load = 1;
>                 } else if (sh & 2) {
> +                    ARCH(5);

5TE.

>                     /* doubleword */
>                     if (sh & 1) {
>                         /* store */

-- PMM