Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 03/22/2011 07:50 AM, Stefan Berger wrote:
> On 03/22/2011 06:40 AM, Avi Kivity wrote:
>> On 03/22/2011 12:23 PM, Stefan Berger wrote:
>>> On 03/22/2011 05:28 AM, Avi Kivity wrote:
>>>> On 03/22/2011 03:46 AM, Anthony Liguori wrote:
>>>>> On 03/21/2011 07:25 PM, Stefan Berger wrote:
>>>>>> On 03/15/2011 10:53 AM, Juan Quintela wrote:
>>>>>>> commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c
>>>>>>>
>>>>>>> only contains half of the fix.  It forgots the save state fix for
>>>>>>> UINT8 indexes.
>>>>>>>
>>>>>>> Anthony, please apply, without this migration using hpet is broken.
>>>>>>> (only current user).
>>>>>> I have just been bisecting the code (from the tip) due to 
>>>>>> suspend/resume problems and it looks like commit 82fa39b7 is 
>>>>>> introducing the suspend/resume problem I am seeing (frozen screen).
>>>>>
>>>>> It's in tip now.
>>>>
>>>> Great, spent some lovely time bisecting and fixing it as well.
>>>>
>>> It doesn't work better now than it did before... Trying a 
>>> suspend/resume while in grub leaves me with a black screen upon 
>>> resume...
>>
>> Well, it fixed it for me (autotest migration tests).
>>
>> Do you mean that 82fa39b7^ works but b784421ce4c doesn't?  What about 
>> b784421ce4c^ with 82fa39b7 reverted?  That will rule out some other bug.
>>
> b784421 works for me. It's the tip that is again broken for 
> suspend/resume, this time pointing to
>
> c995b495b9d6e60ab1e390bd398a22425d0b3c8c is the first bad commit
> commit c995b495b9d6e60ab1e390bd398a22425d0b3c8c
> Author: Jan Kiszka <jan.kiszka@siemens.com>
> Date:   Tue Mar 15 12:26:22 2011 +0100
>
>     x86: Save/restore PAT MSR
>
>     Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>     Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index d78eceb..6384f54 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -491,6 +491,8 @@ static const VMStateDescription vmstate_cpu = {
>          VMSTATE_UINT64_V(xcr0, CPUState, 12),
>          VMSTATE_UINT64_V(xstate_bv, CPUState, 12),
>          VMSTATE_YMMH_REGS_VARS(ymmh_regs, CPUState, CPU_NB_REGS, 12),
> +
> +        VMSTATE_UINT64_V(pat, CPUState, 13),
>          VMSTATE_END_OF_LIST()
>          /* The above list is not sorted /wrt version numbers, watch 
> out! */
>      },
>
> Doesn't look bad, but I get a black screen when resuming while in grub.
>
I think that patch was probably not necessary:

target-i386/machine.c line 452 has this:

         VMSTATE_UINT64_V(pat, CPUState, 5),

then again in line 495:

         VMSTATE_UINT64_V(pat, CPUState, 13),

    Stefan