qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 16/37] target-arm: Implement SP_EL0, SP_EL1


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH v6 16/37] target-arm: Implement SP_EL0, SP_EL1
Date: Mon, 14 Apr 2014 15:56:13 +1000

On Fri, Apr 11, 2014 at 2:15 AM, Peter Maydell <address@hidden> wrote:
> Implement handling for the AArch64 SP_EL0 system register.
> This holds the EL0 stack pointer, and is only accessible when
> it's not being used as the stack pointer, ie when we're in EL1
> and EL1 is using its own stack pointer. We also provide a
> definition of the SP_EL1 register; this isn't guest visible
> as a system register for an implementation like QEMU which
> doesn't provide EL2 or EL3; however it is useful for ensuring
> the underlying state is migrated.
>
> We need to update the state fields in the CPU state whenever
> we switch stack pointers; this happens when we take an exception
> and also when SPSEL is used to change the bit in PSTATE which
> indicates which stack pointer EL1 should use.
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Peter Crosthwaite <address@hidden>

> ---
>  target-arm/cpu.h       |  2 ++
>  target-arm/helper.c    | 34 ++++++++++++++++++++++++++++++++++
>  target-arm/internals.h | 25 +++++++++++++++++++++++++
>  target-arm/kvm64.c     | 37 ++++++++++++++++++++++++++++++++++---
>  target-arm/machine.c   |  7 ++++---
>  target-arm/op_helper.c |  2 +-
>  6 files changed, 100 insertions(+), 7 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ecdd7a7..28b9bda 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -163,6 +163,7 @@ typedef struct CPUARMState {
>      uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
>
>      uint64_t elr_el1; /* AArch64 ELR_EL1 */
> +    uint64_t sp_el[2]; /* AArch64 banked stack pointers */
>
>      /* System control coprocessor (cp15) */
>      struct {
> @@ -434,6 +435,7 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr 
> address, int rw,
>   * Only these are valid when in AArch64 mode; in
>   * AArch32 mode SPSRs are basically CPSR-format.
>   */
> +#define PSTATE_SP (1U)
>  #define PSTATE_M (0xFU)
>  #define PSTATE_nRW (1U << 4)
>  #define PSTATE_F (1U << 6)
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 276ecf2..27a3dc2 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1783,6 +1783,27 @@ static uint64_t aa64_dczid_read(CPUARMState *env, 
> const ARMCPRegInfo *ri)
>      return cpu->dcz_blocksize | dzp_bit;
>  }
>
> +static CPAccessResult sp_el0_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    if (!env->pstate & PSTATE_SP) {
> +        /* Access to SP_EL0 is undefined if it's being used as
> +         * the stack pointer.
> +         */
> +        return CP_ACCESS_TRAP_UNCATEGORIZED;
> +    }
> +    return CP_ACCESS_OK;
> +}
> +
> +static uint64_t spsel_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    return env->pstate & PSTATE_SP;
> +}
> +
> +static void spsel_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> val)
> +{
> +    update_spsel(env, val);
> +}
> +
>  static const ARMCPRegInfo v8_cp_reginfo[] = {
>      /* Minimal set of EL0-visible registers. This will need to be expanded
>       * significantly for system emulation of AArch64 CPUs.
> @@ -1915,6 +1936,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 1,
>        .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, elr_el1) },
> +    /* We rely on the access checks not allowing the guest to write to the
> +     * state field when SPSel indicates that it's being used as the stack
> +     * pointer.
> +     */
> +    { .name = "SP_EL0", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 1, .opc2 = 0,
> +      .access = PL1_RW, .accessfn = sp_el0_access,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .fieldoffset = offsetof(CPUARMState, sp_el[0]) },
> +    { .name = "SPSel", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 2, .opc2 = 0,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .access = PL1_RW, .readfn = spsel_read, .writefn = spsel_write },
>      REGINFO_SENTINEL
>  };
>
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index a527f02..de79dfc 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -60,6 +60,31 @@ enum arm_fprounding {
>
>  int arm_rmode_to_sf(int rmode);
>
> +static inline void update_spsel(CPUARMState *env, uint32_t imm)
> +{
> +    /* Update PSTATE SPSel bit; this requires us to update the
> +     * working stack pointer in xregs[31].
> +     */
> +    if (!((imm ^ env->pstate) & PSTATE_SP)) {
> +        return;
> +    }
> +    env->pstate = deposit32(env->pstate, 0, 1, imm);
> +
> +    /* EL0 has no access rights to update SPSel, and this code
> +     * assumes we are updating SP for EL1 while running as EL1.
> +     */
> +    assert(arm_current_pl(env) == 1);
> +    if (env->pstate & PSTATE_SP) {
> +        /* Switch from using SP_EL0 to using SP_ELx */
> +        env->sp_el[0] = env->xregs[31];
> +        env->xregs[31] = env->sp_el[1];
> +    } else {
> +        /* Switch from SP_EL0 to SP_ELx */
> +        env->sp_el[1] = env->xregs[31];
> +        env->xregs[31] = env->sp_el[0];
> +    }
> +}
> +
>  /* Valid Syndrome Register EC field values */
>  enum arm_exception_class {
>      EC_UNCATEGORIZED          = 0x00,
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index ee72748..39c4364 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -121,8 +121,24 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>          }
>      }
>
> +    /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
> +     * QEMU side we keep the current SP in xregs[31] as well.
> +     */
> +    if (env->pstate & PSTATE_SP) {
> +        env->sp_el[1] = env->xregs[31];
> +    } else {
> +        env->sp_el[0] = env->xregs[31];
> +    }
> +
>      reg.id = AARCH64_CORE_REG(regs.sp);
> -    reg.addr = (uintptr_t) &env->xregs[31];
> +    reg.addr = (uintptr_t) &env->sp_el[0];
> +    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    reg.id = AARCH64_CORE_REG(sp_el1);
> +    reg.addr = (uintptr_t) &env->sp_el[1];
>      ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
>      if (ret) {
>          return ret;
> @@ -152,7 +168,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>      }
>
>      /* TODO:
> -     * SP_EL1
>       * SPSR[]
>       * FP state
>       * system registers
> @@ -180,7 +195,14 @@ int kvm_arch_get_registers(CPUState *cs)
>      }
>
>      reg.id = AARCH64_CORE_REG(regs.sp);
> -    reg.addr = (uintptr_t) &env->xregs[31];
> +    reg.addr = (uintptr_t) &env->sp_el[0];
> +    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    reg.id = AARCH64_CORE_REG(sp_el1);
> +    reg.addr = (uintptr_t) &env->sp_el[1];
>      ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
>      if (ret) {
>          return ret;
> @@ -194,6 +216,15 @@ int kvm_arch_get_registers(CPUState *cs)
>      }
>      pstate_write(env, val);
>
> +    /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the
> +     * QEMU side we keep the current SP in xregs[31] as well.
> +     */
> +    if (env->pstate & PSTATE_SP) {
> +        env->xregs[31] = env->sp_el[1];
> +    } else {
> +        env->xregs[31] = env->sp_el[0];
> +    }
> +
>      reg.id = AARCH64_CORE_REG(regs.pc);
>      reg.addr = (uintptr_t) &env->pc;
>      ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 42b1c90..c2c0780 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -222,9 +222,9 @@ static int cpu_post_load(void *opaque, int version_id)
>
>  const VMStateDescription vmstate_arm_cpu = {
>      .name = "cpu",
> -    .version_id = 15,
> -    .minimum_version_id = 15,
> -    .minimum_version_id_old = 15,
> +    .version_id = 16,
> +    .minimum_version_id = 16,
> +    .minimum_version_id_old = 16,
>      .pre_save = cpu_pre_save,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
> @@ -244,6 +244,7 @@ const VMStateDescription vmstate_arm_cpu = {
>          VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
>          VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
>          VMSTATE_UINT64(env.elr_el1, ARMCPU),
> +        VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 2),
>          /* The length-check must come before the arrays to avoid
>           * incoming data possibly overflowing the array.
>           */
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 51edd90..64a33dd 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -371,7 +371,7 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
> uint32_t imm)
>
>      switch (op) {
>      case 0x05: /* SPSel */
> -        env->pstate = deposit32(env->pstate, 0, 1, imm);
> +        update_spsel(env, imm);
>          break;
>      case 0x1e: /* DAIFSet */
>          env->daif |= (imm << 6) & PSTATE_DAIF;
> --
> 1.9.1
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]