Richard Henderson <richard.hender...@linaro.org> writes: > On 1/16/25 07:06, Ilya Leoshkevich wrote: >> 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 <n...@linux.ibm.com> >>>> Signed-off-by: Ilya Leoshkevich <i...@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. > > You're absolutely right. > > Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
It would still be nice to update the docstrings on the two lookup functions to make it clear what they are for though. -- Alex Bennée Virtualisation Tech Lead @ Linaro
