Patchwork [v2,1/3] Add support for 64bit ARM system registers

login
register
mail settings
Submitter Alexey Starikovskiy
Date March 14, 2012, 1 p.m.
Message ID <4F609652.9070701@gmail.com>
Download mbox | patch
Permalink /patch/146631/
State New
Headers show

Comments

Alexey Starikovskiy - March 14, 2012, 1 p.m.
Signed-off-by: Alexey Starikovskiy<aystarik@gmail.com>
---
  target-arm/cpu.h     |   10 ++++------
  target-arm/helper.c  |   14 +++++++-------
  target-arm/machine.c |   16 ++++++----------
  3 files changed, 17 insertions(+), 23 deletions(-)
Michael Roth - March 14, 2012, 6:57 p.m.
On Wed, Mar 14, 2012 at 05:00:02PM +0400, Alexey Starikovskiy wrote:
> Signed-off-by: Alexey Starikovskiy<aystarik@gmail.com>
> ---
>  target-arm/cpu.h     |   10 ++++------
>  target-arm/helper.c  |   14 +++++++-------
>  target-arm/machine.c |   16 ++++++----------
>  3 files changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 0d9b39c..0298a98 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -117,11 +117,9 @@ typedef struct CPUARMState {
>          uint32_t c1_coproc; /* Coprocessor access register.  */
>          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
>          uint32_t c1_scr; /* secure config register.  */
> -        uint32_t c2_base0; /* MMU translation table base 0.  */
> -        uint32_t c2_base1; /* MMU translation table base 1.  */
> +        uint64_t c2_base0; /* MMU translation table base 0.  */
> +        uint64_t c2_base1; /* MMU translation table base 1.  */
>          uint32_t c2_control; /* MMU translation table base control.  */
> -        uint32_t c2_mask; /* MMU translation table base selection mask.  */
> -        uint32_t c2_base_mask; /* MMU translation table base 0 mask. */
>          uint32_t c2_data; /* MPU data cachable bits.  */
>          uint32_t c2_insn; /* MPU instruction cachable bits.  */
>          uint32_t c3; /* MMU domain access control register
> @@ -131,7 +129,7 @@ typedef struct CPUARMState {
>          uint32_t c6_region[8]; /* MPU base/size registers.  */
>          uint32_t c6_insn; /* Fault address registers.  */
>          uint32_t c6_data;
> -        uint32_t c7_par;  /* Translation result. */
> +        uint64_t c7_par;  /* Translation result. */
>          uint32_t c9_insn; /* Cache lockdown registers.  */
>          uint32_t c9_data;
>          uint32_t c9_pmcr; /* performance monitor control register */
> @@ -455,7 +453,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
>  #define cpu_signal_handler cpu_arm_signal_handler
>  #define cpu_list arm_cpu_list
> 
> -#define CPU_SAVE_VERSION 6
> +#define CPU_SAVE_VERSION 7
> 
>  /* MMU modes definitions */
>  #define MMU_MODE0_SUFFIX _kernel
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index abe1c30..d190104 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -325,7 +325,6 @@ void cpu_reset(CPUARMState *env)
>          }
>      }
>      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> -    env->cp15.c2_base_mask = 0xffffc000u;
>      /* v7 performance monitor control register: same implementor
>       * field as main ID register, and we implement no event counters.
>       */
> @@ -1050,12 +1049,15 @@ static inline int check_ap(CPUState *env, int ap, int domain_prot,
>  static uint32_t get_level1_table_address(CPUState *env, uint32_t address)
>  {
>      uint32_t table;
> +    int t0size = env->cp15.c2_control&  0x7;
> +    uint32_t mask = ~(((uint32_t)0xffffffffu)>>  t0size);
> 
> -    if (address&  env->cp15.c2_mask)
> +    if (address&  mask) {
>          table = env->cp15.c2_base1&  0xffffc000;
> -    else
> -        table = env->cp15.c2_base0&  env->cp15.c2_base_mask;
> -
> +    } else {
> +        mask = ~((uint32_t)0x3fffu>>  t0size);
> +        table = env->cp15.c2_base0&  mask;
> +    }
>      table |= (address>>  18)&  0x3ffc;
>      return table;
>  }
> @@ -1531,8 +1533,6 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn, uint32_t val)
>  	    case 2:
>                  val&= 7;
>                  env->cp15.c2_control = val;
> -		env->cp15.c2_mask = ~(((uint32_t)0xffffffffu)>>  val);
> -                env->cp15.c2_base_mask = ~((uint32_t)0x3fffu>>  val);
>  		break;
>  	    default:
>  		goto bad_reg;
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index f66b8df..8fa738e 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -27,11 +27,9 @@ void cpu_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, env->cp15.c1_coproc);
>      qemu_put_be32(f, env->cp15.c1_xscaleauxcr);
>      qemu_put_be32(f, env->cp15.c1_scr);
> -    qemu_put_be32(f, env->cp15.c2_base0);
> -    qemu_put_be32(f, env->cp15.c2_base1);
> +    qemu_put_be64(f, env->cp15.c2_base0);
> +    qemu_put_be64(f, env->cp15.c2_base1);
>      qemu_put_be32(f, env->cp15.c2_control);
> -    qemu_put_be32(f, env->cp15.c2_mask);
> -    qemu_put_be32(f, env->cp15.c2_base_mask);
>      qemu_put_be32(f, env->cp15.c2_data);
>      qemu_put_be32(f, env->cp15.c2_insn);
>      qemu_put_be32(f, env->cp15.c3);
> @@ -42,7 +40,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>      }
>      qemu_put_be32(f, env->cp15.c6_insn);
>      qemu_put_be32(f, env->cp15.c6_data);
> -    qemu_put_be32(f, env->cp15.c7_par);
> +    qemu_put_be64(f, env->cp15.c7_par);
>      qemu_put_be32(f, env->cp15.c9_insn);
>      qemu_put_be32(f, env->cp15.c9_data);
>      qemu_put_be32(f, env->cp15.c9_pmcr);
> @@ -145,11 +143,9 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>      env->cp15.c1_coproc = qemu_get_be32(f);
>      env->cp15.c1_xscaleauxcr = qemu_get_be32(f);
>      env->cp15.c1_scr = qemu_get_be32(f);
> -    env->cp15.c2_base0 = qemu_get_be32(f);
> -    env->cp15.c2_base1 = qemu_get_be32(f);
> +    env->cp15.c2_base0 = qemu_get_be64(f);
> +    env->cp15.c2_base1 = qemu_get_be64(f);

This breaks old -> new migration. Should CPU_SAVE_VERSION a do this
conditionally on version_id >= CPU_SAVE_VERSION

>      env->cp15.c2_control = qemu_get_be32(f);
> -    env->cp15.c2_mask = qemu_get_be32(f);
> -    env->cp15.c2_base_mask = qemu_get_be32(f);
>      env->cp15.c2_data = qemu_get_be32(f);
>      env->cp15.c2_insn = qemu_get_be32(f);
>      env->cp15.c3 = qemu_get_be32(f);
> @@ -160,7 +156,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>      }
>      env->cp15.c6_insn = qemu_get_be32(f);
>      env->cp15.c6_data = qemu_get_be32(f);
> -    env->cp15.c7_par = qemu_get_be32(f);
> +    env->cp15.c7_par = qemu_get_be64(f);

Here too.

>      env->cp15.c9_insn = qemu_get_be32(f);
>      env->cp15.c9_data = qemu_get_be32(f);
>      env->cp15.c9_pmcr = qemu_get_be32(f);
> 
> 
>
Michael Roth - March 14, 2012, 7:06 p.m.
On Wed, Mar 14, 2012 at 01:57:47PM -0500, Michael Roth wrote:
> On Wed, Mar 14, 2012 at 05:00:02PM +0400, Alexey Starikovskiy wrote:
> > Signed-off-by: Alexey Starikovskiy<aystarik@gmail.com>
> > ---
> >  target-arm/cpu.h     |   10 ++++------
> >  target-arm/helper.c  |   14 +++++++-------
> >  target-arm/machine.c |   16 ++++++----------
> >  3 files changed, 17 insertions(+), 23 deletions(-)
> > 
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 0d9b39c..0298a98 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -117,11 +117,9 @@ typedef struct CPUARMState {
> >          uint32_t c1_coproc; /* Coprocessor access register.  */
> >          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
> >          uint32_t c1_scr; /* secure config register.  */
> > -        uint32_t c2_base0; /* MMU translation table base 0.  */
> > -        uint32_t c2_base1; /* MMU translation table base 1.  */
> > +        uint64_t c2_base0; /* MMU translation table base 0.  */
> > +        uint64_t c2_base1; /* MMU translation table base 1.  */
> >          uint32_t c2_control; /* MMU translation table base control.  */
> > -        uint32_t c2_mask; /* MMU translation table base selection mask.  */
> > -        uint32_t c2_base_mask; /* MMU translation table base 0 mask. */
> >          uint32_t c2_data; /* MPU data cachable bits.  */
> >          uint32_t c2_insn; /* MPU instruction cachable bits.  */
> >          uint32_t c3; /* MMU domain access control register
> > @@ -131,7 +129,7 @@ typedef struct CPUARMState {
> >          uint32_t c6_region[8]; /* MPU base/size registers.  */
> >          uint32_t c6_insn; /* Fault address registers.  */
> >          uint32_t c6_data;
> > -        uint32_t c7_par;  /* Translation result. */
> > +        uint64_t c7_par;  /* Translation result. */
> >          uint32_t c9_insn; /* Cache lockdown registers.  */
> >          uint32_t c9_data;
> >          uint32_t c9_pmcr; /* performance monitor control register */
> > @@ -455,7 +453,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
> >  #define cpu_signal_handler cpu_arm_signal_handler
> >  #define cpu_list arm_cpu_list
> > 
> > -#define CPU_SAVE_VERSION 6
> > +#define CPU_SAVE_VERSION 7
> > 
> >  /* MMU modes definitions */
> >  #define MMU_MODE0_SUFFIX _kernel
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index abe1c30..d190104 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -325,7 +325,6 @@ void cpu_reset(CPUARMState *env)
> >          }
> >      }
> >      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> > -    env->cp15.c2_base_mask = 0xffffc000u;
> >      /* v7 performance monitor control register: same implementor
> >       * field as main ID register, and we implement no event counters.
> >       */
> > @@ -1050,12 +1049,15 @@ static inline int check_ap(CPUState *env, int ap, int domain_prot,
> >  static uint32_t get_level1_table_address(CPUState *env, uint32_t address)
> >  {
> >      uint32_t table;
> > +    int t0size = env->cp15.c2_control&  0x7;
> > +    uint32_t mask = ~(((uint32_t)0xffffffffu)>>  t0size);
> > 
> > -    if (address&  env->cp15.c2_mask)
> > +    if (address&  mask) {
> >          table = env->cp15.c2_base1&  0xffffc000;
> > -    else
> > -        table = env->cp15.c2_base0&  env->cp15.c2_base_mask;
> > -
> > +    } else {
> > +        mask = ~((uint32_t)0x3fffu>>  t0size);
> > +        table = env->cp15.c2_base0&  mask;
> > +    }
> >      table |= (address>>  18)&  0x3ffc;
> >      return table;
> >  }
> > @@ -1531,8 +1533,6 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn, uint32_t val)
> >  	    case 2:
> >                  val&= 7;
> >                  env->cp15.c2_control = val;
> > -		env->cp15.c2_mask = ~(((uint32_t)0xffffffffu)>>  val);
> > -                env->cp15.c2_base_mask = ~((uint32_t)0x3fffu>>  val);
> >  		break;
> >  	    default:
> >  		goto bad_reg;
> > diff --git a/target-arm/machine.c b/target-arm/machine.c
> > index f66b8df..8fa738e 100644
> > --- a/target-arm/machine.c
> > +++ b/target-arm/machine.c
> > @@ -27,11 +27,9 @@ void cpu_save(QEMUFile *f, void *opaque)
> >      qemu_put_be32(f, env->cp15.c1_coproc);
> >      qemu_put_be32(f, env->cp15.c1_xscaleauxcr);
> >      qemu_put_be32(f, env->cp15.c1_scr);
> > -    qemu_put_be32(f, env->cp15.c2_base0);
> > -    qemu_put_be32(f, env->cp15.c2_base1);
> > +    qemu_put_be64(f, env->cp15.c2_base0);
> > +    qemu_put_be64(f, env->cp15.c2_base1);
> >      qemu_put_be32(f, env->cp15.c2_control);
> > -    qemu_put_be32(f, env->cp15.c2_mask);
> > -    qemu_put_be32(f, env->cp15.c2_base_mask);
> >      qemu_put_be32(f, env->cp15.c2_data);
> >      qemu_put_be32(f, env->cp15.c2_insn);
> >      qemu_put_be32(f, env->cp15.c3);
> > @@ -42,7 +40,7 @@ void cpu_save(QEMUFile *f, void *opaque)
> >      }
> >      qemu_put_be32(f, env->cp15.c6_insn);
> >      qemu_put_be32(f, env->cp15.c6_data);
> > -    qemu_put_be32(f, env->cp15.c7_par);
> > +    qemu_put_be64(f, env->cp15.c7_par);
> >      qemu_put_be32(f, env->cp15.c9_insn);
> >      qemu_put_be32(f, env->cp15.c9_data);
> >      qemu_put_be32(f, env->cp15.c9_pmcr);
> > @@ -145,11 +143,9 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
> >      env->cp15.c1_coproc = qemu_get_be32(f);
> >      env->cp15.c1_xscaleauxcr = qemu_get_be32(f);
> >      env->cp15.c1_scr = qemu_get_be32(f);
> > -    env->cp15.c2_base0 = qemu_get_be32(f);
> > -    env->cp15.c2_base1 = qemu_get_be32(f);
> > +    env->cp15.c2_base0 = qemu_get_be64(f);
> > +    env->cp15.c2_base1 = qemu_get_be64(f);
> 
> This breaks old -> new migration. Should CPU_SAVE_VERSION a do this
> conditionally on version_id >= CPU_SAVE_VERSION

Heh, let's try that again. Should *increment* CPU_SAVE_VERSION *and*

Also, historically I guess arm just bails on old->new but that seems wrong.
Either we need to increment though.

> 
> >      env->cp15.c2_control = qemu_get_be32(f);
> > -    env->cp15.c2_mask = qemu_get_be32(f);
> > -    env->cp15.c2_base_mask = qemu_get_be32(f);
> >      env->cp15.c2_data = qemu_get_be32(f);
> >      env->cp15.c2_insn = qemu_get_be32(f);
> >      env->cp15.c3 = qemu_get_be32(f);
> > @@ -160,7 +156,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
> >      }
> >      env->cp15.c6_insn = qemu_get_be32(f);
> >      env->cp15.c6_data = qemu_get_be32(f);
> > -    env->cp15.c7_par = qemu_get_be32(f);
> > +    env->cp15.c7_par = qemu_get_be64(f);
> 
> Here too.
> 
> >      env->cp15.c9_insn = qemu_get_be32(f);
> >      env->cp15.c9_data = qemu_get_be32(f);
> >      env->cp15.c9_pmcr = qemu_get_be32(f);
> > 
> > 
> >
Alexey Starikovskiy - March 14, 2012, 7:09 p.m.
Do I need to do anything beside following or not?

Thanks,
Alex.

On Wed, Mar 14, 2012 at 11:06 PM, Michael Roth
<mdroth@linux.vnet.ibm.com> wrote:
>> > -#define CPU_SAVE_VERSION 6
>> > +#define CPU_SAVE_VERSION 7
Michael Roth - March 14, 2012, 7:38 p.m.
On Wed, Mar 14, 2012 at 10:09:09PM +0300, Alexey Starikovskiy wrote:
> Do I need to do anything beside following or not?

Not sure, ARM folks?

My suggestion would be to lose the the catch-all -EINVAL error we
throw in machine.c:cpu_load() when version_id != CPU_SAVE_VERSION
and only conditionally load 64-bit registers if the source's
version_id >= CPU_SAVE_VERSION. But we've never done that in the
past, and we broke old->new as recently as 2 months ago so I'm not
sure it's worth it since there hasn't been a release since then.

> 
> Thanks,
> Alex.
> 
> On Wed, Mar 14, 2012 at 11:06 PM, Michael Roth
> <mdroth@linux.vnet.ibm.com> wrote:
> >> > -#define CPU_SAVE_VERSION 6
> >> > +#define CPU_SAVE_VERSION 7
>
Peter Maydell - March 14, 2012, 8:38 p.m.
On 14 March 2012 19:38, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> On Wed, Mar 14, 2012 at 10:09:09PM +0300, Alexey Starikovskiy wrote:
>> Do I need to do anything beside following or not?
>
> Not sure, ARM folks?
>
> My suggestion would be to lose the the catch-all -EINVAL error we
> throw in machine.c:cpu_load() when version_id != CPU_SAVE_VERSION
> and only conditionally load 64-bit registers if the source's
> version_id >= CPU_SAVE_VERSION. But we've never done that in the
> past, and we broke old->new as recently as 2 months ago so I'm not
> sure it's worth it since there hasn't been a release since then.

My position at the moment is that old->new migration on ARM is
not supported and further that we make no attempt to avoid version
bumps. At some point as KVM-on-ARM gets towards being complete
we'll have to start worrying about compatibility, but at the moment
I really don't think that either (a) all the devices have tested
and working migration state or (b) we have a CPUState that's actually
in good enough shape[*] that we can reasonably start to insist on
not breaking cross-version migration.

So migration-wise I have no problem with the patch as it stands.
(I have other issues with it I suspect but haven't got to reviewing
it yet.)

[*] as an example of the kind of problem I'd like to see a solution
for, there's no reason that adding support for a new feature like
LPAE should require the migration state for non-LPAE CPUs to change,
but as things stand it does.

-- PMM

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 0d9b39c..0298a98 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -117,11 +117,9 @@  typedef struct CPUARMState {
          uint32_t c1_coproc; /* Coprocessor access register.  */
          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
          uint32_t c1_scr; /* secure config register.  */
-        uint32_t c2_base0; /* MMU translation table base 0.  */
-        uint32_t c2_base1; /* MMU translation table base 1.  */
+        uint64_t c2_base0; /* MMU translation table base 0.  */
+        uint64_t c2_base1; /* MMU translation table base 1.  */
          uint32_t c2_control; /* MMU translation table base control.  */
-        uint32_t c2_mask; /* MMU translation table base selection mask.  */
-        uint32_t c2_base_mask; /* MMU translation table base 0 mask. */
          uint32_t c2_data; /* MPU data cachable bits.  */
          uint32_t c2_insn; /* MPU instruction cachable bits.  */
          uint32_t c3; /* MMU domain access control register
@@ -131,7 +129,7 @@  typedef struct CPUARMState {
          uint32_t c6_region[8]; /* MPU base/size registers.  */
          uint32_t c6_insn; /* Fault address registers.  */
          uint32_t c6_data;
-        uint32_t c7_par;  /* Translation result. */
+        uint64_t c7_par;  /* Translation result. */
          uint32_t c9_insn; /* Cache lockdown registers.  */
          uint32_t c9_data;
          uint32_t c9_pmcr; /* performance monitor control register */
@@ -455,7 +453,7 @@  void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
  #define cpu_signal_handler cpu_arm_signal_handler
  #define cpu_list arm_cpu_list

-#define CPU_SAVE_VERSION 6
+#define CPU_SAVE_VERSION 7

  /* MMU modes definitions */
  #define MMU_MODE0_SUFFIX _kernel
diff --git a/target-arm/helper.c b/target-arm/helper.c
index abe1c30..d190104 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -325,7 +325,6 @@  void cpu_reset(CPUARMState *env)
          }
      }
      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
-    env->cp15.c2_base_mask = 0xffffc000u;
      /* v7 performance monitor control register: same implementor
       * field as main ID register, and we implement no event counters.
       */
@@ -1050,12 +1049,15 @@  static inline int check_ap(CPUState *env, int ap, int domain_prot,
  static uint32_t get_level1_table_address(CPUState *env, uint32_t address)
  {
      uint32_t table;
+    int t0size = env->cp15.c2_control&  0x7;
+    uint32_t mask = ~(((uint32_t)0xffffffffu)>>  t0size);

-    if (address&  env->cp15.c2_mask)
+    if (address&  mask) {
          table = env->cp15.c2_base1&  0xffffc000;
-    else
-        table = env->cp15.c2_base0&  env->cp15.c2_base_mask;
-
+    } else {
+        mask = ~((uint32_t)0x3fffu>>  t0size);
+        table = env->cp15.c2_base0&  mask;
+    }
      table |= (address>>  18)&  0x3ffc;
      return table;
  }
@@ -1531,8 +1533,6 @@  void HELPER(set_cp15)(CPUState *env, uint32_t insn, uint32_t val)
  	    case 2:
                  val&= 7;
                  env->cp15.c2_control = val;
-		env->cp15.c2_mask = ~(((uint32_t)0xffffffffu)>>  val);
-                env->cp15.c2_base_mask = ~((uint32_t)0x3fffu>>  val);
  		break;
  	    default:
  		goto bad_reg;
diff --git a/target-arm/machine.c b/target-arm/machine.c
index f66b8df..8fa738e 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -27,11 +27,9 @@  void cpu_save(QEMUFile *f, void *opaque)
      qemu_put_be32(f, env->cp15.c1_coproc);
      qemu_put_be32(f, env->cp15.c1_xscaleauxcr);
      qemu_put_be32(f, env->cp15.c1_scr);
-    qemu_put_be32(f, env->cp15.c2_base0);
-    qemu_put_be32(f, env->cp15.c2_base1);
+    qemu_put_be64(f, env->cp15.c2_base0);
+    qemu_put_be64(f, env->cp15.c2_base1);
      qemu_put_be32(f, env->cp15.c2_control);
-    qemu_put_be32(f, env->cp15.c2_mask);
-    qemu_put_be32(f, env->cp15.c2_base_mask);
      qemu_put_be32(f, env->cp15.c2_data);
      qemu_put_be32(f, env->cp15.c2_insn);
      qemu_put_be32(f, env->cp15.c3);
@@ -42,7 +40,7 @@  void cpu_save(QEMUFile *f, void *opaque)
      }
      qemu_put_be32(f, env->cp15.c6_insn);
      qemu_put_be32(f, env->cp15.c6_data);
-    qemu_put_be32(f, env->cp15.c7_par);
+    qemu_put_be64(f, env->cp15.c7_par);
      qemu_put_be32(f, env->cp15.c9_insn);
      qemu_put_be32(f, env->cp15.c9_data);
      qemu_put_be32(f, env->cp15.c9_pmcr);
@@ -145,11 +143,9 @@  int cpu_load(QEMUFile *f, void *opaque, int version_id)
      env->cp15.c1_coproc = qemu_get_be32(f);
      env->cp15.c1_xscaleauxcr = qemu_get_be32(f);
      env->cp15.c1_scr = qemu_get_be32(f);
-    env->cp15.c2_base0 = qemu_get_be32(f);
-    env->cp15.c2_base1 = qemu_get_be32(f);
+    env->cp15.c2_base0 = qemu_get_be64(f);
+    env->cp15.c2_base1 = qemu_get_be64(f);
      env->cp15.c2_control = qemu_get_be32(f);
-    env->cp15.c2_mask = qemu_get_be32(f);
-    env->cp15.c2_base_mask = qemu_get_be32(f);
      env->cp15.c2_data = qemu_get_be32(f);
      env->cp15.c2_insn = qemu_get_be32(f);
      env->cp15.c3 = qemu_get_be32(f);
@@ -160,7 +156,7 @@  int cpu_load(QEMUFile *f, void *opaque, int version_id)
      }
      env->cp15.c6_insn = qemu_get_be32(f);
      env->cp15.c6_data = qemu_get_be32(f);
-    env->cp15.c7_par = qemu_get_be32(f);
+    env->cp15.c7_par = qemu_get_be64(f);
      env->cp15.c9_insn = qemu_get_be32(f);
      env->cp15.c9_data = qemu_get_be32(f);
      env->cp15.c9_pmcr = qemu_get_be32(f);