Re: [Qemu-devel] [PATCH v8 20/27] target-arm: make IFSR banked

[Peter Maydell]

On 30 October 2014 21:28, Greg Bellows <address@hidden> wrote:
> From: Fabian Aggeler <address@hidden>
>
> When EL3 is running in AArch32 (or ARMv7 with Security Extensions)
> IFSR has a secure and a non-secure instance.
>
> Signed-off-by: Fabian Aggeler <address@hidden>
> Signed-off-by: Greg Bellows <address@hidden>
> ---
>  target-arm/cpu.h    | 10 +++++++++-
>  target-arm/helper.c |  9 +++++----
>  2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 6d39af1..c44649e 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -251,7 +251,15 @@ typedef struct CPUARMState {
>          uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
>          uint64_t hcr_el2; /* Hypervisor configuration register */
>          uint64_t scr_el3; /* Secure configuration register.  */
> -        uint32_t ifsr_el2; /* Fault status registers.  */
> +        union { /* Fault status registers.  */
> +            struct {
> +                uint32_t ifsr_ns;
> +                uint32_t ifsr_s;
> +            };
> +            struct {
> +                uint32_t ifsr32_el2;
> +            };
> +        };

Again, if we have the struct field we should have the reginfo.

>          uint64_t esr_el[4];
>          uint32_t c6_region[8]; /* MPU base/size registers.  */
>          uint64_t far_el[4]; /* Fault address registers.  */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index eaae534..de355f5 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1653,8 +1653,9 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
>        .fieldoffset = offsetoflow32(CPUARMState, cp15.esr_el[1]),
>        .resetfn = arm_cp_reset_ignore, },
>      { .name = "IFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 1,
> -      .access = PL1_RW,
> -      .fieldoffset = offsetof(CPUARMState, cp15.ifsr_el2), .resetvalue = 0, 
> },
> +      .access = PL1_RW, .resetvalue = 0,
> +      .bank_fieldoffsets = { offsetof(CPUARMState, cp15.ifsr_s),
> +                             offsetof(CPUARMState, cp15.ifsr_ns) } },
>      { .name = "ESR_EL1", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .crn = 5, .crm = 2, .opc1 = 0, .opc2 = 0,
>        .access = PL1_RW,
> @@ -4297,11 +4298,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
>          env->exception.fsr = 2;
>          /* Fall through to prefetch abort.  */
>      case EXCP_PREFETCH_ABORT:
> -        env->cp15.ifsr_el2 = env->exception.fsr;
> +        A32_BANKED_CURRENT_REG_SET(env, ifsr, env->exception.fsr);
>          env->cp15.far_el[1] = deposit64(env->cp15.far_el[1], 32, 32,
>                                          env->exception.vaddress);
>          qemu_log_mask(CPU_LOG_INT, "...with IFSR 0x%x IFAR 0x%x\n",
> -                      env->cp15.ifsr_el2, (uint32_t)env->exception.vaddress);
> +                      env->exception.fsr, (uint32_t)env->exception.vaddress);
>          new_mode = ARM_CPU_MODE_ABT;
>          addr = 0x0c;
>          mask = CPSR_A | CPSR_I;

This will work OK for now; if we want to implement SCR.EA to
allow routing of external aborts to Monitor mode we're going
to have to come up with something cleverer, because that
affects whether we want to write to the secure or nonsecure
copy of the IFSR/IFAR. But at the moment you can only get
a prefetch abort or data abort to the Secure world if you
were already in the Secure world.

thanks
-- PMM