| Message ID | 20250115232022.27332-1-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: Call tcg_tb_insert() for one-insn TBs | expand |
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.
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.
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(-)