On 29 March 2011 17:58, Dmitry Eremin-Solenikov <dbarysh...@gmail.com> wrote:
Looks good, nearly there I think.
> @@ -7172,10 +7191,11 @@ static void disas_arm_insn(CPUState * env,
> DisasContext *s)
> }
> if (insn & (1 << 20)) {
> /* Complete the load. */
> - if (rd == 15)
> + if (rd == 15 && ENABLE_ARCH_4T) {
> gen_bx(s, tmp);
> - else
> + } else {
> store_reg(s, rd, tmp);
> + }
> }
> break;
> case 0x08:
Shouldn't this be ENABLE_ARCH_5T ? Loads to PC are only interworking
in v5T and above.
(But see below...)
> @@ -7229,7 +7249,11 @@ static void disas_arm_insn(CPUState * env,
> DisasContext *s)
> /* load */
> tmp = gen_ld32(addr, IS_USER(s));
> if (i == 15) {
> - gen_bx(s, tmp);
> + if (ENABLE_ARCH_5) {
> + gen_bx(s, tmp);
> + } else {
> + store_reg(s, i, tmp);
> + }
> } else if (user) {
> tmp2 = tcg_const_i32(i);
> gen_helper_set_user_reg(tmp2, tmp);
> @@ -8980,8 +9006,13 @@ static void disas_thumb_insn(CPUState *env,
> DisasContext *s)
> /* write back the new stack pointer */
> store_reg(s, 13, addr);
> /* set the new PC value */
> - if ((insn & 0x0900) == 0x0900)
> - gen_bx(s, tmp);
> + if ((insn & 0x0900) == 0x0900) {
> + if (ENABLE_ARCH_5) {
> + gen_bx(s, tmp);
> + } else {
> + store_reg(s, 15, tmp);
> + }
> + }
> break;
>
> case 1: case 3: case 9: case 11: /* czb */
These two are right, but I think we should have a utility function
(put it next to store_reg_bx()):
/* Variant of store_reg which uses branch&exchange logic when storing
* to r15 in ARM architecture v5T and above. This is used for storing
* the results of a LDR/LDM/POP into r15, and corresponds to the cases
* in the ARM ARM which use the LoadWritePC() pseudocode function.
*/
static inline void store_reg_from_load(CPUState *env, DisasContext *s,
int reg, TCGv var)
{
if (reg == 15 && ENABLE_ARCH_5TE) {
gen_bx(s, var);
} else {
store_reg(s, reg, var);
}
}
Then you can use this in the three code hunks above. (You'll want
to tweak the middle one, you can move it to
if (user) {
...
} else if (i == rn) {
...
} else {
store_reg_from_load(env, s, i, tmp);
}
because if i==15 then user must be false, and if rn == 15 this
is UNPREDICTABLE anyway.)
These comments from last round still hold for this patch:
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
