Re: [Qemu-devel] [RFC PATCH 1/1] target-ppc: Don't invalidate non-supported msr bits

[David Gibson]

On Wed, Nov 29, 2017 at 07:22:19PM +0300, Kurban Mallachiev wrote:
> The msr invalidation code (commits 993eb and 2360b) inverts all
> bits except MSR_TGPR and MSR_HVB. On non PowerPC 601 processors
> this leads to incorrect change of excp_prefix in hreg_store_msr()
> function. The problem is that new msr value get multiplied by msr_mask
> and inverted msr does not, thus values of MSR_EP bit in new msr value
> and inverted msr are distinct, so that excp_prefix changes but should
> not.
> 
> Signed-off-by: Kurban Mallachiev <address@hidden>

So, the whole logic of ppc_store_msr() / hreg_store_msr() looks much
harier than it should be to me.  Nonetheless, this definitely looks
like an improvement over the current code.

Applied to ppc-for-2.11.

Laurent, could this be related to the loadvm state problems you were
seeing in several BZs?

> ---
>  target/ppc/machine.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 384caee800..96113ee881 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -298,9 +298,9 @@ static int cpu_post_load(void *opaque, int version_id)
>          ppc_store_sdr1(env, env->spr[SPR_SDR1]);
>      }
>  
> -    /* Invalidate all msr bits except MSR_TGPR/MSR_HVB before restoring */
> +    /* Invalidate all supported msr bits except MSR_TGPR/MSR_HVB before 
> restoring */
>      msr = env->msr;
> -    env->msr ^= ~((1ULL << MSR_TGPR) | MSR_HVB);
> +    env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
>      ppc_store_msr(env, msr);
>  
>      hreg_compute_mem_idx(env);

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature