| Message ID | 299bd2202b8bda7c319cfd172eb1a7ca5a175d47.1403757527.git.alistair.francis@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 26 June 2014 06:02, Alistair Francis <alistair.francis@xilinx.com> wrote: > Call the new pmccntr_sync() function when there is a possibility > of swapping ELs (I.E. when there is an exception) > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > target-arm/helper-a64.c | 5 +++++ > target-arm/helper.c | 7 +++++++ > target-arm/op_helper.c | 6 ++++++ > 3 files changed, 18 insertions(+), 0 deletions(-) > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index 2b4ce6a..b61174f 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -446,6 +446,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > target_ulong addr = env->cp15.vbar_el[1]; > int i; > > + pmccntr_sync(env); > + > if (arm_current_pl(env) == 0) { > if (env->aarch64) { > addr += 0x400; > @@ -484,6 +486,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > addr += 0x100; > break; > default: > + pmccntr_sync(env); > cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); > } > > @@ -511,4 +514,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > env->pc = addr; > cs->interrupt_request |= CPU_INTERRUPT_EXITTB; > + > + pmccntr_sync(env); > } > diff --git a/target-arm/helper.c b/target-arm/helper.c > index e78c5a7..f05d912 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -3457,6 +3457,8 @@ void arm_cpu_do_interrupt(CPUState *cs) > > assert(!IS_M(env)); > > + pmccntr_sync(env); > + > arm_log_exception(cs->exception_index); > > /* TODO: Vectored interrupt controller. */ > @@ -3487,6 +3489,7 @@ void arm_cpu_do_interrupt(CPUState *cs) > && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) { > env->regs[0] = do_arm_semihosting(env); > qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n"); > + pmccntr_sync(env); > return; > } > } > @@ -3505,6 +3508,7 @@ void arm_cpu_do_interrupt(CPUState *cs) > env->regs[15] += 2; > env->regs[0] = do_arm_semihosting(env); > qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n"); > + pmccntr_sync(env); > return; > } > } > @@ -3548,6 +3552,7 @@ void arm_cpu_do_interrupt(CPUState *cs) > offset = 4; > break; > default: > + pmccntr_sync(env); > cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); > return; /* Never happens. Keep compiler happy. */ > } > @@ -3580,6 +3585,8 @@ void arm_cpu_do_interrupt(CPUState *cs) > env->regs[14] = env->regs[15] + offset; > env->regs[15] = addr; > cs->interrupt_request |= CPU_INTERRUPT_EXITTB; > + > + pmccntr_sync(env); > } > > /* Check section/page access permissions. > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 9c1ef52..07ab30b 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -376,6 +376,8 @@ void HELPER(exception_return)(CPUARMState *env) > uint32_t spsr = env->banked_spsr[spsr_idx]; > int new_el, i; > > + pmccntr_sync(env); > + > if (env->pstate & PSTATE_SP) { > env->sp_el[cur_el] = env->xregs[31]; > } else { > @@ -418,6 +420,8 @@ void HELPER(exception_return)(CPUARMState *env) > env->pc = env->elr_el[cur_el]; > } > > + pmccntr_sync(env); > + > return; > > illegal_return: > @@ -433,6 +437,8 @@ illegal_return: > spsr &= PSTATE_NZCV | PSTATE_DAIF; > spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF); > pstate_write(env, spsr); > + > + pmccntr_sync(env); > } This feels way too fragile to me to be scattering these sync calls all over the place like this. We need to find a better approach than this. thanks -- PMM
On Sat, Aug 2, 2014 at 1:35 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 26 June 2014 06:02, Alistair Francis <alistair.francis@xilinx.com> wrote: >> Call the new pmccntr_sync() function when there is a possibility >> of swapping ELs (I.E. when there is an exception) >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> target-arm/helper-a64.c | 5 +++++ >> target-arm/helper.c | 7 +++++++ >> target-arm/op_helper.c | 6 ++++++ >> 3 files changed, 18 insertions(+), 0 deletions(-) >> >> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c >> index 2b4ce6a..b61174f 100644 >> --- a/target-arm/helper-a64.c >> +++ b/target-arm/helper-a64.c >> @@ -446,6 +446,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs) >> target_ulong addr = env->cp15.vbar_el[1]; >> int i; >> >> + pmccntr_sync(env); >> + >> if (arm_current_pl(env) == 0) { >> if (env->aarch64) { >> addr += 0x400; >> @@ -484,6 +486,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) >> addr += 0x100; >> break; >> default: >> + pmccntr_sync(env); >> cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); >> } >> >> @@ -511,4 +514,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs) >> >> env->pc = addr; >> cs->interrupt_request |= CPU_INTERRUPT_EXITTB; >> + >> + pmccntr_sync(env); >> } >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index e78c5a7..f05d912 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -3457,6 +3457,8 @@ void arm_cpu_do_interrupt(CPUState *cs) >> >> assert(!IS_M(env)); >> >> + pmccntr_sync(env); >> + >> arm_log_exception(cs->exception_index); >> >> /* TODO: Vectored interrupt controller. */ >> @@ -3487,6 +3489,7 @@ void arm_cpu_do_interrupt(CPUState *cs) >> && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) { >> env->regs[0] = do_arm_semihosting(env); >> qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n"); >> + pmccntr_sync(env); >> return; >> } >> } >> @@ -3505,6 +3508,7 @@ void arm_cpu_do_interrupt(CPUState *cs) >> env->regs[15] += 2; >> env->regs[0] = do_arm_semihosting(env); >> qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n"); >> + pmccntr_sync(env); >> return; >> } >> } >> @@ -3548,6 +3552,7 @@ void arm_cpu_do_interrupt(CPUState *cs) >> offset = 4; >> break; >> default: >> + pmccntr_sync(env); >> cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); >> return; /* Never happens. Keep compiler happy. */ >> } >> @@ -3580,6 +3585,8 @@ void arm_cpu_do_interrupt(CPUState *cs) >> env->regs[14] = env->regs[15] + offset; >> env->regs[15] = addr; >> cs->interrupt_request |= CPU_INTERRUPT_EXITTB; >> + >> + pmccntr_sync(env); >> } >> >> /* Check section/page access permissions. >> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c >> index 9c1ef52..07ab30b 100644 >> --- a/target-arm/op_helper.c >> +++ b/target-arm/op_helper.c >> @@ -376,6 +376,8 @@ void HELPER(exception_return)(CPUARMState *env) >> uint32_t spsr = env->banked_spsr[spsr_idx]; >> int new_el, i; >> >> + pmccntr_sync(env); >> + >> if (env->pstate & PSTATE_SP) { >> env->sp_el[cur_el] = env->xregs[31]; >> } else { >> @@ -418,6 +420,8 @@ void HELPER(exception_return)(CPUARMState *env) >> env->pc = env->elr_el[cur_el]; >> } >> >> + pmccntr_sync(env); >> + >> return; >> >> illegal_return: >> @@ -433,6 +437,8 @@ illegal_return: >> spsr &= PSTATE_NZCV | PSTATE_DAIF; >> spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF); >> pstate_write(env, spsr); >> + >> + pmccntr_sync(env); >> } > > This feels way too fragile to me to be scattering these sync > calls all over the place like this. We need to find a better > approach than this. > If we could consolidate all the code paths that actually switch el (both ways) into a helper fn we could then add the pmccntr el-switching side effects (i.e. the sync) to that helper once. Would you accept such an approach? Regards, Peter > thanks > -- PMM >
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index 2b4ce6a..b61174f 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -446,6 +446,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs) target_ulong addr = env->cp15.vbar_el[1]; int i; + pmccntr_sync(env); + if (arm_current_pl(env) == 0) { if (env->aarch64) { addr += 0x400; @@ -484,6 +486,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) addr += 0x100; break; default: + pmccntr_sync(env); cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); } @@ -511,4 +514,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs) env->pc = addr; cs->interrupt_request |= CPU_INTERRUPT_EXITTB; + + pmccntr_sync(env); } diff --git a/target-arm/helper.c b/target-arm/helper.c index e78c5a7..f05d912 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -3457,6 +3457,8 @@ void arm_cpu_do_interrupt(CPUState *cs) assert(!IS_M(env)); + pmccntr_sync(env); + arm_log_exception(cs->exception_index); /* TODO: Vectored interrupt controller. */ @@ -3487,6 +3489,7 @@ void arm_cpu_do_interrupt(CPUState *cs) && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) { env->regs[0] = do_arm_semihosting(env); qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n"); + pmccntr_sync(env); return; } } @@ -3505,6 +3508,7 @@ void arm_cpu_do_interrupt(CPUState *cs) env->regs[15] += 2; env->regs[0] = do_arm_semihosting(env); qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n"); + pmccntr_sync(env); return; } } @@ -3548,6 +3552,7 @@ void arm_cpu_do_interrupt(CPUState *cs) offset = 4; break; default: + pmccntr_sync(env); cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); return; /* Never happens. Keep compiler happy. */ } @@ -3580,6 +3585,8 @@ void arm_cpu_do_interrupt(CPUState *cs) env->regs[14] = env->regs[15] + offset; env->regs[15] = addr; cs->interrupt_request |= CPU_INTERRUPT_EXITTB; + + pmccntr_sync(env); } /* Check section/page access permissions. diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 9c1ef52..07ab30b 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -376,6 +376,8 @@ void HELPER(exception_return)(CPUARMState *env) uint32_t spsr = env->banked_spsr[spsr_idx]; int new_el, i; + pmccntr_sync(env); + if (env->pstate & PSTATE_SP) { env->sp_el[cur_el] = env->xregs[31]; } else { @@ -418,6 +420,8 @@ void HELPER(exception_return)(CPUARMState *env) env->pc = env->elr_el[cur_el]; } + pmccntr_sync(env); + return; illegal_return: @@ -433,6 +437,8 @@ illegal_return: spsr &= PSTATE_NZCV | PSTATE_DAIF; spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF); pstate_write(env, spsr); + + pmccntr_sync(env); } /* ??? Flag setting arithmetic is awkward because we need to do comparisons.
Call the new pmccntr_sync() function when there is a possibility of swapping ELs (I.E. when there is an exception) Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- target-arm/helper-a64.c | 5 +++++ target-arm/helper.c | 7 +++++++ target-arm/op_helper.c | 6 ++++++ 3 files changed, 18 insertions(+), 0 deletions(-)