diff mbox series

[02/10] target/arm: Add missing .cp = 15 to HMAIR1 and HAMAIR1 regdefs

Message ID 20180814124254.5229-3-peter.maydell@linaro.org
State Superseded
Headers show
Series target/arm: Some pieces of support for 32-bit Hyp mode | expand

Commit Message

Peter Maydell Aug. 14, 2018, 12:42 p.m. UTC 
ARMCPRegInfo structs will not default to .cp = 15 if they
are ARM_CP_STATE_BOTH, but not if they are ARM_CP_STATE_AA32
(because a coprocessor number of 0 is valid for AArch32).
We forgot to explicitly set .cp = 15 for the HMAIR1 and
HAMAIR1 regdefs, which meant they would UNDEF when the guest
tried to access them under cp15.

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

---
A quick grep suggests these are the only ones we got wrong.
---
 target/arm/helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.18.0

Comments

Edgar E. Iglesias Aug. 14, 2018, 2:41 p.m. UTC | #1 
On Tue, Aug 14, 2018 at 01:42:46PM +0100, Peter Maydell wrote:
> ARMCPRegInfo structs will not default to .cp = 15 if they

                            ^^^
               I think you've got a typo here


Otherwise:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>




> are ARM_CP_STATE_BOTH, but not if they are ARM_CP_STATE_AA32

> (because a coprocessor number of 0 is valid for AArch32).

> We forgot to explicitly set .cp = 15 for the HMAIR1 and

> HAMAIR1 regdefs, which meant they would UNDEF when the guest

> tried to access them under cp15.

> 

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

> ---

> A quick grep suggests these are the only ones we got wrong.

> ---

>  target/arm/helper.c | 8 ++++----

>  1 file changed, 4 insertions(+), 4 deletions(-)

> 

> diff --git a/target/arm/helper.c b/target/arm/helper.c

> index 2c5e02c0b1a..466c8ae492e 100644

> --- a/target/arm/helper.c

> +++ b/target/arm/helper.c

> @@ -3767,14 +3767,14 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "HMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },

>      { .name = "AMAIR_EL2", .state = ARM_CP_STATE_BOTH,

>        .opc0 = 3, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 0,

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "HAMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "AFSR0_EL2", .state = ARM_CP_STATE_BOTH,

> @@ -3917,7 +3917,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[2]),

>        .resetvalue = 0 },

>      { .name = "HMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_ALIAS,

>        .fieldoffset = offsetofhigh32(CPUARMState, cp15.mair_el[2]) },

>      { .name = "AMAIR_EL2", .state = ARM_CP_STATE_BOTH,

> @@ -3926,7 +3926,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

>        .resetvalue = 0 },

>      /* HAMAIR1 is mapped to AMAIR_EL2[63:32] */

>      { .name = "HAMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "AFSR0_EL2", .state = ARM_CP_STATE_BOTH,

> -- 

> 2.18.0

>
Peter Maydell Aug. 14, 2018, 2:45 p.m. UTC | #2 
On 14 August 2018 at 15:41, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Tue, Aug 14, 2018 at 01:42:46PM +0100, Peter Maydell wrote:

>> ARMCPRegInfo structs will not default to .cp = 15 if they

>                             ^^^

>                I think you've got a typo here


Yep, that 'not' should be deleted.

> Otherwise:

> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


thanks
-- PMM
Luc Michel Aug. 15, 2018, 9:10 a.m. UTC | #3 
On 8/14/18 2:42 PM, Peter Maydell wrote:
> ARMCPRegInfo structs will not default to .cp = 15 if they

"will default" as suggested by Edgar.
> are ARM_CP_STATE_BOTH, but not if they are ARM_CP_STATE_AA32

> (because a coprocessor number of 0 is valid for AArch32).

> We forgot to explicitly set .cp = 15 for the HMAIR1 and

> HAMAIR1 regdefs, which meant they would UNDEF when the guest

> tried to access them under cp15.

> 

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


Reviewed-By: Luc Michel <luc.michel@greensocs.com>


> ---

> A quick grep suggests these are the only ones we got wrong.

> ---

>  target/arm/helper.c | 8 ++++----

>  1 file changed, 4 insertions(+), 4 deletions(-)

> 

> diff --git a/target/arm/helper.c b/target/arm/helper.c

> index 2c5e02c0b1a..466c8ae492e 100644

> --- a/target/arm/helper.c

> +++ b/target/arm/helper.c

> @@ -3767,14 +3767,14 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "HMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },

>      { .name = "AMAIR_EL2", .state = ARM_CP_STATE_BOTH,

>        .opc0 = 3, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 0,

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "HAMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "AFSR0_EL2", .state = ARM_CP_STATE_BOTH,

> @@ -3917,7 +3917,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[2]),

>        .resetvalue = 0 },

>      { .name = "HMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_ALIAS,

>        .fieldoffset = offsetofhigh32(CPUARMState, cp15.mair_el[2]) },

>      { .name = "AMAIR_EL2", .state = ARM_CP_STATE_BOTH,

> @@ -3926,7 +3926,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

>        .resetvalue = 0 },

>      /* HAMAIR1 is mapped to AMAIR_EL2[63:32] */

>      { .name = "HAMAIR1", .state = ARM_CP_STATE_AA32,

> -      .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

> +      .cp = 15, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,

>        .access = PL2_RW, .type = ARM_CP_CONST,

>        .resetvalue = 0 },

>      { .name = "AFSR0_EL2", .state = ARM_CP_STATE_BOTH,

>
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2c5e02c0b1a..466c8ae492e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3767,14 +3767,14 @@  static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
       .access = PL2_RW, .type = ARM_CP_CONST,
       .resetvalue = 0 },
     { .name = "HMAIR1", .state = ARM_CP_STATE_AA32,
-      .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,
+      .cp = 15, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "AMAIR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 0,
       .access = PL2_RW, .type = ARM_CP_CONST,
       .resetvalue = 0 },
     { .name = "HAMAIR1", .state = ARM_CP_STATE_AA32,
-      .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,
+      .cp = 15, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,
       .access = PL2_RW, .type = ARM_CP_CONST,
       .resetvalue = 0 },
     { .name = "AFSR0_EL2", .state = ARM_CP_STATE_BOTH,
@@ -3917,7 +3917,7 @@  static const ARMCPRegInfo el2_cp_reginfo[] = {
       .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[2]),
       .resetvalue = 0 },
     { .name = "HMAIR1", .state = ARM_CP_STATE_AA32,
-      .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,
+      .cp = 15, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 1,
       .access = PL2_RW, .type = ARM_CP_ALIAS,
       .fieldoffset = offsetofhigh32(CPUARMState, cp15.mair_el[2]) },
     { .name = "AMAIR_EL2", .state = ARM_CP_STATE_BOTH,
@@ -3926,7 +3926,7 @@  static const ARMCPRegInfo el2_cp_reginfo[] = {
       .resetvalue = 0 },
     /* HAMAIR1 is mapped to AMAIR_EL2[63:32] */
     { .name = "HAMAIR1", .state = ARM_CP_STATE_AA32,
-      .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,
+      .cp = 15, .opc1 = 4, .crn = 10, .crm = 3, .opc2 = 1,
       .access = PL2_RW, .type = ARM_CP_CONST,
       .resetvalue = 0 },
     { .name = "AFSR0_EL2", .state = ARM_CP_STATE_BOTH,