On 1/28/21 8:58 AM, Alex Bennée wrote: > Looking at the function here I wonder if we should be worried about the > thumb state? Peter? > > static void arm_cpu_set_pc(CPUState *cs, vaddr value) > { > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; > > if (is_a64(env)) { > env->pc = value; > env->thumb = 0; > } else { > env->regs[15] = value & ~1; > env->thumb = value & 1; > } > }
Plausible. You could possibly test this via gdbstub, as there are not many other users. I think it would be of the form: (gdb) call foo() where foo is a thumb function. > #ifdef CONFIG_TCG > void arm_cpu_synchronize_from_tb(CPUState *cs, > const TranslationBlock *tb) > { > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; > > /* > * It's OK to look at env for the current mode here, because it's > * never possible for an AArch64 TB to chain to an AArch32 TB. > */ > if (is_a64(env)) { > env->pc = tb->pc; > } else { > env->regs[15] = tb->pc; > } > } > #endif /* CONFIG_TCG */ This function need only handle any state that is "deferred" across goto_tb. This is almost always simply the pc, e.g. if (use_goto_tb(s, dest)) { tcg_gen_goto_tb(n); gen_set_pc_im(s, dest); tcg_gen_exit_tb(s->base.tb, n); A few targets do a bit more than that, especially vs delayed branches, but ARM does not. But there should be no thumb state that ought to be updated here. r~