Re: [Qemu-devel] [PATCH QEMU] target-arm: Add support for SPSR_(ABT|UND|IRQ|FIQ)

[Peter Maydell <peter.maydell@linaro.org>]

On 15 October 2015 at 05:41, Soren Brinkmann <soren.brinkmann@xilinx.com> wrote:
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> Hi,
>
> I recently came across some code that caused undefined instruction exceptions
> when executing instructions 'mrs     x11, spsr_abt' and the like. I'm not sure I
> get the full picture, but it seems QEMU already keeps the state for those SPSR
> registers and all that might be missing is exposing those registers to the
> guest.

Well, these are only accessible from EL2 or EL3, and we don't implement
those yet for 64-bit...

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 83679970b432..0c64c0588115 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3281,6 +3281,22 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .type = ARM_CP_ALIAS,
>        .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) },
> +    { .name = "SPSR_IRQ", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_ALIAS,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 0,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[4]) },
> +    { .name = "SPSR_ABT", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_ALIAS,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 1,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[2]) },
> +    { .name = "SPSR_UND", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_ALIAS,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 2,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[3]) },
> +    { .name = "SPSR_FIQ", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_ALIAS,
> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 3, .opc2 = 3,
> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[5]) },
>      { .name = "VBAR_EL2", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
>        .access = PL2_RW, .writefn = vbar_write,

These look OK, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I wonder if we should define some ARM_BANK_* constants for the
indexes into the banked_* arrays?

Providing the 32-bit versions of these would be nice, but they aren't
implemented as system registers and they're kind of complicated, so
we'll leave that for now.

thanks
-- PMM