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

[Dmitry Eremin-Solenikov]

On 3/25/11, Peter Maydell <address@hidden> wrote:
> 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 :-))

Oops. According to cpuid they indeed are V4T.

>
>> @@ -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);

Ack. I just wanted to be insn-by-insn clear, rather than disabling
full blocks.

> 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.

... and fix the V4T PUSH containing PC.

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

Can we assume (maybe temporarily) that all v5 are also v5TE?
It seems it's currently done so, and I don't want to be too intrusive.

I'll need to dig more into this...

-- 
With best wishes
Dmitry