[Qemu-devel] [PATCH for-2.6] ppc: Fix migration of the XER register

[Qemu-devel] [PATCH for-2.6] ppc: Fix migration of the XER register

[Thomas Huth]

env->xer only holds the lower bits of the XER register nowadays, the
SO, OV and CA bits are stored in separate variables (see the function
cpu_write_xer() for details). Since the migration code currently only
reads the "xer" variable, the upper bits are lost during migration.
Fix it by using cpu_read_xer() instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 In case somebody wants to test this, this problem can easily be
 seen with my SPRs kvm-unit-test on KVM running on a P8 machine:
 https://patchwork.ozlabs.org/patch/607981/

 target-ppc/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 692121e..46684fb 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -136,7 +136,7 @@ static void cpu_pre_save(void *opaque)
 
     env->spr[SPR_LR] = env->lr;
     env->spr[SPR_CTR] = env->ctr;
-    env->spr[SPR_XER] = env->xer;
+    env->spr[SPR_XER] = cpu_read_xer(env);
 #if defined(TARGET_PPC64)
     env->spr[SPR_CFAR] = env->cfar;
 #endif
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.6] ppc: Fix migration of the XER register

[Mark Cave-Ayland]

On 15/04/16 10:03, Thomas Huth wrote:

> env->xer only holds the lower bits of the XER register nowadays, the
> SO, OV and CA bits are stored in separate variables (see the function
> cpu_write_xer() for details). Since the migration code currently only
> reads the "xer" variable, the upper bits are lost during migration.
> Fix it by using cpu_read_xer() instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

This looks like the pre_save counterpart to the post_load patch I sent
in January (see 6a9620e60cc1b16dba9ee9d9d8cb374e4303c072) so I'm fairly
sure this is right.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [Qemu-devel] [PATCH for-2.6] ppc: Fix migration of the XER register

[Thomas Huth]

On 15.04.2016 12:17, Mark Cave-Ayland wrote:
> On 15/04/16 10:03, Thomas Huth wrote:
> 
>> env->xer only holds the lower bits of the XER register nowadays, the
>> SO, OV and CA bits are stored in separate variables (see the function
>> cpu_write_xer() for details). Since the migration code currently only
>> reads the "xer" variable, the upper bits are lost during migration.
>> Fix it by using cpu_read_xer() instead.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> This looks like the pre_save counterpart to the post_load patch I sent
> in January (see 6a9620e60cc1b16dba9ee9d9d8cb374e4303c072) so I'm fairly
> sure this is right.

Ah, ok, that's why the load part was already right :-)

Looks like the cpu_read/write_xer() functions have originally been
introduced in da91a00f ("Split out SO, OV, CA fields from XER"), and
this patch also used it for the load and save functions in machine.c.
However, a little bit later, the functions had been changed to use
VMState instead (see a90db158 - "Convert ppc cpu savevm to
VMStateDescription"), and the cpu_read/write_xer() calls accidentally
got dropped in this patch. So introducing them again now is really the
right thing to do.

> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

 Thanks!
  Thomas

Re: [Qemu-devel] [PATCH for-2.6] ppc: Fix migration of the XER register

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

On Fri, Apr 15, 2016 at 11:03:00AM +0200, Thomas Huth wrote:
> env->xer only holds the lower bits of the XER register nowadays, the
> SO, OV and CA bits are stored in separate variables (see the function
> cpu_write_xer() for details). Since the migration code currently only
> reads the "xer" variable, the upper bits are lost during migration.
> Fix it by using cpu_read_xer() instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  In case somebody wants to test this, this problem can easily be
>  seen with my SPRs kvm-unit-test on KVM running on a P8 machine:
>  https://patchwork.ozlabs.org/patch/607981/
> 
>  target-ppc/machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to ppc-for-2.6, thanks.

> 
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index 692121e..46684fb 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -136,7 +136,7 @@ static void cpu_pre_save(void *opaque)
>  
>      env->spr[SPR_LR] = env->lr;
>      env->spr[SPR_CTR] = env->ctr;
> -    env->spr[SPR_XER] = env->xer;
> +    env->spr[SPR_XER] = cpu_read_xer(env);
>  #if defined(TARGET_PPC64)
>      env->spr[SPR_CFAR] = env->cfar;
>  #endif

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]