Re: [PATCH] accel/tcg: Call tcg_tb_insert() for one-insn TBs

[Richard Henderson]

On 1/15/25 15:20, Ilya Leoshkevich wrote:
> Currently single-insn TBs created from I/O memory are not added to
> region_trees. Therefore, when they generate exceptions, they are not
> handled by cpu_restore_state_from_tb(). For x86 this is not a problem,
> because x86_restore_state_to_opc() only restores pc and cc, which are
> already correct. However, on several other architectures,
> restore_state_to_opc() restores more registers, and guests can notice
> incorrect values.
>
> Fix by always calling tcg_tb_insert(). This may increase the size of
> region_trees, but tcg_region_reset_all() clears it once code_gen_buffer
> fills up, so it will not grow uncontrollably.
>
> Co-developed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   accel/tcg/translate-all.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 453eb20ec95..6333302813e 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -531,23 +531,23 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>           tb_reset_jump(tb, 1);
>       }
>   
> +    /*
> +     * Insert TB into the corresponding region tree before publishing it
> +     * through QHT. Otherwise rewinding happened in the TB might fail to
> +     * lookup itself using host PC.
> +     */
> +    tcg_tb_insert(tb);

I think what we need is to mark the tb CF_INVALID before inserting it. That way we'll 
never match in tb_lookup (comparing guest state, including cflags), but *will* find it in 
tcg_tb_lookup (comparing host_pc).


r~