Re: [Qemu-ppc] [PATCH 5/7] ppc: force update of all msr bits in cpu_post_load

[Mark Cave-Ayland]

On 20/01/15 15:01, Paolo Bonzini wrote:

> On 23/12/2014 01:36, Mark Cave-Ayland wrote:
>> Since env->msr has already been restored by the time cpu_post_load is called,
>> make sure that ppc_store_msr() is explicitly called with all msr bits marked
>> as invalid.
>>
>> This solves the issue where MSR flags aren't set correctly when restoring a 
>> VM
>> snapshot, in particular the internal env->excp_prefix value when MSR_EP has
>> been altered by a guest.
>>
>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>> ---
>>  target-ppc/machine.c |    8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>> index c801b82..9669063 100644
>> --- a/target-ppc/machine.c
>> +++ b/target-ppc/machine.c
>> @@ -159,6 +159,7 @@ static int cpu_post_load(void *opaque, int version_id)
>>      PowerPCCPU *cpu = opaque;
>>      CPUPPCState *env = &cpu->env;
>>      int i;
>> +    target_ulong msr;
>>  
>>      /*
>>       * We always ignore the source PVR. The user or management
>> @@ -190,7 +191,12 @@ static int cpu_post_load(void *opaque, int version_id)
>>          /* Restore htab_base and htab_mask variables */
>>          ppc_store_sdr1(env, env->spr[SPR_SDR1]);
>>      }
>> -    hreg_compute_hflags(env);
>> +
>> +    /* Mark all msr bits invalid before restoring */
>> +    msr = env->msr;
>> +    env->msr = ~env->msr;
>> +    ppc_store_msr(env, msr);
>> +
>>      hreg_compute_mem_idx(env);
>>  
>>      return 0;
>>
> 
> This is wrong for this line of hreg_store_msr:
> 
>     if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
>                  ((value ^ env->msr) & (1 << MSR_TGPR)))) {
>         /* Swap temporary saved registers with GPRs */
>         hreg_swap_gpr_tgpr(env);
>     }
> 
> I think you need to change the ~ with "^ ~(1 << MSR_TGPR)".

It took me a little time to decipher how that worked, but yes it seems
fine in my tests here. I'll resend the series with with your Reviewed-by
added to the previous patch.


Many thanks,

Mark.