[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] cpu: flush TB cache when loading VMState
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] cpu: flush TB cache when loading VMState |
Date: |
Wed, 10 Jan 2018 18:32:05 +0000 |
On 10 January 2018 at 17:49, Richard Henderson
<address@hidden> 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 <address@hidden>
>> Signed-off-by: Maria Klimushenkova <address@hidden>
>> ---
>> 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