[Qemu-devel] [PATCH 0/4] target-arm: Initial ARMv8 extended EL support

[Qemu-devel] [PATCH 0/4] target-arm: Initial ARMv8 extended EL support

[Greg Bellows]

These patches add extended EL support for ARMv8. Specifically the following
changes were made:

- Fix issue in RVBAR_EL1 CP register definition
- Add missing RVBAR_ELx CP register definitions
- Add missing SP_ELx CP register definitions
- Add reset support to start in highest EL

Greg Bellows (4):
  target-arm: Fix RVBAR_EL1 register encoding
  target-arm: Add extended RVBAR support
  target-arm: Change reset to highest available EL
  target-arm: Add missing SP_ELx register definition

 hw/arm/boot.c       | 10 ++++++++++
 target-arm/cpu.c    | 10 +++++++++-
 target-arm/helper.c | 45 +++++++++++++++++++++++++++++++++++++--------
 3 files changed, 56 insertions(+), 9 deletions(-)

--
1.8.3.2

[Qemu-devel] [PATCH 1/4] target-arm: Fix RVBAR_EL1 register encoding

[Greg Bellows]

Fix the RVBAR_EL1 CP register opc2 encoding from 2 to 1

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 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 1a5e067..c9b1c08 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3055,7 +3055,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         };
         ARMCPRegInfo rvbar = {
             .name = "RVBAR_EL1", .state = ARM_CP_STATE_AA64,
-            .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 2,
+            .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
             .type = ARM_CP_CONST, .access = PL1_R, .resetvalue = cpu->rvbar
         };
         define_one_arm_cp_reg(cpu, &rvbar);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 1/4] target-arm: Fix RVBAR_EL1 register encoding

[Peter Maydell]

On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> Fix the RVBAR_EL1 CP register opc2 encoding from 2 to 1
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> ---
>  target-arm/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I assume you checked that Linux still boots (ie we're not
accidentally relying on this regdef for RMR_EL1).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/4] target-arm: Fix RVBAR_EL1 register encoding

[Greg Bellows]

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

On Jan 23, 2015 8:56 AM, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Fix the RVBAR_EL1 CP register opc2 encoding from 2 to 1
> >
> > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> > ---
> >  target-arm/helper.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> I assume you checked that Linux still boots (ie we're not
> accidentally relying on this regdef for RMR_EL1).
>
Yeah, I booted linux on virt/a57 without issue.  I'll double check the
reliance on rmr el1.

> thanks
> -- PMM

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

Re: [Qemu-devel] [PATCH 1/4] target-arm: Fix RVBAR_EL1 register encoding

[Peter Maydell]

On 23 January 2015 at 14:59, Greg Bellows <greg.bellows@linaro.org> wrote:
>
> On Jan 23, 2015 8:56 AM, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>> I assume you checked that Linux still boots (ie we're not
>> accidentally relying on this regdef for RMR_EL1).
>>
> Yeah, I booted linux on virt/a57 without issue.  I'll double check the
> reliance on rmr el1.

If it booted we're fine.

-- PMM

[Qemu-devel] [PATCH 2/4] target-arm: Add extended RVBAR support

[Greg Bellows]

Added RVBAR_EL2 and RVBAR_EL3 CP register support.  All RVBAR_EL# registers
point to the same location and only the highest EL version exists at any one
time.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 target-arm/helper.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index c9b1c08..d5f0997 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3053,17 +3053,30 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .resetvalue = cpu->mvfr2 },
             REGINFO_SENTINEL
         };
-        ARMCPRegInfo rvbar = {
-            .name = "RVBAR_EL1", .state = ARM_CP_STATE_AA64,
-            .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
-            .type = ARM_CP_CONST, .access = PL1_R, .resetvalue = cpu->rvbar
-        };
-        define_one_arm_cp_reg(cpu, &rvbar);
+        /* RVBAR_EL1 is only implemented if EL1 is the highest EL */
+        if (!arm_feature(env, ARM_FEATURE_EL3) &&
+            !arm_feature(env, ARM_FEATURE_EL2)) {
+            ARMCPRegInfo rvbar = {
+                .name = "RVBAR_EL1", .state = ARM_CP_STATE_AA64,
+                .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
+                .type = ARM_CP_CONST, .access = PL1_R, .resetvalue = cpu->rvbar
+            };
+            define_one_arm_cp_reg(cpu, &rvbar);
+        }
         define_arm_cp_regs(cpu, v8_idregs);
         define_arm_cp_regs(cpu, v8_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_EL2)) {
         define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
+        /* RVBAR_EL2 is only implemented if EL2 is the highest EL */
+        if (!arm_feature(env, ARM_FEATURE_EL3)) {
+            ARMCPRegInfo rvbar = {
+                .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
+                .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 1,
+                .type = ARM_CP_CONST, .access = PL2_R, .resetvalue = cpu->rvbar
+            };
+            define_one_arm_cp_reg(cpu, &rvbar);
+        }
     } else {
         /* If EL2 is missing but higher ELs are enabled, we need to
          * register the no_el2 reginfos.
@@ -3074,6 +3087,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     }
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         define_arm_cp_regs(cpu, el3_cp_reginfo);
+        ARMCPRegInfo rvbar = {
+            .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,
+            .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 1,
+            .type = ARM_CP_CONST, .access = PL3_R, .resetvalue = cpu->rvbar
+        };
+        define_one_arm_cp_reg(cpu, &rvbar);
     }
     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 2/4] target-arm: Add extended RVBAR support

[Peter Maydell]

On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> Added RVBAR_EL2 and RVBAR_EL3 CP register support.  All RVBAR_EL# registers
> point to the same location and only the highest EL version exists at any one
> time.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

>      if (arm_feature(env, ARM_FEATURE_EL2)) {
>          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> +        /* RVBAR_EL2 is only implemented if EL2 is the highest EL */
> +        if (!arm_feature(env, ARM_FEATURE_EL3)) {
> +            ARMCPRegInfo rvbar = {
> +                .name = "RVBAR_EL3", .state = ARM_CP_STATE_AA64,

Should be "RVBAR_EL2"...

> +                .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 1,
> +                .type = ARM_CP_CONST, .access = PL2_R, .resetvalue = cpu->rvbar
> +            };
> +            define_one_arm_cp_reg(cpu, &rvbar);
> +        }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 3/4] target-arm: Change reset to highest available EL

[Greg Bellows]

Update to arm_cpu_reset() to reset into the highest available exception level
based on the set ARM features.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/boot.c    | 10 ++++++++++
 target-arm/cpu.c | 10 +++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 52ebd8b..148011b 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -464,6 +464,16 @@ static void do_cpu_reset(void *opaque)
              * requested.
              */
             if (arm_feature(env, ARM_FEATURE_EL3) && !info->secure_boot) {
+                /* AArch64 is defined to come out of reset into EL3 if enabled.
+                 * If we are booting Linux then we need to adjust our EL as
+                 * Linux expects us to be EL1.  AArch32 resets into SVC, which
+                 * Linux expects, so no privilege/exception level to adjust.
+                 */
+                if (env->aarch64) {
+                    env->pstate = PSTATE_MODE_EL1h;
+                }
+
+                /* Linux expects non-secure state */
                 env->cp15.scr_el3 |= SCR_NS;
             }
 
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 285947f..6793596 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -113,7 +113,15 @@ static void arm_cpu_reset(CPUState *s)
         /* and to the FP/Neon instructions */
         env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
 #else
-        env->pstate = PSTATE_MODE_EL1h;
+        /* Reset into the highest available EL */
+        if (arm_feature(env, ARM_FEATURE_EL3)) {
+            env->pstate = PSTATE_MODE_EL3h;
+            env->cp15.scr_el3 &= ~SCR_NS;
+        } else if (arm_feature(env, ARM_FEATURE_EL3)) {
+            env->pstate = PSTATE_MODE_EL2h;
+        } else {
+            env->pstate = PSTATE_MODE_EL1h;
+        }
         env->pc = cpu->rvbar;
 #endif
     } else {
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 3/4] target-arm: Change reset to highest available EL

[Peter Maydell]

On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> Update to arm_cpu_reset() to reset into the highest available exception level
> based on the set ARM features.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> ---
>  hw/arm/boot.c    | 10 ++++++++++
>  target-arm/cpu.c | 10 +++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 52ebd8b..148011b 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -464,6 +464,16 @@ static void do_cpu_reset(void *opaque)
>               * requested.
>               */
>              if (arm_feature(env, ARM_FEATURE_EL3) && !info->secure_boot) {

We also need to handle the "have EL3, secure_boot set, AArch64" case,
for which purposes we need to boot Linux in secure EL1.
Or we could declare that invalid and complain to the user, if you
prefer; it's not a case we actually have to support right now. But
trying to boot the kernel in secure EL3 AArch64 is not going to work...

> +                /* AArch64 is defined to come out of reset into EL3 if enabled.
> +                 * If we are booting Linux then we need to adjust our EL as
> +                 * Linux expects us to be EL1.  AArch32 resets into SVC, which
> +                 * Linux expects, so no privilege/exception level to adjust.
> +                 */
> +                if (env->aarch64) {
> +                    env->pstate = PSTATE_MODE_EL1h;
> +                }

We might as well make this be "EL2 if you have it, else EL1"
while we're adjusting this code.

> +
> +                /* Linux expects non-secure state */
>                  env->cp15.scr_el3 |= SCR_NS;
>              }
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 285947f..6793596 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -113,7 +113,15 @@ static void arm_cpu_reset(CPUState *s)
>          /* and to the FP/Neon instructions */
>          env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
>  #else
> -        env->pstate = PSTATE_MODE_EL1h;
> +        /* Reset into the highest available EL */
> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            env->pstate = PSTATE_MODE_EL3h;
> +            env->cp15.scr_el3 &= ~SCR_NS;

...why is this code changing the NS bit?

> +        } else if (arm_feature(env, ARM_FEATURE_EL3)) {

ARM_FEATURE_EL2, surely?

> +            env->pstate = PSTATE_MODE_EL2h;
> +        } else {
> +            env->pstate = PSTATE_MODE_EL1h;
> +        }
>          env->pc = cpu->rvbar;
>  #endif
>      } else {
> --

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/4] target-arm: Change reset to highest available EL

[Greg Bellows]

On Fri, Jan 23, 2015 at 9:05 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> wrote:
>> Update to arm_cpu_reset() to reset into the highest available exception level
>> based on the set ARM features.
>>
>> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
>> ---
>>  hw/arm/boot.c    | 10 ++++++++++
>>  target-arm/cpu.c | 10 +++++++++-
>>  2 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 52ebd8b..148011b 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -464,6 +464,16 @@ static void do_cpu_reset(void *opaque)
>>               * requested.
>>               */
>>              if (arm_feature(env, ARM_FEATURE_EL3) && !info->secure_boot) {
>
> We also need to handle the "have EL3, secure_boot set, AArch64" case,
> for which purposes we need to boot Linux in secure EL1.
> Or we could declare that invalid and complain to the user, if you
> prefer; it's not a case we actually have to support right now. But
> trying to boot the kernel in secure EL3 AArch64 is not going to work...

Added case.

>
>> +                /* AArch64 is defined to come out of reset into EL3 if enabled.
>> +                 * If we are booting Linux then we need to adjust our EL as
>> +                 * Linux expects us to be EL1.  AArch32 resets into SVC, which
>> +                 * Linux expects, so no privilege/exception level to adjust.
>> +                 */
>> +                if (env->aarch64) {
>> +                    env->pstate = PSTATE_MODE_EL1h;
>> +                }
>
> We might as well make this be "EL2 if you have it, else EL1"
> while we're adjusting this code.

Added.

>
>> +
>> +                /* Linux expects non-secure state */
>>                  env->cp15.scr_el3 |= SCR_NS;
>>              }
>>
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index 285947f..6793596 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -113,7 +113,15 @@ static void arm_cpu_reset(CPUState *s)
>>          /* and to the FP/Neon instructions */
>>          env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
>>  #else
>> -        env->pstate = PSTATE_MODE_EL1h;
>> +        /* Reset into the highest available EL */
>> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
>> +            env->pstate = PSTATE_MODE_EL3h;
>> +            env->cp15.scr_el3 &= ~SCR_NS;
>
> ...why is this code changing the NS bit?

I was thinking that I needed to reset it, but that should be taken
care of by the CP reg reset.  Removed.

>
>> +        } else if (arm_feature(env, ARM_FEATURE_EL3)) {
>
> ARM_FEATURE_EL2, surely?

Yes, Fixed.

>
>> +            env->pstate = PSTATE_MODE_EL2h;
>> +        } else {
>> +            env->pstate = PSTATE_MODE_EL1h;
>> +        }
>>          env->pc = cpu->rvbar;
>>  #endif
>>      } else {
>> --
>
> thanks
> -- PMM

[Qemu-devel] [PATCH 4/4] target-arm: Add missing SP_ELx register definition

[Greg Bellows]

Added CP register definitions for SP_EL1 and SP_EL2.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 target-arm/helper.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index d5f0997..ae7394d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2096,7 +2096,7 @@ static uint64_t aa64_dczid_read(CPUARMState *env, const ARMCPRegInfo *ri)
     return cpu->dcz_blocksize | dzp_bit;
 }
 
-static CPAccessResult sp_el0_access(CPUARMState *env, const ARMCPRegInfo *ri)
+static CPAccessResult sp_el_access(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     if (!(env->pstate & PSTATE_SP)) {
         /* Access to SP_EL0 is undefined if it's being used as
@@ -2326,9 +2326,14 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
      */
     { .name = "SP_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 1, .opc2 = 0,
-      .access = PL1_RW, .accessfn = sp_el0_access,
+      .access = PL1_RW, .accessfn = sp_el_access,
       .type = ARM_CP_NO_MIGRATE,
       .fieldoffset = offsetof(CPUARMState, sp_el[0]) },
+    { .name = "SP_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 1, .opc2 = 0,
+      .access = PL2_RW, .accessfn = sp_el_access,
+      .type = ARM_CP_NO_MIGRATE,
+      .fieldoffset = offsetof(CPUARMState, sp_el[1]) },
     { .name = "SPSel", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 2, .opc2 = 0,
       .type = ARM_CP_NO_MIGRATE,
@@ -2410,6 +2415,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
       .access = PL2_RW, .writefn = vbar_write,
       .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
       .resetvalue = 0 },
+    { .name = "SP_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
+      .access = PL3_RW, .accessfn = sp_el_access,
+      .type = ARM_CP_NO_MIGRATE,
+      .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
     REGINFO_SENTINEL
 };
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 4/4] target-arm: Add missing SP_ELx register definition

[Peter Maydell]

On 23 January 2015 at 14:49, Greg Bellows <greg.bellows@linaro.org> wrote:
> Added CP register definitions for SP_EL1 and SP_EL2.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> ---
>  target-arm/helper.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d5f0997..ae7394d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2096,7 +2096,7 @@ static uint64_t aa64_dczid_read(CPUARMState *env, const ARMCPRegInfo *ri)
>      return cpu->dcz_blocksize | dzp_bit;
>  }
>
> -static CPAccessResult sp_el0_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +static CPAccessResult sp_el_access(CPUARMState *env, const ARMCPRegInfo *ri)

This isn't necessary. The access function for SP_EL0 is there to
check for the "if SPSel.SP is 0 then any access to SP_EL0 using MSR
or MRS is UNDEFINED" case in the spec (ie using MSR/MRS to mess with
the stack pointer currently in use is forbidden). But this isn't
needed for the SP_EL1/2/3, because the RW access permissions ensure
that you can't get at SP_EL1 when you're using it as your SP (and
so the spec for those registers doesn't have any similar clause).
So they can work with just .access and no .accessfn.

>  {
>      if (!(env->pstate & PSTATE_SP)) {
>          /* Access to SP_EL0 is undefined if it's being used as
> @@ -2326,9 +2326,14 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>       */
>      { .name = "SP_EL0", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 1, .opc2 = 0,
> -      .access = PL1_RW, .accessfn = sp_el0_access,
> +      .access = PL1_RW, .accessfn = sp_el_access,
>        .type = ARM_CP_NO_MIGRATE,
>        .fieldoffset = offsetof(CPUARMState, sp_el[0]) },
> +    { .name = "SP_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .accessfn = sp_el_access,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .fieldoffset = offsetof(CPUARMState, sp_el[1]) },
>      { .name = "SPSel", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 2, .opc2 = 0,
>        .type = ARM_CP_NO_MIGRATE,
> @@ -2410,6 +2415,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>        .access = PL2_RW, .writefn = vbar_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
>        .resetvalue = 0 },
> +    { .name = "SP_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
> +      .access = PL3_RW, .accessfn = sp_el_access,
> +      .type = ARM_CP_NO_MIGRATE,
> +      .fieldoffset = offsetof(CPUARMState, sp_el[2]) },

Otherwise OK.

thanks
-- PMM