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>
> ---

This needs something else.  The reason why they're not insertted is that they're not valid 
for a second execution.  We need to not find them in the search tree.


r~

>   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);
> +
>       /*
>        * If the TB is not associated with a physical RAM page then it must be
>        * a temporary one-insn TB, and we have nothing left to do. Return early
> -     * before attempting to link to other TBs or add to the lookup table.
> +     * before attempting to link to other TBs.
>        */
>       if (tb_page_addr0(tb) == -1) {
>           assert_no_pages_locked();
>           return tb;
>       }
>   
> -    /*
> -     * 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);
> -
>       /*
>        * No explicit memory barrier is required -- tb_link_page() makes the
>        * TB visible in a consistent state.