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

[Peter Maydell]

On 24 March 2011 22:07, Dmitry Eremin-Solenikov <address@hidden> wrote:
> Currently target-arm/ assumes at least ARMv5 core. Add support for
> handling also ARMv4/ARMv4T. This changes the following instructions:

Mostly looks good; comments below.

> @@ -161,6 +179,8 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
> id)
>         break;
>     case ARM_CPUID_TI915T:
>     case ARM_CPUID_TI925T:
> +        set_feature(env, ARM_FEATURE_V4T);
> +        set_feature(env, ARM_FEATURE_V5);
>         set_feature(env, ARM_FEATURE_OMAPCP);
>         env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring.  */
>         env->cp15.c0_cachetype = 0x5109149;

As far as I can tell from google these are based on the ARM9TDMI
which means they're ARMv4T and so shouldn't have the V5 feature set.
(You can legitimately feel disgruntled that whoever added these didn't
do the v4T stuff properly :-))

> @@ -6129,6 +6131,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>                 }
>             }
>             /* Otherwise PLD; v5TE+ */
> +            ARCH(5);
>             return;
>         }
>         if (((insn & 0x0f70f000) == 0x0450f000) ||

Rather than adding ARCH() lines here and in some of the following
hunks it would be simpler to change the

    if (cond == 0xf){
        /* Unconditional instructions.  */

to:

if (cond == 0xf) {
 /* In ARMv3 and v4 the NV condition is UNPREDICTABLE; we
  * choose to UNDEF. In ARMv5 and above the space is used
  * for miscellaneous unconditional instructions.
  */
 ARCH(5);

Some bits that are missing from this patch:

You need to guard the Thumb BKPT and BLX decodes
with ARCH(5) as they're not in v4T.

The CPSR Q bit needs to RAZ/WI on v4 and v4T.

For v4 you need to make sure that the core can't get into
thumb mode at all. So feature guards in gen_bx_imm() and
gen_bx(), make sure PSR masks prevent the T bit getting set,
and check helper.c for anything that sets env->thumb from
somewhere else...

-- PMM