Patchwork [v2] arm gic saving/loading fix

login
register
mail settings
Submitter Dmitry Koshelev
Date Oct. 19, 2011, 2:10 p.m.
Message ID <CAKUHwZqVz1egRsgTtf_=Voe66EkL8n0kETW5zEFK5f=JHq_K6w@mail.gmail.com>
Download mbox | patch
Permalink /patch/120638/
State New
Headers show

Comments

Dmitry Koshelev - Oct. 19, 2011, 2:10 p.m.
irq_target field saving/loading is in the wrong loop
version bump

Signed-off-by: Dmitry Koshelev <karagiozis@gmail.com>
---
 hw/arm_gic.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

         for (j = 0; j < GIC_NIRQ; j++)
@@ -715,6 +712,9 @@ static int gic_load(QEMUFile *f, void *opaque, int
version_id)
         s->priority2[i] = qemu_get_be32(f);
     }
     for (i = 0; i < GIC_NIRQ; i++) {
+#ifndef NVIC
+        s->irq_target[i] = qemu_get_be32(f);
+#endif
         s->irq_state[i].enabled = qemu_get_byte(f);
         s->irq_state[i].pending = qemu_get_byte(f);
         s->irq_state[i].active = qemu_get_byte(f);
@@ -744,5 +744,5 @@ static void gic_init(gic_state *s)
     s->iomemtype = cpu_register_io_memory(gic_dist_readfn,
                                           gic_dist_writefn, s);
     gic_reset(s);
-    register_savevm(NULL, "arm_gic", -1, 1, gic_save, gic_load, s);
+    register_savevm(NULL, "arm_gic", -1, 2, gic_save, gic_load, s);
 }
Andreas Färber - Oct. 19, 2011, 4:22 p.m.
Dmitry,

It would be nice to cc the people that have cared to review previous
versions of the patch, Peter Maydell and me. Gets you quicker replies.

Functional ARM patches are being reviewed by our ARM gurus and not by
Stefan Hajnoczi, so please drop qemu-trivial from the cc list, as
pointed out before.

Am 19.10.2011 16:10, schrieb Dmitry Koshelev:
> irq_target field saving/loading is in the wrong loop
> version bump
> 
> Signed-off-by: Dmitry Koshelev <karagiozis@gmail.com>

From my side this version looks okay now, with a check for loading and a
bump for registration. Being no expert for the old savevm format,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

> ---
>  hw/arm_gic.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
> index 8286a28..d0747cf 100644
> --- a/hw/arm_gic.c
> +++ b/hw/arm_gic.c
> @@ -662,9 +662,6 @@ static void gic_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, s->enabled);
>      for (i = 0; i < NUM_CPU(s); i++) {
>          qemu_put_be32(f, s->cpu_enabled[i]);
> -#ifndef NVIC
> -        qemu_put_be32(f, s->irq_target[i]);
> -#endif
>          for (j = 0; j < 32; j++)
>              qemu_put_be32(f, s->priority1[j][i]);
>          for (j = 0; j < GIC_NIRQ; j++)
> @@ -678,6 +675,9 @@ static void gic_save(QEMUFile *f, void *opaque)
>          qemu_put_be32(f, s->priority2[i]);
>      }
>      for (i = 0; i < GIC_NIRQ; i++) {
> +#ifndef NVIC
> +        qemu_put_be32(f, s->irq_target[i]);
> +#endif
>          qemu_put_byte(f, s->irq_state[i].enabled);
>          qemu_put_byte(f, s->irq_state[i].pending);
>          qemu_put_byte(f, s->irq_state[i].active);
> @@ -693,15 +693,12 @@ static int gic_load(QEMUFile *f, void *opaque,
> int version_id)
>      int i;
>      int j;
> 
> -    if (version_id != 1)
> +    if (version_id != 2)
>          return -EINVAL;
> 
>      s->enabled = qemu_get_be32(f);
>      for (i = 0; i < NUM_CPU(s); i++) {
>          s->cpu_enabled[i] = qemu_get_be32(f);
> -#ifndef NVIC
> -        s->irq_target[i] = qemu_get_be32(f);
> -#endif
>          for (j = 0; j < 32; j++)
>              s->priority1[j][i] = qemu_get_be32(f);
>          for (j = 0; j < GIC_NIRQ; j++)
> @@ -715,6 +712,9 @@ static int gic_load(QEMUFile *f, void *opaque, int
> version_id)
>          s->priority2[i] = qemu_get_be32(f);
>      }
>      for (i = 0; i < GIC_NIRQ; i++) {
> +#ifndef NVIC
> +        s->irq_target[i] = qemu_get_be32(f);
> +#endif
>          s->irq_state[i].enabled = qemu_get_byte(f);
>          s->irq_state[i].pending = qemu_get_byte(f);
>          s->irq_state[i].active = qemu_get_byte(f);
> @@ -744,5 +744,5 @@ static void gic_init(gic_state *s)
>      s->iomemtype = cpu_register_io_memory(gic_dist_readfn,
>                                            gic_dist_writefn, s);
>      gic_reset(s);
> -    register_savevm(NULL, "arm_gic", -1, 1, gic_save, gic_load, s);
> +    register_savevm(NULL, "arm_gic", -1, 2, gic_save, gic_load, s);
>  }
>
Peter Maydell - Oct. 19, 2011, 6:21 p.m.
On 19 October 2011 15:10, Dmitry Koshelev <karaghiozis@gmail.com> wrote:
> irq_target field saving/loading is in the wrong loop
> version bump
>
> Signed-off-by: Dmitry Koshelev <karagiozis@gmail.com>
> ---
>  hw/arm_gic.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
>
> @@ -744,5 +744,5 @@ static void gic_init(gic_state *s)
>     s->iomemtype = cpu_register_io_memory(gic_dist_readfn,
>                                           gic_dist_writefn, s);
>     gic_reset(s);
> -    register_savevm(NULL, "arm_gic", -1, 1, gic_save, gic_load, s);
> +    register_savevm(NULL, "arm_gic", -1, 2, gic_save, gic_load, s);
>  }
>

This doesn't apply against master -- can you rebase, please?

Since you need to resend anyway, convention for git commit messages
is to put the filename in the summary line, something like

 hw/arm_gic.c: Fix save/load of irq_target array

Also, if at all possible, can you send via a mailer which doesn't
wrap long lines? Otherwise I have to fix them at this end which
is very tedious...

thanks
-- PMM

Patch

diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index 8286a28..d0747cf 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -662,9 +662,6 @@  static void gic_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, s->enabled);
     for (i = 0; i < NUM_CPU(s); i++) {
         qemu_put_be32(f, s->cpu_enabled[i]);
-#ifndef NVIC
-        qemu_put_be32(f, s->irq_target[i]);
-#endif
         for (j = 0; j < 32; j++)
             qemu_put_be32(f, s->priority1[j][i]);
         for (j = 0; j < GIC_NIRQ; j++)
@@ -678,6 +675,9 @@  static void gic_save(QEMUFile *f, void *opaque)
         qemu_put_be32(f, s->priority2[i]);
     }
     for (i = 0; i < GIC_NIRQ; i++) {
+#ifndef NVIC
+        qemu_put_be32(f, s->irq_target[i]);
+#endif
         qemu_put_byte(f, s->irq_state[i].enabled);
         qemu_put_byte(f, s->irq_state[i].pending);
         qemu_put_byte(f, s->irq_state[i].active);
@@ -693,15 +693,12 @@  static int gic_load(QEMUFile *f, void *opaque,
int version_id)
     int i;
     int j;

-    if (version_id != 1)
+    if (version_id != 2)
         return -EINVAL;

     s->enabled = qemu_get_be32(f);
     for (i = 0; i < NUM_CPU(s); i++) {
         s->cpu_enabled[i] = qemu_get_be32(f);
-#ifndef NVIC
-        s->irq_target[i] = qemu_get_be32(f);
-#endif
         for (j = 0; j < 32; j++)
             s->priority1[j][i] = qemu_get_be32(f);