Re: [PATCH 3/3] accel/tcg: Make TCGCPUOps::cpu_exec_halt mandatory

On 3/6/24 18:09, Peter Maydell wrote:
> Now that all targets set TCGCPUOps::cpu_exec_halt, we can make it
> mandatory and remove the fallback handling that calls cpu_has_work.
> 
> Signed-off-by: Peter Maydell <[email protected]>
> ---
>   include/hw/core/tcg-cpu-ops.h | 9 ++++++---
>   accel/tcg/cpu-exec.c          | 7 +------
>   2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index 099de3375e3..34318cf0e60 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -122,10 +122,13 @@ struct TCGCPUOps {
>        * to do when the CPU is in the halted state.
>        *
>        * Return true to indicate that the CPU should now leave halt, false
> -     * if it should remain in the halted state.
> +     * if it should remain in the halted state. (This should generally
> +     * be the same value that cpu_has_work() would return.)
>        *
> -     * If this method is not provided, the default is to do nothing, and
> -     * to leave halt if cpu_has_work() returns true.
> +     * This method must be provided. If the target does not need to
> +     * do anything special for halt, the same function used for its
> +     * CPUClass::has_work method can be used here, as they have the
> +     * same function signature.

With retrospective, has_work() should be idempotent, taking a const
CPUState, while cpu_exec_halt() is expected to change CPUState
internal fields.

>        */
>       bool (*cpu_exec_halt)(CPUState *cpu);
>       /**
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 6711b58e0b2..8be4d2a1330 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -682,13 +682,8 @@ static inline bool cpu_handle_halt(CPUState *cpu)
>   #ifndef CONFIG_USER_ONLY
>       if (cpu->halted) {
>           const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops;
> -        bool leave_halt;
> +        bool leave_halt = tcg_ops->cpu_exec_halt(cpu);
>   
> -        if (tcg_ops->cpu_exec_halt) {
> -            leave_halt = tcg_ops->cpu_exec_halt(cpu);
> -        } else {
> -            leave_halt = cpu_has_work(cpu);
> -        }
>           if (!leave_halt) {
>               return true;
>           }