Re: [PATCH 08/11] target/i386: fix INHIBIT_IRQ/TF/RF handling for PAUSE

[Richard Henderson <richard.henderson@linaro.org>]

On 6/4/24 02:18, Paolo Bonzini wrote:
> PAUSE uses DISAS_NORETURN because the corresponding helper
> calls cpu_loop_exit().  However, while HLT clear HF_INHIBIT_IRQ_MASK
> to correctly handle "STI; HLT", the same is missing from PAUSE.
> And also gen_eob() clears HF_RF_MASK and synthesizes a #DB exception
> if single-step is active; none of this is done by HLT and PAUSE.
> Start fixing PAUSE, HLT will follow.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/misc_helper.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/i386/tcg/misc_helper.c b/target/i386/tcg/misc_helper.c
> index 8316d42ffcd..ed4cda8001e 100644
> --- a/target/i386/tcg/misc_helper.c
> +++ b/target/i386/tcg/misc_helper.c
> @@ -92,6 +92,10 @@ G_NORETURN void helper_pause(CPUX86State *env)
>   {
>       CPUState *cs = env_cpu(env);
>   
> +    /* Do gen_eob() tasks before going back to the main loop.  */
> +    do_end_instruction(env);
> +    helper_rechecking_single_step(env);
> +
>       /* Just let another CPU run.  */
>       cs->exception_index = EXCP_INTERRUPT;
>       cpu_loop_exit(cs);

Perhaps it would be better to do

void helper_cpu_exit(CPUX86State *env)
{
     cpu_exit(env_cpu(env));
}

static void gen_PAUSE(...)
{
     helper_cpu_exit(tcg_env);
     s->base.is_jmp = DISAS_EOB_NEXT;
}

to keep from replicating gen_eob?

Anyway, this is correct, so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~