On 10 January 2018 at 17:49, Richard Henderson <richard.hender...@linaro.org> wrote: > On 01/10/2018 05:48 AM, Pavel Dovgalyuk wrote: >> Flushing TB cache is required because TBs key in the cache may match >> different code which existed in the previous state. >> >> Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru> >> Signed-off-by: Maria Klimushenkova <maria.klimushenk...@ispras.ru> >> --- >> exec.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/exec.c b/exec.c >> index 4722e52..ff31e71 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -622,6 +622,7 @@ static int cpu_common_post_load(void *opaque, int >> version_id) >> version_id is increased. */ >> cpu->interrupt_request &= ~0x01; >> tlb_flush(cpu); >> + tb_flush(cpu); > > I'm not necessarily objecting, but what do you mean by "may match different > code"? > > What this patch suggests is that the inputs to the computation of TB->FLAGS > are > different for some unspecified reason. Without further explanation, to me > this > suggests a bug in vmstate save/restore.
Yeah, this looks a little fishy. If there's code in the TB cache which would be wrong for the freshly-reset (or whatever) CPU after a VM state load, then it could just as easily be wrong for a 2nd CPU in an SMP config. I used to think it was OK to have the generated code bake in some information that wasn't in tb_flags as long as you then did a tb_flush when that information changed, but I realized I was wrong about that (because of the SMP issue). git grep suggests we do still have a few places in targets that are calling tb_flush(), but I think we should try to fix those. thanks -- PMM
