Re: [Qemu-devel] [PATCH v4 03/21] target-arm: Define exception record for AArch64 exceptions

[Peter Crosthwaite <peter.crosthwaite@xilinx.com>]

On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

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