On 10/24/22 15:24, Richard Henderson wrote:
> Add a tcg_ops hook to replace the restore_state_to_opc
> function call. Because these generic hooks cannot depend
> on target-specific types, temporarily, copy the current
> target_ulong data[] into uint64_t d64[].
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> include/exec/exec-all.h | 2 +-
> include/hw/core/tcg-cpu-ops.h | 11 +++++++++++
> accel/tcg/translate-all.c | 24 ++++++++++++++++++++++--
> 3 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index e5f8b224a5..a772e8cbdc 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -40,7 +40,7 @@ typedef ram_addr_t tb_page_addr_t;
> #endif
>
> void restore_state_to_opc(CPUArchState *env, TranslationBlock *tb,
> - target_ulong *data);
> + target_ulong *data) __attribute__((weak));
Hi Richard, doesn't matter much since this is removed later on, but I wonder
why the need for attribute weak here?
I don't see you overloading this function in later patches..
Thanks,
Claudio
>
> /**
> * cpu_restore_state:
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index 78c6c6635d..20e3c0ffbb 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -31,6 +31,17 @@ struct TCGCPUOps {
> * function to restore all the state, and register it here.
> */
> void (*synchronize_from_tb)(CPUState *cpu, const TranslationBlock *tb);
> + /**
> + * @restore_state_to_opc: Synchronize state from INDEX_op_start_insn
> + *
> + * This is called when we unwind state in the middle of a TB,
> + * usually before raising an exception. Set all part of the CPU
> + * state which are tracked insn-by-insn in the target-specific
> + * arguments to start_insn, passed as @data.
> + */
> + void (*restore_state_to_opc)(CPUState *cpu, const TranslationBlock *tb,
> + const uint64_t *data);
> +
> /** @cpu_exec_enter: Callback for cpu_exec preparation */
> void (*cpu_exec_enter)(CPUState *cpu);
> /** @cpu_exec_exit: Callback for cpu_exec cleanup */
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 4ed75a13e1..19cd23e9a0 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -329,7 +329,6 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
> TranslationBlock *tb,
> {
> target_ulong data[TARGET_INSN_START_WORDS];
> uintptr_t host_pc = (uintptr_t)tb->tc.ptr;
> - CPUArchState *env = cpu->env_ptr;
> const uint8_t *p = tb->tc.ptr + tb->tc.size;
> int i, j, num_insns = tb->icount;
> #ifdef CONFIG_PROFILER
> @@ -368,7 +367,20 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
> TranslationBlock *tb,
> and shift if to the number of actually executed instructions */
> cpu_neg(cpu)->icount_decr.u16.low += num_insns - i;
> }
> - restore_state_to_opc(env, tb, data);
> +
> + {
> + const struct TCGCPUOps *ops = cpu->cc->tcg_ops;
> + __typeof(ops->restore_state_to_opc) restore =
> ops->restore_state_to_opc;
> + if (restore) {
> + uint64_t d64[TARGET_INSN_START_WORDS];
> + for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
> + d64[i] = data[i];
> + }
> + restore(cpu, tb, d64);
> + } else {
> + restore_state_to_opc(cpu->env_ptr, tb, data);
> + }
> + }
>
> #ifdef CONFIG_PROFILER
> qatomic_set(&prof->restore_time,
> @@ -380,6 +392,14 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
> TranslationBlock *tb,
>
> bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
> {
> + /*
> + * The pc update associated with restore without exit will
> + * break the relative pc adjustments performed by TARGET_TB_PCREL.
> + */
> + if (TARGET_TB_PCREL) {
> + assert(will_exit);
> + }
> +
> /*
> * The host_pc has to be in the rx region of the code buffer.
> * If it is not we will not be able to resolve it here.
