diff mbox series

target/arm: Fix register definitions for VMIDR and VMPIDR

Message ID 20180215182421.23788-1-peter.maydell@linaro.org
State Superseded
Headers show
Series target/arm: Fix register definitions for VMIDR and VMPIDR | expand

Commit Message

Peter Maydell Feb. 15, 2018, 6:24 p.m. UTC
The register definitions for VMIDR and VMPIDR have separate
reginfo structs for the AArch32 and AArch64 registers. However
the 32-bit versions are wrong:
 * they use offsetof instead of offsetoflow32 to mark where
   the 32-bit value lives in the uint64_t CPU state field
 * they don't mark themselves as ARM_CP_ALIAS

In particular this means that if you try to use an Arm guest CPU
which enables EL2 on a big-endian host it will assert at reset:
 target/arm/cpu.c:114: cp_reg_check_reset: Assertion `oldvalue == newvalue' failed.

because the reset of the 32-bit register writes to the top
half of the uint64_t.

Correct the errors in the structures.

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

---
This is necessary for 'make check' to pass on big endian
systems with the 'raspi3' board enabled, which is the
first board which has an EL2-enabled-by-default CPU.
---
 target/arm/helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.16.1

Comments

Philippe Mathieu-Daudé Feb. 16, 2018, 1:25 p.m. UTC | #1
On 02/15/2018 03:24 PM, Peter Maydell wrote:
> The register definitions for VMIDR and VMPIDR have separate

> reginfo structs for the AArch32 and AArch64 registers. However

> the 32-bit versions are wrong:

>  * they use offsetof instead of offsetoflow32 to mark where

>    the 32-bit value lives in the uint64_t CPU state field


Maybe we can replace all offsetof() by offsetof64() and forbid the use
of offsetof() in this file, this will make A32 endian/alignment errors
more apparent.

>  * they don't mark themselves as ARM_CP_ALIAS

> 

> In particular this means that if you try to use an Arm guest CPU

> which enables EL2 on a big-endian host it will assert at reset:

>  target/arm/cpu.c:114: cp_reg_check_reset: Assertion `oldvalue == newvalue' failed.

> 

> because the reset of the 32-bit register writes to the top

> half of the uint64_t.

> 

> Correct the errors in the structures.

> 

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

> ---

> This is necessary for 'make check' to pass on big endian

> systems with the 'raspi3' board enabled, which is the

> first board which has an EL2-enabled-by-default CPU.

> ---

>  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 e7586fcf6c..e27957df38 100644

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

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

> @@ -5068,8 +5068,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)

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

>                .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,

>                .access = PL2_RW, .accessfn = access_el3_aa32ns,

> -              .resetvalue = cpu->midr,

> -              .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },

> +              .resetvalue = cpu->midr, .type = ARM_CP_ALIAS,

> +              .fieldoffset = offsetoflow32(CPUARMState, cp15.vpidr_el2) },

>              { .name = "VPIDR_EL2", .state = ARM_CP_STATE_AA64,

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

>                .access = PL2_RW, .resetvalue = cpu->midr,

> @@ -5077,8 +5077,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)

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

>                .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,

>                .access = PL2_RW, .accessfn = access_el3_aa32ns,

> -              .resetvalue = vmpidr_def,

> -              .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },

> +              .resetvalue = vmpidr_def, .type = ARM_CP_ALIAS,

> +              .fieldoffset = offsetoflow32(CPUARMState, cp15.vmpidr_el2) },

>              { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_AA64,

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

>                .access = PL2_RW,

> 


After enjoying scrolling a 6666p specs,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index e7586fcf6c..e27957df38 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5068,8 +5068,8 @@  void register_cp_regs_for_features(ARMCPU *cpu)
             { .name = "VPIDR", .state = ARM_CP_STATE_AA32,
               .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,
               .access = PL2_RW, .accessfn = access_el3_aa32ns,
-              .resetvalue = cpu->midr,
-              .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
+              .resetvalue = cpu->midr, .type = ARM_CP_ALIAS,
+              .fieldoffset = offsetoflow32(CPUARMState, cp15.vpidr_el2) },
             { .name = "VPIDR_EL2", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,
               .access = PL2_RW, .resetvalue = cpu->midr,
@@ -5077,8 +5077,8 @@  void register_cp_regs_for_features(ARMCPU *cpu)
             { .name = "VMPIDR", .state = ARM_CP_STATE_AA32,
               .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
               .access = PL2_RW, .accessfn = access_el3_aa32ns,
-              .resetvalue = vmpidr_def,
-              .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
+              .resetvalue = vmpidr_def, .type = ARM_CP_ALIAS,
+              .fieldoffset = offsetoflow32(CPUARMState, cp15.vmpidr_el2) },
             { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
               .access = PL2_RW,