On 20/11/18 14:54, Peter Maydell wrote:
On 13 November 2018 at 16:52, Samuel Ortiz <sa...@linux.intel.com> wrote:In preparation for supporting TCG disablement on ARM, we move all TCG related v7m helpers and APIs into their own file (m_helper.c for all v*-m helpers). arm_v7m_cpu_do_interrupt pulls a large number of static functions out of helper.c into m_helper.c because it is TCG dependent.Signed-off-by: Samuel Ortiz <sa...@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com> Reviewed-by: Robert Bradford <robert.bradf...@intel.com> --- target/arm/internals.h | 37 + target/arm/helper.c | 2209 +++----------------------------------- target/arm/m_helper.c | 1892 ++++++++++++++++++++++++++++++++ target/arm/Makefile.objs | 2 +- 4 files changed, 2086 insertions(+), 2054 deletions(-) create mode 100644 target/arm/m_helper.c+/* Function used to synchronize QEMU's AArch64 register set with AArch32 + * register set. This is necessary when switching between AArch32 and AArch64 + * execution state. + */ +void aarch64_sync_32_to_64(CPUARMState *env) { - uint32_t new_ss_msp, new_ss_psp; + int i; + uint32_t mode = env->uncached_cpsr & CPSR_M; - if (env->v7m.secure == new_secstate) { - return; + /* We can blanket copy R[0:7] to X[0:7] */ + for (i = 0; i < 8; i++) { + env->xregs[i] = env->regs[i]; } - /* All the banked state is accessed by looking at env->v7m.secure - * except for the stack pointer; rearrange the SP appropriately. + /* Unless we are in FIQ mode, x8-x12 come from the user registers r8-r12. + * Otherwise, they come from the banked user regs. */ - new_ss_msp = env->v7m.other_ss_msp; - new_ss_psp = env->v7m.other_ss_psp; - - if (v7m_using_psp(env)) { - env->v7m.other_ss_psp = env->regs[13]; - env->v7m.other_ss_msp = env->v7m.other_sp; - } else { - env->v7m.other_ss_msp = env->regs[13]; - env->v7m.other_ss_psp = env->v7m.other_sp; - } - - env->v7m.secure = new_secstate; - - if (v7m_using_psp(env)) { - env->regs[13] = new_ss_psp; - env->v7m.other_sp = new_ss_msp; + if (mode == ARM_CPU_MODE_FIQ) { + for (i = 8; i < 13; i++) { + env->xregs[i] = env->usr_regs[i - 8]; + } } else { - env->regs[13] = new_ss_msp; - env->v7m.other_sp = new_ss_psp; + for (i = 8; i < 13; i++) { + env->xregs[i] = env->regs[i]; + } } -} -void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) -{ - /* Handle v7M BXNS: - * - if the return value is a magic value, do exception return (like BX) - * - otherwise bit 0 of the return value is the target security state + /* Registers x13-x23 are the various mode SP and FP registers. Registers + * r13 and r14 are only copied if we are in that mode, otherwise we copy + * from the mode banked register. */ - uint32_t min_magic; - - if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { - /* Covers FNC_RETURN and EXC_RETURN magic */ - min_magic = FNC_RETURN_MIN_MAGIC; + if (mode == ARM_CPU_MODE_USR || mode == ARM_CPU_MODE_SYS) { + env->xregs[13] = env->regs[13]; + env->xregs[14] = env->regs[14]; } else { - /* EXC_RETURN magic only */ - min_magic = EXC_RETURN_MIN_MAGIC; + env->xregs[13] = env->banked_r13[bank_number(ARM_CPU_MODE_USR)]; + /* HYP is an exception in that it is copied from r14 */ + if (mode == ARM_CPU_MODE_HYP) { + env->xregs[14] = env->regs[14]; + } else { + env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)]; + } }This part of the patch is a mess to read. I suspect this is a combination of (a) your git not being configured to use a better diff algorithm than the default (try "algorithm = histogram" in the [diff] section of your .gitconfig), and it doing an effective revert of 593cfa2b637b92d37 by accident.
I did the review offline, applying the series then looking at each commit with gitk, this is why I did not notice this.
It's also an absolutely enormous patch, even for a "just
moving code" patch, which makes it hard to review even
with diff --color-moved. Maybe it would be better in two
pieces ("helper routines for various insns" and "exception
handling functions" seems like a workable split).
Good idea. Regards, Phil.
