[PATCHv2,5/7] target-ppc: force update of msr bits in cpu_post_load

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 except
MSR_TGPR 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 <mark.cave-ayland@ilande.co.uk>
---
 target-ppc/machine.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

On 21.01.15 17:01, 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 except
> MSR_TGPR 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 <mark.cave-ayland@ilande.co.uk>
> ---
>  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..fc8ddcd 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 msr bits except MSR_TGPR invalid before restoring */
> +    msr = env->msr;
> +    env->msr ^= ~(1 << MSR_TGPR);

Doesn't this need to be 1ULL?


Alex

> +    ppc_store_msr(env, msr);
> +
>      hreg_compute_mem_idx(env);
>  
>      return 0;
>

On 22/01/15 13:37, Alexander Graf wrote:

> On 21.01.15 17:01, 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 except
>> MSR_TGPR 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 <mark.cave-ayland@ilande.co.uk>
>> ---
>>  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..fc8ddcd 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 msr bits except MSR_TGPR invalid before restoring */
>> +    msr = env->msr;
>> +    env->msr ^= ~(1 << MSR_TGPR);
> 
> Doesn't this need to be 1ULL?

Yes, you're probably right. Are you able to fix this, or do you need me
to respin a v3?


ATB,

Mark.

> Am 26.01.2015 um 22:41 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
> 
>> On 22/01/15 13:37, Alexander Graf wrote:
>> 
>>> On 21.01.15 17:01, 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 except
>>> MSR_TGPR 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 <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> 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..fc8ddcd 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 msr bits except MSR_TGPR invalid before restoring */
>>> +    msr = env->msr;
>>> +    env->msr ^= ~(1 << MSR_TGPR);
>> 
>> Doesn't this need to be 1ULL?
> 
> Yes, you're probably right. Are you able to fix this, or do you need me
> to respin a v3?

Depends on your answer to the other mail ;). If this is as far as you want to push the patches for now I can certainly apply it and change the line as I go.


Alex