Peter Maydell <peter.mayd...@linaro.org> writes: > On Thu, 16 Jan 2025 at 11:48, Ilya Leoshkevich <i...@linux.ibm.com> wrote: >> >> On Thu, 2025-01-16 at 11:06 +0000, Peter Maydell wrote: >> > The original reported problem here seems to me like it's a >> > problem with whatever target's frontend code this is. >> > This is a single instruction TB, so either: >> > * the generated code for it completes the insn without >> > raising an exception (no problem) >> > * the generated code for it should raise an exception >> > without having modified the CPU state (so there would >> > be nothing to do for restore_state_to_opc) >> > >> > It sounds like the target is generating code which does >> > something like: >> > * do part of the instruction, including updating some of >> > the CPU state >> > * then decide it needs to raise an exception, and rely on >> > the restore_state_to_opc handling to undo the state updates >> > it did previously >> > >> > The assumption of the "throwaway single insn TB" is that >> > you don't do that (i.e. that restore_state_to_opc is only >> > there for the benefit of multi-insn TBs). > >> The problem is not a partial state update in an instruction, but rather >> that on some targets restore_state_to_opc is more than just a >> "restore" - it is also "prepare for handling an exception", i.e.: >> >> - arm: exception.syndrome >> - hppa: unwind_breg, psw_n >> - mips: btarget >> - openrisc: ppc >> - riscv: excp_uw2 >> - s390x: int_pgm_ilen >> >> Some of these may be wrong due to unfamiliarity with the respective >> architectures, sorry - but this illustrates the idea. > > Ah, yes, thanks for the clear explanation. The "throw away > the TB" design didn't consider that (or vice-versa).
We can certainly do with better docstrings for tcg_tb_lookup (via the region tree) and tb_lookup (using cache and/or QHT) to make it clear the difference between the two. I don't think we should ever use tcg_tb_lookup for the purposes of executing a TB, just for resolution. We have a few spare CF_ flags so maybe we could have a CF_RUNONCE flag which is set for these TBs and assert its not set in tb_lookup along with the current CF_INVALID flag. We could possibly set CF_INVALID before executing the TB as we don't check the tb state from tb_gen_code() before executing it but I guess that might be a little too magic. Rich, WDYT? > > thanks > -- PMM -- Alex Bennée Virtualisation Tech Lead @ Linaro
