From 5313b1aaaccca99b2fe5a03fc6b3bfad65597b22 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Thu, 16 Jan 2025 22:31:33 +0100 Subject: [PATCH] accel/tcg: Call tcg_tb_insert() for one-insn TBs MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently one-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 already have the correct values if the first TB instruction causes an exception. However, on several other architectures, restore_state_to_opc() is not stricly limited to state restoration and affects some exception-related registers, where guests can notice incorrect values, for example: - arm's exception.syndrome; - hppa's unwind_breg; - riscv's excp_uw2; - s390x's int_pgm_ilen. 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. Do not call tb_link_page(), which would add such TBs to the QHT, to prevent tb_lookup() from finding them. These TBs are single-use, since subsequent reads from I/O memory may return different values; they are not removed from code_gen_buffer only in order to keep things simple. Co-developed-by: Nina Schoetterl-Glausch Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson Message-ID: <20250116213214.5695-2-iii@linux.ibm.com> --- accel/tcg/translate-all.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 453eb20ec9..7ec1c53f24 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -531,23 +531,32 @@ 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. + * a temporary one-insn TB. + * + * Such TBs must be added to region trees in order to make sure that + * restore_state_to_opc() - which on some architectures is not limited to + * rewinding, but also affects exception handling! - is called when such a + * TB causes an exception. + * + * At the same time, temporary one-insn TBs must be executed at most once, + * because subsequent reads from, e.g., I/O memory may return different + * values. So return early before attempting to link to other TBs or add + * to the QHT. */ 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. -- 2.30.2