From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47357) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPNfw-00041S-QX for qemu-devel@nongnu.org; Sun, 16 Mar 2014 22:53:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPNfs-0000Du-Ai for qemu-devel@nongnu.org; Sun, 16 Mar 2014 22:53:16 -0400 Received: from mail-we0-f176.google.com ([74.125.82.176]:37256) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPNfs-0000AT-1o for qemu-devel@nongnu.org; Sun, 16 Mar 2014 22:53:12 -0400 Received: by mail-we0-f176.google.com with SMTP id x48so4067461wes.35 for ; Sun, 16 Mar 2014 19:53:11 -0700 (PDT) MIME-Version: 1.0 Sender: peter.crosthwaite@petalogix.com In-Reply-To: <1394134385-1727-4-git-send-email-peter.maydell@linaro.org> References: <1394134385-1727-1-git-send-email-peter.maydell@linaro.org> <1394134385-1727-4-git-send-email-peter.maydell@linaro.org> Date: Mon, 17 Mar 2014 12:53:10 +1000 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH v4 03/21] target-arm: Define exception record for AArch64 exceptions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Rob Herring , Patch Tracking , Michael Matz , Claudio Fontana , Alexander Graf , "qemu-devel@nongnu.org Developers" , Laurent Desnogues , Dirk Mueller , Will Newton , =?ISO-8859-1?Q?Alex_Benn=E9e?= , "kvmarm@lists.cs.columbia.edu" , Christoffer Dall , Richard Henderson On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell wrote: > For AArch32 exceptions, the only information provided about > the cause of an exception is the individual exception type (data > abort, undef, etc), which we store in env->exception_index. For > AArch64, the CPU provides much more detail about the cause of > the exception, which can be found in the syndrome register. > Create a set of fields in CPUARMState which must be filled in > whenever an exception is raised, so that exception entry can > correctly fill in the syndrome register for the guest. > This includes the information which in AArch32 appears in > the DFAR and IFAR (fault address registers) and the DFSR > and IFSR (fault status registers) for data aborts and > prefetch aborts, since if we end up taking the MMU fault > to AArch64 rather than AArch32 this will need to end up > in different system registers. > > This patch does a refactoring which moves the setting of the > AArch32 DFAR/DFSR/IFAR/IFSR from the point where the exception > is raised to the point where it is taken. (This is no change > for cores with an MMU, retains the existing clearly incorrect > behaviour for ARM946 of trashing the MP access permissions > registers which share the c5_data and c5_insn state fields, > and has no effect for v7M because we don't implement its > MPU fault status or address registers.) > > As a side effect of the cleanup we fix a bug in the AArch64 > linux-user mode code where we were passing a 64 bit fault > address through the 32 bit c6_data/c6_insn fields: it now > goes via the always-64-bit exception.vaddress. > > Signed-off-by: Peter Maydell Reviewed-by: Peter Crosthwaite > --- > linux-user/main.c | 56 ++++++++++++++++++++++------------------------------ > target-arm/cpu.h | 15 ++++++++++++++ > target-arm/helper.c | 23 ++++++++++++--------- > target-arm/machine.c | 3 +++ > 4 files changed, 56 insertions(+), 41 deletions(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 9192977..a17ee47 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -483,17 +483,17 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env) > addr = env->regs[2]; > > if (get_user_u64(oldval, env->regs[0])) { > - env->cp15.c6_data = env->regs[0]; > + env->exception.vaddress = env->regs[0]; > goto segv; > }; > > if (get_user_u64(newval, env->regs[1])) { > - env->cp15.c6_data = env->regs[1]; > + env->exception.vaddress = env->regs[1]; > goto segv; > }; > > if (get_user_u64(val, addr)) { > - env->cp15.c6_data = addr; > + env->exception.vaddress = addr; > goto segv; > } > > @@ -501,7 +501,7 @@ static void arm_kernel_cmpxchg64_helper(CPUARMState *env) > val = newval; > > if (put_user_u64(val, addr)) { > - env->cp15.c6_data = addr; > + env->exception.vaddress = addr; > goto segv; > }; > > @@ -523,7 +523,7 @@ segv: > info.si_errno = 0; > /* XXX: check env->error_code */ > info.si_code = TARGET_SEGV_MAPERR; > - info._sifields._sigfault._addr = env->cp15.c6_data; > + info._sifields._sigfault._addr = env->exception.vaddress; > queue_signal(env, info.si_signo, &info); > > end_exclusive(); > @@ -620,14 +620,14 @@ static int do_strex(CPUARMState *env) > abort(); > } > if (segv) { > - env->cp15.c6_data = addr; > + env->exception.vaddress = addr; > goto done; > } > if (size == 3) { > uint32_t valhi; > segv = get_user_u32(valhi, addr + 4); > if (segv) { > - env->cp15.c6_data = addr + 4; > + env->exception.vaddress = addr + 4; > goto done; > } > val = deposit64(val, 32, 32, valhi); > @@ -650,14 +650,14 @@ static int do_strex(CPUARMState *env) > break; > } > if (segv) { > - env->cp15.c6_data = addr; > + env->exception.vaddress = addr; > goto done; > } > if (size == 3) { > val = env->regs[(env->exclusive_info >> 12) & 0xf]; > segv = put_user_u32(val, addr + 4); > if (segv) { > - env->cp15.c6_data = addr + 4; > + env->exception.vaddress = addr + 4; > goto done; > } > } > @@ -832,12 +832,14 @@ void cpu_loop(CPUARMState *env) > case EXCP_INTERRUPT: > /* just indicate that signals should be handled asap */ > break; > + case EXCP_STREX: > + if (!do_strex(env)) { > + break; > + } > + /* fall through for segv */ > case EXCP_PREFETCH_ABORT: > - addr = env->cp15.c6_insn; > - goto do_segv; > case EXCP_DATA_ABORT: > - addr = env->cp15.c6_data; > - do_segv: > + addr = env->exception.vaddress; > { > info.si_signo = SIGSEGV; > info.si_errno = 0; > @@ -865,12 +867,6 @@ void cpu_loop(CPUARMState *env) > if (do_kernel_trap(env)) > goto error; > break; > - case EXCP_STREX: > - if (do_strex(env)) { > - addr = env->cp15.c6_data; > - goto do_segv; > - } > - break; > default: > error: > fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n", > @@ -933,7 +929,7 @@ static int do_strex_a64(CPUARMState *env) > abort(); > } > if (segv) { > - env->cp15.c6_data = addr; > + env->exception.vaddress = addr; > goto error; > } > if (val != env->exclusive_val) { > @@ -946,7 +942,7 @@ static int do_strex_a64(CPUARMState *env) > segv = get_user_u64(val, addr + 8); > } > if (segv) { > - env->cp15.c6_data = addr + (size == 2 ? 4 : 8); > + env->exception.vaddress = addr + (size == 2 ? 4 : 8); > goto error; > } > if (val != env->exclusive_high) { > @@ -981,7 +977,7 @@ static int do_strex_a64(CPUARMState *env) > segv = put_user_u64(val, addr + 8); > } > if (segv) { > - env->cp15.c6_data = addr + (size == 2 ? 4 : 8); > + env->exception.vaddress = addr + (size == 2 ? 4 : 8); > goto error; > } > } > @@ -1037,12 +1033,14 @@ void cpu_loop(CPUARMState *env) > info._sifields._sigfault._addr = env->pc; > queue_signal(env, info.si_signo, &info); > break; > + case EXCP_STREX: > + if (!do_strex_a64(env)) { > + break; > + } > + /* fall through for segv */ > case EXCP_PREFETCH_ABORT: > - addr = env->cp15.c6_insn; > - goto do_segv; > case EXCP_DATA_ABORT: > - addr = env->cp15.c6_data; > - do_segv: > + addr = env->exception.vaddress; > info.si_signo = SIGSEGV; > info.si_errno = 0; > /* XXX: check env->error_code */ > @@ -1060,12 +1058,6 @@ void cpu_loop(CPUARMState *env) > queue_signal(env, info.si_signo, &info); > } > break; > - case EXCP_STREX: > - if (do_strex_a64(env)) { > - addr = env->cp15.c6_data; > - goto do_segv; > - } > - break; > default: > fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n", > trapnr); > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 45eb6a2..9982e47 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -229,6 +229,21 @@ typedef struct CPUARMState { > int pending_exception; > } v7m; > > + /* Information associated with an exception about to be taken: > + * code which raises an exception must set env->exception_index and > + * the relevant parts of this structure; the cpu_do_interrupt function > + * will then set the guest-visible registers as part of the exception > + * entry process. > + */ > + struct { > + uint32_t syndrome; /* AArch64 format syndrome register */ > + uint32_t fsr; /* AArch32 format fault status register info */ > + uint64_t vaddress; /* virtual addr associated with exception, if any */ > + /* If we implement EL2 we will also need to store information > + * about the intermediate physical address for stage 2 faults. > + */ > + } exception; > + > /* Thumb-2 EE state. */ > uint32_t teecr; > uint32_t teehbr; > diff --git a/target-arm/helper.c b/target-arm/helper.c > index f7168c1..2fa01ae 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -2610,12 +2610,11 @@ void arm_cpu_do_interrupt(CPUState *cs) > int cpu_arm_handle_mmu_fault (CPUARMState *env, target_ulong address, int rw, > int mmu_idx) > { > + env->exception.vaddress = address; > if (rw == 2) { > env->exception_index = EXCP_PREFETCH_ABORT; > - env->cp15.c6_insn = address; > } else { > env->exception_index = EXCP_DATA_ABORT; > - env->cp15.c6_data = address; > } > return 1; > } > @@ -2820,6 +2819,9 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) > return; > case EXCP_PREFETCH_ABORT: > case EXCP_DATA_ABORT: > + /* TODO: if we implemented the MPU registers, this is where we > + * should set the MMFAR, etc from exception.fsr and exception.vaddress. > + */ > armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); > return; > case EXCP_BKPT: > @@ -2934,9 +2936,11 @@ void arm_cpu_do_interrupt(CPUState *cs) > return; > } > } > - env->cp15.c5_insn = 2; > + env->exception.fsr = 2; > /* Fall through to prefetch abort. */ > case EXCP_PREFETCH_ABORT: > + env->cp15.c5_insn = env->exception.fsr; > + env->cp15.c6_insn = env->exception.vaddress; > qemu_log_mask(CPU_LOG_INT, "...with IFSR 0x%x IFAR 0x%x\n", > env->cp15.c5_insn, env->cp15.c6_insn); > new_mode = ARM_CPU_MODE_ABT; > @@ -2945,6 +2949,8 @@ void arm_cpu_do_interrupt(CPUState *cs) > offset = 4; > break; > case EXCP_DATA_ABORT: > + env->cp15.c5_data = env->exception.fsr; > + env->cp15.c6_data = env->exception.vaddress; > qemu_log_mask(CPU_LOG_INT, "...with DFSR 0x%x DFAR 0x%x\n", > env->cp15.c5_data, env->cp15.c6_data); > new_mode = ARM_CPU_MODE_ABT; > @@ -3593,16 +3599,15 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, target_ulong address, > } > > if (access_type == 2) { > - env->cp15.c5_insn = ret; > - env->cp15.c6_insn = address; > env->exception_index = EXCP_PREFETCH_ABORT; > } else { > - env->cp15.c5_data = ret; > - if (access_type == 1 && arm_feature(env, ARM_FEATURE_V6)) > - env->cp15.c5_data |= (1 << 11); > - env->cp15.c6_data = address; > + if (access_type == 1 && arm_feature(env, ARM_FEATURE_V6)) { > + ret |= (1 << 11); > + } > env->exception_index = EXCP_DATA_ABORT; > } > + env->exception.vaddress = address; > + env->exception.fsr = ret; > return 1; > } > > diff --git a/target-arm/machine.c b/target-arm/machine.c > index 8f9e7d4..fc8825e 100644 > --- a/target-arm/machine.c > +++ b/target-arm/machine.c > @@ -257,6 +257,9 @@ const VMStateDescription vmstate_arm_cpu = { > VMSTATE_UINT64(env.exclusive_val, ARMCPU), > VMSTATE_UINT64(env.exclusive_high, ARMCPU), > VMSTATE_UINT64(env.features, ARMCPU), > + VMSTATE_UINT32(env.exception.syndrome, ARMCPU), > + VMSTATE_UINT32(env.exception.fsr, ARMCPU), > + VMSTATE_UINT64(env.exception.vaddress, ARMCPU), > VMSTATE_TIMER(gt_timer[GTIMER_PHYS], ARMCPU), > VMSTATE_TIMER(gt_timer[GTIMER_VIRT], ARMCPU), > VMSTATE_END_OF_LIST() > -- > 1.9.0 > >