[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] target/arm: Rebuild hflags at el changes an
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] target/arm: Rebuild hflags at el changes and MSR writes |
Date: |
Tue, 19 Feb 2019 14:48:36 +0000 |
User-agent: |
mu4e 1.1.0; emacs 26.1 |
Richard Henderson <address@hidden> writes:
> Now setting, but not relying upon, env->hflags.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> target/arm/internals.h | 1 +
> linux-user/syscall.c | 1 +
> target/arm/cpu.c | 1 +
> target/arm/helper-a64.c | 3 +++
> target/arm/helper.c | 2 ++
> target/arm/machine.c | 1 +
> target/arm/op_helper.c | 1 +
> target/arm/translate-a64.c | 6 +++++-
> target/arm/translate.c | 14 ++++++++++++--
> 9 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 8c1b813364..235f4fafec 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -970,5 +970,6 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env,
> uint64_t va,
>
> uint32_t rebuild_hflags_a32(CPUARMState *env, int el);
> uint32_t rebuild_hflags_a64(CPUARMState *env, int el);
> +void rebuild_hflags_any(CPUARMState *env);
>
> #endif
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5bbb72f3d5..123f342bdc 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9691,6 +9691,7 @@ static abi_long do_syscall1(void *cpu_env, int num,
> abi_long arg1,
> aarch64_sve_narrow_vq(env, vq);
> }
> env->vfp.zcr_el[1] = vq - 1;
> + arm_rebuild_hflags(env);
> ret = vq * 16;
> }
> return ret;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index edf6e0e1f1..e4da513eb3 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -390,6 +390,7 @@ static void arm_cpu_reset(CPUState *s)
>
> hw_breakpoint_update_all(cpu);
> hw_watchpoint_update_all(cpu);
> + arm_rebuild_hflags(env);
> }
>
> bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
> index 101fa6d3ea..f51cb75444 100644
> --- a/target/arm/helper-a64.c
> +++ b/target/arm/helper-a64.c
> @@ -995,6 +995,7 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t
> new_pc)
> } else {
> env->regs[15] = new_pc & ~0x3;
> }
> + rebuild_hflags_a32(env, new_el);
> qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to "
> "AArch32 EL%d PC 0x%" PRIx32 "\n",
> cur_el, new_el, env->regs[15]);
> @@ -1006,10 +1007,12 @@ void HELPER(exception_return)(CPUARMState *env,
> uint64_t new_pc)
> }
> aarch64_restore_sp(env, new_el);
> env->pc = new_pc;
> + rebuild_hflags_a64(env, new_el);
> qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to "
> "AArch64 EL%d PC 0x%" PRIx64 "\n",
> cur_el, new_el, env->pc);
> }
> +
> /*
> * Note that cur_el can never be 0. If new_el is 0, then
> * el0_a64 is return_to_aa64, else el0_a64 is ignored.
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 7a77f53ba8..d8249f0eae 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -9081,6 +9081,7 @@ static void take_aarch32_exception(CPUARMState *env,
> int new_mode,
> env->regs[14] = env->regs[15] + offset;
> }
> env->regs[15] = newpc;
> + env->hflags = rebuild_hflags_a32(env, arm_current_el(env));
> }
>
> static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
> @@ -9426,6 +9427,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>
> pstate_write(env, PSTATE_DAIF | new_mode);
> env->aarch64 = 1;
> + env->hflags = rebuild_hflags_a64(env, new_el);
> aarch64_restore_sp(env, new_el);
>
> env->pc = addr;
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index b292549614..61889801dd 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -743,6 +743,7 @@ static int cpu_post_load(void *opaque, int version_id)
> if (!kvm_enabled()) {
> pmu_op_finish(&cpu->env);
> }
> + arm_rebuild_hflags(&cpu->env);
>
> return 0;
> }
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index c998eadfaa..8d459d9a84 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -571,6 +571,7 @@ void HELPER(cpsr_write_eret)(CPUARMState *env, uint32_t
> val)
> */
> env->regs[15] &= (env->thumb ? ~1 : ~3);
>
> + rebuild_hflags_a32(env, arm_current_el(env));
> qemu_mutex_lock_iothread();
> arm_call_el_change_hook(arm_env_get_cpu(env));
> qemu_mutex_unlock_iothread();
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index e002251ac6..ad665c1609 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -1841,11 +1841,15 @@ static void handle_sys(DisasContext *s, uint32_t
> insn, bool isread,
> /* I/O operations must end the TB here (whether read or write) */
> gen_io_end();
> s->base.is_jmp = DISAS_UPDATE;
> - } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
> + }
> + if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
Does this potentially introduce a icount bug (or fix an existing bug)?
> /* We default to ending the TB on a coprocessor register write,
> * but allow this to be suppressed by the register definition
> * (usually only necessary to work around guest bugs).
> */
> + TCGv_i32 tcg_el = tcg_const_i32(s->current_el);
> + gen_helper_rebuild_hflags_a64(cpu_env, tcg_el);
> + tcg_temp_free_i32(tcg_el);
> s->base.is_jmp = DISAS_UPDATE;
> }
> }
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 66cf28c8cb..36842a27b3 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8432,6 +8432,8 @@ static int disas_coproc_insn(DisasContext *s, uint32_t
> insn)
> ri = get_arm_cp_reginfo(s->cp_regs,
> ENCODE_CP_REG(cpnum, is64, s->ns, crn, crm, opc1, opc2));
> if (ri) {
> + bool need_exit_tb;
> +
> /* Check access permissions */
> if (!cp_access_ok(s->current_el, ri, isread)) {
> return 1;
> @@ -8604,15 +8606,23 @@ static int disas_coproc_insn(DisasContext *s,
> uint32_t insn)
> }
> }
>
> + need_exit_tb = false;
I'd of initialised this with false up above but I guess that's more a
stylistic choice.
> if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type &
> ARM_CP_IO)) {
> /* I/O operations must end the TB here (whether read or write) */
> gen_io_end();
> - gen_lookup_tb(s);
> - } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
> + need_exit_tb = true;
> + }
> + if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
> /* We default to ending the TB on a coprocessor register write,
> * but allow this to be suppressed by the register definition
> * (usually only necessary to work around guest bugs).
> */
> + TCGv_i32 tcg_el = tcg_const_i32(s->current_el);
> + gen_helper_rebuild_hflags_a32(cpu_env, tcg_el);
> + tcg_temp_free_i32(tcg_el);
> + need_exit_tb = true;
> + }
> + if (need_exit_tb) {
> gen_lookup_tb(s);
> }
Reviewed-by: Alex Bennée <address@hidden>
--
Alex Bennée
- [Qemu-devel] [PATCH 0/4] target/arm: Reduce overhead of cpu_get_tb_cpu_state, Richard Henderson, 2019/02/13
- [Qemu-devel] [PATCH 3/4] target/arm: Assert hflags is correct in cpu_get_tb_cpu_state, Richard Henderson, 2019/02/13
- [Qemu-devel] [PATCH 2/4] target/arm: Rebuild hflags at el changes and MSR writes, Richard Henderson, 2019/02/13
- [Qemu-devel] [PATCH 1/4] target/arm: Split out recompute_hflags et al, Richard Henderson, 2019/02/13
- [Qemu-devel] [PATCH 4/4] target/arm: Rely on hflags correct in cpu_get_tb_cpu_state, Richard Henderson, 2019/02/13
- Re: [Qemu-devel] [PATCH 0/4] target/arm: Reduce overhead of cpu_get_tb_cpu_state, Laurent Desnogues, 2019/02/14
- Re: [Qemu-devel] [PATCH 0/4] target/arm: Reduce overhead of cpu_get_tb_cpu_state, Alex Bennée, 2019/02/14