diff mbox

[v3,19/22] target-arm: A64: Register VBAR_EL2

Message ID 1400491383-6725-20-git-send-email-edgar.iglesias@gmail.com
State New
Headers show

Commit Message

Edgar E. Iglesias May 19, 2014, 9:23 a.m. UTC 
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/helper.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

Comments

Peter Crosthwaite May 20, 2014, 9:02 a.m. UTC | #1 
On Mon, May 19, 2014 at 7:23 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/cpu.h    |  2 +-
>  target-arm/helper.c | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 7322e03..693ad0f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -198,7 +198,7 @@ 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 vbar_el[2]; /* 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.  */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5a2073e..7ca63a8 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2077,6 +2077,15 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>      REGINFO_SENTINEL
>  };
>
> +/* Used to describe the behaviour of EL2 regs when EL2 does not exist.  */
> +static const ARMCPRegInfo v8_no_el2_cp_reginfo[] = {

Should it be v8_el3_no_el2_cp_reginfo?

Otherwise,

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

> +    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
> +      .access = PL2_RW,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +    REGINFO_SENTINEL
> +};
> +
>  static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>      { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
> @@ -2087,6 +2096,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[2]),
> +      .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>
> @@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
>      if (arm_feature(env, ARM_FEATURE_EL2)) {
>          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> +    } else {
> +        /* If EL2 is missing but higher ELs are enabled, we need to
> +         * register the no_el2 reginfos.  */
> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
> +        }
>      }
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> --
> 1.8.3.2
>
>
Edgar E. Iglesias May 21, 2014, 1:01 a.m. UTC | #2 
On Tue, May 20, 2014 at 07:02:37PM +1000, Peter Crosthwaite wrote:
> On Mon, May 19, 2014 at 7:23 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/cpu.h    |  2 +-
> >  target-arm/helper.c | 20 ++++++++++++++++++++
> >  2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 7322e03..693ad0f 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -198,7 +198,7 @@ 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 vbar_el[2]; /* 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.  */
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 5a2073e..7ca63a8 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2077,6 +2077,15 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
> >      REGINFO_SENTINEL
> >  };
> >
> > +/* Used to describe the behaviour of EL2 regs when EL2 does not exist.  */
> > +static const ARMCPRegInfo v8_no_el2_cp_reginfo[] = {
> 
> Should it be v8_el3_no_el2_cp_reginfo?

I've changed it to v8_el3_no_el2_cp_reginfo.

Thanks,
Edgar

> 
> Otherwise,
> 
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Peter Maydell May 21, 2014, 7:22 p.m. UTC | #3 
On 19 May 2014 10:23, 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>

> @@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
>      if (arm_feature(env, ARM_FEATURE_EL2)) {
>          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> +    } else {
> +        /* If EL2 is missing but higher ELs are enabled, we need to
> +         * register the no_el2 reginfos.  */

Very nitpicky, but my preferred multiline comment style puts the
final "*/" on its own line rather than at the end of the preceding.

> +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
> +        }
>      }
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> --
> 1.8.3.2

thanks
-- PMM
Edgar E. Iglesias May 22, 2014, 1:11 a.m. UTC | #4 
On Wed, May 21, 2014 at 08:22:27PM +0100, Peter Maydell wrote:
> On 19 May 2014 10:23, 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>
> 
> > @@ -2356,6 +2370,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> >      }
> >      if (arm_feature(env, ARM_FEATURE_EL2)) {
> >          define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
> > +    } else {
> > +        /* If EL2 is missing but higher ELs are enabled, we need to
> > +         * register the no_el2 reginfos.  */
> 
> Very nitpicky, but my preferred multiline comment style puts the
> final "*/" on its own line rather than at the end of the preceding.

I changed it to your style.

Personally, I try to avoid these kind of requests because I feel it
adds unnecessary friction to upstreaming but I have no problems
with fixing it up, so feel free to keep on "nitpicking" :-)

Thanks,
Edgar


> 
> > +        if (arm_feature(env, ARM_FEATURE_EL3)) {
> > +            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
> > +        }
> >      }
> >      if (arm_feature(env, ARM_FEATURE_EL3)) {
> >          define_arm_cp_regs(cpu, v8_el3_cp_reginfo);
> > --
> > 1.8.3.2
> 
> thanks
> -- PMM
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7322e03..693ad0f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -198,7 +198,7 @@  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 vbar_el[2]; /* 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.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5a2073e..7ca63a8 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2077,6 +2077,15 @@  static const ARMCPRegInfo v8_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+/* Used to describe the behaviour of EL2 regs when EL2 does not exist.  */
+static const ARMCPRegInfo v8_no_el2_cp_reginfo[] = {
+    { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
+      .access = PL2_RW,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+    REGINFO_SENTINEL
+};
+
 static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
     { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_NO_MIGRATE,
@@ -2087,6 +2096,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[2]),
+      .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
@@ -2356,6 +2370,12 @@  void register_cp_regs_for_features(ARMCPU *cpu)
     }
     if (arm_feature(env, ARM_FEATURE_EL2)) {
         define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
+    } else {
+        /* If EL2 is missing but higher ELs are enabled, we need to
+         * register the no_el2 reginfos.  */
+        if (arm_feature(env, ARM_FEATURE_EL3)) {
+            define_arm_cp_regs(cpu, v8_no_el2_cp_reginfo);
+        }
     }
     if (arm_feature(env, ARM_FEATURE_EL3)) {
         define_arm_cp_regs(cpu, v8_el3_cp_reginfo);