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

[Ilya Leoshkevich]

On Thu, 2025-01-16 at 06:54 -0800, Richard Henderson wrote:
> 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~

How can tb_lookup() find it? With this change, it is inserted into
region_trees, but not into tb_ctx.htable - this is done by
tb_link_page(), which is not called. And because it's not in
tb_ctx.htable, it can't end up in tb_jmp_cache either.