[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH] target/arm: Fix register definitions for VMIDR an
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-arm] [PATCH] target/arm: Fix register definitions for VMIDR and VMPIDR |
Date: |
Fri, 16 Feb 2018 10:25:38 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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 <address@hidden>
> ---
> 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é <address@hidden>