[Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Hi,

I've been doing some work on modeling parts of EL2 and 3 + some of
the system-wide virtualization features for ARMv8. A lot is missing
but I've got a series with enough to for example run KVM A64 guests
on top of EL3 firmware inside emulated QEMU A64 VMs.
I'm working on cleaning things up and plan to send patches and publish
things as I go.

This series does a first round of preparations. Most of it has little
or no user visible impact as the EL2 and 3 features remain disabled.

For the arrayification of the various EL regs that do not exist for
EL0, I've used index macros. I don't really have a preference here,
so if you have better/less ugly ideas or preferences I'm happy to change
the pattern (getters/setters or whatever).

Happy to address any comments people may have.

Cheers,
Edgar

Edgar E. Iglesias (22):
  target-arm: A64: Add friendly logging of PSTATE A and I flags
  target-arm: Make elr_el1 an array
  target-arm: Make esr_el1 an array
  target-arm: c12_vbar -> vbar_el[]
  target-arm: Add arm_el_to_mmu_idx()
  target-arm: Move get_mem_index to translate.h
  target-arm: A64: Add SP entries for EL2 and 3
  target-arm: A64: Add ELR entries for EL2 and 3
  target-arm: Add SPSR entries for EL2/HYP and EL3/MON
  target-arm: A64: Introduce arm64_banked_spsr_index()
  target-arm: Add a feature flag for EL2
  target-arm: Add a feature flag for EL3
  target-arm: Register EL2 versions of ELR and SPSR
  target-arm: Register EL3 versions of ELR and SPSR
  target-arm: A64: Forbid ERET to increase the EL
  target-arm: A64: Forbid ERET to unimplemented ELs
  target-arm: A64: Generalize ERET to various ELs
  target-arm: A64: Generalize update_spsel for the various ELs
  target-arm: Add storage for VBAR_EL2 and 3
  target-arm: Make vbar_write writeback to any CPREG
  target-arm: A64: Register VBAR_EL2
  target-arm: A64: Register VBAR_EL3

 target-arm/cpu.h           | 38 +++++++++++++++++++++-----
 target-arm/helper-a64.c    | 15 ++++++-----
 target-arm/helper.c        | 67 +++++++++++++++++++++++++++++++++++++++-------
 target-arm/internals.h     | 25 ++++++++++++-----
 target-arm/kvm64.c         |  4 +--
 target-arm/machine.c       | 12 ++++-----
 target-arm/op_helper.c     | 19 ++++++++-----
 target-arm/translate-a64.c | 13 +++------
 target-arm/translate.h     |  5 ++++
 9 files changed, 145 insertions(+), 53 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH v1 01/22] target-arm: A64: Add friendly logging of PSTATE A and I flags

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/translate-a64.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index b62db4d..4f8246f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -137,8 +137,10 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
             cpu_fprintf(f, " ");
         }
     }
-    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
+    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c%c%c)\n",
                 psr,
+                psr & PSTATE_A ? 'A' : '-',
+                psr & PSTATE_I ? 'I' : '-',
                 psr & PSTATE_N ? 'N' : '-',
                 psr & PSTATE_Z ? 'Z' : '-',
                 psr & PSTATE_C ? 'C' : '-',
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 01/22] target-arm: A64: Add friendly logging of PSTATE A and I flags

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/translate-a64.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index b62db4d..4f8246f 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -137,8 +137,10 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>              cpu_fprintf(f, " ");
>          }
>      }
> -    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
> +    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c%c%c)\n",

Should delimit (just a space I think) between DAIF and NZCV
components. ARM ARM repeatedly refers to these two groups of four as
single item suggesting they are two logical groupings of 4 bits each.

>                  psr,
> +                psr & PSTATE_A ? 'A' : '-',
> +                psr & PSTATE_I ? 'I' : '-',

And should the full DAIF be added for completeness?

Regards,
Peter

>                  psr & PSTATE_N ? 'N' : '-',
>                  psr & PSTATE_Z ? 'Z' : '-',
>                  psr & PSTATE_C ? 'C' : '-',
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 01/22] target-arm: A64: Add friendly logging of PSTATE A and I flags

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/translate-a64.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index b62db4d..4f8246f 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -137,8 +137,10 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>              cpu_fprintf(f, " ");
>          }
>      }
> -    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
> +    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c%c%c)\n",
>                  psr,
> +                psr & PSTATE_A ? 'A' : '-',
> +                psr & PSTATE_I ? 'I' : '-',
>                  psr & PSTATE_N ? 'N' : '-',
>                  psr & PSTATE_Z ? 'Z' : '-',
>                  psr & PSTATE_C ? 'C' : '-',

Why A and I ? In particular in QEMU the A bit is always zero
because we don't do System Errors (aka asynchronous
external aborts), and it's weird to show I but not F. The
idea of splitting out NZCV is really that (as with the A32/T32
state dump) they're the most useful bits for immediately
figuring out code flow); anything else you can fish out of
the hex value by hand if you really need it. I think you can
make a case for "decode only a small set of key bits" or
for "completely decode the whole register", but I'm not
sure adding only two more bits makes sense.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 01/22] target-arm: A64: Add friendly logging of PSTATE A and I flags

[Edgar E. Iglesias]

On Wed, May 07, 2014 at 09:50:27AM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/translate-a64.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> > index b62db4d..4f8246f 100644
> > --- a/target-arm/translate-a64.c
> > +++ b/target-arm/translate-a64.c
> > @@ -137,8 +137,10 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
> >              cpu_fprintf(f, " ");
> >          }
> >      }
> > -    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c)\n",
> > +    cpu_fprintf(f, "PSTATE=%08x (flags %c%c%c%c%c%c)\n",
> >                  psr,
> > +                psr & PSTATE_A ? 'A' : '-',
> > +                psr & PSTATE_I ? 'I' : '-',
> >                  psr & PSTATE_N ? 'N' : '-',
> >                  psr & PSTATE_Z ? 'Z' : '-',
> >                  psr & PSTATE_C ? 'C' : '-',
> 
> Why A and I ? In particular in QEMU the A bit is always zero
> because we don't do System Errors (aka asynchronous
> external aborts), and it's weird to show I but not F. The
> idea of splitting out NZCV is really that (as with the A32/T32
> state dump) they're the most useful bits for immediately
> figuring out code flow); anything else you can fish out of
> the hex value by hand if you really need it. I think you can
> make a case for "decode only a small set of key bits" or
> for "completely decode the whole register", but I'm not
> sure adding only two more bits makes sense.

Hi,

TBH I didn't give this much thought. I used the I flag while
debugging some virq stuff and probably added A while stumbling
in the dark at some point..

For v2 I've added the DAIF flags with delimiter if thats not good
enough I suggest removing the patch, it's not very important.

Cheers,
Edgar

[Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

No functional change.
Prepares for future additions of the EL2 and 3 versions of this reg.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h        | 3 ++-
 target-arm/helper-a64.c | 4 ++--
 target-arm/helper.c     | 3 ++-
 target-arm/kvm64.c      | 4 ++--
 target-arm/machine.c    | 2 +-
 target-arm/op_helper.c  | 6 +++---
 6 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c83f249..eb7a0f5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -162,7 +162,8 @@ typedef struct CPUARMState {
     uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
     uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
 
-    uint64_t elr_el1; /* AArch64 ELR_EL1 */
+#define ELR_EL_IDX(x) (x - 1)
+    uint64_t elr_el[1]; /* AArch64 exception link regs  */
     uint64_t sp_el[2]; /* AArch64 banked stack pointers */
 
     /* System control coprocessor (cp15) */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index bf921cc..5adf2b5 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -491,13 +491,13 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
         env->banked_spsr[0] = pstate_read(env);
         env->sp_el[arm_current_pl(env)] = env->xregs[31];
         env->xregs[31] = env->sp_el[1];
-        env->elr_el1 = env->pc;
+        env->elr_el[ELR_EL_IDX(1)] = env->pc;
     } else {
         env->banked_spsr[0] = cpsr_read(env);
         if (!env->thumb) {
             env->cp15.esr_el1 |= 1 << 25;
         }
-        env->elr_el1 = env->regs[15];
+        env->elr_el[ELR_EL_IDX(1)] = env->regs[15];
 
         for (i = 0; i < 15; i++) {
             env->xregs[i] = env->regs[i];
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3be917c..3457d3e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2055,7 +2055,8 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
     { .name = "ELR_EL1", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 1,
-      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, elr_el1) },
+      .access = PL1_RW,
+      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(1)]) },
     { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index e115879..da376cf 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -161,7 +161,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     }
 
     reg.id = AARCH64_CORE_REG(elr_el1);
-    reg.addr = (uintptr_t) &env->elr_el1;
+    reg.addr = (uintptr_t) &env->elr_el[ELR_EL_IDX(1)];
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
     if (ret) {
         return ret;
@@ -241,7 +241,7 @@ int kvm_arch_get_registers(CPUState *cs)
     }
 
     reg.id = AARCH64_CORE_REG(elr_el1);
-    reg.addr = (uintptr_t) &env->elr_el1;
+    reg.addr = (uintptr_t) &env->elr_el[ELR_EL_IDX(1)];
     ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
     if (ret) {
         return ret;
diff --git a/target-arm/machine.c b/target-arm/machine.c
index b967223..8b299a0 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -243,7 +243,7 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
         VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
-        VMSTATE_UINT64(env.elr_el1, ARMCPU),
+        VMSTATE_UINT64(env.elr_el[ELR_EL_IDX(1)], 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 fb90676..21545d0 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -406,7 +406,7 @@ void HELPER(exception_return)(CPUARMState *env)
             env->regs[i] = env->xregs[i];
         }
 
-        env->regs[15] = env->elr_el1 & ~0x1;
+        env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
     } else {
         new_el = extract32(spsr, 2, 2);
         if (new_el > 1) {
@@ -424,7 +424,7 @@ void HELPER(exception_return)(CPUARMState *env)
         env->aarch64 = 1;
         pstate_write(env, spsr);
         env->xregs[31] = env->sp_el[new_el];
-        env->pc = env->elr_el1;
+        env->pc = env->elr_el[ELR_EL_IDX(1)];
     }
 
     return;
@@ -438,7 +438,7 @@ illegal_return:
      * no change to exception level, execution state or stack pointer
      */
     env->pstate |= PSTATE_IL;
-    env->pc = env->elr_el1;
+    env->pc = env->elr_el[ELR_EL_IDX(1)];
     spsr &= PSTATE_NZCV | PSTATE_DAIF;
     spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
     pstate_write(env, spsr);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> No functional change.
> Prepares for future additions of the EL2 and 3 versions of this reg.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h        | 3 ++-
>  target-arm/helper-a64.c | 4 ++--
>  target-arm/helper.c     | 3 ++-
>  target-arm/kvm64.c      | 4 ++--
>  target-arm/machine.c    | 2 +-
>  target-arm/op_helper.c  | 6 +++---
>  6 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c83f249..eb7a0f5 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -162,7 +162,8 @@ typedef struct CPUARMState {
>      uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
>      uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
>
> -    uint64_t elr_el1; /* AArch64 ELR_EL1 */
> +#define ELR_EL_IDX(x) (x - 1)
> +    uint64_t elr_el[1]; /* AArch64 exception link regs  */

Is it perhaps just easier to waste the space and always pad these
EL-banked CP arrays out to length 4 you can just use literal numbers
in the code? Probably make life easier when introspecting the CPU
state in GDB too.

Regards,
Peter

>      uint64_t sp_el[2]; /* AArch64 banked stack pointers */
>
>      /* System control coprocessor (cp15) */
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index bf921cc..5adf2b5 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -491,13 +491,13 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>          env->banked_spsr[0] = pstate_read(env);
>          env->sp_el[arm_current_pl(env)] = env->xregs[31];
>          env->xregs[31] = env->sp_el[1];
> -        env->elr_el1 = env->pc;
> +        env->elr_el[ELR_EL_IDX(1)] = env->pc;
>      } else {
>          env->banked_spsr[0] = cpsr_read(env);
>          if (!env->thumb) {
>              env->cp15.esr_el1 |= 1 << 25;
>          }
> -        env->elr_el1 = env->regs[15];
> +        env->elr_el[ELR_EL_IDX(1)] = env->regs[15];
>
>          for (i = 0; i < 15; i++) {
>              env->xregs[i] = env->regs[i];
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 3be917c..3457d3e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2055,7 +2055,8 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>      { .name = "ELR_EL1", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 1,
> -      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, elr_el1) },
> +      .access = PL1_RW,
> +      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(1)]) },
>      { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index e115879..da376cf 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -161,7 +161,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>      }
>
>      reg.id = AARCH64_CORE_REG(elr_el1);
> -    reg.addr = (uintptr_t) &env->elr_el1;
> +    reg.addr = (uintptr_t) &env->elr_el[ELR_EL_IDX(1)];
>      ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
>      if (ret) {
>          return ret;
> @@ -241,7 +241,7 @@ int kvm_arch_get_registers(CPUState *cs)
>      }
>
>      reg.id = AARCH64_CORE_REG(elr_el1);
> -    reg.addr = (uintptr_t) &env->elr_el1;
> +    reg.addr = (uintptr_t) &env->elr_el[ELR_EL_IDX(1)];
>      ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
>      if (ret) {
>          return ret;
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index b967223..8b299a0 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -243,7 +243,7 @@ const VMStateDescription vmstate_arm_cpu = {
>          VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
>          VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
>          VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
> -        VMSTATE_UINT64(env.elr_el1, ARMCPU),
> +        VMSTATE_UINT64(env.elr_el[ELR_EL_IDX(1)], 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 fb90676..21545d0 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -406,7 +406,7 @@ void HELPER(exception_return)(CPUARMState *env)
>              env->regs[i] = env->xregs[i];
>          }
>
> -        env->regs[15] = env->elr_el1 & ~0x1;
> +        env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
>      } else {
>          new_el = extract32(spsr, 2, 2);
>          if (new_el > 1) {
> @@ -424,7 +424,7 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
>          env->xregs[31] = env->sp_el[new_el];
> -        env->pc = env->elr_el1;
> +        env->pc = env->elr_el[ELR_EL_IDX(1)];
>      }
>
>      return;
> @@ -438,7 +438,7 @@ illegal_return:
>       * no change to exception level, execution state or stack pointer
>       */
>      env->pstate |= PSTATE_IL;
> -    env->pc = env->elr_el1;
> +    env->pc = env->elr_el[ELR_EL_IDX(1)];
>      spsr &= PSTATE_NZCV | PSTATE_DAIF;
>      spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
>      pstate_write(env, spsr);
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Edgar E. Iglesias]

On Wed, May 07, 2014 at 03:10:54PM +1000, Peter Crosthwaite wrote:
> On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > No functional change.
> > Prepares for future additions of the EL2 and 3 versions of this reg.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h        | 3 ++-
> >  target-arm/helper-a64.c | 4 ++--
> >  target-arm/helper.c     | 3 ++-
> >  target-arm/kvm64.c      | 4 ++--
> >  target-arm/machine.c    | 2 +-
> >  target-arm/op_helper.c  | 6 +++---
> >  6 files changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index c83f249..eb7a0f5 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -162,7 +162,8 @@ typedef struct CPUARMState {
> >      uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
> >      uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
> >
> > -    uint64_t elr_el1; /* AArch64 ELR_EL1 */
> > +#define ELR_EL_IDX(x) (x - 1)
> > +    uint64_t elr_el[1]; /* AArch64 exception link regs  */
> 
> Is it perhaps just easier to waste the space and always pad these
> EL-banked CP arrays out to length 4 you can just use literal numbers
> in the code? Probably make life easier when introspecting the CPU
> state in GDB too.

Thanks Peter,

I've fixed all your comments except this one. I considered this
pattern but avoided it due to the bloating of CPUARMState. Anyway,
I'm happy to change to this full array allocation if others agree.

PMM, what is your preference on this?

Best regards,
Edgar


> 
> Regards,
> Peter
> 
> >      uint64_t sp_el[2]; /* AArch64 banked stack pointers */
> >
> >      /* System control coprocessor (cp15) */
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index bf921cc..5adf2b5 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -491,13 +491,13 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >          env->banked_spsr[0] = pstate_read(env);
> >          env->sp_el[arm_current_pl(env)] = env->xregs[31];
> >          env->xregs[31] = env->sp_el[1];
> > -        env->elr_el1 = env->pc;
> > +        env->elr_el[ELR_EL_IDX(1)] = env->pc;
> >      } else {
> >          env->banked_spsr[0] = cpsr_read(env);
> >          if (!env->thumb) {
> >              env->cp15.esr_el1 |= 1 << 25;
> >          }
> > -        env->elr_el1 = env->regs[15];
> > +        env->elr_el[ELR_EL_IDX(1)] = env->regs[15];
> >
> >          for (i = 0; i < 15; i++) {
> >              env->xregs[i] = env->regs[i];
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 3be917c..3457d3e 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2055,7 +2055,8 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
> >      { .name = "ELR_EL1", .state = ARM_CP_STATE_AA64,
> >        .type = ARM_CP_NO_MIGRATE,
> >        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 1,
> > -      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, elr_el1) },
> > +      .access = PL1_RW,
> > +      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(1)]) },
> >      { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64,
> >        .type = ARM_CP_NO_MIGRATE,
> >        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
> > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> > index e115879..da376cf 100644
> > --- a/target-arm/kvm64.c
> > +++ b/target-arm/kvm64.c
> > @@ -161,7 +161,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >      }
> >
> >      reg.id = AARCH64_CORE_REG(elr_el1);
> > -    reg.addr = (uintptr_t) &env->elr_el1;
> > +    reg.addr = (uintptr_t) &env->elr_el[ELR_EL_IDX(1)];
> >      ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> >      if (ret) {
> >          return ret;
> > @@ -241,7 +241,7 @@ int kvm_arch_get_registers(CPUState *cs)
> >      }
> >
> >      reg.id = AARCH64_CORE_REG(elr_el1);
> > -    reg.addr = (uintptr_t) &env->elr_el1;
> > +    reg.addr = (uintptr_t) &env->elr_el[ELR_EL_IDX(1)];
> >      ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> >      if (ret) {
> >          return ret;
> > diff --git a/target-arm/machine.c b/target-arm/machine.c
> > index b967223..8b299a0 100644
> > --- a/target-arm/machine.c
> > +++ b/target-arm/machine.c
> > @@ -243,7 +243,7 @@ const VMStateDescription vmstate_arm_cpu = {
> >          VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
> >          VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
> >          VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
> > -        VMSTATE_UINT64(env.elr_el1, ARMCPU),
> > +        VMSTATE_UINT64(env.elr_el[ELR_EL_IDX(1)], 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 fb90676..21545d0 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -406,7 +406,7 @@ void HELPER(exception_return)(CPUARMState *env)
> >              env->regs[i] = env->xregs[i];
> >          }
> >
> > -        env->regs[15] = env->elr_el1 & ~0x1;
> > +        env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
> >      } else {
> >          new_el = extract32(spsr, 2, 2);
> >          if (new_el > 1) {
> > @@ -424,7 +424,7 @@ void HELPER(exception_return)(CPUARMState *env)
> >          env->aarch64 = 1;
> >          pstate_write(env, spsr);
> >          env->xregs[31] = env->sp_el[new_el];
> > -        env->pc = env->elr_el1;
> > +        env->pc = env->elr_el[ELR_EL_IDX(1)];
> >      }
> >
> >      return;
> > @@ -438,7 +438,7 @@ illegal_return:
> >       * no change to exception level, execution state or stack pointer
> >       */
> >      env->pstate |= PSTATE_IL;
> > -    env->pc = env->elr_el1;
> > +    env->pc = env->elr_el[ELR_EL_IDX(1)];
> >      spsr &= PSTATE_NZCV | PSTATE_DAIF;
> >      spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
> >      pstate_write(env, spsr);
> > --
> > 1.8.3.2
> >
> >

Re: [Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Peter Maydell]

On 8 May 2014 01:13, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Wed, May 07, 2014 at 03:10:54PM +1000, Peter Crosthwaite wrote:
>> On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
>> <edgar.iglesias@gmail.com> wrote:
>> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>> >
>> > No functional change.
>> > Prepares for future additions of the EL2 and 3 versions of this reg.
>> >
>> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> > ---
>> >  target-arm/cpu.h        | 3 ++-
>> >  target-arm/helper-a64.c | 4 ++--
>> >  target-arm/helper.c     | 3 ++-
>> >  target-arm/kvm64.c      | 4 ++--
>> >  target-arm/machine.c    | 2 +-
>> >  target-arm/op_helper.c  | 6 +++---
>> >  6 files changed, 12 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> > index c83f249..eb7a0f5 100644
>> > --- a/target-arm/cpu.h
>> > +++ b/target-arm/cpu.h
>> > @@ -162,7 +162,8 @@ typedef struct CPUARMState {
>> >      uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
>> >      uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
>> >
>> > -    uint64_t elr_el1; /* AArch64 ELR_EL1 */
>> > +#define ELR_EL_IDX(x) (x - 1)
>> > +    uint64_t elr_el[1]; /* AArch64 exception link regs  */
>>
>> Is it perhaps just easier to waste the space and always pad these
>> EL-banked CP arrays out to length 4 you can just use literal numbers
>> in the code? Probably make life easier when introspecting the CPU
>> state in GDB too.
>
> Thanks Peter,
>
> I've fixed all your comments except this one. I considered this
> pattern but avoided it due to the bloating of CPUARMState. Anyway,
> I'm happy to change to this full array allocation if others agree.
>
> PMM, what is your preference on this?

I think I'd rather we just padded the array to length 4.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Edgar E. Iglesias]

On Fri, May 16, 2014 at 03:19:36PM +0100, Peter Maydell wrote:
> On 8 May 2014 01:13, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Wed, May 07, 2014 at 03:10:54PM +1000, Peter Crosthwaite wrote:
> >> On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
> >> <edgar.iglesias@gmail.com> wrote:
> >> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >> >
> >> > No functional change.
> >> > Prepares for future additions of the EL2 and 3 versions of this reg.
> >> >
> >> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >> > ---
> >> >  target-arm/cpu.h        | 3 ++-
> >> >  target-arm/helper-a64.c | 4 ++--
> >> >  target-arm/helper.c     | 3 ++-
> >> >  target-arm/kvm64.c      | 4 ++--
> >> >  target-arm/machine.c    | 2 +-
> >> >  target-arm/op_helper.c  | 6 +++---
> >> >  6 files changed, 12 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> >> > index c83f249..eb7a0f5 100644
> >> > --- a/target-arm/cpu.h
> >> > +++ b/target-arm/cpu.h
> >> > @@ -162,7 +162,8 @@ typedef struct CPUARMState {
> >> >      uint32_t condexec_bits; /* IT bits.  cpsr[15:10,26:25].  */
> >> >      uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
> >> >
> >> > -    uint64_t elr_el1; /* AArch64 ELR_EL1 */
> >> > +#define ELR_EL_IDX(x) (x - 1)
> >> > +    uint64_t elr_el[1]; /* AArch64 exception link regs  */
> >>
> >> Is it perhaps just easier to waste the space and always pad these
> >> EL-banked CP arrays out to length 4 you can just use literal numbers
> >> in the code? Probably make life easier when introspecting the CPU
> >> state in GDB too.
> >
> > Thanks Peter,
> >
> > I've fixed all your comments except this one. I considered this
> > pattern but avoided it due to the bloating of CPUARMState. Anyway,
> > I'm happy to change to this full array allocation if others agree.
> >
> > PMM, what is your preference on this?
> 
> I think I'd rather we just padded the array to length 4.

Sounds good, I'll change the pattern.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> No functional change.
> Prepares for future additions of the EL2 and 3 versions of this reg.

> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -243,7 +243,7 @@ const VMStateDescription vmstate_arm_cpu = {
>          VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
>          VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
>          VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
> -        VMSTATE_UINT64(env.elr_el1, ARMCPU),
> +        VMSTATE_UINT64(env.elr_el[ELR_EL_IDX(1)], 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.

Do we add the rest of the elr_el[] array to the vmstate in
a later patch?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 02/22] target-arm: Make elr_el1 an array

[Edgar E. Iglesias]

On Fri, May 16, 2014 at 03:22:40PM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > No functional change.
> > Prepares for future additions of the EL2 and 3 versions of this reg.
> 
> > --- a/target-arm/machine.c
> > +++ b/target-arm/machine.c
> > @@ -243,7 +243,7 @@ const VMStateDescription vmstate_arm_cpu = {
> >          VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
> >          VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
> >          VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
> > -        VMSTATE_UINT64(env.elr_el1, ARMCPU),
> > +        VMSTATE_UINT64(env.elr_el[ELR_EL_IDX(1)], 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.
> 
> Do we add the rest of the elr_el[] array to the vmstate in
> a later patch?

Yes. I can squash the patches if you prefer but I did find it useful to
do some of these changes in steps of non-functional change followed
by the actual usage/addition of EL2/3 while developing.

Cheers,
Edgar

[Qemu-devel] [PATCH v1 03/22] target-arm: Make esr_el1 an array

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

No functional change.
Prepares for future addtion of EL2 and 3 versions of this reg.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h        |  3 ++-
 target-arm/helper-a64.c |  4 ++--
 target-arm/helper.c     | 11 ++++++-----
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index eb7a0f5..2a068ec 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -186,7 +186,8 @@ typedef struct CPUARMState {
         uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
         uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
         uint32_t ifsr_el2; /* Fault status registers.  */
-        uint64_t esr_el1;
+#define ESR_EL_IDX(x) (x - 1)
+        uint64_t esr_el[1];
         uint32_t c6_region[8]; /* MPU base/size registers.  */
         uint64_t far_el1; /* Fault address registers.  */
         uint64_t par_el1;  /* Translation result. */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 5adf2b5..4bee075 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -464,7 +464,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
                       env->exception.syndrome);
     }
 
-    env->cp15.esr_el1 = env->exception.syndrome;
+    env->cp15.esr_el[ESR_EL_IDX(1)] = env->exception.syndrome;
     env->cp15.far_el1 = env->exception.vaddress;
 
     switch (cs->exception_index) {
@@ -495,7 +495,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     } else {
         env->banked_spsr[0] = cpsr_read(env);
         if (!env->thumb) {
-            env->cp15.esr_el1 |= 1 << 25;
+            env->cp15.esr_el[ESR_EL_IDX(1)] |= 1 << 25;
         }
         env->elr_el[ELR_EL_IDX(1)] = env->regs[15];
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3457d3e..c86fab6 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1452,7 +1452,7 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_NO_MIGRATE,
-      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el1),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[ESR_EL_IDX(1)]),
       .resetfn = arm_cp_reset_ignore, },
     { .name = "IFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 1,
       .access = PL1_RW,
@@ -1460,7 +1460,8 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     { .name = "ESR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .crn = 5, .crm = 2, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW,
-      .fieldoffset = offsetof(CPUARMState, cp15.esr_el1), .resetvalue = 0, },
+      .fieldoffset = offsetof(CPUARMState, cp15.esr_el[ESR_EL_IDX(1)]),
+      .resetvalue = 0, },
     { .name = "TTBR0_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.ttbr0_el1),
@@ -1521,7 +1522,7 @@ static void omap_cachemaint_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static const ARMCPRegInfo omap_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = CP_ANY,
       .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_RW, .type = ARM_CP_OVERRIDE,
-      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el1),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[ESR_EL_IDX(1)]),
       .resetvalue = 0, },
     { .name = "", .cp = 15, .crn = 15, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_NOP },
@@ -3331,11 +3332,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
         offset = 4;
         break;
     case EXCP_DATA_ABORT:
-        env->cp15.esr_el1 = env->exception.fsr;
+        env->cp15.esr_el[ESR_EL_IDX(1)] = env->exception.fsr;
         env->cp15.far_el1 = deposit64(env->cp15.far_el1, 0, 32,
                                       env->exception.vaddress);
         qemu_log_mask(CPU_LOG_INT, "...with DFSR 0x%x DFAR 0x%x\n",
-                      (uint32_t)env->cp15.esr_el1,
+                      (uint32_t)env->cp15.esr_el[ESR_EL_IDX(1)],
                       (uint32_t)env->exception.vaddress);
         new_mode = ARM_CPU_MODE_ABT;
         addr = 0x10;
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 04/22] target-arm: c12_vbar -> vbar_el[]

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

No functional change.
Preparation for adding EL2 and 3 versions of this reg.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h        | 3 ++-
 target-arm/helper-a64.c | 2 +-
 target-arm/helper.c     | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 2a068ec..ff86250 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -200,7 +200,8 @@ typedef struct CPUARMState {
         uint32_t c9_pmuserenr; /* perf monitor user enable */
         uint32_t c9_pminten; /* perf monitor interrupt enables */
         uint64_t mair_el1;
-        uint64_t c12_vbar; /* vector base address register */
+#define VBAR_EL_IDX(x) (x - 1)
+        uint64_t vbar_el[1]; /* vector base address register */
         uint32_t c13_fcse; /* FCSE PID.  */
         uint64_t contextidr_el1; /* Context ID.  */
         uint64_t tpidr_el0; /* User RW Thread register.  */
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 4bee075..10bd1fc 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -443,7 +443,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
-    target_ulong addr = env->cp15.c12_vbar;
+    target_ulong addr = env->cp15.vbar_el[VBAR_EL_IDX(1)];
     int i;
 
     if (arm_current_pl(env) == 0) {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index c86fab6..baeaa28 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
      * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7
      * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.)
      */
-    env->cp15.c12_vbar = value & ~0x1FULL;
+    env->cp15.vbar_el[VBAR_EL_IDX(1)] = value & ~0x1FULL;
 }
 
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
@@ -766,7 +766,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
     { .name = "VBAR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 12, .crm = 0, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .writefn = vbar_write,
-      .fieldoffset = offsetof(CPUARMState, cp15.c12_vbar),
+      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[VBAR_EL_IDX(1)]),
       .resetvalue = 0 },
     { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
@@ -3373,7 +3373,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
          * and is never in monitor mode this feature is always active.
          * Note: only bits 31:5 are valid.
          */
-        addr += env->cp15.c12_vbar;
+        addr += env->cp15.vbar_el[VBAR_EL_IDX(1)];
     }
     switch_mode (env, new_mode);
     env->spsr = cpsr_read(env);
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 05/22] target-arm: Add arm_el_to_mmu_idx()

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Maps a given EL to the corresponding MMU index.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h           | 21 ++++++++++++++++++++-
 target-arm/translate-a64.c |  8 ++------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ff86250..938f389 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
 #define MMU_MODE0_SUFFIX _kernel
 #define MMU_MODE1_SUFFIX _user
 #define MMU_USER_IDX 1
+static inline int arm_el_to_mmu_idx(int current_el)
+{
+#ifdef CONFIG_USER_ONLY
+    return MMU_USER_IDX;
+#else
+    switch (current_el) {
+    case 0:
+        return MMU_USER_IDX;
+    case 1:
+        return 0;
+    default:
+        /* Unsupported EL.  */
+        assert(0);
+        return 0;
+    }
+#endif
+}
+
 static inline int cpu_mmu_index (CPUARMState *env)
 {
-    return arm_current_pl(env) ? 0 : 1;
+    int cur_el = arm_current_pl(env);
+    return arm_el_to_mmu_idx(cur_el);
 }
 
 #include "exec/cpu-all.h"
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 4f8246f..8523e76 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -164,13 +164,9 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     }
 }
 
-static int get_mem_index(DisasContext *s)
+static inline int get_mem_index(DisasContext *s)
 {
-#ifdef CONFIG_USER_ONLY
-    return 1;
-#else
-    return s->user;
-#endif
+    return arm_el_to_mmu_idx(s->current_pl);
 }
 
 void gen_a64_set_pc_im(uint64_t val)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 05/22] target-arm: Add arm_el_to_mmu_idx()

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Maps a given EL to the corresponding MMU index.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h           | 21 ++++++++++++++++++++-
>  target-arm/translate-a64.c |  8 ++------
>  2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ff86250..938f389 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
>  #define MMU_MODE0_SUFFIX _kernel
>  #define MMU_MODE1_SUFFIX _user
>  #define MMU_USER_IDX 1

Blank line here.

Otherwise:

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

> +static inline int arm_el_to_mmu_idx(int current_el)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return MMU_USER_IDX;
> +#else
> +    switch (current_el) {
> +    case 0:
> +        return MMU_USER_IDX;
> +    case 1:
> +        return 0;
> +    default:
> +        /* Unsupported EL.  */
> +        assert(0);
> +        return 0;
> +    }
> +#endif
> +}
> +
>  static inline int cpu_mmu_index (CPUARMState *env)
>  {
> -    return arm_current_pl(env) ? 0 : 1;
> +    int cur_el = arm_current_pl(env);
> +    return arm_el_to_mmu_idx(cur_el);
>  }
>
>  #include "exec/cpu-all.h"
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 4f8246f..8523e76 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -164,13 +164,9 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>      }
>  }
>
> -static int get_mem_index(DisasContext *s)
> +static inline int get_mem_index(DisasContext *s)
>  {
> -#ifdef CONFIG_USER_ONLY
> -    return 1;
> -#else
> -    return s->user;
> -#endif
> +    return arm_el_to_mmu_idx(s->current_pl);
>  }
>
>  void gen_a64_set_pc_im(uint64_t val)
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 05/22] target-arm: Add arm_el_to_mmu_idx()

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Maps a given EL to the corresponding MMU index.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h           | 21 ++++++++++++++++++++-
>  target-arm/translate-a64.c |  8 ++------
>  2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ff86250..938f389 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
>  #define MMU_MODE0_SUFFIX _kernel
>  #define MMU_MODE1_SUFFIX _user
>  #define MMU_USER_IDX 1
> +static inline int arm_el_to_mmu_idx(int current_el)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return MMU_USER_IDX;
> +#else
> +    switch (current_el) {
> +    case 0:
> +        return MMU_USER_IDX;
> +    case 1:
> +        return 0;
> +    default:
> +        /* Unsupported EL.  */
> +        assert(0);
> +        return 0;
> +    }
> +#endif

Can we just make the EL and the MMU index the same thing,
or is secure-vs-nonsecure going to need its own MMU
indexes anyway?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 05/22] target-arm: Add arm_el_to_mmu_idx()

[Edgar E. Iglesias]

On Fri, May 16, 2014 at 03:24:42PM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Maps a given EL to the corresponding MMU index.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h           | 21 ++++++++++++++++++++-
> >  target-arm/translate-a64.c |  8 ++------
> >  2 files changed, 22 insertions(+), 7 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index ff86250..938f389 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
> >  #define MMU_MODE0_SUFFIX _kernel
> >  #define MMU_MODE1_SUFFIX _user
> >  #define MMU_USER_IDX 1
> > +static inline int arm_el_to_mmu_idx(int current_el)
> > +{
> > +#ifdef CONFIG_USER_ONLY
> > +    return MMU_USER_IDX;
> > +#else
> > +    switch (current_el) {
> > +    case 0:
> > +        return MMU_USER_IDX;
> > +    case 1:
> > +        return 0;
> > +    default:
> > +        /* Unsupported EL.  */
> > +        assert(0);
> > +        return 0;
> > +    }
> > +#endif
> 
> Can we just make the EL and the MMU index the same thing,
> or is secure-vs-nonsecure going to need its own MMU
> indexes anyway?

Right, I did the conversion to 1:1 mapping at an early stage
but avoided it as we will need an indirect mapping for
Secure EL0/1 anyway.

I still have the patches around but they will conflict with the other
trustzone patches on the list so I'd rather avoid them for now.

Thanks,
Edgar

Re: [Qemu-devel] [PATCH v1 05/22] target-arm: Add arm_el_to_mmu_idx()

[Alexander Graf]

On 17.05.14 00:10, Edgar E. Iglesias wrote:
> On Fri, May 16, 2014 at 03:24:42PM +0100, Peter Maydell wrote:
>> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>>
>>> Maps a given EL to the corresponding MMU index.
>>>
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>>> ---
>>>   target-arm/cpu.h           | 21 ++++++++++++++++++++-
>>>   target-arm/translate-a64.c |  8 ++------
>>>   2 files changed, 22 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> index ff86250..938f389 100644
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
>>>   #define MMU_MODE0_SUFFIX _kernel
>>>   #define MMU_MODE1_SUFFIX _user
>>>   #define MMU_USER_IDX 1
>>> +static inline int arm_el_to_mmu_idx(int current_el)
>>> +{
>>> +#ifdef CONFIG_USER_ONLY
>>> +    return MMU_USER_IDX;
>>> +#else
>>> +    switch (current_el) {
>>> +    case 0:
>>> +        return MMU_USER_IDX;
>>> +    case 1:
>>> +        return 0;
>>> +    default:
>>> +        /* Unsupported EL.  */
>>> +        assert(0);
>>> +        return 0;
>>> +    }
>>> +#endif
>> Can we just make the EL and the MMU index the same thing,
>> or is secure-vs-nonsecure going to need its own MMU
>> indexes anyway?
> Right, I did the conversion to 1:1 mapping at an early stage
> but avoided it as we will need an indirect mapping for
> Secure EL0/1 anyway.

How often do we switch between secure and non-secure? If it doesn't 
happen all that often, we could just flush the TLB on every transition.


Alex

Re: [Qemu-devel] [PATCH v1 05/22] target-arm: Add arm_el_to_mmu_idx()

[Edgar E. Iglesias]

On Sat, May 17, 2014 at 12:13:06AM +0200, Alexander Graf wrote:
> 
> On 17.05.14 00:10, Edgar E. Iglesias wrote:
> >On Fri, May 16, 2014 at 03:24:42PM +0100, Peter Maydell wrote:
> >>On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> >>>From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >>>
> >>>Maps a given EL to the corresponding MMU index.
> >>>
> >>>Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >>>---
> >>>  target-arm/cpu.h           | 21 ++++++++++++++++++++-
> >>>  target-arm/translate-a64.c |  8 ++------
> >>>  2 files changed, 22 insertions(+), 7 deletions(-)
> >>>
> >>>diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> >>>index ff86250..938f389 100644
> >>>--- a/target-arm/cpu.h
> >>>+++ b/target-arm/cpu.h
> >>>@@ -1086,9 +1086,28 @@ static inline CPUARMState *cpu_init(const char *cpu_model)
> >>>  #define MMU_MODE0_SUFFIX _kernel
> >>>  #define MMU_MODE1_SUFFIX _user
> >>>  #define MMU_USER_IDX 1
> >>>+static inline int arm_el_to_mmu_idx(int current_el)
> >>>+{
> >>>+#ifdef CONFIG_USER_ONLY
> >>>+    return MMU_USER_IDX;
> >>>+#else
> >>>+    switch (current_el) {
> >>>+    case 0:
> >>>+        return MMU_USER_IDX;
> >>>+    case 1:
> >>>+        return 0;
> >>>+    default:
> >>>+        /* Unsupported EL.  */
> >>>+        assert(0);
> >>>+        return 0;
> >>>+    }
> >>>+#endif
> >>Can we just make the EL and the MMU index the same thing,
> >>or is secure-vs-nonsecure going to need its own MMU
> >>indexes anyway?
> >Right, I did the conversion to 1:1 mapping at an early stage
> >but avoided it as we will need an indirect mapping for
> >Secure EL0/1 anyway.
> 
> How often do we switch between secure and non-secure? If it doesn't happen
> all that often, we could just flush the TLB on every transition.

That's an option. I think this mostly affects aarch32 as that is where the
TTBR regs are banked. For aarch64 I think the world switching FW has
to rewrite the TTBR regs leading to TLB flushes anyway with current code.
IIUC..

I can include a switch over to 1:1 mapping between EL and MMU-idx if
preferd. I already have the patches but they will make this series
conflict alot more with the TZ patches on list.

Thanks,
Edgar

[Qemu-devel] [PATCH v1 06/22] target-arm: Move get_mem_index to translate.h

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

So that it can be shared with the A32 code in the future.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/translate-a64.c | 5 -----
 target-arm/translate.h     | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 8523e76..93fc4a5 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -164,11 +164,6 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     }
 }
 
-static inline int get_mem_index(DisasContext *s)
-{
-    return arm_el_to_mmu_idx(s->current_pl);
-}
-
 void gen_a64_set_pc_im(uint64_t val)
 {
     tcg_gen_movi_i64(cpu_pc, val);
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 34328f4..db6f0af 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -52,6 +52,11 @@ static inline int arm_dc_feature(DisasContext *dc, int feature)
     return (dc->features & (1ULL << feature)) != 0;
 }
 
+static inline int get_mem_index(DisasContext *s)
+{
+    return arm_el_to_mmu_idx(s->current_pl);
+}
+
 /* target-specific extra values for is_jmp */
 /* These instructions trap after executing, so the A32/T32 decoder must
  * defer them until after the conditional execution state has been updated.
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 07/22] target-arm: A64: Add SP entries for EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h     | 2 +-
 target-arm/machine.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 938f389..7bac416 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -164,7 +164,7 @@ typedef struct CPUARMState {
 
 #define ELR_EL_IDX(x) (x - 1)
     uint64_t elr_el[1]; /* AArch64 exception link regs  */
-    uint64_t sp_el[2]; /* AArch64 banked stack pointers */
+    uint64_t sp_el[4]; /* AArch64 banked stack pointers */
 
     /* System control coprocessor (cp15) */
     struct {
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 8b299a0..6a87ef6 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 = 17,
-    .minimum_version_id = 17,
-    .minimum_version_id_old = 17,
+    .version_id = 18,
+    .minimum_version_id = 18,
+    .minimum_version_id_old = 18,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -244,7 +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_el[ELR_EL_IDX(1)], ARMCPU),
-        VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 2),
+        VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 4),
         /* The length-check must come before the arrays to avoid
          * incoming data possibly overflowing the array.
          */
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 08/22] target-arm: A64: Add ELR entries for EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h     | 2 +-
 target-arm/machine.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7bac416..fd8ce70 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -163,7 +163,7 @@ typedef struct CPUARMState {
     uint64_t daif; /* exception masks, in the bits they are in in PSTATE */
 
 #define ELR_EL_IDX(x) (x - 1)
-    uint64_t elr_el[1]; /* AArch64 exception link regs  */
+    uint64_t elr_el[3]; /* AArch64 exception link regs  */
     uint64_t sp_el[4]; /* AArch64 banked stack pointers */
 
     /* System control coprocessor (cp15) */
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 6a87ef6..92ac621 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 = 18,
-    .minimum_version_id = 18,
-    .minimum_version_id_old = 18,
+    .version_id = 19,
+    .minimum_version_id = 19,
+    .minimum_version_id_old = 19,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -243,7 +243,7 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
         VMSTATE_UINT32_ARRAY(env.fiq_regs, ARMCPU, 5),
-        VMSTATE_UINT64(env.elr_el[ELR_EL_IDX(1)], ARMCPU),
+        VMSTATE_UINT64_ARRAY(env.elr_el, ARMCPU, 3),
         VMSTATE_UINT64_ARRAY(env.sp_el, ARMCPU, 4),
         /* The length-check must come before the arrays to avoid
          * incoming data possibly overflowing the array.
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 09/22] target-arm: Add SPSR entries for EL2/HYP and EL3/MON

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h     | 4 +++-
 target-arm/helper.c  | 4 ++++
 target-arm/machine.c | 8 ++++----
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index fd8ce70..6e6625b 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -143,7 +143,7 @@ typedef struct CPUARMState {
     uint32_t spsr;
 
     /* Banked registers.  */
-    uint64_t banked_spsr[6];
+    uint64_t banked_spsr[8];
     uint32_t banked_r13[6];
     uint32_t banked_r14[6];
 
@@ -566,7 +566,9 @@ enum arm_cpu_mode {
   ARM_CPU_MODE_FIQ = 0x11,
   ARM_CPU_MODE_IRQ = 0x12,
   ARM_CPU_MODE_SVC = 0x13,
+  ARM_CPU_MODE_MON = 0x16,
   ARM_CPU_MODE_ABT = 0x17,
+  ARM_CPU_MODE_HYP = 0x1a,
   ARM_CPU_MODE_UND = 0x1b,
   ARM_CPU_MODE_SYS = 0x1f
 };
diff --git a/target-arm/helper.c b/target-arm/helper.c
index baeaa28..ba1830d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3078,6 +3078,10 @@ int bank_number(int mode)
         return 4;
     case ARM_CPU_MODE_FIQ:
         return 5;
+    case ARM_CPU_MODE_HYP:
+        return 6;
+    case ARM_CPU_MODE_MON:
+        return 7;
     }
     hw_error("bank number requested for bad CPSR mode value 0x%x\n", mode);
 }
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 92ac621..e95be47 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 = 19,
-    .minimum_version_id = 19,
-    .minimum_version_id_old = 19,
+    .version_id = 20,
+    .minimum_version_id = 20,
+    .minimum_version_id_old = 20,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -238,7 +238,7 @@ const VMStateDescription vmstate_arm_cpu = {
             .offset = 0,
         },
         VMSTATE_UINT32(env.spsr, ARMCPU),
-        VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 6),
+        VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
         VMSTATE_UINT32_ARRAY(env.banked_r13, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
         VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 09/22] target-arm: Add SPSR entries for EL2/HYP and EL3/MON

[Edgar E. Iglesias]

On Tue, May 06, 2014 at 04:08:13PM +1000, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Noticed I missed updating cpu_mode_names[]
Queued an update to translate.c for v2.

Cheers,
Edgar

> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h     | 4 +++-
>  target-arm/helper.c  | 4 ++++
>  target-arm/machine.c | 8 ++++----
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fd8ce70..6e6625b 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -143,7 +143,7 @@ typedef struct CPUARMState {
>      uint32_t spsr;
>  
>      /* Banked registers.  */
> -    uint64_t banked_spsr[6];
> +    uint64_t banked_spsr[8];
>      uint32_t banked_r13[6];
>      uint32_t banked_r14[6];
>  
> @@ -566,7 +566,9 @@ enum arm_cpu_mode {
>    ARM_CPU_MODE_FIQ = 0x11,
>    ARM_CPU_MODE_IRQ = 0x12,
>    ARM_CPU_MODE_SVC = 0x13,
> +  ARM_CPU_MODE_MON = 0x16,
>    ARM_CPU_MODE_ABT = 0x17,
> +  ARM_CPU_MODE_HYP = 0x1a,
>    ARM_CPU_MODE_UND = 0x1b,
>    ARM_CPU_MODE_SYS = 0x1f
>  };
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index baeaa28..ba1830d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3078,6 +3078,10 @@ int bank_number(int mode)
>          return 4;
>      case ARM_CPU_MODE_FIQ:
>          return 5;
> +    case ARM_CPU_MODE_HYP:
> +        return 6;
> +    case ARM_CPU_MODE_MON:
> +        return 7;
>      }
>      hw_error("bank number requested for bad CPSR mode value 0x%x\n", mode);
>  }
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 92ac621..e95be47 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 = 19,
> -    .minimum_version_id = 19,
> -    .minimum_version_id_old = 19,
> +    .version_id = 20,
> +    .minimum_version_id = 20,
> +    .minimum_version_id_old = 20,
>      .pre_save = cpu_pre_save,
>      .post_load = cpu_post_load,
>      .fields = (VMStateField[]) {
> @@ -238,7 +238,7 @@ const VMStateDescription vmstate_arm_cpu = {
>              .offset = 0,
>          },
>          VMSTATE_UINT32(env.spsr, ARMCPU),
> -        VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 6),
> +        VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
>          VMSTATE_UINT32_ARRAY(env.banked_r13, ARMCPU, 6),
>          VMSTATE_UINT32_ARRAY(env.banked_r14, ARMCPU, 6),
>          VMSTATE_UINT32_ARRAY(env.usr_regs, ARMCPU, 5),
> -- 
> 1.8.3.2
> 

[Qemu-devel] [PATCH v1 10/22] target-arm: A64: Introduce arm64_banked_spsr_index()

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add arm64_banked_spsr_index(), used to map an Exception Level
to an index in the baked_spsr array.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper-a64.c |  5 +++--
 target-arm/internals.h  | 14 ++++++++++++++
 target-arm/op_helper.c  |  3 ++-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 10bd1fc..415efbe 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -444,6 +444,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     ARMCPU *cpu = ARM_CPU(cs);
     CPUARMState *env = &cpu->env;
     target_ulong addr = env->cp15.vbar_el[VBAR_EL_IDX(1)];
+    unsigned int spsr_idx = arm64_banked_spsr_index(1);
     int i;
 
     if (arm_current_pl(env) == 0) {
@@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     }
 
     if (is_a64(env)) {
-        env->banked_spsr[0] = pstate_read(env);
+        env->banked_spsr[spsr_idx] = pstate_read(env);
         env->sp_el[arm_current_pl(env)] = env->xregs[31];
         env->xregs[31] = env->sp_el[1];
         env->elr_el[ELR_EL_IDX(1)] = env->pc;
     } else {
-        env->banked_spsr[0] = cpsr_read(env);
+        env->banked_spsr[spsr_idx] = cpsr_read(env);
         if (!env->thumb) {
             env->cp15.esr_el[ESR_EL_IDX(1)] |= 1 << 25;
         }
diff --git a/target-arm/internals.h b/target-arm/internals.h
index d63a975..7c39946 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -75,6 +75,20 @@ static inline void arm_log_exception(int idx)
  */
 #define GTIMER_SCALE 16
 
+/*
+ * For aarch64, map a given EL to an index in the banked_spsr array.
+ */
+static inline unsigned int arm64_banked_spsr_index(unsigned int el)
+{
+    static const unsigned int map[3] = {
+        [0] = 0, /* EL1.  */
+        [1] = 6, /* EL2.  */
+        [2] = 7, /* EL3.  */
+    };
+    assert(el >= 1 && el <= 3);
+    return map[el - 1];
+}
+
 int bank_number(int mode);
 void switch_mode(CPUARMState *, int);
 void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 21545d0..dd9e4fc 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -386,7 +386,8 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
 
 void HELPER(exception_return)(CPUARMState *env)
 {
-    uint32_t spsr = env->banked_spsr[0];
+    unsigned int spsr_idx = arm64_banked_spsr_index(1);
+    uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el, i;
 
     if (env->pstate & PSTATE_SP) {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 10/22] target-arm: A64: Introduce arm64_banked_spsr_index()

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add arm64_banked_spsr_index(), used to map an Exception Level
> to an index in the baked_spsr array.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper-a64.c |  5 +++--
>  target-arm/internals.h  | 14 ++++++++++++++
>  target-arm/op_helper.c  |  3 ++-
>  3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 10bd1fc..415efbe 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -444,6 +444,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
>      target_ulong addr = env->cp15.vbar_el[VBAR_EL_IDX(1)];
> +    unsigned int spsr_idx = arm64_banked_spsr_index(1);
>      int i;
>
>      if (arm_current_pl(env) == 0) {
> @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      }
>
>      if (is_a64(env)) {
> -        env->banked_spsr[0] = pstate_read(env);
> +        env->banked_spsr[spsr_idx] = pstate_read(env);
>          env->sp_el[arm_current_pl(env)] = env->xregs[31];
>          env->xregs[31] = env->sp_el[1];
>          env->elr_el[ELR_EL_IDX(1)] = env->pc;
>      } else {
> -        env->banked_spsr[0] = cpsr_read(env);
> +        env->banked_spsr[spsr_idx] = cpsr_read(env);
>          if (!env->thumb) {
>              env->cp15.esr_el[ESR_EL_IDX(1)] |= 1 << 25;
>          }
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index d63a975..7c39946 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -75,6 +75,20 @@ static inline void arm_log_exception(int idx)
>   */
>  #define GTIMER_SCALE 16
>
> +/*
> + * For aarch64, map a given EL to an index in the banked_spsr array.
> + */
> +static inline unsigned int arm64_banked_spsr_index(unsigned int el)
> +{
> +    static const unsigned int map[3] = {
> +        [0] = 0, /* EL1.  */
> +        [1] = 6, /* EL2.  */
> +        [2] = 7, /* EL3.  */

You could just change the [] indicies to [1], [2], [3]

> +    };
> +    assert(el >= 1 && el <= 3);
> +    return map[el - 1];

And drop this subtraction.

Regards,
Peter

> +}
> +
>  int bank_number(int mode);
>  void switch_mode(CPUARMState *, int);
>  void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 21545d0..dd9e4fc 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -386,7 +386,8 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>
>  void HELPER(exception_return)(CPUARMState *env)
>  {
> -    uint32_t spsr = env->banked_spsr[0];
> +    unsigned int spsr_idx = arm64_banked_spsr_index(1);
> +    uint32_t spsr = env->banked_spsr[spsr_idx];
>      int new_el, i;
>
>      if (env->pstate & PSTATE_SP) {
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 10/22] target-arm: A64: Introduce arm64_banked_spsr_index()

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add arm64_banked_spsr_index(), used to map an Exception Level
> to an index in the baked_spsr array.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper-a64.c |  5 +++--
>  target-arm/internals.h  | 14 ++++++++++++++
>  target-arm/op_helper.c  |  3 ++-
>  3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 10bd1fc..415efbe 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -444,6 +444,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
>      target_ulong addr = env->cp15.vbar_el[VBAR_EL_IDX(1)];
> +    unsigned int spsr_idx = arm64_banked_spsr_index(1);
>      int i;
>
>      if (arm_current_pl(env) == 0) {
> @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>      }
>
>      if (is_a64(env)) {
> -        env->banked_spsr[0] = pstate_read(env);
> +        env->banked_spsr[spsr_idx] = pstate_read(env);
>          env->sp_el[arm_current_pl(env)] = env->xregs[31];
>          env->xregs[31] = env->sp_el[1];
>          env->elr_el[ELR_EL_IDX(1)] = env->pc;
>      } else {
> -        env->banked_spsr[0] = cpsr_read(env);
> +        env->banked_spsr[spsr_idx] = cpsr_read(env);
>          if (!env->thumb) {
>              env->cp15.esr_el[ESR_EL_IDX(1)] |= 1 << 25;
>          }

This looks bogus -- the function you've added is only
valid if we are taking the exception from AArch64, but we
use the spsr_idx in the from-AArch32 case as well.

> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index d63a975..7c39946 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -75,6 +75,20 @@ static inline void arm_log_exception(int idx)
>   */
>  #define GTIMER_SCALE 16
>
> +/*
> + * For aarch64, map a given EL to an index in the banked_spsr array.

In comments, "AArch64", please.

> + */
> +static inline unsigned int arm64_banked_spsr_index(unsigned int el)

"aarch64", not "arm64", please.

> +{
> +    static const unsigned int map[3] = {
> +        [0] = 0, /* EL1.  */
> +        [1] = 6, /* EL2.  */
> +        [2] = 7, /* EL3.  */
> +    };
> +    assert(el >= 1 && el <= 3);
> +    return map[el - 1];
> +}
> +

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 10/22] target-arm: A64: Introduce arm64_banked_spsr_index()

[Edgar E. Iglesias]

On Fri, May 16, 2014 at 03:31:16PM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Add arm64_banked_spsr_index(), used to map an Exception Level
> > to an index in the baked_spsr array.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/helper-a64.c |  5 +++--
> >  target-arm/internals.h  | 14 ++++++++++++++
> >  target-arm/op_helper.c  |  3 ++-
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> > index 10bd1fc..415efbe 100644
> > --- a/target-arm/helper-a64.c
> > +++ b/target-arm/helper-a64.c
> > @@ -444,6 +444,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >      ARMCPU *cpu = ARM_CPU(cs);
> >      CPUARMState *env = &cpu->env;
> >      target_ulong addr = env->cp15.vbar_el[VBAR_EL_IDX(1)];
> > +    unsigned int spsr_idx = arm64_banked_spsr_index(1);
> >      int i;
> >
> >      if (arm_current_pl(env) == 0) {
> > @@ -488,12 +489,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> >      }
> >
> >      if (is_a64(env)) {
> > -        env->banked_spsr[0] = pstate_read(env);
> > +        env->banked_spsr[spsr_idx] = pstate_read(env);
> >          env->sp_el[arm_current_pl(env)] = env->xregs[31];
> >          env->xregs[31] = env->sp_el[1];
> >          env->elr_el[ELR_EL_IDX(1)] = env->pc;
> >      } else {
> > -        env->banked_spsr[0] = cpsr_read(env);
> > +        env->banked_spsr[spsr_idx] = cpsr_read(env);
> >          if (!env->thumb) {
> >              env->cp15.esr_el[ESR_EL_IDX(1)] |= 1 << 25;
> >          }
> 
> This looks bogus -- the function you've added is only
> valid if we are taking the exception from AArch64, but we
> use the spsr_idx in the from-AArch32 case as well.

Good catch, thanks!

> 
> > diff --git a/target-arm/internals.h b/target-arm/internals.h
> > index d63a975..7c39946 100644
> > --- a/target-arm/internals.h
> > +++ b/target-arm/internals.h
> > @@ -75,6 +75,20 @@ static inline void arm_log_exception(int idx)
> >   */
> >  #define GTIMER_SCALE 16
> >
> > +/*
> > + * For aarch64, map a given EL to an index in the banked_spsr array.
> 
> In comments, "AArch64", please.
> 
> > + */
> > +static inline unsigned int arm64_banked_spsr_index(unsigned int el)
> 
> "aarch64", not "arm64", please.


Will fix

Cheers,
Edgar

[Qemu-devel] [PATCH v1 11/22] target-arm: Add a feature flag for EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6e6625b..d2e52d4 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -636,6 +636,7 @@ enum arm_features {
     ARM_FEATURE_CBAR, /* has cp15 CBAR */
     ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
     ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
+    ARM_FEATURE_EL2, /* has EL2 Virtualization support */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 11/22] target-arm: Add a feature flag for EL2

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

> ---
>  target-arm/cpu.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 6e6625b..d2e52d4 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -636,6 +636,7 @@ enum arm_features {
>      ARM_FEATURE_CBAR, /* has cp15 CBAR */
>      ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
>      ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
> +    ARM_FEATURE_EL2, /* has EL2 Virtualization support */
>  };
>
>  static inline int arm_feature(CPUARMState *env, int feature)
> --
> 1.8.3.2
>
>

[Qemu-devel] [PATCH v1 12/22] target-arm: Add a feature flag for EL3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d2e52d4..34e8f7c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -637,6 +637,7 @@ enum arm_features {
     ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
     ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
     ARM_FEATURE_EL2, /* has EL2 Virtualization support */
+    ARM_FEATURE_EL3, /* has EL3 Secure monitor support */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 12/22] target-arm: Add a feature flag for EL3

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

> ---
>  target-arm/cpu.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index d2e52d4..34e8f7c 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -637,6 +637,7 @@ enum arm_features {
>      ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
>      ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
>      ARM_FEATURE_EL2, /* has EL2 Virtualization support */
> +    ARM_FEATURE_EL3, /* has EL3 Secure monitor support */
>  };
>
>  static inline int arm_feature(CPUARMState *env, int feature)
> --
> 1.8.3.2
>
>

[Qemu-devel] [PATCH v1 13/22] target-arm: Register EL2 versions of ELR and SPSR

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index ba1830d..8efc340 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2078,6 +2078,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
+    { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
+      .access = PL2_RW,
+      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(2)]) },
+    { .name = "SPSR_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
+      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
+    REGINFO_SENTINEL
+};
+
 static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                         uint64_t value)
 {
@@ -2321,6 +2334,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, v8_idregs);
         define_arm_cp_regs(cpu, v8_cp_reginfo);
         define_aarch64_debug_regs(cpu);
+
+        if (arm_feature(env, ARM_FEATURE_EL2)) {
+            define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
+        }
     }
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         /* These are the MPU registers prior to PMSAv6. Any new
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 13/22] target-arm: Register EL2 versions of ELR and SPSR

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ba1830d..8efc340 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2078,6 +2078,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>      REGINFO_SENTINEL
>  };
>
> +static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> +    { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
> +      .access = PL2_RW,
> +      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(2)]) },
> +    { .name = "SPSR_EL2", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
> +    REGINFO_SENTINEL
> +};
> +
>  static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                          uint64_t value)
>  {
> @@ -2321,6 +2334,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>          define_arm_cp_regs(cpu, v8_idregs);
>          define_arm_cp_regs(cpu, v8_cp_reginfo);
>          define_aarch64_debug_regs(cpu);
> +
> +        if (arm_feature(env, ARM_FEATURE_EL2)) {
> +            define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> +        }

I think this should be outside the if ARM_FEATURE_V8 for consistency.
None of the other per-feature CP register defs are nested within the
ifferry for their ARM version. Detecting the invalid combination of
ARM_FEATURE_EL2 and pre V8 should probably be an assertion done in
arm_cpu_realizefn().

Otherwise:

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

>      }
>      if (arm_feature(env, ARM_FEATURE_MPU)) {
>          /* These are the MPU registers prior to PMSAv6. Any new
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 13/22] target-arm: Register EL2 versions of ELR and SPSR

[Peter Maydell]

On 7 May 2014 07:02, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>
>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> ---
>>  target-arm/helper.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index ba1830d..8efc340 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -2078,6 +2078,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>>      REGINFO_SENTINEL
>>  };
>>
>> +static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>> +    { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>> +      .type = ARM_CP_NO_MIGRATE,
>> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
>> +      .access = PL2_RW,
>> +      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(2)]) },
>> +    { .name = "SPSR_EL2", .state = ARM_CP_STATE_AA64,
>> +      .type = ARM_CP_NO_MIGRATE,
>> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
>> +    REGINFO_SENTINEL
>> +};
>> +
>>  static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>                          uint64_t value)
>>  {
>> @@ -2321,6 +2334,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>>          define_arm_cp_regs(cpu, v8_idregs);
>>          define_arm_cp_regs(cpu, v8_cp_reginfo);
>>          define_aarch64_debug_regs(cpu);
>> +
>> +        if (arm_feature(env, ARM_FEATURE_EL2)) {
>> +            define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
>> +        }
>
> I think this should be outside the if ARM_FEATURE_V8 for consistency.
> None of the other per-feature CP register defs are nested within the
> ifferry for their ARM version. Detecting the invalid combination of
> ARM_FEATURE_EL2 and pre V8 should probably be an assertion done in
> arm_cpu_realizefn().

ARM_FEATURE_EL2 + pre-V8 should be valid, that seems the obvious
way to indicate "v7 CPU with virt extensions".

As it happens registering these reginfo on a pre-v8 CPU is
harmless -- they'll just never be accessible.

thanks
-- PMM

[Qemu-devel] [PATCH v1 14/22] target-arm: Register EL3 versions of ELR and SPSR

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8efc340..65daeaf 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2091,6 +2091,19 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
+    { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 1,
+      .access = PL3_RW,
+      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(3)]) },
+    { .name = "SPSR_EL3", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_NO_MIGRATE,
+      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0,
+      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) },
+    REGINFO_SENTINEL
+};
+
 static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                         uint64_t value)
 {
@@ -2338,6 +2351,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         if (arm_feature(env, ARM_FEATURE_EL2)) {
             define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
         }
+        if (arm_feature(env, ARM_FEATURE_EL3)) {
+            define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
+        }
     }
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         /* These are the MPU registers prior to PMSAv6. Any new
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 14/22] target-arm: Register EL3 versions of ELR and SPSR

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Same as last patch,

Otherwise:

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

> ---
>  target-arm/helper.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 8efc340..65daeaf 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2091,6 +2091,19 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>      REGINFO_SENTINEL
>  };
>
> +static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
> +    { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 1,
> +      .access = PL3_RW,
> +      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(3)]) },
> +    { .name = "SPSR_EL3", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0,
> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) },
> +    REGINFO_SENTINEL
> +};
> +
>  static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                          uint64_t value)
>  {
> @@ -2338,6 +2351,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>          if (arm_feature(env, ARM_FEATURE_EL2)) {
>              define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
>          }
> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> +        }
>      }
>      if (arm_feature(env, ARM_FEATURE_MPU)) {
>          /* These are the MPU registers prior to PMSAv6. Any new
> --
> 1.8.3.2
>
>

[Qemu-devel] [PATCH v1 15/22] target-arm: A64: Forbid ERET to increase the EL

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index dd9e4fc..770c776 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -389,6 +389,7 @@ void HELPER(exception_return)(CPUARMState *env)
     unsigned int spsr_idx = arm64_banked_spsr_index(1);
     uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el, i;
+    int cur_el = arm_current_pl(env);
 
     if (env->pstate & PSTATE_SP) {
         env->sp_el[1] = env->xregs[31];
@@ -410,6 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
         env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
     } else {
         new_el = extract32(spsr, 2, 2);
+        if (new_el > cur_el) {
+            /* Disallow returns to higher ELs than the current one.  */
+            goto illegal_return;
+        }
         if (new_el > 1) {
             /* Return to unimplemented EL */
             goto illegal_return;
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 15/22] target-arm: A64: Forbid ERET to increase the EL

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

> ---
>  target-arm/op_helper.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index dd9e4fc..770c776 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -389,6 +389,7 @@ void HELPER(exception_return)(CPUARMState *env)
>      unsigned int spsr_idx = arm64_banked_spsr_index(1);
>      uint32_t spsr = env->banked_spsr[spsr_idx];
>      int new_el, i;
> +    int cur_el = arm_current_pl(env);
>
>      if (env->pstate & PSTATE_SP) {
>          env->sp_el[1] = env->xregs[31];
> @@ -410,6 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
>      } else {
>          new_el = extract32(spsr, 2, 2);
> +        if (new_el > cur_el) {
> +            /* Disallow returns to higher ELs than the current one.  */
> +            goto illegal_return;
> +        }
>          if (new_el > 1) {
>              /* Return to unimplemented EL */
>              goto illegal_return;
> --
> 1.8.3.2
>
>

[Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Check for EL2 support before returning to it.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 770c776..f1ae05e 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
         env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
     } else {
         new_el = extract32(spsr, 2, 2);
-        if (new_el > cur_el) {
+        if (new_el > cur_el
+            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
             /* Disallow returns to higher ELs than the current one.  */
-            goto illegal_return;
-        }
-        if (new_el > 1) {
-            /* Return to unimplemented EL */
+            /* Disallow returns to unimplemented ELs.  */
             goto illegal_return;
         }
         if (extract32(spsr, 1, 1)) {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Check for EL2 support before returning to it.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

> ---
>  target-arm/op_helper.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 770c776..f1ae05e 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
>      } else {
>          new_el = extract32(spsr, 2, 2);
> -        if (new_el > cur_el) {
> +        if (new_el > cur_el
> +            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
>              /* Disallow returns to higher ELs than the current one.  */
> -            goto illegal_return;
> -        }
> -        if (new_el > 1) {
> -            /* Return to unimplemented EL */
> +            /* Disallow returns to unimplemented ELs.  */
>              goto illegal_return;
>          }
>          if (extract32(spsr, 1, 1)) {
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Check for EL2 support before returning to it.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/op_helper.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 770c776..f1ae05e 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
>      } else {
>          new_el = extract32(spsr, 2, 2);
> -        if (new_el > cur_el) {
> +        if (new_el > cur_el
> +            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
>              /* Disallow returns to higher ELs than the current one.  */
> -            goto illegal_return;
> -        }
> -        if (new_el > 1) {
> -            /* Return to unimplemented EL */
> +            /* Disallow returns to unimplemented ELs.  */

Merge the comments rather than having two one-liners one after
the other, please.
 /* Disallow return to an EL which is unimplemented or higher
  * than the current one.
  */

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs

[Edgar E. Iglesias]

On Wed, May 07, 2014 at 10:00:11AM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Check for EL2 support before returning to it.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/op_helper.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> > index 770c776..f1ae05e 100644
> > --- a/target-arm/op_helper.c
> > +++ b/target-arm/op_helper.c
> > @@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env)
> >          env->regs[15] = env->elr_el[ELR_EL_IDX(1)] & ~0x1;
> >      } else {
> >          new_el = extract32(spsr, 2, 2);
> > -        if (new_el > cur_el) {
> > +        if (new_el > cur_el
> > +            || (new_el == 2 && !arm_feature(env, ARM_FEATURE_EL2))) {
> >              /* Disallow returns to higher ELs than the current one.  */
> > -            goto illegal_return;
> > -        }
> > -        if (new_el > 1) {
> > -            /* Return to unimplemented EL */
> > +            /* Disallow returns to unimplemented ELs.  */
> 
> Merge the comments rather than having two one-liners one after
> the other, please.
>  /* Disallow return to an EL which is unimplemented or higher
>   * than the current one.
>   */

Fixed for v2, thanks.

Cheers,
Edgar

[Qemu-devel] [PATCH v1 17/22] target-arm: A64: Generalize ERET to various ELs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Adds support for ERET to Aarch64 EL2 and 3.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/op_helper.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index f1ae05e..8494f7f 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -386,13 +386,14 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
 
 void HELPER(exception_return)(CPUARMState *env)
 {
-    unsigned int spsr_idx = arm64_banked_spsr_index(1);
-    uint32_t spsr = env->banked_spsr[spsr_idx];
-    int new_el, i;
     int cur_el = arm_current_pl(env);
+    unsigned int spsr_idx = arm64_banked_spsr_index(cur_el);
+    uint32_t spsr;
+    int new_el, i;
 
+    spsr = env->banked_spsr[spsr_idx];
     if (env->pstate & PSTATE_SP) {
-        env->sp_el[1] = env->xregs[31];
+        env->sp_el[cur_el] = env->xregs[31];
     } else {
         env->sp_el[0] = env->xregs[31];
     }
@@ -428,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env)
         env->aarch64 = 1;
         pstate_write(env, spsr);
         env->xregs[31] = env->sp_el[new_el];
-        env->pc = env->elr_el[ELR_EL_IDX(1)];
+        env->pc = env->elr_el[ELR_EL_IDX(cur_el)];
     }
 
     return;
@@ -442,7 +443,7 @@ illegal_return:
      * no change to exception level, execution state or stack pointer
      */
     env->pstate |= PSTATE_IL;
-    env->pc = env->elr_el[ELR_EL_IDX(1)];
+    env->pc = env->elr_el[ELR_EL_IDX(cur_el)];
     spsr &= PSTATE_NZCV | PSTATE_DAIF;
     spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
     pstate_write(env, spsr);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 17/22] target-arm: A64: Generalize ERET to various ELs

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Adds support for ERET to Aarch64 EL2 and 3.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/op_helper.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index f1ae05e..8494f7f 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -386,13 +386,14 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm)
>
>  void HELPER(exception_return)(CPUARMState *env)
>  {
> -    unsigned int spsr_idx = arm64_banked_spsr_index(1);
> -    uint32_t spsr = env->banked_spsr[spsr_idx];
> -    int new_el, i;
>      int cur_el = arm_current_pl(env);
> +    unsigned int spsr_idx = arm64_banked_spsr_index(cur_el);
> +    uint32_t spsr;
> +    int new_el, i;
>
> +    spsr = env->banked_spsr[spsr_idx];

Why change to split declaration and assignment (amongst the other
all-in-one's above)?

Otherwise:

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

>      if (env->pstate & PSTATE_SP) {
> -        env->sp_el[1] = env->xregs[31];
> +        env->sp_el[cur_el] = env->xregs[31];
>      } else {
>          env->sp_el[0] = env->xregs[31];
>      }
> @@ -428,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env)
>          env->aarch64 = 1;
>          pstate_write(env, spsr);
>          env->xregs[31] = env->sp_el[new_el];
> -        env->pc = env->elr_el[ELR_EL_IDX(1)];
> +        env->pc = env->elr_el[ELR_EL_IDX(cur_el)];
>      }
>
>      return;
> @@ -442,7 +443,7 @@ illegal_return:
>       * no change to exception level, execution state or stack pointer
>       */
>      env->pstate |= PSTATE_IL;
> -    env->pc = env->elr_el[ELR_EL_IDX(1)];
> +    env->pc = env->elr_el[ELR_EL_IDX(cur_el)];
>      spsr &= PSTATE_NZCV | PSTATE_DAIF;
>      spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
>      pstate_write(env, spsr);
> --
> 1.8.3.2
>
>

[Qemu-devel] [PATCH v1 18/22] target-arm: A64: Generalize update_spsel for the various ELs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/internals.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/target-arm/internals.h b/target-arm/internals.h
index 7c39946..5d802db 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -107,6 +107,7 @@ int arm_rmode_to_sf(int rmode);
 
 static inline void update_spsel(CPUARMState *env, uint32_t imm)
 {
+    unsigned int cur_el = arm_current_pl(env);
     /* Update PSTATE SPSel bit; this requires us to update the
      * working stack pointer in xregs[31].
      */
@@ -115,17 +116,15 @@ static inline void update_spsel(CPUARMState *env, uint32_t imm)
     }
     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);
+    /* EL0 has no access rights to update SPSel.  */
+    assert(cur_el >= 1 && cur_el <= 3);
     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];
+        env->xregs[31] = env->sp_el[cur_el];
     } else {
         /* Switch from SP_EL0 to SP_ELx */
-        env->sp_el[1] = env->xregs[31];
+        env->sp_el[cur_el] = env->xregs[31];
         env->xregs[31] = env->sp_el[0];
     }
 }
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 18/22] target-arm: A64: Generalize update_spsel for the various ELs

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

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

> ---
>  target-arm/internals.h | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index 7c39946..5d802db 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -107,6 +107,7 @@ int arm_rmode_to_sf(int rmode);
>
>  static inline void update_spsel(CPUARMState *env, uint32_t imm)
>  {
> +    unsigned int cur_el = arm_current_pl(env);
>      /* Update PSTATE SPSel bit; this requires us to update the
>       * working stack pointer in xregs[31].
>       */
> @@ -115,17 +116,15 @@ static inline void update_spsel(CPUARMState *env, uint32_t imm)
>      }
>      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);
> +    /* EL0 has no access rights to update SPSel.  */
> +    assert(cur_el >= 1 && cur_el <= 3);
>      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];
> +        env->xregs[31] = env->sp_el[cur_el];
>      } else {
>          /* Switch from SP_EL0 to SP_ELx */
> -        env->sp_el[1] = env->xregs[31];
> +        env->sp_el[cur_el] = env->xregs[31];
>          env->xregs[31] = env->sp_el[0];
>      }
>  }
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 18/22] target-arm: A64: Generalize update_spsel for the various ELs

[Richard Henderson]

On 05/05/2014 11:08 PM, Edgar E. Iglesias wrote:
> -    /* 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);
> +    /* EL0 has no access rights to update SPSel.  */
> +    assert(cur_el >= 1 && cur_el <= 3);

The old comment makes it clear that we're not supposed to get here when running
as EL0.  The new comment makes this look like a possible DoS attack.


r~

Re: [Qemu-devel] [PATCH v1 18/22] target-arm: A64: Generalize update_spsel for the various ELs

[Edgar E. Iglesias]

On Tue, May 13, 2014 at 10:32:09AM -0700, Richard Henderson wrote:
> On 05/05/2014 11:08 PM, Edgar E. Iglesias wrote:
> > -    /* 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);
> > +    /* EL0 has no access rights to update SPSel.  */
> > +    assert(cur_el >= 1 && cur_el <= 3);
> 
> The old comment makes it clear that we're not supposed to get here when running
> as EL0.  The new comment makes this look like a possible DoS attack.

I've changed it to the following for next version:
    /* We rely on illegal updates to SPsel from EL0 to get trapped
     * at translation time.
     */

If you have better suggestions I'm happy to update.

Thanks,
Edgar

Re: [Qemu-devel] [PATCH v1 18/22] target-arm: A64: Generalize update_spsel for the various ELs

[Richard Henderson]

On 05/13/2014 06:18 PM, Edgar E. Iglesias wrote:
> I've changed it to the following for next version:
>     /* We rely on illegal updates to SPsel from EL0 to get trapped
>      * at translation time.
>      */
> 
> If you have better suggestions I'm happy to update.

Excellent, thanks.


r~

[Qemu-devel] [PATCH v1 19/22] target-arm: Add storage for VBAR_EL2 and 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 34e8f7c..88dfdcb 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -201,7 +201,7 @@ typedef struct CPUARMState {
         uint32_t c9_pminten; /* perf monitor interrupt enables */
         uint64_t mair_el1;
 #define VBAR_EL_IDX(x) (x - 1)
-        uint64_t vbar_el[1]; /* vector base address register */
+        uint64_t vbar_el[3]; /* vector base address register */
         uint32_t c13_fcse; /* FCSE PID.  */
         uint64_t contextidr_el1; /* Context ID.  */
         uint64_t tpidr_el0; /* User RW Thread register.  */
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 19/22] target-arm: Add storage for VBAR_EL2 and 3

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 34e8f7c..88dfdcb 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -201,7 +201,7 @@ typedef struct CPUARMState {
>          uint32_t c9_pminten; /* perf monitor interrupt enables */
>          uint64_t mair_el1;
>  #define VBAR_EL_IDX(x) (x - 1)
> -        uint64_t vbar_el[1]; /* vector base address register */
> +        uint64_t vbar_el[3]; /* vector base address register */
>          uint32_t c13_fcse; /* FCSE PID.  */
>          uint64_t contextidr_el1; /* Context ID.  */
>          uint64_t tpidr_el0; /* User RW Thread register.  */
> --
> 1.8.3.2

You should just fold this into whichever patch causes us to
start actually accessing vbar_el[] for other ELs.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 19/22] target-arm: Add storage for VBAR_EL2 and 3

[Edgar E. Iglesias]

On Fri, May 16, 2014 at 03:40:22PM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 34e8f7c..88dfdcb 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -201,7 +201,7 @@ typedef struct CPUARMState {
> >          uint32_t c9_pminten; /* perf monitor interrupt enables */
> >          uint64_t mair_el1;
> >  #define VBAR_EL_IDX(x) (x - 1)
> > -        uint64_t vbar_el[1]; /* vector base address register */
> > +        uint64_t vbar_el[3]; /* vector base address register */
> >          uint32_t c13_fcse; /* FCSE PID.  */
> >          uint64_t contextidr_el1; /* Context ID.  */
> >          uint64_t tpidr_el0; /* User RW Thread register.  */
> > --
> > 1.8.3.2
> 
> You should just fold this into whichever patch causes us to
> start actually accessing vbar_el[] for other ELs.

Agreed, will squash them.

Thanks,
Edgar

[Qemu-devel] [PATCH v1 20/22] target-arm: Make vbar_write writeback to any CPREG

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 65daeaf..2406058 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
      * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7
      * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.)
      */
-    env->cp15.vbar_el[VBAR_EL_IDX(1)] = value & ~0x1FULL;
+    CPREG_FIELD64(env, ri) = value & ~0x1FULL;
 }
 
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 20/22] target-arm: Make vbar_write writeback to any CPREG

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 65daeaf..2406058 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri,
>       * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7
>       * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.)
>       */
> -    env->cp15.vbar_el[VBAR_EL_IDX(1)] = value & ~0x1FULL;
> +    CPREG_FIELD64(env, ri) = value & ~0x1FULL;

Use raw_write() to implement CP register writing (check
vmsa_ttbr_write for example).

Regards,
Peter

>  }
>
>  static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> --
> 1.8.3.2
>
>

[Qemu-devel] [PATCH v1 21/22] target-arm: A64: Register VBAR_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2406058..6e3f5fa 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2088,6 +2088,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
       .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
+    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
+      .access = PL2_RW, .writefn = vbar_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]),
+      .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 21/22] target-arm: A64: Register VBAR_EL2

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2406058..6e3f5fa 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2088,6 +2088,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
> +    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> +      .access = PL2_RW, .writefn = vbar_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]),

This [1] is smoewhat misleading, and should either use the VBAR_EL_IDX
macro or if changing over to always-four array, just [2].

Regards,
Peter

> +      .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 21/22] target-arm: A64: Register VBAR_EL2

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2406058..6e3f5fa 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2088,6 +2088,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
> +    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> +      .access = PL2_RW, .writefn = vbar_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]),
> +      .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };

ARM ARM says that if EL2 isn't implemented then this register
should be RES0 from EL3 (ie not UNDEF).

thanks
-- PMM

[Qemu-devel] [PATCH v1 22/22] target-arm: A64: Register VBAR_EL3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 6e3f5fa..b6dac25 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2106,6 +2106,11 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
       .type = ARM_CP_NO_MIGRATE,
       .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0,
       .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) },
+    { .name = "VBAR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 0,
+      .access = PL3_RW, .writefn = vbar_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
+      .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 22/22] target-arm: A64: Register VBAR_EL3

[Peter Crosthwaite]

On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 6e3f5fa..b6dac25 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2106,6 +2106,11 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>        .type = ARM_CP_NO_MIGRATE,
>        .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) },
> +    { .name = "VBAR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 0,
> +      .access = PL3_RW, .writefn = vbar_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),

Same comment as P21.

Regards,
Peter

> +      .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Peter Maydell]

On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> I've been doing some work on modeling parts of EL2 and 3 + some of
> the system-wide virtualization features for ARMv8. A lot is missing
> but I've got a series with enough to for example run KVM A64 guests
> on top of EL3 firmware inside emulated QEMU A64 VMs.
> I'm working on cleaning things up and plan to send patches and publish
> things as I go.

So before I start reviewing this, how does it relate to the
Samsung series for AArch32 trustzone (EL3) support that was
posted last year? In Linaro we've been planning to rework that
and integrate it upstream...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Edgar E. Iglesias]

On Tue, May 06, 2014 at 08:58:43AM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Hi,
> >
> > I've been doing some work on modeling parts of EL2 and 3 + some of
> > the system-wide virtualization features for ARMv8. A lot is missing
> > but I've got a series with enough to for example run KVM A64 guests
> > on top of EL3 firmware inside emulated QEMU A64 VMs.
> > I'm working on cleaning things up and plan to send patches and publish
> > things as I go.
> 
> So before I start reviewing this, how does it relate to the
> Samsung series for AArch32 trustzone (EL3) support that was
> posted last year? In Linaro we've been planning to rework that
> and integrate it upstream...
>

Hi Peter,

AFAICT the series have some minor overlap but mostly they complement each other.
The aarch64 EL3 support I've got so far is very limited. Has mode switching,
separate page tables, SMC etc and that kind of things but no S/NS state yet.
The A64 security state parts can be implemented on top of the Samsung series.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Aggeler Fabian]

Hi

I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to send them some time this week. I am currently rebasing them when I noticed Edgar’s patches. Is there some branch with the patches so I could rebase on them?

Thanks,
Fabian

On 07 May 2014, at 05:46, Edgar E. Iglesias <edgar.iglesias@gmail.com<mailto:edgar.iglesias@gmail.com>> wrote:

On Tue, May 06, 2014 at 08:58:43AM +0100, Peter Maydell wrote:
On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com<mailto:edgar.iglesias@gmail.com>> wrote:
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com<mailto:edgar.iglesias@xilinx.com>>

Hi,

I've been doing some work on modeling parts of EL2 and 3 + some of
the system-wide virtualization features for ARMv8. A lot is missing
but I've got a series with enough to for example run KVM A64 guests
on top of EL3 firmware inside emulated QEMU A64 VMs.
I'm working on cleaning things up and plan to send patches and publish
things as I go.

So before I start reviewing this, how does it relate to the
Samsung series for AArch32 trustzone (EL3) support that was
posted last year? In Linaro we've been planning to rework that
and integrate it upstream...


Hi Peter,

AFAICT the series have some minor overlap but mostly they complement each other.
The aarch64 EL3 support I've got so far is very limited. Has mode switching,
separate page tables, SMC etc and that kind of things but no S/NS state yet.
The A64 security state parts can be implemented on top of the Samsung series.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Peter Maydell]

On 12 May 2014 20:13, Aggeler  Fabian <aggelerf@student.ethz.ch> wrote:
> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to
> send them some time this week. I am currently rebasing them when I noticed Edgar’s
> patches. Is there some branch with the patches so I could rebase on them?

Hmm, that makes about three lots of people trying to do similar things
at this point. We should try to coordinate so we don't duplicate work.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Aggeler Fabian]

I see. What is Greg Bellows working on exactly? Also peripherals like TZASC, TZPC,...? My plan is to focus on them now if no one else is working on them. What do you suggest to minimize overlap?

Thanks,
Fabian
________________________________________
From: Peter Maydell [peter.maydell@linaro.org]
Sent: Monday, May 12, 2014 10:39 PM
To: Aggeler  Fabian
Cc: Edgar E. Iglesias; Rob Herring; Peter Crosthwaite; QEMU Developers; Alexander Graf; John Williams; Alex Bennée; Greg Bellows
Subject: Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

On 12 May 2014 20:13, Aggeler  Fabian <aggelerf@student.ethz.ch> wrote:
> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to
> send them some time this week. I am currently rebasing them when I noticed Edgar’s
> patches. Is there some branch with the patches so I could rebase on them?

Hmm, that makes about three lots of people trying to do similar things
at this point. We should try to coordinate so we don't duplicate work.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

Hi Fabian,

I too had been updating the core TZ patches provided by Samsung.  From
looking at your changes I see a lot of similarities in our code with the
exception being the mechanism for banked register support.  The difference
being that your approach is a bit more explicit in the declaration of the
banked registers.  Whereas my approach was to update the banked registers
once all the other registers were registered.  Both approaches I believe
work.

I spoke with Peter M. and he and I are okay with your approach.  I will be
looking closer at your patches today and making comments.

One thing that held me up from committing sooner was testing my changes.
 Do you have a good approach for testing the changes?

Regards,

Greg


On 14 May 2014 03:58, Aggeler Fabian <aggelerf@student.ethz.ch> wrote:

> I see. What is Greg Bellows working on exactly? Also peripherals like
> TZASC, TZPC,...? My plan is to focus on them now if no one else is working
> on them. What do you suggest to minimize overlap?
>
> Thanks,
> Fabian
> ________________________________________
> From: Peter Maydell [peter.maydell@linaro.org]
> Sent: Monday, May 12, 2014 10:39 PM
> To: Aggeler  Fabian
> Cc: Edgar E. Iglesias; Rob Herring; Peter Crosthwaite; QEMU Developers;
> Alexander Graf; John Williams; Alex Bennée; Greg Bellows
> Subject: Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for
> A64 EL2 and 3
>
> On 12 May 2014 20:13, Aggeler  Fabian <aggelerf@student.ethz.ch> wrote:
> > I’ve been reworking the Samsung patches as part of my Master thesis and
> I wanted to
> > send them some time this week. I am currently rebasing them when I
> noticed Edgar’s
> > patches. Is there some branch with the patches so I could rebase on them?
>
> Hmm, that makes about three lots of people trying to do similar things
> at this point. We should try to coordinate so we don't duplicate work.
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 2566 bytes --]

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Aggeler Fabian]

Hi Greg

Thanks for your comments. I still have to work through them. I am using OpenVirtualization in secure world, which then switches to a Linux kernel in non-secure world to test the patches. What about you?

Best,
Fabian

On 14 May 2014, at 15:55, Greg Bellows <greg.bellows@linaro.org<mailto:greg.bellows@linaro.org>> wrote:

Hi Fabian,

I too had been updating the core TZ patches provided by Samsung.  From looking at your changes I see a lot of similarities in our code with the exception being the mechanism for banked register support.  The difference being that your approach is a bit more explicit in the declaration of the banked registers.  Whereas my approach was to update the banked registers once all the other registers were registered.  Both approaches I believe work.

I spoke with Peter M. and he and I are okay with your approach.  I will be looking closer at your patches today and making comments.

One thing that held me up from committing sooner was testing my changes.  Do you have a good approach for testing the changes?

Regards,

Greg


On 14 May 2014 03:58, Aggeler Fabian <aggelerf@student.ethz.ch<mailto:aggelerf@student.ethz.ch>> wrote:
I see. What is Greg Bellows working on exactly? Also peripherals like TZASC, TZPC,...? My plan is to focus on them now if no one else is working on them. What do you suggest to minimize overlap?

Thanks,
Fabian
________________________________________
From: Peter Maydell [peter.maydell@linaro.org<mailto:peter.maydell@linaro.org>]
Sent: Monday, May 12, 2014 10:39 PM
To: Aggeler  Fabian
Cc: Edgar E. Iglesias; Rob Herring; Peter Crosthwaite; QEMU Developers; Alexander Graf; John Williams; Alex Bennée; Greg Bellows
Subject: Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

On 12 May 2014 20:13, Aggeler  Fabian <aggelerf@student.ethz.ch<mailto:aggelerf@student.ethz.ch>> wrote:
> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to
> send them some time this week. I am currently rebasing them when I noticed Edgar’s
> patches. Is there some branch with the patches so I could rebase on them?

Hmm, that makes about three lots of people trying to do similar things
at this point. We should try to coordinate so we don't duplicate work.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Sergey Fedorov]

On 15.05.2014 13:28, Aggeler Fabian wrote:
> Hi Greg
>
> Thanks for your comments. I still have to work through them. I am using OpenVirtualization in secure world, which then switches to a Linux kernel in non-secure world to test the patches. What about you?
>
> Best,
> Fabian

Hi, Fabian, are there some secure OS with secure user-space tasks which
can be used for testing whether world switching is performed correctly?

Regards, Sergey.

>
> On 14 May 2014, at 15:55, Greg Bellows <greg.bellows@linaro.org<mailto:greg.bellows@linaro.org>> wrote:
>
> Hi Fabian,
>
> I too had been updating the core TZ patches provided by Samsung.  From looking at your changes I see a lot of similarities in our code with the exception being the mechanism for banked register support.  The difference being that your approach is a bit more explicit in the declaration of the banked registers.  Whereas my approach was to update the banked registers once all the other registers were registered.  Both approaches I believe work.
>
> I spoke with Peter M. and he and I are okay with your approach.  I will be looking closer at your patches today and making comments.
>
> One thing that held me up from committing sooner was testing my changes.  Do you have a good approach for testing the changes?
>
> Regards,
>
> Greg
>
>
> On 14 May 2014 03:58, Aggeler Fabian <aggelerf@student.ethz.ch<mailto:aggelerf@student.ethz.ch>> wrote:
> I see. What is Greg Bellows working on exactly? Also peripherals like TZASC, TZPC,...? My plan is to focus on them now if no one else is working on them. What do you suggest to minimize overlap?
>
> Thanks,
> Fabian
> ________________________________________
> From: Peter Maydell [peter.maydell@linaro.org<mailto:peter.maydell@linaro.org>]
> Sent: Monday, May 12, 2014 10:39 PM
> To: Aggeler  Fabian
> Cc: Edgar E. Iglesias; Rob Herring; Peter Crosthwaite; QEMU Developers; Alexander Graf; John Williams; Alex Bennée; Greg Bellows
> Subject: Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3
>
> On 12 May 2014 20:13, Aggeler  Fabian <aggelerf@student.ethz.ch<mailto:aggelerf@student.ethz.ch>> wrote:
>> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to
>> send them some time this week. I am currently rebasing them when I noticed Edgar’s
>> patches. Is there some branch with the patches so I could rebase on them?
> Hmm, that makes about three lots of people trying to do similar things
> at this point. We should try to coordinate so we don't duplicate work.
>
> thanks
> -- PMM
>
>
>

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Christopher Covington]

Hi Sergey,

On 05/15/2014 05:45 AM, Sergey Fedorov wrote:
> On 15.05.2014 13:28, Aggeler Fabian wrote:
>> Hi Greg
>>
>> Thanks for your comments. I still have to work through them. I am using OpenVirtualization in secure world, which then switches to a Linux kernel in non-secure world to test the patches. What about you?
>>
>> Best,
>> Fabian
> 
> Hi, Fabian, are there some secure OS with secure user-space tasks which
> can be used for testing whether world switching is performed correctly?

In my experience, A32 and A64 Linux runs fine in secure supervisor mode /
EL1_S (KVM, if enabled, will fail gracefully). A32 and A64 Linux applications
run fine in secure user mode / EL0_S.

For A64 EL3 there's https://github.com/ARM-software/arm-trusted-firmware,
although I've yet to use it myself. It advertises an EL1 test payload.

Christopher

>>
>> On 14 May 2014, at 15:55, Greg Bellows <greg.bellows@linaro.org<mailto:greg.bellows@linaro.org>> wrote:
>>
>> Hi Fabian,
>>
>> I too had been updating the core TZ patches provided by Samsung.  From looking at your changes I see a lot of similarities in our code with the exception being the mechanism for banked register support.  The difference being that your approach is a bit more explicit in the declaration of the banked registers.  Whereas my approach was to update the banked registers once all the other registers were registered.  Both approaches I believe work.
>>
>> I spoke with Peter M. and he and I are okay with your approach.  I will be looking closer at your patches today and making comments.
>>
>> One thing that held me up from committing sooner was testing my changes.  Do you have a good approach for testing the changes?
>>
>> Regards,
>>
>> Greg
>>
>>
>> On 14 May 2014 03:58, Aggeler Fabian <aggelerf@student.ethz.ch<mailto:aggelerf@student.ethz.ch>> wrote:
>> I see. What is Greg Bellows working on exactly? Also peripherals like TZASC, TZPC,...? My plan is to focus on them now if no one else is working on them. What do you suggest to minimize overlap?
>>
>> Thanks,
>> Fabian
>> ________________________________________
>> From: Peter Maydell [peter.maydell@linaro.org<mailto:peter.maydell@linaro.org>]
>> Sent: Monday, May 12, 2014 10:39 PM
>> To: Aggeler  Fabian
>> Cc: Edgar E. Iglesias; Rob Herring; Peter Crosthwaite; QEMU Developers; Alexander Graf; John Williams; Alex Bennée; Greg Bellows
>> Subject: Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3
>>
>> On 12 May 2014 20:13, Aggeler  Fabian <aggelerf@student.ethz.ch<mailto:aggelerf@student.ethz.ch>> wrote:
>>> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to
>>> send them some time this week. I am currently rebasing them when I noticed Edgar’s
>>> patches. Is there some branch with the patches so I could rebase on them?
>> Hmm, that makes about three lots of people trying to do similar things
>> at this point. We should try to coordinate so we don't duplicate work.
>>
>> thanks
>> -- PMM
>>
>>
>>
> 
> 


-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Edgar E. Iglesias]

On Wed, May 14, 2014 at 08:58:09AM +0000, Aggeler  Fabian wrote:
> I see. What is Greg Bellows working on exactly? Also peripherals like TZASC, TZPC,...? My plan is to focus on them now if no one else is working on them. What do you suggest to minimize overlap?

Hi,

With regards to my patches our overlap is minimal. I dont think I will have
any problems rebasing on top of your stuff.

I've got aarch64 VIRQ/GICv2 virtualization support, Timer updates,
MMU 2nd stage translation + a few hax here and there. Will hopefully
post next week. My focus has been on getting KVM to run within
emulated QEMU. Not so much on EL3 and TrustZone.

WRT coordination, I'm happy to rebase if other patches go in before
mine in any area I've mentioned. IMO, the best help right now would be
to actually get some of the basic and few conflicting patches
reviewed & merged. 

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Peter Crosthwaite]

On Tue, May 13, 2014 at 5:13 AM, Aggeler  Fabian
<aggelerf@student.ethz.ch> wrote:
> Hi
>
> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to send them some time this week. I am currently rebasing them when I noticed Edgar’s patches. Is there some branch with the patches so I could rebase on them?
>

The patches tool is very good for getting someones work off list as a
local branch.

https://github.com/aliguori/patches

Regards,
Peter

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Edgar E. Iglesias]

On Mon, May 12, 2014 at 07:13:45PM +0000, Aggeler  Fabian wrote:
> Hi
> 
> I’ve been reworking the Samsung patches as part of my Master thesis and I wanted to send them some time this week. I am currently rebasing them when I noticed Edgar’s patches. Is there some branch with the patches so I could rebase on them?

Hi,

I've put my v2 here:
git://repo.or.cz/qemu/cris-port.git edgar/a64/elx-refactor.2

I'm busy with some other things now but plan to send out more patches
next week.

Best regards,
Edgar



> 
> Thanks,
> Fabian
> 
> On 07 May 2014, at 05:46, Edgar E. Iglesias <edgar.iglesias@gmail.com<mailto:edgar.iglesias@gmail.com>> wrote:
> 
> On Tue, May 06, 2014 at 08:58:43AM +0100, Peter Maydell wrote:
> On 6 May 2014 07:08, Edgar E. Iglesias <edgar.iglesias@gmail.com<mailto:edgar.iglesias@gmail.com>> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com<mailto:edgar.iglesias@xilinx.com>>
> 
> Hi,
> 
> I've been doing some work on modeling parts of EL2 and 3 + some of
> the system-wide virtualization features for ARMv8. A lot is missing
> but I've got a series with enough to for example run KVM A64 guests
> on top of EL3 firmware inside emulated QEMU A64 VMs.
> I'm working on cleaning things up and plan to send patches and publish
> things as I go.
> 
> So before I start reviewing this, how does it relate to the
> Samsung series for AArch32 trustzone (EL3) support that was
> posted last year? In Linaro we've been planning to rework that
> and integrate it upstream...
> 
> 
> Hi Peter,
> 
> AFAICT the series have some minor overlap but mostly they complement each other.
> The aarch64 EL3 support I've got so far is very limited. Has mode switching,
> separate page tables, SMC etc and that kind of things but no S/NS state yet.
> The A64 security state parts can be implemented on top of the Samsung series.
> 
> Cheers,
> Edgar
> 

Re: [Qemu-devel] [PATCH v1 00/22] target-arm: Preparations for A64 EL2 and 3

[Alexander Graf]

On 05/06/2014 08:08 AM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Hi,
>
> I've been doing some work on modeling parts of EL2 and 3 + some of
> the system-wide virtualization features for ARMv8. A lot is missing
> but I've got a series with enough to for example run KVM A64 guests
> on top of EL3 firmware inside emulated QEMU A64 VMs.
> I'm working on cleaning things up and plan to send patches and publish
> things as I go.
>
> This series does a first round of preparations. Most of it has little
> or no user visible impact as the EL2 and 3 features remain disabled.
>
> For the arrayification of the various EL regs that do not exist for
> EL0, I've used index macros. I don't really have a preference here,
> so if you have better/less ugly ideas or preferences I'm happy to change
> the pattern (getters/setters or whatever).
>
> Happy to address any comments people may have.

I've only had a rough glimpse at the patches, but so far things look 
pretty sane.


Alex